First updates following code review on RM-1945 & RM-1946. Thanks Roy & Tom!

Renamed a public service method to getClassificationLevels.
Removed a redundant test method.
Code tidying.
Various internal renames to help readability.
Slight javadoc improvements.
Also some trivial changes like fixing typos and copyright years etc.
Removing warnings from within AuthenticationUtil blocks allows IntelliJ to fold them to Java 8 closure format. (fistpump).  


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@100020 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
Neil McErlean
2015-03-24 21:15:22 +00:00
parent ff4fb3b7d0
commit 0cfc1a98bf
6 changed files with 33 additions and 49 deletions

View File

@@ -46,7 +46,7 @@
<property name="authenticationUtil" ref="rm.authenticationUtil"/> <property name="authenticationUtil" ref="rm.authenticationUtil"/>
</bean> </bean>
<!-- Bootstap the message property files --> <!-- Bootstrap the message property files -->
<bean id="org_alfresco_module_rm_resourceBundles" class="org.alfresco.i18n.ResourceBundleBootstrapComponent"> <bean id="org_alfresco_module_rm_resourceBundles" class="org.alfresco.i18n.ResourceBundleBootstrapComponent">
<property name="resourceBundles"> <property name="resourceBundles">
<list> <list>

View File

@@ -42,7 +42,7 @@
<property name="objectDefinitionSource"> <property name="objectDefinitionSource">
<value> <value>
<![CDATA[ <![CDATA[
org.alfresco.module.org_alfresco_module_rm.classification.ClassificationService.getApplicableLevels=RM_ALLOW org.alfresco.module.org_alfresco_module_rm.classification.ClassificationService.getClassificationLevels=RM_ALLOW
org.alfresco.module.org_alfresco_module_rm.classification.ClassificationService.*=RM_DENY org.alfresco.module.org_alfresco_module_rm.classification.ClassificationService.*=RM_DENY
]]> ]]>
</value> </value>

View File

@@ -32,7 +32,9 @@ public interface ClassificationService
{ {
/** /**
* Returns an immutable list of the defined classification levels. * Returns an immutable list of the defined classification levels.
* @return classification levels in descending order from highest to lowest. * @return classification levels in descending order from highest to lowest
* (where fewer users have access to the highest classification levels
* and therefore access to the most restricted documents).
*/ */
public List<ClassificationLevel> getApplicableLevels(); public List<ClassificationLevel> getClassificationLevels();
} }

View File

