Merged in DSC-1256 (pull request DSpace#1280)

[DSC-1256] Avoid NPE checking bitstream content when metadata are missing on cris layout Approved-by: Stefano Maffei
4Science · Mar 26, 2024 · f768bbe · f768bbe
2 parents 73beed3 + 61e11de
commit f768bbe
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 32 deletions.
diff --git a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java
@@ -216,28 +216,6 @@ public Thumbnail getThumbnail(Context context, Item item) throws SQLException {
         if (thumbnail != null) {
             return thumbnail;
         }
-        // If no thumbnail is retrieved by the first strategy
-        // then use the fallback strategy
-        Bitstream thumbBitstream = null;
-        List<Bundle> originalBundles = getBundles(item, "ORIGINAL");
-        Bitstream primaryBitstream = null;
-        if (CollectionUtils.isNotEmpty(originalBundles)) {
-            primaryBitstream = originalBundles.get(0).getPrimaryBitstream();
-        }
-        if (primaryBitstream == null) {
-            primaryBitstream = bitstreamService.getFirstBitstream(item, "ORIGINAL");
-        }
-        if (primaryBitstream != null) {
-            thumbBitstream = bitstreamService.getThumbnail(context, primaryBitstream);
-            if (thumbBitstream == null) {
-                thumbBitstream = bitstreamService.getFirstBitstream(item, "THUMBNAIL");
-            }
-        }
-
-        if (thumbBitstream != null) {
-            return new Thumbnail(thumbBitstream, primaryBitstream);
-        }
-
         return null;
     }
 
@@ -252,7 +230,27 @@ private Thumbnail thumbnailLayoutTabConfigurationStrategy(Context context, Item
 
         List<CrisLayoutField> thumbFields = getThumbnailFields(crisLayoutTabs);
         if (CollectionUtils.isEmpty(thumbFields)) {
-            return null;
+            // If no thumbnail is retrieved by the first strategy
+            // then use the fallback strategy
+            Bitstream thumbBitstream = null;
+            List<Bundle> originalBundles = getBundles(item, "ORIGINAL");
+            Bitstream primaryBitstream = null;
+            if (CollectionUtils.isNotEmpty(originalBundles)) {
+                primaryBitstream = originalBundles.get(0).getPrimaryBitstream();
+            }
+            if (primaryBitstream == null) {
+                primaryBitstream = bitstreamService.getFirstBitstream(item, "ORIGINAL");
+            }
+            if (primaryBitstream != null) {
+                thumbBitstream = bitstreamService.getThumbnail(context, primaryBitstream);
+                if (thumbBitstream == null) {
+                    thumbBitstream = bitstreamService.getFirstBitstream(item, "THUMBNAIL");
+                }
+            }
+
+            if (thumbBitstream != null) {
+                return new Thumbnail(thumbBitstream, primaryBitstream);
+            }
         }
         return retrieveThumbnailFromFields(context, item, thumbFields);
     }
@@ -312,9 +310,13 @@ private Thumbnail retrieveThumbnail(Context context, Item item, String bundle,
         if (CollectionUtils.isNotEmpty(bundles)) {
             Optional<Bitstream> primaryBitstream = bundles.get(0).getBitstreams().stream().filter(bitstream -> {
                 return bitstream.getMetadata().stream().anyMatch(metadataValue -> {
-                    return metadataValue.getMetadataField().getID() == metadataField.getID()
-                        && metadataValue.getValue() != null
-                        && metadataValue.getValue().equalsIgnoreCase(value);
+                    if (metadataField != null) {
+                        return metadataValue.getMetadataField().getID() == metadataField.getID()
+                            && metadataValue.getValue() != null
+                            && metadataValue.getValue().equalsIgnoreCase(value);
+                    } else {
+                        return true;
+                    }
                 });
             }).findFirst();
             if (primaryBitstream.isEmpty()) {

diff --git a/dspace-api/src/main/java/org/dspace/layout/service/impl/CrisLayoutBoxServiceImpl.java b/dspace-api/src/main/java/org/dspace/layout/service/impl/CrisLayoutBoxServiceImpl.java
@@ -9,6 +9,7 @@
 
 import java.sql.SQLException;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -208,13 +209,10 @@ private boolean isMetadataFieldPresent(DSpaceObject item, MetadataField metadata
     }
 
     private boolean isBitstreamPresent(Context context, Item item, CrisLayoutFieldBitstream field) {
-
-        Map<String, String> filters = Map.of();
-
-        if (field.getMetadataField() != null) {
-            filters = Map.of(field.getMetadataField().toString('.'), field.getMetadataValue());
+        Map<String, String> filters = new HashMap<>();
+        if (field.getMetadataField() != null && StringUtils.isNotBlank(field.getMetadataValue())) {
+            filters.put(field.getMetadataField().toString('.'), field.getMetadataValue());
         }
-
         try {
             return bitstreamService.findShowableByItem(context, item.getID(), field.getBundle(), filters).size() > 0;
         } catch (SQLException e) {

diff --git a/dspace-api/src/test/java/org/dspace/layout/service/impl/CrisLayoutBoxServiceImplTest.java b/dspace-api/src/test/java/org/dspace/layout/service/impl/CrisLayoutBoxServiceImplTest.java
@@ -214,6 +214,36 @@ public void testHasNoContentWithBoxWithBitstream() throws SQLException {
 
     }
 
+    @Test
+    public void testHasContentWithBoxWithBitstreamWithBlankMetadataValue() throws SQLException {
+
+        MetadataField titleField = metadataField("dc", "title", null);
+        MetadataField typeField = metadataField("dc", "type", null);
+
+        Item item = item();
+
+        Bitstream bitstream = mock(Bitstream.class);
+
+        CrisLayoutFieldBitstream fieldBitstream = new CrisLayoutFieldBitstream();
+        fieldBitstream.setBundle("ORIGINAL");
+        fieldBitstream.setMetadataField(typeField);
+        fieldBitstream.setMetadataValue(null);
+
+        CrisLayoutBox box = new CrisLayoutBox();
+        box.addLayoutField(crisLayoutField(titleField));
+        box.addLayoutField(fieldBitstream);
+        box.setShortname("Main Box");
+        box.setType("METADATA");
+
+        when(bitstreamService.findShowableByItem(context, item.getID(), "ORIGINAL", Map.of()))
+                .thenReturn(List.of(bitstream));
+
+        assertThat(crisLayoutBoxService.hasContent(context, box, item), is(true));
+
+        verify(bitstreamService).findShowableByItem(context, item.getID(), "ORIGINAL", Map.of());
+
+    }
+
     @Test
     public void testHasMetricsBoxContent() throws SQLException {