Fix for ALF-20023 Recent Sites and Favourite Sites in copy/move pickers empty

So the bug was caused by 2 problems: one in ScriptPreferenceService and one in person.sites.get.js
The first problem was that ScriptPreferenceService constructs raw Mozilla NativeObjects - a very unusual practice in itself - and it does not provide a default value as required by ECMA 9.1 from the ECMA standard. I've added this code to the NativeObject (can't change the type to ScriptableHashMap<K, V> or similar as the API is published).
I've also fixed the same bug (unreported, possibly never apparent) in a jbpm class.
The second problem was that person.sites.get.js simply could never return preferences data if the caller provided a filter (favourites or recents) and did not also provide a page size.
There was a logic error in the algorithm such that the size defaulted to 0 thus providing a 'page' of zero results as a default.
I assume this is a merge error from cloud, but I don't know.
I added tests for the JavaScript API's favourites/recents filter calls and logging here and there.
Also deleted some dead code.



git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55474 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

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,6 +60,8 @@ import org.json.JSONObject;
  */
 public class PreferenceServiceImpl implements PreferenceService
 {
+    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<String, Serializable> getPreferences(String userName, String preferenceFilter)
     {
+        if (log.isTraceEnabled()) { log.trace("getPreferences(" + userName + ", " + preferenceFilter + ")"); }
+        
         Map<String, Serializable> preferences = new TreeMap<String, Serializable>();
         
         try
         {
-        	Set<String> siteIds = new HashSet<String>();
-
 	        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;
     }
     

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<String, Serializable> prefs = this.preferenceService.getPreferences(userName, preferenceFilter);        
-        NativeObject result = new NativeObject();
+        NativeObject result = new NativeObjectDV();
         
         for (Map.Entry<String, Serializable> 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

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();
+    }
 }

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<String, Object> model = new HashMap<String, Object>();
-                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);

source/test-resources/org/alfresco/repo/preference/script/test_preferenceService.js

@@ -6,9 +6,9 @@ function testPreferences()
     preferences.comp1.value1 = "value1";
     preferences.comp1.value2 = 12;
     
-	preferenceService.setPreferences(username, preferences);
+    preferenceService.setPreferences(username1, preferences);
     
-	var result = preferenceService.getPreferences(username);
+    var result = preferenceService.getPreferences(username1);
     
     test.assertNotNull(result);
     test.assertEquals("myValue", result.myValue);
@@ -23,9 +23,9 @@ function testPreferences()
     preferences.comp1.value1 = "changed";
     preferences.comp1.value2 = 1001;
     
-	preferenceService.setPreferences(username, preferences);
+    preferenceService.setPreferences(username1, preferences);
     
-	result = preferenceService.getPreferences(username);	
+    result = preferenceService.getPreferences(username1);   
     test.assertNotNull(result);
     test.assertEquals("myValue", result.myValue);
     test.assertEquals("changed", result.comp1.value1);
@@ -33,24 +33,77 @@ function testPreferences()
     test.assertEquals("value1", result.comp2.value1);
     test.assertEquals(3.142, result.comp2.value2);
     
-	preferenceService.clearPreferences(username, "comp1");
+    preferenceService.clearPreferences(username1, "comp1");
     
-	result = preferenceService.getPreferences(username);	
+    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(username);
+    preferenceService.clearPreferences(username1);
     
-	result = preferenceService.getPreferences(username);	
+    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
+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();