@@ -38,9 +38,9 @@ public class ClassificationServiceImpl extends ServiceBaseImpl
{ {
private static Log logger = LogFactory.getLog(ClassificationServiceImpl.class); private static Log logger = LogFactory.getLog(ClassificationServiceImpl.class);
private static final String[] ATTRIBUTE_KEYS = new String[] { "org.alfresco", private static final String[] LEVELS_KEY = new String[] { "org.alfresco",
"module.org_alfresco_module_rm", "module.org_alfresco_module_rm",
"classification.levels" }; "classification.levels" };
public static final String DEFAULT_CONFIG_LOCATION = public static final String DEFAULT_CONFIG_LOCATION =
"/alfresco/module/org_alfresco_module_rm/classification/rm-classification-levels.json"; "/alfresco/module/org_alfresco_module_rm/classification/rm-classification-levels.json";
@@ -56,24 +56,24 @@ public class ClassificationServiceImpl extends ServiceBaseImpl
void initConfiguredClassificationLevels() void initConfiguredClassificationLevels()
{ {
final List<ClassificationLevel> allPersistedLevels = getPersistedLevels(); final List<ClassificationLevel> allPersistedLevels = getPersistedLevels();
final List<ClassificationLevel> configuredLevels = getConfiguredLevels(); final List<ClassificationLevel> configurationLevels = getConfigurationLevels();
if (logger.isDebugEnabled()) if (logger.isDebugEnabled())
{ {
// Note! We cannot log the level names or even the size of these lists for security reasons. // 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("Persisted classification levels: " + loggableStatusOf(allPersistedLevels));
logger.debug("Configured classification levels: " + loggableStatusOf(configuredLevels)); logger.debug("Classpath classification levels: " + loggableStatusOf(configurationLevels));
} }
if (configuredLevels == null || configuredLevels.isEmpty()) if (configurationLevels == null || configurationLevels.isEmpty())
{ {
throw new MissingConfiguration("Classification level configuration is missing."); throw new MissingConfiguration("Classification level configuration is missing.");
} }
else if ( !configuredLevels.equals(allPersistedLevels)) else if ( !configurationLevels.equals(allPersistedLevels))
{ {
attributeService.setAttribute((Serializable) configuredLevels, ATTRIBUTE_KEYS); attributeService.setAttribute((Serializable) configurationLevels, LEVELS_KEY);
this.configuredLevels = configuredLevels; this.configuredLevels = configurationLevels;
} }
else else
{ {
@@ -96,21 +96,23 @@ public class ClassificationServiceImpl extends ServiceBaseImpl
List<ClassificationLevel> getPersistedLevels() { List<ClassificationLevel> getPersistedLevels() {
return authenticationUtil.runAsSystem(new AuthenticationUtil.RunAsWork<List<ClassificationLevel>>() return authenticationUtil.runAsSystem(new AuthenticationUtil.RunAsWork<List<ClassificationLevel>>()
{ {
@Override public List<ClassificationLevel> doWork() throws Exception @Override
@SuppressWarnings("unchecked")
public List<ClassificationLevel> doWork() throws Exception
{ {
return (List<ClassificationLevel>) attributeService.getAttribute(ATTRIBUTE_KEYS); return (List<ClassificationLevel>) attributeService.getAttribute(LEVELS_KEY);
} }
}); });
} }
/** Gets the list (in descending order) of classification levels - as defined in the system configuration. */ /** Gets the list (in descending order) of classification levels - as defined in the system configuration. */
List<ClassificationLevel> getConfiguredLevels() List<ClassificationLevel> getConfigurationLevels()
{ {
return config.getConfiguredLevels(); return config.getConfiguredLevels();
} }
@Override @Override
public List<ClassificationLevel> getApplicableLevels() public List<ClassificationLevel> getClassificationLevels()
{ {
return configuredLevels == null ? Collections.<ClassificationLevel>emptyList() : return configuredLevels == null ? Collections.<ClassificationLevel>emptyList() :
Collections.unmodifiableList(configuredLevels); Collections.unmodifiableList(configuredLevels);

View File

@@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2005-2014 Alfresco Software Limited. * Copyright (C) 2005-2015 Alfresco Software Limited.
* *
* This file is part of Alfresco * This file is part of Alfresco
* *
@@ -57,7 +57,7 @@ class Configuration
List<ClassificationLevel> result; List<ClassificationLevel> result;
try (final InputStream in = this.getClass().getResourceAsStream(configLocation)) try (final InputStream in = this.getClass().getResourceAsStream(configLocation))
{ {
if ( in == null) { result = Collections.emptyList(); } if (in == null) { result = Collections.emptyList(); }
else else
{ {
final String jsonString = IOUtils.toString(in); final String jsonString = IOUtils.toString(in);
@@ -65,7 +65,8 @@ class Configuration
result = new ArrayList<>(jsonArray.length()); result = new ArrayList<>(jsonArray.length());
for (int i = 0; i < jsonArray.length(); i++) { for (int i = 0; i < jsonArray.length(); i++)
{
final JSONObject nextObj = jsonArray.getJSONObject(i); final JSONObject nextObj = jsonArray.getJSONObject(i);
final String name = nextObj.getString("name"); final String name = nextObj.getString("name");
final String displayLabelKey = nextObj.getString("displayLabel"); final String displayLabelKey = nextObj.getString("displayLabel");

View File

@@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2005-2014 Alfresco Software Limited. * Copyright (C) 2005-2015 Alfresco Software Limited.
* *
* This file is part of Alfresco * This file is part of Alfresco
* *
@@ -31,7 +31,6 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
/** /**
* Unit tests for {@link ClassificationServiceImpl}. * Unit tests for {@link ClassificationServiceImpl}.
* *
@@ -63,21 +62,11 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest
ClassificationLevel.class.getSimpleName(), idsAndLabels.length)); ClassificationLevel.class.getSimpleName(), idsAndLabels.length));
} }
final int resultLength = idsAndLabels.length / 2; final List<ClassificationLevel> levels = new ArrayList<>(idsAndLabels.length / 2);
final String[] ids = new String[resultLength];
final String[] labels = new String[resultLength];
for (int i = 0; i < idsAndLabels.length; i++) for (int i = 0; i < idsAndLabels.length; i += 2)
{ {
if (i % 2 == 0) { ids[i / 2] = idsAndLabels[i]; } levels.add(new ClassificationLevel(idsAndLabels[i], idsAndLabels[i+1]));
else { labels[(i - 1) / 2] = idsAndLabels[i]; }
}
final List<ClassificationLevel> levels = new ArrayList<>(resultLength);
for (int i = 0; i < resultLength; i++)
{
levels.add(new ClassificationLevel(ids[i], labels[i]));
} }
return levels; return levels;
@@ -94,17 +83,7 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest
classificationService.initConfiguredClassificationLevels(); classificationService.initConfiguredClassificationLevels();
assertEquals(DEFAULT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels()); assertEquals(DEFAULT_CLASSIFICATION_LEVELS, classificationService.getClassificationLevels());
}
@Test public void alternativeConfigurationVanillaSystem()
{
classificationService = new TestClassificationService(null, ALT_CLASSIFICATION_LEVELS);
classificationService.setAttributeService(mockedAttributeService);
classificationService.initConfiguredClassificationLevels();
assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels());
} }
@Test public void alternativeConfigurationPreviouslyStartedSystem() @Test public void alternativeConfigurationPreviouslyStartedSystem()
@@ -114,7 +93,7 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest
classificationService.initConfiguredClassificationLevels(); classificationService.initConfiguredClassificationLevels();
assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels()); assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getClassificationLevels());
} }
@Test (expected=MissingConfiguration.class) @Test (expected=MissingConfiguration.class)
@@ -141,6 +120,6 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest
} }
@Override List<ClassificationLevel> getPersistedLevels() { return persisted; } @Override List<ClassificationLevel> getPersistedLevels() { return persisted; }
@Override List<ClassificationLevel> getConfiguredLevels() { return configured; } @Override List<ClassificationLevel> getConfigurationLevels() { return configured; }
} }
} }