[MNT-21901] Prevent update preferences concurrency errors (#821)

* [MNT-21901] Added PersistenceException to retry exceptions. Added validation to get retry cause

* [MNT-21901] Check exception message only if it is JavaScriptException

* [MNT-21901] Removed unit test from repository. Added new test on remote-api. Changed ExceptionStackUtil validation.

* [MNT-21901] Improved JavascriptException validation. Added exception delimiter to prevent accepting exceptions with the same partial name.

core/src/main/java/org/alfresco/error/ExceptionStackUtil.java

@@ -25,6 +25,9 @@ package org.alfresco.error;
  */
 public class ExceptionStackUtil
 {
+    private static final String JAVASCRIPT_EXCEPTION = "org.mozilla.javascript.JavaScriptException";
+    private static final String EXCEPTION_DELIMITER = ":";
+
     /**
      * Searches through the exception stack of the given throwable to find any instance
      * of the possible cause.  The top-level throwable will also be tested.
@@ -38,10 +41,17 @@ public class ExceptionStackUtil
     {
         while (throwable != null)
         {
+            Class<?> throwableClass = throwable.getClass();
+
+            boolean isJavaScriptException = throwableClass.getName().contains(JAVASCRIPT_EXCEPTION);
+            String throwableMsg = throwable.getMessage() != null ? throwable.getMessage() : "";
+
             for (Class<?> possibleCauseClass : possibleCauses)
             {
-                Class<?> throwableClass = throwable.getClass();
+                String possibleCauseClassName = possibleCauseClass.getName();
-                if (possibleCauseClass.isAssignableFrom(throwableClass))
+
+                if (possibleCauseClass.isAssignableFrom(throwableClass)
+                        || (isJavaScriptException && throwableMsg.contains(possibleCauseClassName + EXCEPTION_DELIMITER)))
                 {
                     // We have a match
                     return throwable;

remote-api/src/test/java/org/alfresco/repo/web/scripts/preference/PreferenceServiceTest.java

@@ -55,10 +55,14 @@ public class PreferenceServiceTest extends BaseWebScriptTest
     private PersonService personService;
 
     private static final String USER_ONE = "PreferenceTestOne" + System.currentTimeMillis();
-
+    private static final String USER_TWO = "PreferenceTestTwo" + System.currentTimeMillis();
     private static final String USER_BAD = "PreferenceTestBad" + System.currentTimeMillis();
 
-    private static final String URL = "/api/people/" + USER_ONE + "/preferences";;
+    private static final String URL = "/api/people/" + USER_ONE + "/preferences";
+    private static final String URL2 = "/api/people/" + USER_TWO + "/preferences";
+
+    private static final int NUM_THREADS = 3;
+    private static Integer THREADS_FINISHED = 0;
 
     @Override
     protected void setUp() throws Exception
@@ -75,6 +79,7 @@ public class PreferenceServiceTest extends BaseWebScriptTest
 
         // Create users
         createUser(USER_ONE);
+        createUser(USER_TWO);
         createUser(USER_BAD);
 
         // Do tests as user one
@@ -103,7 +108,6 @@ public class PreferenceServiceTest extends BaseWebScriptTest
     {
         super.tearDown();
         this.authenticationComponent.setCurrentUser(AuthenticationUtil.getAdminUserName());
-
     }
 
     public void testPreferences() throws Exception
@@ -193,6 +197,64 @@ public class PreferenceServiceTest extends BaseWebScriptTest
         sendRequest(new GetRequest("/api/people/noExistUser/preferences"), 404);
     }
 
+    public void testPreferencesMNT21901() throws Exception
+    {
+        String[] body = {
+                "{\"org\":{\"alfresco\":{\"share\":{\"forum\":{\"summary\":{\"dashlet\":{\"component-1-5\":{\"topics\":\"mine\"}}}}}}}}",
+                "{\"org\":{\"alfresco\":{\"share\":{\"forum\":{\"summary\":{\"dashlet\":{\"component-2-5\":{\"topics\":\"mine\"}}}}}}}}",
+                "{\"org\":{\"alfresco\":{\"share\":{\"forum\":{\"summary\":{\"dashlet\":{\"component-2-5\":{\"history\":\"1\"}}}}}}}}" 
+        };
+
+        Thread[] threads = new Thread[NUM_THREADS];
+
+        for (int i = 0; i < NUM_THREADS; i++)
+        {
+            UpdatePreferencesThread t = new UpdatePreferencesThread(i, body[i]);
+            threads[i] = t;
+            t.start();
+        }
+
+        for (int i = 0; i < threads.length; i++)
+        {
+            threads[i].join();
+        }
+
+        authenticationComponent.setCurrentUser(USER_ONE);
+
+        if (THREADS_FINISHED != NUM_THREADS)
+        {
+            fail("An error has occurred when updating preferences in concurrency context");
+        }
+    }
+
+    private class UpdatePreferencesThread extends Thread
+    {
+        private String body;
+
+        UpdatePreferencesThread(int threadNum, String body)
+        {
+            super(UpdatePreferencesThread.class.getSimpleName() + "-" + threadNum);
+            this.body = body;
+        }
+
+        @Override
+        public void run()
+        {
+            authenticationComponent.setCurrentUser(USER_TWO);
+
+            try
+            {
+                Response resp = sendRequest(new PostRequest(URL2, body, "application/json"), 200);
+                assertEquals(0, resp.getContentLength());
+                THREADS_FINISHED++;
+            }
+            catch (Exception e)
+            {
+                // Intentionally empty
+            }
+        }
+    }
+
     private JSONObject getPreferenceObj() throws JSONException
     {
         JSONObject jsonObject = new JSONObject();
@@ -208,5 +270,4 @@ public class PreferenceServiceTest extends BaseWebScriptTest
         assertEquals(10, jsonObject.get("numberValue"));
         assertEquals(BigDecimal.valueOf(3.142), jsonObject.get("numberValue2"));
     }
-
 }

repository/src/main/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java

@@ -51,6 +51,7 @@ import org.alfresco.service.license.LicenseIntegrityException;
 import org.alfresco.util.LockHelper.LockTryException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.ibatis.exceptions.PersistenceException;
 import org.apache.ibatis.exceptions.TooManyResultsException;
 import org.springframework.aop.MethodBeforeAdvice;
 import org.springframework.aop.framework.ProxyFactory;
@@ -105,7 +106,8 @@ public class RetryingTransactionHelper
                     DataIntegrityViolationException.class,
                     LicenseIntegrityException.class,
                     TooManyResultsException.class,              // Expected one result but found multiple (bad key alert)
-                    LockTryException.class
+                    LockTryException.class,
+                    PersistenceException.class
                     };
      
         List<Class<?>> retryExceptions = new ArrayList<Class<?>>();

repository/src/test/resources/org/alfresco/repo/preference/script/test_preferenceService2.js

@@ -0,0 +1,11 @@
+function testPreferences()
+{
+    var preferences = {
+		"org.alfresco.share.forum.summary.dashlet.component-1-3.history": "1"
+    };
+
+    preferenceService.setPreferences(username, preferences);
+}
+
+// Execute tests
+testPreferences();