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..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 @@ -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: + *

* } * * @author Michael B. Klein @@ -59,11 +61,12 @@ 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']"; + "'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"; @@ -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", "")); } @@ -178,15 +181,23 @@ 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(); - int total = 0; - List choices = new ArrayList(); + 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; + List choices; try { NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); total = results.getLength(); @@ -202,15 +213,23 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera @Override public Choices getBestMatch(String text, String locale) { init(locale); - log.debug("Getting best matches for '" + text + "'"); - String xpathExpression = ""; + 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++) { - xpathExpression += - String.format(valueTemplate, textHierarchy[i].replaceAll("'", "'")); + xpathExpressionBuilder.append(String.format(valueTemplate, "$var" + i)); } + String xpathExpression = xpathExpressionBuilder.toString(); XPath xpath = XPathFactory.newInstance().newXPath(); - List choices = new ArrayList(); + 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; try { NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); choices = getChoicesFromNodeList(results, 0, 1); @@ -258,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); } @@ -282,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; } @@ -302,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; @@ -321,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; @@ -386,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); @@ -409,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; } @@ -436,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 43bd20cc15..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 @@ -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,14 +135,16 @@ 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); } /** - * 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 { @@ -157,14 +162,16 @@ 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); } /** - * 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 { @@ -179,6 +186,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); }