diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFile.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFile.java new file mode 100644 index 000000000..c93862128 --- /dev/null +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFile.java @@ -0,0 +1,261 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual; + +import java.io.IOException; +import java.io.Reader; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.OutputKeys; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamResult; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +/** + * Parses a configuration file for a custom list view, and makes the information + * readily accessible. + * + * If not editing the page, include clauses that will filter out statements with + * missing linked individual or other critical information missing (e.g., anchor + * and url on a link). If editing, omit those clauses so the incomplete + * statements will show, and can be edited or removed. + * + * We might want to refine this based on whether the user can edit the statement + * in question, but that would require a completely different approach: include + * the statement in the query results, and then during the postprocessing phase, + * check the editing policy, and remove the statement if it's not editable. We + * would not preprocess the query, as here. + * + * If not collating by subgroup, omit clauses that only involve subgroups. + */ +public class CustomListViewConfigFile { + private static final Log log = LogFactory + .getLog(CustomListViewConfigFile.class); + + private static final String TAG_CONSTRUCT = "query-construct"; + private static final String TAG_SELECT = "query-select"; + private static final String TAG_TEMPLATE = "template"; + private static final String TAG_POSTPROCESSOR = "postprocessor"; + private static final String TAG_COLLATED = "collated"; + private static final String TAG_CRITICAL = "critical-data-required"; + + // Will not be null. This is mutable, but don't modify it. Clone it and + // modify the clone. + private final Element selectQueryElement; + + // The set might be empty but will not be null. Each query will not be empty + // or null. + private final Set constructQueries; + + // Will not be empty or null. + private final String templateName; + + // Might be empty but will not be null. + private final String postprocessorName; + + CustomListViewConfigFile(Reader reader) throws InvalidConfigFileException { + Document doc = parseDocument(reader); + selectQueryElement = parseSelectQuery(doc); + constructQueries = parseConstructQueries(doc); + templateName = parseTemplateName(doc); + postprocessorName = parsePostprocessorName(doc); + } + + private Document parseDocument(Reader reader) + throws InvalidConfigFileException { + if (reader == null) { + throw new InvalidConfigFileException("Config file reader is null."); + } + + try { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + DocumentBuilder db = dbf.newDocumentBuilder(); + Document doc = db.parse(new InputSource(reader)); + return doc; + } catch (ParserConfigurationException e) { + throw new InvalidConfigFileException("Problem with XML parser.", e); + } catch (SAXException e) { + throw new InvalidConfigFileException( + "Config file is not valid XML: " + e.getMessage(), e); + } catch (IOException e) { + throw new InvalidConfigFileException("Unable to read config file.", + e); + } + } + + private Element parseSelectQuery(Document doc) + throws InvalidConfigFileException { + Element element = getExactlyOneElement(doc, TAG_SELECT); + elementMustNotBeEmpty(element); + return element; + } + + private Set parseConstructQueries(Document doc) { + Set queries = new HashSet(); + for (Element element : getElements(doc, TAG_CONSTRUCT)) { + String content = element.getTextContent(); + if (!content.trim().isEmpty()) { + queries.add(content); + } + } + return queries; + } + + private String parseTemplateName(Document doc) + throws InvalidConfigFileException { + Element element = getExactlyOneElement(doc, TAG_TEMPLATE); + elementMustNotBeEmpty(element); + return element.getTextContent(); + } + + private String parsePostprocessorName(Document doc) + throws InvalidConfigFileException { + Element element = getZeroOrOneElement(doc, TAG_POSTPROCESSOR); + if (element == null) { + return ""; + } else { + return element.getTextContent(); + } + } + + private List getElements(Document doc, String tagName) { + List list = new ArrayList(); + NodeList nodes = doc.getElementsByTagName(tagName); + if (nodes != null) { + for (int i = 0; i < nodes.getLength(); i++) { + list.add((Element) nodes.item(i)); + } + } + return list; + } + + private Element getExactlyOneElement(Document doc, String tagName) + throws InvalidConfigFileException { + List elements = getElements(doc, tagName); + + if (elements.size() == 1) { + return elements.get(0); + } else if (elements.isEmpty()) { + throw new InvalidConfigFileException("Config file must contain a " + + tagName + " element"); + } else { + throw new InvalidConfigFileException( + "Config file may not contain more than one " + tagName + + " element"); + } + } + + private Element getZeroOrOneElement(Document doc, String tagName) + throws InvalidConfigFileException { + List elements = getElements(doc, tagName); + if (elements.size() == 1) { + return elements.get(0); + } else if (elements.isEmpty()) { + return null; + } else { + throw new InvalidConfigFileException( + "Config file may not contain more than one " + tagName + + " element"); + } + } + + private void elementMustNotBeEmpty(Element element) + throws InvalidConfigFileException { + String contents = element.getTextContent(); + if (contents.trim().isEmpty()) { + throw new InvalidConfigFileException("In a config file, the <" + + element.getTagName() + "> element must not be empty."); + } + } + + private void removeChildElements(Element element, String tagName) { + /* + * When we remove a child from the element, it disappears from the + * NodeList. We process the NodeList in reverse order, so we won't be + * disturbed by nodes disappearing off the end. Strange. + */ + NodeList doomed = element.getElementsByTagName(tagName); + for (int i = doomed.getLength() - 1; i >= 0; i--) { + element.removeChild(doomed.item(i)); + } + } + + public String getSelectQuery(boolean collated, boolean editing) { + Element cloned = (Element) selectQueryElement.cloneNode(true); + + if (!collated) { + removeChildElements(cloned, TAG_COLLATED); + } + if (editing) { + removeChildElements(cloned, TAG_CRITICAL); + } + + return cloned.getTextContent(); + } + + public Set getConstructQueries() { + return constructQueries; + } + + public String getTemplateName() { + return templateName; + } + + public String getPostprocessorName() { + return postprocessorName; + } + + @Override + public String toString() { + return "CustomListViewConfigFile[selectQuery='" + + formatXmlNode(selectQueryElement) + "', constructQueries=" + + constructQueries + ", templateName='" + templateName + + "', postprocessorName='" + postprocessorName + "']"; + } + + private String formatXmlNode(Node node) { + try { + // Set up the output transformer + Transformer t = TransformerFactory.newInstance().newTransformer(); + t.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); + t.setOutputProperty(OutputKeys.INDENT, "yes"); + + // Print the DOM node + StringWriter sw = new StringWriter(); + StreamResult result = new StreamResult(sw); + DOMSource source = new DOMSource(node); + t.transform(source, result); + return sw.toString(); + } catch (TransformerException e) { + return "Failed to format XML node: " + node.getTextContent(); + } + } + + public static class InvalidConfigFileException extends Exception { + public InvalidConfigFileException(String message) { + super(message); + } + + public InvalidConfigFileException(String message, Throwable cause) { + super(message, cause); + } + } +} diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel.java index d8893c187..3e9918eab 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel.java @@ -3,10 +3,10 @@ package edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual; import java.io.File; +import java.io.FileReader; import java.io.IOException; import java.lang.reflect.Constructor; import java.util.ArrayList; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -14,15 +14,9 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; - import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.w3c.dom.Document; -import org.w3c.dom.Node; -import org.w3c.dom.NodeList; import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper; import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestActionConstants; @@ -341,13 +335,6 @@ public abstract class ObjectPropertyTemplateModel extends PropertyTemplateModel private static final String CONFIG_FILE_PATH = "/config/"; private static final String DEFAULT_CONFIG_FILE_NAME = "listViewConfig-default.xml"; - private static final String NODE_NAME_QUERY_CONSTRUCT = "query-construct"; - private static final String NODE_NAME_QUERY_SELECT = "query-select"; - private static final String NODE_NAME_TEMPLATE = "template"; - private static final String NODE_NAME_POSTPROCESSOR = "postprocessor"; - private static final String NODE_NAME_COLLATED = "collated"; - private static final String NODE_NAME_CRITICAL_DATA_REQUIRED = "critical-data-required"; - /* NB The default post-processor is not the same as the post-processor for the default view. The latter * actually defines its own post-processor, whereas the default post-processor is used for custom views * that don't define a post-processor, to ensure that the standard post-processing applies. @@ -442,79 +429,38 @@ public abstract class ObjectPropertyTemplateModel extends PropertyTemplateModel private void setValuesFromConfigFile(String configFilePath, ObjectProperty op, WebappDaoFactory wdf, boolean editing) { - - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - DocumentBuilder db; - - try { - db = dbf.newDocumentBuilder(); - Document doc = db.parse(configFilePath); - String propertyUri = op.getURI(); - - // Required values - selectQuery = getSelectQuery(doc, propertyUri, editing); - - templateName = getConfigValue(doc, NODE_NAME_TEMPLATE, propertyUri); - - // Optional values - constructQueries = getConfigValues(doc, NODE_NAME_QUERY_CONSTRUCT, propertyUri); - - String postprocessorName = getConfigValue(doc, NODE_NAME_POSTPROCESSOR, propertyUri); + + try { + FileReader reader = new FileReader(configFilePath); + CustomListViewConfigFile configFileContents = new CustomListViewConfigFile(reader); + boolean collated = ObjectPropertyTemplateModel.this instanceof CollatedObjectPropertyTemplateModel; - if (StringUtils.isBlank(postprocessorName)) { - log.debug("No postprocessor specified for property " + propertyUri + ". Using default postprocessor."); - postprocessorName = DEFAULT_POSTPROCESSOR; - } - try { - getPostProcessor(postprocessorName, wdf); - } catch (Exception e) { - if (! postprocessorName.equals(DEFAULT_POSTPROCESSOR)) { - log.debug("Cannot find postprocessor specified for property " + propertyUri + ". Using default postprocessor."); - postprocessorName = DEFAULT_POSTPROCESSOR; - getPostProcessor(postprocessorName, wdf); - } - } - - } catch (Exception e) { - log.error("Error processing config file " + configFilePath, e); - } - } - - private String getSelectQuery(Document doc, String propertyUri, boolean editing) { - Node selectQueryNode = doc.getElementsByTagName(NODE_NAME_QUERY_SELECT).item(0); - String value = null; - if (selectQueryNode != null) { - boolean collated = ObjectPropertyTemplateModel.this instanceof CollatedObjectPropertyTemplateModel; - /* If not editing the page (editing == false), hide statements with missing linked individual or other - * critical information missing (e.g., anchor and url on a link); otherwise, show these statements. - * We might want to refine this based on whether the user can edit the statement in question, but that - * would require a completely different approach: include the statement in the query results, and then during the - * postprocessing phase, check the editing policy, and remove the statement if it's not editable. We would not - * preprocess the query, as here. - */ - boolean criticalDataRequired = !editing; - NodeList children = selectQueryNode.getChildNodes(); - int childCount = children.getLength(); - value = ""; - for (int i = 0; i < childCount; i++) { - Node node = children.item(i); - if (node.getNodeName().equals(NODE_NAME_COLLATED)) { - if (collated) { - value += node.getChildNodes().item(0).getNodeValue(); - } // else ignore this node - } else if (node.getNodeName().equals(NODE_NAME_CRITICAL_DATA_REQUIRED)) { - if (criticalDataRequired) { - value += node.getChildNodes().item(0).getNodeValue(); - } // else ignore this node - } else { - value += node.getNodeValue(); - } - } - log.debug("Found config parameter " + NODE_NAME_QUERY_SELECT + " for object property " + propertyUri + " with value " + value); - } else { - log.error("No value found for config parameter " + NODE_NAME_QUERY_SELECT + " for object property " + propertyUri); - } - return value; + selectQuery = configFileContents.getSelectQuery(collated, editing); + templateName = configFileContents.getTemplateName(); + constructQueries = configFileContents.getConstructQueries(); + + String postprocessorName = configFileContents.getPostprocessorName(); + + if (StringUtils.isBlank(postprocessorName)) { + log.debug("No postprocessor specified for property " + + propertyUri + ". Using default postprocessor."); + postprocessorName = DEFAULT_POSTPROCESSOR; + } + try { + getPostProcessor(postprocessorName, wdf); + } catch (Exception e) { + if (!postprocessorName.equals(DEFAULT_POSTPROCESSOR)) { + log.debug("Cannot find postprocessor specified for property " + + propertyUri + + ". Using default postprocessor."); + postprocessorName = DEFAULT_POSTPROCESSOR; + getPostProcessor(postprocessorName, wdf); + } + } + + } catch (Exception e) { + log.error("Error processing config file " + configFilePath, e); + } } private void getPostProcessor(String name, WebappDaoFactory wdf) throws Exception { @@ -523,36 +469,6 @@ public abstract class ObjectPropertyTemplateModel extends PropertyTemplateModel postprocessor = (ObjectPropertyDataPostProcessor) constructor.newInstance(ObjectPropertyTemplateModel.this, wdf); } - private String getConfigValue(Document doc, String nodeName, String propertyUri) { - Node node = doc.getElementsByTagName(nodeName).item(0); - String value = null; - if (node != null) { - value = node.getChildNodes().item(0).getNodeValue(); - log.debug("Found config parameter " + nodeName + " for object property " + propertyUri + " with value " + value); - } else { - log.debug("No value found for config parameter " + nodeName + " for object property " + propertyUri); - } - return value; - } - - private Set getConfigValues(Document doc, String nodeName, String propertyUri) { - Set values = null; - NodeList nodes = doc.getElementsByTagName(nodeName); - int nodeCount = nodes.getLength(); - if (nodeCount > 0) { - values = new HashSet(nodeCount); - for (int i = 0; i < nodeCount; i++) { - Node node = nodes.item(i); - String value = node.getChildNodes().item(0).getNodeValue(); - values.add(value); - log.debug("Found config parameter " + nodeName + " for object property " + propertyUri + " with value " + value); - } - } else { - log.debug("No values found for config parameter " + nodeName + " for object property " + propertyUri); - } - return values; - } - private String getConfigFilePath(String filename) { return servletContext.getRealPath(CONFIG_FILE_PATH + filename); } diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFileTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFileTest.java new file mode 100644 index 000000000..90590ba4b --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/CustomListViewConfigFileTest.java @@ -0,0 +1,267 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; +import java.util.Arrays; +import java.util.HashSet; + +import org.hamcrest.Matcher; +import org.junit.Rule; +import org.junit.Test; +import org.junit.matchers.JUnitMatchers; +import org.junit.rules.ExpectedException; + +import edu.cornell.mannlib.vitro.testing.AbstractTestClass; +import edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual.CustomListViewConfigFile.InvalidConfigFileException; + +/** + * Note: when testing, an "empty" element may be self-closing, or with + * zero-length text content, or with only blank text content. + */ +public class CustomListViewConfigFileTest extends AbstractTestClass { + /** + * Use this XML to test the methods that strip tags from the select clause. + * + * If not collated, omit the "collated" tag. If editing, omit the + * "critical-data-required" tag. + */ + private static final String XML_WITH_RICH_SELECT_CLAUSE = "" + + "SELECT" + + " collated1" + + " collated2" + + " critical" + + "" + + "" + + ""; + + /** + * In general, we expect no exception, but individual tests may override, + * like this: + * + * 
+	 * thrown.expect(InvalidConfigurationException.class);
+	 * thrown.expectMessage("Bozo");
+	 *
+ */ + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private CustomListViewConfigFile configFile; + + // ---------------------------------------------------------------------- + // Test failures + // ---------------------------------------------------------------------- + + @Test + public void readerIsNull() throws InvalidConfigFileException { + expectException("Config file reader is null."); + configFile = new CustomListViewConfigFile(null); + } + + @Test + public void readerThrowsIOException() throws InvalidConfigFileException { + expectException("Unable to read config file."); + configFile = new CustomListViewConfigFile(new ExplodingReader()); + } + + @Test + public void invalidXml() throws InvalidConfigFileException { + suppressSyserr(); // catch the error report from the XML parser + expectException(JUnitMatchers + .containsString("Config file is not valid XML:")); + readConfigFile(""); + } + + @Test + public void selectQueryMissing() throws InvalidConfigFileException { + expectException("Config file must contain a query-select element"); + readConfigFile("" + + "" + ""); + } + + @Test + public void selectQueryMultiple() throws InvalidConfigFileException { + expectException("Config file may not contain more than one query-select element"); + readConfigFile("" + + "SELECT" + + "ANOTHER" + + "" + ""); + } + + @Test + public void selectQueryEmpty() throws InvalidConfigFileException { + expectException("In a config file, the element must not be empty."); + readConfigFile("" + "" + + "" + ""); + } + + @Test + public void templateNameMissing() throws InvalidConfigFileException { + expectException("Config file must contain a template element"); + readConfigFile("" + + "SELECT" + ""); + } + + @Test + public void templateNameMultiple() throws InvalidConfigFileException { + expectException("Config file may not contain more than one template element"); + readConfigFile("" + + "SELECT" + + "" + + "" + ""); + } + + @Test + public void templateNameEmpty() throws InvalidConfigFileException { + expectException("In a config file, the