resolve Copilot comments

Author: kuchtiak-ufal

dspace-api/src/main/java/org/dspace/health/MetadataCheck.java

@@ -385,14 +385,19 @@ private void addMessage(String validationType,
          *                        comparing to the frequency of the new message
          */
         private void replaceMessage(Message message, StoredMessagesInfo storedMessagesInfo, int dispersionQuota) {
-            int highestMessageFrequency = storedMessagesInfo.getHighestFrequency();
+            Map<String, List<String>> storedMessages = storedMessagesInfo.getStoredMessages();
+
+            // calculate the highest frequency of messages for any short message in stored messages,
+            String messageKeyWithHighestFrequency = Objects.requireNonNull(getMessageWithHighestCount(storedMessages));
+            int highestMessageFrequency = storedMessages.get(messageKeyWithHighestFrequency).size();
+
+            // int highestMessageFrequency = storedMessagesInfo.getHighestFrequency();
             if (highestMessageFrequency <= 1) {
                 // no replacement, as there are no messages with the frequency higher than 1, so the replacement
                 // of any message would not increase the diversity of messages in stored messages
                 return;
             }
             String messageKey = message.getMessageKey();
-            Map<String, List<String>> storedMessages = storedMessagesInfo.getStoredMessages();
             List<String> storedMessagesForMessageKey = storedMessages.get(messageKey);
 
             if (storedMessagesForMessageKey != null &&
@@ -402,11 +407,10 @@ private void replaceMessage(Message message, StoredMessagesInfo storedMessagesIn
                 return;
             }
 
-            // recalculate the highest frequency of messages for any short message in storedmessages,
+            // recalculate the highest frequency of messages for any short message in stored messages,
             // because it can be changed after each replacement
-            String messageKeyWithHighestFrequency = Objects.requireNonNull(getMessageWithHighestCount(storedMessages));
+            messageKeyWithHighestFrequency = Objects.requireNonNull(getMessageWithHighestCount(storedMessages));
             highestMessageFrequency = storedMessages.get(messageKeyWithHighestFrequency).size();
-            storedMessagesInfo.setHighestFrequency(highestMessageFrequency);
 
             if (highestMessageFrequency <= 1) {
                 // no replacement, as there are no messages with the frequency higher than 1 anymore
@@ -455,27 +459,17 @@ private static String getMessageWithHighestCount(Map<String, List<String>> store
      */
     private static class StoredMessagesInfo {
         private int count;
-        private int highestFrequency;
         private final Map<String, List<String>> storedMessages;
 
         StoredMessagesInfo() {
             this.count = 0;
-            this.highestFrequency = Integer.MAX_VALUE;
             storedMessages = new TreeMap<>();
         }
 
         public int getCount() {
             return count;
         }
 
-        public int getHighestFrequency() {
-            return highestFrequency;
-        }
-
-        public void setHighestFrequency(int highestFrequency) {
-            this.highestFrequency = highestFrequency;
-        }
-
         public Map<String, List<String>> getStoredMessages() {
             return storedMessages;
         }

dspace-api/src/main/resources/metadata-check-patterns.json

@@ -25,7 +25,7 @@
       "^There are bitstreams but incomplete rights metadata."
     ],
     "dc.description": [
-      "^contains suspicious dc.description.uri metadata"
+      "^contains suspicious [dc.description.uri] metadata"
     ],
     "local.branding": [
       "^local.branding "

dspace-api/src/test/java/org/dspace/scripts/HealthReportIT.java

@@ -40,7 +40,6 @@
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
 import org.dspace.content.Item;
-import org.dspace.content.MetadataValue;
 import org.dspace.content.ReportResult;
 import org.dspace.content.clarin.ClarinLicense;
 import org.dspace.content.clarin.ClarinLicenseLabel;
@@ -399,108 +398,109 @@ public void testMetadataCheckWithRestrictedReportSize() throws Exception {
         configurationService.setProperty("healthcheck.metadata.max-errors-to-show", 8);
         configurationService.setProperty("healthcheck.metadata.error-dispersion-quota", 1);
 
-        Community community = CommunityBuilder.createCommunity(context)
-                .withName("Community")
-                .build();
-
-        Collection collection = CollectionBuilder.createCollection(context, community)
-                .withName("Collection")
-                .withSubmitterGroup(eperson)
-                .build();
-
-        Item item1 = ItemBuilder.createItem(context, collection)
-                .withTitle("Test item 1")
-                .withType("corpus")
-                .withSubject("Test subject")
-                .withMetadata("local", "branding", null, "Community")
-                .build();
-
-        Item item2 = ItemBuilder.createItem(context, collection)
-                .withTitle("Test item 2")
-                .withType("toolService")
-                .withSubject("Test subject")
-                .withMetadata("local", "branding", null, "Community")
-                .withMetadata("dc", "relation", "replaces", findItemUri(item1))
-                .build();
+        try {
+            Community community = CommunityBuilder.createCommunity(context)
+                    .withName("Community")
+                    .build();
 
-        ItemBuilder.createItem(context, collection)
-                .withTitle("Test item 3")
-                .withType("toolService")
-                .withSubject("Test subject")
-                .withMetadata("local", "branding", null, "Community")
-                .withMetadata("dc", "relation", "isreplacedby", findItemUri(item2))
-                .build();
+            Collection collection = CollectionBuilder.createCollection(context, community)
+                    .withName("Collection")
+                    .withSubmitterGroup(eperson)
+                    .build();
 
-        // create 4 items with missing title
-        for (int i = 0; i < 4; i++) {
-            ItemBuilder.createItem(context, collection)
-                    .withType("toolService")
+            Item item1 = ItemBuilder.createItem(context, collection)
+                    .withTitle("Test item 1")
+                    .withType("corpus")
                     .withSubject("Test subject")
                     .withMetadata("local", "branding", null, "Community")
                     .build();
-        }
 
-        // create 4 items with missing type
-        for (int i = 4; i < 8; i++) {
-            ItemBuilder.createItem(context, collection)
-                    .withTitle("Test Item " + i)
+            Item item2 = ItemBuilder.createItem(context, collection)
+                    .withTitle("Test item 2")
+                    .withType("toolService")
                     .withSubject("Test subject")
                     .withMetadata("local", "branding", null, "Community")
+                    .withMetadata("dc", "relation", "replaces", findItemUri(item1))
                     .build();
-        }
 
-        // create 4 items with duplicate type
-        for (int i = 8; i < 12; i++) {
             ItemBuilder.createItem(context, collection)
-                    .withTitle("Test Item " + i)
+                    .withTitle("Test item 3")
                     .withType("toolService")
-                    .withType("corpus")
                     .withSubject("Test subject")
                     .withMetadata("local", "branding", null, "Community")
+                    .withMetadata("dc", "relation", "isreplacedby", findItemUri(item2))
                     .build();
-        }
-
-        TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
 
-        // with "health-report -c 5", only Metadata check is running
-        String[] args = new String[]{"health-report", "-c", "5"};
-        ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl), testDSpaceRunnableHandler, kernelImpl);
-
-        assertThat(testDSpaceRunnableHandler.getErrorMessages(), empty());
-        List<String> messages = testDSpaceRunnableHandler.getInfoMessages();
+            // create 4 items with missing title
+            for (int i = 0; i < 4; i++) {
+                ItemBuilder.createItem(context, collection)
+                        .withType("toolService")
+                        .withSubject("Test subject")
+                        .withMetadata("local", "branding", null, "Community")
+                        .build();
+            }
 
-        assertThat(messages, hasSize(1));
-        assertThat(messages.get(0), containsString("dc.relation issues:  " + " ".repeat(15) + "2"));
-        assertThat(messages.get(0), containsString("dc.title issues:     " + " ".repeat(15) + "4"));
-        assertThat(messages.get(0), containsString("dc.type issues:      " + " ".repeat(15) + "4"));
-        assertThat(messages.get(0), containsString("duplicate value issues:" + " ".repeat(13) + "4"));
-        assertThat(messages.get(0), containsString("Error count total:   " + " ".repeat(14) + "14"));
+            // create 4 items with missing type
+            for (int i = 4; i < 8; i++) {
+                ItemBuilder.createItem(context, collection)
+                        .withTitle("Test Item " + i)
+                        .withSubject("Test subject")
+                        .withMetadata("local", "branding", null, "Community")
+                        .build();
+            }
 
-        assertThat(messages.get(0), containsString("Errors:"));
+            // create 4 items with duplicate type
+            for (int i = 8; i < 12; i++) {
+                ItemBuilder.createItem(context, collection)
+                        .withTitle("Test Item " + i)
+                        .withType("toolService")
+                        .withType("corpus")
+                        .withSubject("Test subject")
+                        .withMetadata("local", "branding", null, "Community")
+                        .build();
+            }
 
-        // check if dc.type error is present exactly 2 times
-        assertThat(StringUtils.countMatches(messages.get(0), "Does not have dc.type metadata"), is(2));
-        // check if dc.title error is present exactly 2 times
-        assertThat(StringUtils.countMatches(messages.get(0), "Item has no dc.title metadata"), is(2));
-        // check id duplicate value error is present exactly 2 times
-        assertThat(StringUtils.countMatches(messages.get(0), "value [dc.type] is present multiple times"), is(2));
-
-        // check if all dc.relation errors are present
-        assertThat(StringUtils.countMatches(messages.get(0), "does not refer back via dc.relation.replaces"), is(1));
-        assertThat(
-                StringUtils.countMatches(messages.get(0), "does not refer back via dc.relation.isreplacedby"), is(1));
-        assertThat(messages.get(0), containsString("and more..."));
-
-        configurationService.setProperty("healthcheck.metadata.max-errors-to-show", null);
-        configurationService.setProperty("healthcheck.metadata.error-dispersion-quota", null);
+            TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
+
+            // with "health-report -c 5", only Metadata check is running
+            String[] args = new String[]{"health-report", "-c", "5"};
+            ScriptLauncher.handleScript(args,
+                    ScriptLauncher.getConfig(kernelImpl), testDSpaceRunnableHandler, kernelImpl);
+
+            assertThat(testDSpaceRunnableHandler.getErrorMessages(), empty());
+            List<String> messages = testDSpaceRunnableHandler.getInfoMessages();
+
+            assertThat(messages, hasSize(1));
+            assertThat(messages.get(0), containsString("dc.relation issues:  " + " ".repeat(15) + "2"));
+            assertThat(messages.get(0), containsString("dc.title issues:     " + " ".repeat(15) + "4"));
+            assertThat(messages.get(0), containsString("dc.type issues:      " + " ".repeat(15) + "4"));
+            assertThat(messages.get(0), containsString("duplicate value issues:" + " ".repeat(13) + "4"));
+            assertThat(messages.get(0), containsString("Error count total:   " + " ".repeat(14) + "14"));
+
+            assertThat(messages.get(0), containsString("Errors:"));
+
+            // check if dc.type error is present exactly 2 times
+            assertThat(StringUtils.countMatches(messages.get(0), "Does not have dc.type metadata"), is(2));
+            // check if dc.title error is present exactly 2 times
+            assertThat(StringUtils.countMatches(messages.get(0), "Item has no dc.title metadata"), is(2));
+            // check if duplicate value error is present exactly 2 times
+            assertThat(StringUtils.countMatches(messages.get(0), "value [dc.type] is present multiple times"), is(2));
+
+            // check if all dc.relation errors are present
+            assertThat(StringUtils.countMatches(
+                    messages.get(0), "does not refer back via dc.relation.replaces"), is(1));
+            assertThat(StringUtils.countMatches(
+                    messages.get(0), "does not refer back via dc.relation.isreplacedby"), is(1));
+            assertThat(messages.get(0), containsString("and more..."));
+        } finally {
+            configurationService.setProperty("healthcheck.metadata.max-errors-to-show", null);
+            configurationService.setProperty("healthcheck.metadata.error-dispersion-quota", null);
+        }
     }
 
     private String findItemUri(Item item) {
-        return item.getMetadata().stream()
-                .filter(metadataValue -> "dc_identifier_uri".equals(metadataValue.getMetadataField().toString()))
-                .findFirst()
-                .map(MetadataValue::getValue)
-                .orElse(null);
+        return ContentServiceFactory.getInstance().getItemService()
+                .getMetadataFirstValue(item, "dc", "identifier", "uri", Item.ANY);
     }
 
     ReportResult findLastReportResult(List<ReportResult> reportResults) {