From a734159ceea29986bf6dd450c7feb9c321242c2a Mon Sep 17 00:00:00 2001 From: Tom Page Date: Fri, 27 Mar 2015 16:43:59 +0000 Subject: [PATCH] RM-2027 Unit testing for classification reason loading. +review RM @nmcerlean git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@100355 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../ClassificationServiceImpl.java | 40 +++++----- .../ClassificationServiceImplUnitTest.java | 77 ++++++++++++------- 2 files changed, 70 insertions(+), 47 deletions(-) diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java index 0904d2787f..231cfab0aa 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java @@ -36,21 +36,17 @@ import org.slf4j.LoggerFactory; public class ClassificationServiceImpl extends ServiceBaseImpl implements ClassificationService { - private static final Logger LOGGER = LoggerFactory.getLogger(ClassificationServiceImpl.class); - private static final String[] LEVELS_KEY = new String[] { "org.alfresco", "module.org_alfresco_module_rm", "classification.levels" }; private static final String[] REASONS_KEY = new String[] { "org.alfresco", - "module.org_alfresco_module_rm", - "classification.reasons" }; + "module.org_alfresco_module_rm", + "classification.reasons" }; + private static Logger logger = LoggerFactory.getLogger(ClassificationServiceImpl.class); - - static final String DEFAULT_CONFIG_DIRECTORY = "/alfresco/module/org_alfresco_module_rm/classification/"; - static final String DEFAULT_LEVELS_FILE = DEFAULT_CONFIG_DIRECTORY - + "rm-classification-levels.json"; - static final String DEFAULT_REASONS_FILE = DEFAULT_CONFIG_DIRECTORY - + "rm-classification-reasons.json"; + static final String DEFAULT_CONFIG_DIRECTORY = "/alfresco/module/org_alfresco_module_rm/classification/"; + static final String DEFAULT_LEVELS_FILE = DEFAULT_CONFIG_DIRECTORY + "rm-classification-levels.json"; + static final String DEFAULT_REASONS_FILE = DEFAULT_CONFIG_DIRECTORY + "rm-classification-reasons.json"; private AttributeService attributeService; // TODO What about other code (e.g. REST API) accessing the AttrService? @@ -66,9 +62,16 @@ public class ClassificationServiceImpl extends ServiceBaseImpl this.config = new Configuration(DEFAULT_LEVELS_FILE, DEFAULT_REASONS_FILE); } - ClassificationServiceImpl(Configuration config) + /** + * Package protected constructor, primarily for unit testing purposes. + * + * @param config The object from which configuration options will be read. + * @param logger The class logger (note - this will be set statically). + */ + ClassificationServiceImpl(Configuration config, Logger logger) { this.config = config; + ClassificationServiceImpl.logger = logger; } public void setAttributeService(AttributeService service) { this.attributeService = service; } @@ -78,9 +81,9 @@ public class ClassificationServiceImpl extends ServiceBaseImpl final List allPersistedLevels = getPersistedLevels(); final List configurationLevels = getConfigurationLevels(); - // Note! We cannot log the level names or even the size of these lists for security reasons. - LOGGER.debug("Persisted classification levels: {}", loggableStatusOf(allPersistedLevels)); - LOGGER.debug("Classpath classification levels: {}", loggableStatusOf(configurationLevels)); + // Note! We cannot log the level names or even the size of these lists for security reasons. + logger.debug("Persisted classification levels: {}", loggableStatusOf(allPersistedLevels)); + logger.debug("Classpath classification levels: {}", loggableStatusOf(configurationLevels)); if (configurationLevels == null || configurationLevels.isEmpty()) { @@ -102,8 +105,8 @@ public class ClassificationServiceImpl extends ServiceBaseImpl final List classpathReasons = getConfigurationReasons(); // Note! We cannot log the reasons or even the size of these lists for security reasons. - LOGGER.debug("Persisted classification reasons: {}", loggableStatusOf(persistedReasons)); - LOGGER.debug("Classpath classification reasons: {}", loggableStatusOf(classpathReasons)); + logger.debug("Persisted classification reasons: {}", loggableStatusOf(persistedReasons)); + logger.debug("Classpath classification reasons: {}", loggableStatusOf(classpathReasons)); if (isEmpty(persistedReasons)) { @@ -118,7 +121,7 @@ public class ClassificationServiceImpl extends ServiceBaseImpl { if (isEmpty(classpathReasons) || !classpathReasons.equals(persistedReasons)) { - LOGGER.warn("Classification reasons configured in classpath do not match those stored in Alfresco." + + logger.warn("Classification reasons configured in classpath do not match those stored in Alfresco." + "Alfresco will use the unchanged values stored in the database."); // RM-2073 says that we should log a warning and proceed normally. } @@ -191,4 +194,5 @@ public class ClassificationServiceImpl extends ServiceBaseImpl { return configuredReasons == null ? Collections.emptyList() : Collections.unmodifiableList(configuredReasons); - }} + } +} diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java index 550a415603..90bc3e20e5 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java @@ -18,18 +18,7 @@ */ package org.alfresco.module.org_alfresco_module_rm.classification; -import org.alfresco.module.org_alfresco_module_rm.classification.ClassificationServiceException.MissingConfiguration; -import org.alfresco.module.org_alfresco_module_rm.test.util.MockAuthenticationUtilHelper; -import org.alfresco.module.org_alfresco_module_rm.util.AuthenticationUtil; -import org.alfresco.service.cmr.attributes.AttributeService; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; - -import java.io.Serializable; -import java.util.ArrayList; -import java.util.List; - +import static java.util.Arrays.asList; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -37,10 +26,19 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; -import static org.junit.Assert.fail; -import static java.util.Arrays.asList; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; + +import org.alfresco.module.org_alfresco_module_rm.classification.ClassificationServiceException.MissingConfiguration; +import org.alfresco.module.org_alfresco_module_rm.test.util.MockAuthenticationUtilHelper; +import org.alfresco.module.org_alfresco_module_rm.util.AuthenticationUtil; +import org.alfresco.service.cmr.attributes.AttributeService; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; /** * Unit tests for {@link ClassificationServiceImpl}. @@ -58,8 +56,10 @@ public class ClassificationServiceImplUnitTest "Executive Management", "EM", "Employee", "E", "Public", "P"); - private static final List PLACEHOLDER_CLASSIFICATION_REASONS = asList(new ClassificationReason("r1", "l1"), - new ClassificationReason("r2", "l2")); + private static final List PLACEHOLDER_CLASSIFICATION_REASONS = asList(new ClassificationReason("id1", "label1"), + new ClassificationReason("id2", "label2")); + private static final List ALTERNATIVE_CLASSIFICATION_REASONS = asList(new ClassificationReason("id8", "label8"), + new ClassificationReason("id9", "label9")); /** * A convenience method for turning lists of level id Strings into lists * of {@code ClassificationLevel} objects. @@ -90,13 +90,15 @@ public class ClassificationServiceImplUnitTest private AttributeService mockedAttributeService = mock(AttributeService.class); private AuthenticationUtil mockedAuthenticationUtil; private Configuration mockConfig = mock(Configuration.class); + /** Using a mock logger in the class so that we can verify some of the logging requirements. */ + private Logger mockLogger = mock(Logger.class); @Before public void setUp() { - reset(mockConfig, mockedAttributeService); + reset(mockConfig, mockedAttributeService, mockLogger); mockedAuthenticationUtil = MockAuthenticationUtilHelper.create(); - classificationService = new ClassificationServiceImpl(mockConfig); + classificationService = new ClassificationServiceImpl(mockConfig, mockLogger); classificationService.setAttributeService(mockedAttributeService); classificationService.setAuthenticationUtil(mockedAuthenticationUtil); } @@ -146,27 +148,44 @@ public class ClassificationServiceImplUnitTest anyString(), anyString(), anyString()); } - @Ignore ("This test is currently failing. Needs to be fixed.") // FIXME - @Test public void previouslyStartedSystemShouldProceedNormallyIfConfiguredReasonsHaveNotChanged() + @Test public void checkAttributesNotTouchedIfConfiguredReasonsHaveNotChanged() { - // There are existing classification reasons stored in the AttributeService. + // The classification reasons stored are the same values that are found on the classpath. when(mockedAttributeService.getAttribute(anyString(), anyString(), anyString())).thenReturn((Serializable)PLACEHOLDER_CLASSIFICATION_REASONS); - - // We'll use a small set of placeholder classification reasons. when(mockConfig.getConfiguredReasons()).thenReturn(PLACEHOLDER_CLASSIFICATION_REASONS); classificationService.initConfiguredClassificationReasons(); - // This line added to try and work out what the interaction *is*. - verifyZeroInteractions(mockedAttributeService); - verify(mockedAttributeService, never()).setAttribute(any(Serializable.class), anyString(), anyString(), anyString()); } - @Ignore ("To be implemented") // TODO + /** + * Check that if the reasons supplied on the classpath differ from those already persisted then a warning is logged + * and no change is made to the persisted reasons. + */ @Test public void previouslyStartedSystemShouldWarnIfConfiguredReasonsHaveChanged() { - fail("TODO"); + // The classification reasons stored are different from those found on the classpath. + when(mockedAttributeService.getAttribute(anyString(), anyString(), anyString())).thenReturn( + (Serializable) PLACEHOLDER_CLASSIFICATION_REASONS); + when(mockConfig.getConfiguredReasons()).thenReturn(ALTERNATIVE_CLASSIFICATION_REASONS); + + classificationService.initConfiguredClassificationReasons(); + + verify(mockLogger).warn("Classification reasons configured in classpath do not match those stored in Alfresco." + + "Alfresco will use the unchanged values stored in the database."); + verify(mockedAttributeService, never()).setAttribute(any(Serializable.class), anyString(), anyString(), + anyString()); + } + + @Test(expected=MissingConfiguration.class) + public void noReasonsFoundCausesException() + { + when(mockedAttributeService.getAttribute(anyString(), anyString(), anyString())).thenReturn( + (Serializable) null); + when(mockConfig.getConfiguredReasons()).thenReturn(null); + + classificationService.initConfiguredClassificationReasons(); } }