diff --git a/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java b/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java index 75cf994869..6a7f412e32 100644 --- a/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java +++ b/source/java/org/alfresco/repo/preference/PreferenceServiceImpl.java @@ -21,11 +21,9 @@ package org.alfresco.repo.preference; import java.io.Serializable; import java.util.ArrayList; import java.util.Date; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; import org.alfresco.error.AlfrescoRuntimeException; @@ -50,6 +48,8 @@ import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.cmr.security.PersonService; import org.alfresco.util.ISO8601DateFormat; import org.alfresco.util.Pair; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.json.JSONException; import org.json.JSONObject; @@ -60,7 +60,9 @@ import org.json.JSONObject; */ public class PreferenceServiceImpl implements PreferenceService { - private static final String SHARE_SITES_PREFERENCE_KEY = "org.alfresco.share.sites.favourites."; + private static final Log log = LogFactory.getLog(PreferenceServiceImpl.class); + + private static final String SHARE_SITES_PREFERENCE_KEY = "org.alfresco.share.sites.favourites."; private static final int SHARE_SITES_PREFERENCE_KEY_LEN = SHARE_SITES_PREFERENCE_KEY.length(); private static final String EXT_SITES_PREFERENCE_KEY = "org.alfresco.ext.sites.favourites."; @@ -192,12 +194,12 @@ public class PreferenceServiceImpl implements PreferenceService @SuppressWarnings({ "unchecked" }) public Map getPreferences(String userName, String preferenceFilter) { + if (log.isTraceEnabled()) { log.trace("getPreferences(" + userName + ", " + preferenceFilter + ")"); } + Map preferences = new TreeMap(); try { - Set siteIds = new HashSet(); - JSONObject jsonPrefs = getPreferencesObject(userName); if(jsonPrefs != null) { @@ -220,7 +222,6 @@ public class PreferenceServiceImpl implements PreferenceService String siteId = key.substring(SHARE_SITES_PREFERENCE_KEY_LEN, idx); StringBuilder sb = new StringBuilder(SHARE_SITES_PREFERENCE_KEY); sb.append(siteId); - siteIds.add(siteId); key = sb.toString(); } @@ -231,7 +232,6 @@ public class PreferenceServiceImpl implements PreferenceService StringBuilder sb = new StringBuilder(EXT_SITES_PREFERENCE_KEY); sb.append(siteId); sb.append(".createdAt"); - siteIds.add(siteId); key = sb.toString(); } else if(preferences.containsKey(key)) @@ -254,9 +254,11 @@ public class PreferenceServiceImpl implements PreferenceService } catch (JSONException exception) { - throw new AlfrescoRuntimeException("Can not get preferences for " + userName + " because there was an error pasing the JSON data.", exception); + throw new AlfrescoRuntimeException("Can not get preferences for " + userName + " because there was an error parsing the JSON data.", exception); } + if (log.isTraceEnabled()) { log.trace("result = " + preferences); } + return preferences; } diff --git a/source/java/org/alfresco/repo/preference/script/ScriptPreferenceService.java b/source/java/org/alfresco/repo/preference/script/ScriptPreferenceService.java index 77dca21728..f3f3a3fd32 100644 --- a/source/java/org/alfresco/repo/preference/script/ScriptPreferenceService.java +++ b/source/java/org/alfresco/repo/preference/script/ScriptPreferenceService.java @@ -68,8 +68,10 @@ public class ScriptPreferenceService extends BaseScopableProcessorExtension public NativeObject getPreferences(String userName, String preferenceFilter) { + // It's a tad unusual to return a NativeObject like this - at least within Alfresco. + // But we can't change it to e.g. a ScriptableHashMap as the API is published. Map prefs = this.preferenceService.getPreferences(userName, preferenceFilter); - NativeObject result = new NativeObject(); + NativeObject result = new NativeObjectDV(); for (Map.Entry entry : prefs.entrySet()) { @@ -80,6 +82,16 @@ public class ScriptPreferenceService extends BaseScopableProcessorExtension return result; } + /** + * This extension of NativeObject adds a default value. See ALF-20023 for some background. + */ + private static class NativeObjectDV extends NativeObject + { + private static final long serialVersionUID = 1L; + + @Override public Object getDefaultValue(@SuppressWarnings("rawtypes") Class typeHint) { return toString(); } + } + private void setPrefValue(String[] keys, Serializable value, NativeObject object) { NativeObject currentObject = object; @@ -96,7 +108,7 @@ public class ScriptPreferenceService extends BaseScopableProcessorExtension Object temp = currentObject.get(key, currentObject); if (temp == null || temp instanceof NativeObject == false) { - newObject = new NativeObject(); + newObject = new NativeObjectDV(); currentObject.put(key, currentObject, newObject); } else diff --git a/source/java/org/alfresco/repo/workflow/jbpm/NullScope.java b/source/java/org/alfresco/repo/workflow/jbpm/NullScope.java index b6aa12a107..59117d003a 100644 --- a/source/java/org/alfresco/repo/workflow/jbpm/NullScope.java +++ b/source/java/org/alfresco/repo/workflow/jbpm/NullScope.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2010 Alfresco Software Limited. + * Copyright (C) 2005-2013 Alfresco Software Limited. * * This file is part of Alfresco * @@ -37,4 +37,8 @@ public class NullScope extends NativeObject return INSTANCE; } + @Override public Object getDefaultValue(@SuppressWarnings("rawtypes") Class hint) + { + return INSTANCE.toString(); + } } diff --git a/source/test-java/org/alfresco/repo/preference/PreferenceServiceImplTest.java b/source/test-java/org/alfresco/repo/preference/PreferenceServiceImplTest.java index b04129aac0..11928edabc 100644 --- a/source/test-java/org/alfresco/repo/preference/PreferenceServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/preference/PreferenceServiceImplTest.java @@ -69,7 +69,7 @@ public class PreferenceServiceImplTest // JUnit rule to initialise the default Alfresco spring configuration @ClassRule public static ApplicationContextInit APP_CONTEXT_INIT = new ApplicationContextInit(); - private static final String USERNAME2 = "userBad"; + private static final String USERNAME2 = "username2"; // Rules to create test users. Note that this class is unusual in that we do *NOT* want to reuse users across test methods. public AlfrescoPerson testUser1 = new AlfrescoPerson(APP_CONTEXT_INIT); @@ -219,7 +219,8 @@ public class PreferenceServiceImplTest { // This test is running as user1 and the JavaScript needs to know that. Map model = new HashMap(); - model.put("username", testUser1.getUsername()); + model.put("username1", testUser1.getUsername()); + model.put("username2", testUser2.getUsername()); ScriptLocation location = new ClasspathScriptLocation("org/alfresco/repo/preference/script/test_preferenceService.js"); SCRIPT_SERVICE.executeScript(location, model); diff --git a/source/test-resources/org/alfresco/repo/preference/script/test_preferenceService.js b/source/test-resources/org/alfresco/repo/preference/script/test_preferenceService.js index 55abae91fb..8504b72bc0 100644 --- a/source/test-resources/org/alfresco/repo/preference/script/test_preferenceService.js +++ b/source/test-resources/org/alfresco/repo/preference/script/test_preferenceService.js @@ -1,56 +1,109 @@ function testPreferences() { - var preferences = new Object(); - preferences.myValue = "myValue"; - preferences.comp1 = new Object(); - preferences.comp1.value1 = "value1"; - preferences.comp1.value2 = 12; - - preferenceService.setPreferences(username, preferences); - - var result = preferenceService.getPreferences(username); - - test.assertNotNull(result); - test.assertEquals("myValue", result.myValue); - test.assertEquals("value1", result.comp1.value1); - test.assertEquals(12, result.comp1.value2); - - preferences = new Object(); - preferences.comp2 = new Object(); - preferences.comp2.value1 = "value1"; - preferences.comp2.value2 = 3.142; - preferences.comp1 = new Object(); - preferences.comp1.value1 = "changed"; - preferences.comp1.value2 = 1001; - - preferenceService.setPreferences(username, preferences); - - result = preferenceService.getPreferences(username); - test.assertNotNull(result); - test.assertEquals("myValue", result.myValue); - test.assertEquals("changed", result.comp1.value1); - test.assertEquals(1001, result.comp1.value2); - test.assertEquals("value1", result.comp2.value1); - test.assertEquals(3.142, result.comp2.value2); - - preferenceService.clearPreferences(username, "comp1"); - - result = preferenceService.getPreferences(username); - test.assertNotNull(result); - test.assertEquals("myValue", result.myValue); - test.assertEquals("undefined", result.comp1); - test.assertEquals("value1", result.comp2.value1); - test.assertEquals(3.142, result.comp2.value2); - - preferenceService.clearPreferences(username); - - result = preferenceService.getPreferences(username); - test.assertNotNull(result); - test.assertEquals("undefined", result.myValue); - test.assertEquals("undefined", result.comp1); - test.assertEquals("undefined", result.comp2); - + var preferences = new Object(); + preferences.myValue = "myValue"; + preferences.comp1 = new Object(); + preferences.comp1.value1 = "value1"; + preferences.comp1.value2 = 12; + + preferenceService.setPreferences(username1, preferences); + + var result = preferenceService.getPreferences(username1); + + test.assertNotNull(result); + test.assertEquals("myValue", result.myValue); + test.assertEquals("value1", result.comp1.value1); + test.assertEquals(12, result.comp1.value2); + + preferences = new Object(); + preferences.comp2 = new Object(); + preferences.comp2.value1 = "value1"; + preferences.comp2.value2 = 3.142; + preferences.comp1 = new Object(); + preferences.comp1.value1 = "changed"; + preferences.comp1.value2 = 1001; + + preferenceService.setPreferences(username1, preferences); + + result = preferenceService.getPreferences(username1); + test.assertNotNull(result); + test.assertEquals("myValue", result.myValue); + test.assertEquals("changed", result.comp1.value1); + test.assertEquals(1001, result.comp1.value2); + test.assertEquals("value1", result.comp2.value1); + test.assertEquals(3.142, result.comp2.value2); + + preferenceService.clearPreferences(username1, "comp1"); + + result = preferenceService.getPreferences(username1); + test.assertNotNull(result); + test.assertEquals("myValue", result.myValue); + test.assertEquals("undefined", result.comp1); + test.assertEquals("value1", result.comp2.value1); + test.assertEquals(3.142, result.comp2.value2); + + preferenceService.clearPreferences(username1); + + result = preferenceService.getPreferences(username1); + test.assertNotNull(result); + test.assertEquals("undefined", result.myValue); + test.assertEquals("undefined", result.comp1); + test.assertEquals("undefined", result.comp2); } -// Execute test's -testPreferences(); \ No newline at end of file +function testGetPreferencesWithFilters_Alf20023() +{ + // Intentionally using a string rather than Date instance below as debugging shows that that is what the Java service receives. + var preferences = + { + "org.alfresco.share.sites.favourites.one" : true, + "org.alfresco.ext.sites.favourites.one.createdAt" : "2013-09-16T08:45:27.246Z" + }; + + preferenceService.setPreferences(username1, preferences); + + var result = preferenceService.getPreferences(username1); + test.assertNotNull(result, 'get preferences returned null'); + // Note that the PreferenceService will restructure some Alfresco-specific JSON keys into deep object structures. + // See CLOUD-1518 for some details on why this is. + // + // 1. Test the restructured data is there + test.assertEquals(true, result.org.alfresco.share.sites.favourites.one); + test.assertEquals("2013-09-16T08:45:27.246Z", result.org.alfresco.ext.sites.favourites.one.createdAt); + // 2. And now test that the API correctly returns data whose key triggered a restructure. + // So get by key, specifying a 'flat' key, which still returns a deep JSON object. + var favouriteSites = preferenceService.getPreferences(username1, "org.alfresco.share.sites.favourites"); + test.assertEquals(true, favouriteSites.org.alfresco.share.sites.favourites.one); + + preferences = + { + "org.alfresco.share.sites.recent._0" : "one" + }; + + preferenceService.setPreferences(username1, preferences); + result = preferenceService.getPreferences(username1); + test.assertNotNull(result, 'get preferences returned null'); + test.assertEquals("one", result.org.alfresco.share.sites.recent._0); + + // Simply getting these values is enough to ensure ALF-20023 is not regressed. + var favs = preferenceService.getPreferences(username1, 'org.alfresco.share.sites.favourites'); + var recents = preferenceService.getPreferences(username1, 'org.alfresco.share.sites.recent'); +} + +function testGettingAnotherUsersPreferencesShouldRaiseAnException() +{ + try + { + preferenceService.getPreferences(username2); + } + catch (e) + { + return; + } + test.fail("Expected exception not thrown: should not be able to access other users' preferences."); +} + +// Execute tests +testPreferences(); +testGettingAnotherUsersPreferencesShouldRaiseAnException(); +testGetPreferencesWithFilters_Alf20023(); \ No newline at end of file