From e94f0a9cb3d43c1ed7af44683202737f598681d5 Mon Sep 17 00:00:00 2001 From: Zeroday BYTE Date: Thu, 17 Jul 2025 02:06:29 +0700 Subject: [PATCH 1/4] Update DSpaceControlledVocabulary.java --- .../authority/DSpaceControlledVocabulary.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java index 4e30559e1c..ff91a58c6f 100644 --- a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java +++ b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java @@ -178,13 +178,21 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera public Choices getMatches(String text, int start, int limit, String locale) { init(locale); log.debug("Getting matches for '" + text + "'"); - String xpathExpression = ""; String[] textHierarchy = text.split(hierarchyDelimiter, -1); + StringBuilder xpathExpressionBuilder = new StringBuilder(); for (int i = 0; i < textHierarchy.length; i++) { - xpathExpression += - String.format(xpathTemplate, textHierarchy[i].replaceAll("'", "'").toLowerCase()); + xpathExpressionBuilder.append(String.format(xpathTemplate, "$var" + i)); } + String xpathExpression = xpathExpressionBuilder.toString(); XPath xpath = XPathFactory.newInstance().newXPath(); + xpath.setXPathVariableResolver(variableName -> { + String varName = variableName.getLocalPart(); + if (varName.startsWith("var")) { + int index = Integer.parseInt(varName.substring(3)); + return textHierarchy[index].toLowerCase(); + } + throw new IllegalArgumentException("Unexpected variable: " + varName); + }); int total = 0; List choices = new ArrayList(); try { @@ -203,13 +211,21 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera public Choices getBestMatch(String text, String locale) { init(locale); log.debug("Getting best matches for '" + text + "'"); - String xpathExpression = ""; String[] textHierarchy = text.split(hierarchyDelimiter, -1); + StringBuilder xpathExpressionBuilder = new StringBuilder(); for (int i = 0; i < textHierarchy.length; i++) { - xpathExpression += - String.format(valueTemplate, textHierarchy[i].replaceAll("'", "'")); + xpathExpressionBuilder.append(String.format(valueTemplate, "$var" + i)); } + String xpathExpression = xpathExpressionBuilder.toString(); XPath xpath = XPathFactory.newInstance().newXPath(); + xpath.setXPathVariableResolver(variableName -> { + String varName = variableName.getLocalPart(); + if (varName.startsWith("var")) { + int index = Integer.parseInt(varName.substring(3)); + return textHierarchy[index]; + } + throw new IllegalArgumentException("Unexpected variable: " + varName); + }); List choices = new ArrayList(); try { NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); From c781ba278093f7c80b8b68c601309fe90a9b85f0 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Thu, 24 Jul 2025 08:32:50 -0400 Subject: [PATCH 2/4] 'No match' should be test failure, not index error. --- .../content/authority/DSpaceControlledVocabularyTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java b/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java index 43bd20cc15..5eb56428f7 100644 --- a/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java +++ b/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java @@ -8,6 +8,7 @@ package org.dspace.content.authority; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import java.io.IOException; @@ -86,6 +87,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { CoreServiceFactory.getInstance().getPluginService().getNamedPlugin(Class.forName(PLUGIN_INTERFACE), "farm"); assertNotNull(instance); Choices result = instance.getMatches(text, start, limit, locale); + assertNotEquals("At least one match expected", 0, result.values.length); assertEquals("north 40", result.values[0].value); } @@ -110,6 +112,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { "countries"); assertNotNull(instance); Choices result = instance.getMatches(labelPart, start, limit, null); + assertNotEquals("At least one match expected", 0, result.values.length); assertEquals(idValue, result.values[0].value); assertEquals("Algeria", result.values[0].label); } @@ -132,6 +135,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { "countries"); assertNotNull(instance); Choices result = instance.getBestMatch(idValue, null); + assertNotEquals("At least one match expected", 0, result.values.length); assertEquals(idValue, result.values[0].value); assertEquals("Algeria", result.values[0].label); } @@ -157,6 +161,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { "countries"); assertNotNull(instance); Choices result = instance.getMatches(labelPart, start, limit, "de"); + assertNotEquals("At least one match expected", 0, result.values.length); assertEquals(idValue, result.values[0].value); assertEquals("Algerien", result.values[0].label); } @@ -179,6 +184,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { "countries"); assertNotNull(instance); Choices result = instance.getBestMatch(idValue, "de"); + assertNotEquals("At least one match expected", 0, result.values.length); assertEquals(idValue, result.values[0].value); assertEquals("Algerien", result.values[0].label); } From 7deaf1cca5c35b721726be1fa7b8a0aa86647938 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Thu, 24 Jul 2025 11:51:53 -0400 Subject: [PATCH 3/4] Variables in XPath expressions should not be quoted. Documentation cleanup. Clean up many IDE warnings. --- .../authority/DSpaceControlledVocabulary.java | 66 +++++++++---------- .../DSpaceControlledVocabularyTest.java | 14 ++-- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java index ff91a58c6f..ccfb156790 100644 --- a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java +++ b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java @@ -35,23 +35,25 @@ import org.xml.sax.InputSource; * from {@code ${dspace.dir}/config/controlled-vocabularies/*.xml} and turns * them into autocompleting authorities. * - * Configuration: This MUST be configured as a self-named plugin, e.g.: {@code - * plugin.selfnamed.org.dspace.content.authority.ChoiceAuthority = \ + *

Configuration: This MUST be configured as a self-named plugin, e.g.: {@code + * plugin.selfnamed.org.dspace.content.authority.ChoiceAuthority = * org.dspace.content.authority.DSpaceControlledVocabulary * } * - * It AUTOMATICALLY configures a plugin instance for each XML file in the + *

It AUTOMATICALLY configures a plugin instance for each XML file in the * controlled vocabularies directory. The name of the plugin is the basename of * the file; e.g., {@code ${dspace.dir}/config/controlled-vocabularies/nsi.xml} * would generate a plugin called "nsi". * - * Each configured plugin comes with three configuration options: {@code - * vocabulary.plugin._plugin_.hierarchy.store = - * # Store entire hierarchy along with selected value. Default: TRUE - * vocabulary.plugin._plugin_.hierarchy.suggest = - * # Display entire hierarchy in the suggestion list. Default: TRUE - * vocabulary.plugin._plugin_.delimiter = "" - * # Delimiter to use when building hierarchy strings. Default: "::" + *

Each configured plugin comes with three configuration options: + *

    + *
  • {@code vocabulary.plugin._plugin_.hierarchy.store = + * # Store entire hierarchy along with selected value. Default: TRUE}
    • + *
  • {@code vocabulary.plugin._plugin_.hierarchy.suggest = + * # Display entire hierarchy in the suggestion list. Default: TRUE}
    • + *
  • {@code vocabulary.plugin._plugin_.delimiter = "" + * # Delimiter to use when building hierarchy strings. Default: "::"}
    • + *
* } * * @author Michael B. Klein @@ -59,12 +61,13 @@ import org.xml.sax.InputSource; public class DSpaceControlledVocabulary extends SelfNamedPlugin implements HierarchicalAuthority { - private static Logger log = org.apache.logging.log4j.LogManager.getLogger(DSpaceControlledVocabulary.class); + private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(); protected static String xpathTemplate = "//node[contains(translate(@label,'ABCDEFGHIJKLMNOPQRSTUVWXYZ'," + - "'abcdefghijklmnopqrstuvwxyz'),'%s')]"; - protected static String idTemplate = "//node[@id = '%s']"; - protected static String labelTemplate = "//node[@label = '%s']"; - protected static String idParentTemplate = "//node[@id = '%s']/parent::isComposedBy/parent::node"; + "'abcdefghijklmnopqrstuvwxyz'),%s)]"; + protected static String idTemplate = "//node[@id = %s]"; + protected static String idTemplateQuoted = "//node[@id = '%s']"; + protected static String labelTemplate = "//node[@label = %s]"; + protected static String idParentTemplate = "//node[@id = %s]/parent::isComposedBy/parent::node"; protected static String rootTemplate = "/node"; protected static String idAttribute = "id"; protected static String labelAttribute = "label"; @@ -110,7 +113,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera File.separator + "config" + File.separator + "controlled-vocabularies"; String[] xmlFiles = (new File(vocabulariesPath)).list(new xmlFilter()); - List names = new ArrayList(); + List names = new ArrayList<>(); for (String filename : xmlFiles) { names.add((new File(filename)).getName().replace(".xml", "")); } @@ -193,8 +196,8 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } throw new IllegalArgumentException("Unexpected variable: " + varName); }); - int total = 0; - List choices = new ArrayList(); + int total; + List choices; try { NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); total = results.getLength(); @@ -210,7 +213,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera @Override public Choices getBestMatch(String text, String locale) { init(locale); - log.debug("Getting best matches for '" + text + "'"); + log.debug("Getting best matches for {}'", text); String[] textHierarchy = text.split(hierarchyDelimiter, -1); StringBuilder xpathExpressionBuilder = new StringBuilder(); for (int i = 0; i < textHierarchy.length; i++) { @@ -226,7 +229,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } throw new IllegalArgumentException("Unexpected variable: " + varName); }); - List choices = new ArrayList(); + List choices; try { NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); choices = getChoicesFromNodeList(results, 0, 1); @@ -298,15 +301,12 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } private boolean isRootElement(Node node) { - if (node != null && node.getOwnerDocument().getDocumentElement().equals(node)) { - return true; - } - return false; + return node != null && node.getOwnerDocument().getDocumentElement().equals(node); } private Node getNode(String key, String locale) throws XPathExpressionException { init(locale); - String xpathExpression = String.format(idTemplate, key); + String xpathExpression = String.format(idTemplateQuoted, key); Node node = getNodeFromXPath(xpathExpression); return node; } @@ -318,7 +318,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } private List getChoicesFromNodeList(NodeList results, int start, int limit) { - List choices = new ArrayList(); + List choices = new ArrayList<>(); for (int i = 0; i < results.getLength(); i++) { if (i < start) { continue; @@ -337,17 +337,17 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera private Map addOtherInformation(String parentCurr, String noteCurr, List childrenCurr, String authorityCurr) { - Map extras = new HashMap(); + Map extras = new HashMap<>(); if (StringUtils.isNotBlank(parentCurr)) { extras.put("parent", parentCurr); } if (StringUtils.isNotBlank(noteCurr)) { extras.put("note", noteCurr); } - if (childrenCurr.size() > 0) { - extras.put("hasChildren", "true"); - } else { + if (childrenCurr.isEmpty()) { extras.put("hasChildren", "false"); + } else { + extras.put("hasChildren", "true"); } extras.put("id", authorityCurr); return extras; @@ -402,7 +402,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } private List getChildren(Node node) { - List children = new ArrayList(); + List children = new ArrayList<>(); NodeList childNodes = node.getChildNodes(); for (int ci = 0; ci < childNodes.getLength(); ci++) { Node firstChild = childNodes.item(ci); @@ -425,7 +425,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera private boolean isSelectable(Node node) { Node selectableAttr = node.getAttributes().getNamedItem("selectable"); if (null != selectableAttr) { - return Boolean.valueOf(selectableAttr.getNodeValue()); + return Boolean.parseBoolean(selectableAttr.getNodeValue()); } else { // Default is true return true; } @@ -452,7 +452,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera } private Choices getChoicesByXpath(String xpathExpression, int start, int limit) { - List choices = new ArrayList(); + List choices = new ArrayList<>(); XPath xpath = XPathFactory.newInstance().newXPath(); try { Node parentNode = (Node) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODE); diff --git a/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java b/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java index 5eb56428f7..7012556ceb 100644 --- a/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java +++ b/dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java @@ -141,9 +141,10 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { } /** - * Test of getMatches method of class - * DSpaceControlledVocabulary using a localized controlled vocabulary with valid locale parameter (localized - * label returned) + * Test of getMatches method of class DSpaceControlledVocabulary + * using a localized controlled vocabulary with valid locale parameter + * (localized label returned). + * @throws java.lang.ClassNotFoundException if class under test cannot be found. */ @Test public void testGetMatchesGermanLocale() throws ClassNotFoundException { @@ -167,9 +168,10 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest { } /** - * Test of getBestMatch method of class - * DSpaceControlledVocabulary using a localized controlled vocabulary with valid locale parameter (localized - * label returned) + * Test of getBestMatch method of class DSpaceControlledVocabulary + * using a localized controlled vocabulary with valid locale parameter + * (localized label returned). + * @throws java.lang.ClassNotFoundException if class under test cannot be found. */ @Test public void testGetBestMatchIdValueGermanLocale() throws ClassNotFoundException { From dac494191c5079912cdc3db1f6af70f1cfdd7c43 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Thu, 24 Jul 2025 14:01:57 -0400 Subject: [PATCH 4/4] Correct some assumptions about what should be quoted. --- .../dspace/content/authority/DSpaceControlledVocabulary.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java index ccfb156790..b6e1bc213d 100644 --- a/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java +++ b/dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java @@ -67,7 +67,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera protected static String idTemplate = "//node[@id = %s]"; protected static String idTemplateQuoted = "//node[@id = '%s']"; protected static String labelTemplate = "//node[@label = %s]"; - protected static String idParentTemplate = "//node[@id = %s]/parent::isComposedBy/parent::node"; + protected static String idParentTemplate = "//node[@id = '%s']/parent::isComposedBy/parent::node"; protected static String rootTemplate = "/node"; protected static String idAttribute = "id"; protected static String labelAttribute = "label"; @@ -277,7 +277,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera @Override public Choices getChoicesByParent(String authorityName, String parentId, int start, int limit, String locale) { init(locale); - String xpathExpression = String.format(idTemplate, parentId); + String xpathExpression = String.format(idTemplateQuoted, parentId); return getChoicesByXpath(xpathExpression, start, limit); }