From 3c2932eb0b8fb83f15a77540e9b7994ecf66280f Mon Sep 17 00:00:00 2001 From: mohamed eskander Date: Tue, 14 Nov 2023 14:56:43 +0200 Subject: [PATCH 1/2] [DSC-1256] Avoid NPE checking bitstream content when metadata are missing on cris layout --- .../org/dspace/content/ItemServiceImpl.java | 54 ++++++++++--------- .../impl/CrisLayoutBoxServiceImpl.java | 6 ++- 2 files changed, 33 insertions(+), 27 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 index cfad7f87dba..aa29bb4f0f9 100644 --- a/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/ItemServiceImpl.java @@ -212,28 +212,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 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; } @@ -248,7 +226,27 @@ private Thumbnail thumbnailLayoutTabConfigurationStrategy(Context context, Item List 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 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); } @@ -308,9 +306,13 @@ private Thumbnail retrieveThumbnail(Context context, Item item, String bundle, if (CollectionUtils.isNotEmpty(bundles)) { Optional 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 index 269dd660185..c15a2f792bc 100644 --- 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,7 +209,10 @@ private boolean isMetadataFieldPresent(DSpaceObject item, MetadataField metadata } private boolean isBitstreamPresent(Context context, Item item, CrisLayoutFieldBitstream field) { - Map filters = Map.of(field.getMetadataField().toString('.'), field.getMetadataValue()); + Map 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) { From 50ec83cc943ed46e3ff2df8a0105c7b3eb39dee1 Mon Sep 17 00:00:00 2001 From: eskander Date: Tue, 26 Mar 2024 12:17:18 +0200 Subject: [PATCH 2/2] [DSC-1256] add new test case when metadata value is null --- .../impl/CrisLayoutBoxServiceImplTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) 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 index fd61aeb0a76..722882589aa 100644 --- 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 {