From 1f96d551f5f6d5890c66d8d3e7e396826bc13af7 Mon Sep 17 00:00:00 2001 From: j2blake Date: Wed, 22 Feb 2012 20:29:50 +0000 Subject: [PATCH] NIHVIVO-3628 Clean up the logic that instantiates the postprocessor. Log a more specific error message if there is a problem. --- .../ObjectPropertyTemplateModel.java | 67 ++++++++++++------- ...yTemplateModel_PropertyListConfigTest.java | 34 +++++----- 2 files changed, 56 insertions(+), 45 deletions(-) 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 3e9918eab..1c25ca2b3 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 @@ -338,9 +338,9 @@ public abstract class ObjectPropertyTemplateModel extends PropertyTemplateModel /* 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. + * + * edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual.DefaultObjectPropertyDataPostProcessor */ - private static final String DEFAULT_POSTPROCESSOR = - "edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual.DefaultObjectPropertyDataPostProcessor"; private boolean isDefaultConfig; private Set constructQueries; @@ -440,35 +440,50 @@ public abstract class ObjectPropertyTemplateModel extends PropertyTemplateModel 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); - } - } - + postprocessor = getPostProcessor(postprocessorName, ObjectPropertyTemplateModel.this, wdf, configFilePath); } catch (Exception e) { log.error("Error processing config file " + configFilePath, e); } } - private void getPostProcessor(String name, WebappDaoFactory wdf) throws Exception { - Class postprocessorClass = Class.forName(name); - Constructor constructor = postprocessorClass.getConstructor(ObjectPropertyTemplateModel.class, WebappDaoFactory.class); - postprocessor = (ObjectPropertyDataPostProcessor) constructor.newInstance(ObjectPropertyTemplateModel.this, wdf); - } - + private ObjectPropertyDataPostProcessor getPostProcessor( + String className, + ObjectPropertyTemplateModel optm, + WebappDaoFactory wdf, String configFilePath) { + try { + if (StringUtils.isBlank(className)) { + return new DefaultObjectPropertyDataPostProcessor(optm, wdf); + } + + Class clazz = Class.forName(className); + Constructor constructor = clazz.getConstructor(ObjectPropertyTemplateModel.class, WebappDaoFactory.class); + return (ObjectPropertyDataPostProcessor) constructor.newInstance(optm, wdf); + } catch (ClassNotFoundException e) { + log.error("Error processing config file '" + configFilePath + + "': can't load postprocessor class '" + className + + "'. " + "Using default postprocessor.", e); + return new DefaultObjectPropertyDataPostProcessor(optm, wdf); + } catch (NoSuchMethodException e) { + log.error("Error processing config file '" + configFilePath + + "': postprocessor class '" + className + + "' does not have a constructor that takes " + + "ObjectPropertyTemplateModel and WebappDaoFactory. " + + "Using default postprocessor.", e); + return new DefaultObjectPropertyDataPostProcessor(optm, wdf); + } catch (ClassCastException e) { + log.error("Error processing config file '" + configFilePath + + "': postprocessor class '" + className + "' does " + + "not implement ObjectPropertyDataPostProcessor. " + + "Using default postprocessor.", e); + return new DefaultObjectPropertyDataPostProcessor(optm, wdf); + } catch (Exception e) { + log.error("Error processing config file '" + configFilePath + + "': can't create postprocessor instance of class '" + + className + "'. " + "Using default postprocessor.", e); + return new DefaultObjectPropertyDataPostProcessor(optm, wdf); + } + } + 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/ObjectPropertyTemplateModel_PropertyListConfigTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel_PropertyListConfigTest.java index 541b61758..01b4c8c54 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel_PropertyListConfigTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/ObjectPropertyTemplateModel_PropertyListConfigTest.java @@ -281,8 +281,6 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends @Test public void templateNodeIsEmpty() throws InvalidConfigurationException { - // TODO fix this so it doesn't throw a NullPointerException - use - // textValue() or something captureLogsFromOPTM(); op = buildOperation("templateNodeIsEmpty"); @@ -403,13 +401,9 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends @Test public void postProcessorNameEmpty() throws InvalidConfigurationException { - captureLogsFromOPTM(); - op = buildOperation("postProcessorNameEmpty"); optm = new NonCollatingOPTM(op, subject, vreq, false); - // TODO This should not cause an error. If it did, it should not swallow - // the exception. It should use the default PP. assertPostProcessorClass("pp name empty", DefaultObjectPropertyDataPostProcessor.class); } @@ -418,14 +412,12 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends public void postProcessorClassNotFound() throws InvalidConfigurationException { captureLogsFromOPTM(); - setLoggerLevel(ObjectPropertyTemplateModel.class, Level.DEBUG); op = buildOperation("postProcessorClassNotFound"); optm = new NonCollatingOPTM(op, subject, vreq, false); - // TODO this should log an error. assertLogMessagesContains("pp class not found", - "Cannot find postprocessor specified"); + "java.lang.ClassNotFoundException"); assertPostProcessorClass("pp class not found", DefaultObjectPropertyDataPostProcessor.class); } @@ -434,14 +426,12 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends public void postProcessorClassIsNotSuitable() throws InvalidConfigurationException { captureLogsFromOPTM(); - setLoggerLevel(ObjectPropertyTemplateModel.class, Level.DEBUG); op = buildOperation("postProcessorClassNotSuitable"); optm = new NonCollatingOPTM(op, subject, vreq, false); - // TODO this should log an error. assertLogMessagesContains("pp doesn't implement required interface", - "Cannot find postprocessor specified"); + "java.lang.ClassCastException"); assertPostProcessorClass("pp doesn't implement required interface", DefaultObjectPropertyDataPostProcessor.class); } @@ -450,14 +440,12 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends public void postProcessorClassHasWrongConstructor() throws InvalidConfigurationException { captureLogsFromOPTM(); - setLoggerLevel(ObjectPropertyTemplateModel.class, Level.DEBUG); op = buildOperation("postProcessorWrongConstructor"); optm = new NonCollatingOPTM(op, subject, vreq, false); - // TODO this should log an error. assertLogMessagesContains("pp has wrong constructor", - "Cannot find postprocessor specified"); + "java.lang.NoSuchMethodException"); assertPostProcessorClass("pp has wrong constructor", DefaultObjectPropertyDataPostProcessor.class); } @@ -466,14 +454,12 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends public void postProcessorConstructorThrowsAnException() throws InvalidConfigurationException { captureLogsFromOPTM(); - setLoggerLevel(ObjectPropertyTemplateModel.class, Level.DEBUG); op = buildOperation("postProcessorConstructorThrowsException"); optm = new NonCollatingOPTM(op, subject, vreq, false); - // TODO this should log an error. assertLogMessagesContains("pp throws an exception", - "Cannot find postprocessor specified"); + "java.lang.reflect.InvocationTargetException"); assertPostProcessorClass("pp throws an exception", DefaultObjectPropertyDataPostProcessor.class); } @@ -636,8 +622,18 @@ public class ObjectPropertyTemplateModel_PropertyListConfigTest extends } + /** Does not implement the required interface. */ + public static class ClassNotSuitable { + @SuppressWarnings("unused") + public ClassNotSuitable(ObjectPropertyTemplateModel optm, + WebappDaoFactory wadf) { + // Do nothing. + } + + } + /** Does not have a constructor with the correct arguments */ - public static class PostProcessorHasWrongConstructor implements + public static class PostProcessorWrongConstructor implements ObjectPropertyDataPostProcessor { @Override