SAIL-348 : Reorder rules returns 200 OK but with error message

- This fix includees some refactoring
 - Tests updated



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

config/alfresco/templates/webscripts/org/alfresco/repository/rule/actionQueue.post.json.ftl

@@ -1,3 +1,4 @@
+
 {
 	"data" : 
 	{

source/java/org/alfresco/repo/web/scripts/rule/AbstractRuleWebScript.java

@@ -19,26 +19,25 @@
 package org.alfresco.repo.web.scripts.rule;
 
 import java.io.Serializable;
-import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.servlet.http.HttpServletResponse;
 
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.repo.action.ActionConditionImpl;
 import org.alfresco.repo.action.ActionImpl;
 import org.alfresco.repo.action.CompositeActionImpl;
 import org.alfresco.service.cmr.action.Action;
 import org.alfresco.service.cmr.action.ActionCondition;
-import org.alfresco.service.cmr.action.ActionConditionDefinition;
-import org.alfresco.service.cmr.action.ActionDefinition;
 import org.alfresco.service.cmr.action.ActionService;
 import org.alfresco.service.cmr.action.ParameterDefinition;
+import org.alfresco.service.cmr.action.ParameterizedItemDefinition;
 import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
+import org.alfresco.service.cmr.dictionary.DictionaryException;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -74,10 +73,6 @@ public abstract class AbstractRuleWebScript extends DeclarativeWebScript
     protected FileFolderService fileFolderService;
     protected NamespaceService namespaceService;
 
-    private static final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
-
-    private static Map<String, Map<String, QName>> propertyTypes = null;
-
     /**
      * Sets the node service instance
      * 
@@ -166,147 +161,6 @@ public abstract class AbstractRuleWebScript extends DeclarativeWebScript
         return nodeRef;
     }
 
-    protected QName getPropertyType(String name, boolean isAction, String propertyName)
-    {
-        QName result = null;
-
-        if (propertyTypes == null)
-        {
-            propertyTypes = new HashMap<String, Map<String, QName>>();
-
-            // get parameters for all action definitions
-            List<ActionDefinition> actionDefinitions = actionService.getActionDefinitions();
-
-            for (ActionDefinition actionDefinition : actionDefinitions)
-            {
-                List<ParameterDefinition> parameterDefinitions = actionDefinition.getParameterDefinitions();
-                Map<String, QName> parameters = new HashMap<String, QName>();
-
-                for (ParameterDefinition parameterDefinition : parameterDefinitions)
-                {
-                    String parameterName = parameterDefinition.getName();
-                    QName parameterType = parameterDefinition.getType();
-                    parameters.put(parameterName, parameterType);
-                }
-
-                try
-                {
-                    // cache parameter
-                    lock.writeLock().lock();
-                    propertyTypes.put(actionDefinition.getName(), parameters);
-                }
-                finally
-                {
-                    lock.writeLock().unlock();
-                }
-            }
-
-            // get parameters for all action condition definitions
-            List<ActionConditionDefinition> actionConditionDefinitions = actionService.getActionConditionDefinitions();
-
-            for (ActionConditionDefinition actionConditionDefinition : actionConditionDefinitions)
-            {
-                List<ParameterDefinition> parameterDefinitions = actionConditionDefinition.getParameterDefinitions();
-                Map<String, QName> parameters = new HashMap<String, QName>();
-
-                for (ParameterDefinition parameterDefinition : parameterDefinitions)
-                {
-                    String parameterName = parameterDefinition.getName();
-                    QName parameterType = parameterDefinition.getType();
-                    parameters.put(parameterName, parameterType);
-                }
-
-                try
-                {
-                    lock.writeLock().lock();
-                    propertyTypes.put(actionConditionDefinition.getName(), parameters);
-                }
-                finally
-                {
-                    lock.writeLock().unlock();
-                }
-            }
-        }
-
-        if (propertyTypes.containsKey(name))
-        {
-            Map<String, QName> parameters;
-            try
-            {
-                lock.readLock().lock();
-                parameters = propertyTypes.get(name);
-            }
-            finally
-            {
-                lock.readLock().unlock();
-            }
-            result = parameters.get(propertyName);
-            return result;
-        }
-        else
-        {
-            if (isAction)
-            {
-                ActionDefinition actionDefinition = actionService.getActionDefinition(name);
-                List<ParameterDefinition> parameterDefinitions = actionDefinition.getParameterDefinitions();
-                Map<String, QName> parameters = new HashMap<String, QName>();
-
-                for (ParameterDefinition parameterDefinition : parameterDefinitions)
-                {
-                    String parameterName = parameterDefinition.getName();
-                    QName parameterType = parameterDefinition.getType();
-                    parameters.put(parameterName, parameterType);
-
-                    if (parameterName.equals(propertyName))
-                    {
-                        result = parameterType;
-                    }
-                }
-
-                try
-                {
-                    // cache parameter
-                    lock.writeLock().lock();
-                    propertyTypes.put(name, parameters);
-                }
-                finally
-                {
-                    lock.writeLock().unlock();
-                }
-            }
-            else
-            {
-                ActionConditionDefinition actionConditionDefinition = actionService.getActionConditionDefinition(name);
-                List<ParameterDefinition> parameterDefinitions = actionConditionDefinition.getParameterDefinitions();
-                Map<String, QName> parameters = new HashMap<String, QName>();
-
-                for (ParameterDefinition parameterDefinition : parameterDefinitions)
-                {
-                    String parameterName = parameterDefinition.getName();
-                    QName parameterType = parameterDefinition.getType();
-                    parameters.put(parameterName, parameterType);
-
-                    if (parameterName.equals(propertyName))
-                    {
-                        result = parameterType;
-                    }
-                }
-
-                try
-                {
-                    lock.writeLock().lock();
-                    propertyTypes.put(name, parameters);
-                }
-                finally
-                {
-                    lock.writeLock().unlock();
-                }
-            }
-        }
-
-        return result;
-    }
-
     protected Rule parseJsonRule(JSONObject jsonRule) throws JSONException
     {
         Rule result = new Rule();
@@ -459,82 +313,99 @@ public abstract class AbstractRuleWebScript extends DeclarativeWebScript
         return result;
     }
 
-    @SuppressWarnings("unchecked")
     protected Map<String, Serializable> parseJsonParameterValues(JSONObject jsonParameterValues, String name, boolean isAction) throws JSONException
     {
         Map<String, Serializable> parameterValues = new HashMap<String, Serializable>();
 
         // get parameters names
         JSONArray names = jsonParameterValues.names();
-
         if (names == null)
         {
             return null;
         }
 
+        // Get the action or condition definition
+        ParameterizedItemDefinition definition = null;
+        if (isAction == true)
+        {
+            definition = actionService.getActionDefinition(name);
+        }
+        else
+        {
+            definition = actionService.getActionConditionDefinition(name);
+        }
+        if (definition == null)
+        {
+            throw new AlfrescoRuntimeException("Could not find defintion for action/condition " + name);
+        }
+
         for (int i = 0; i < names.length(); i++)
         {
             String propertyName = names.getString(i);
             Object propertyValue = jsonParameterValues.get(propertyName);
-
+            
-            // get parameter repository type
+            // Get the parameter definition we care about
-            QName typeQName = getPropertyType(name, isAction, propertyName);
+            ParameterDefinition paramDef = definition.getParameterDefintion(propertyName);
-
+            if (paramDef == null)
-            if (typeQName == null)
             {
-                if (propertyValue.toString().equals("true") || propertyValue.toString().equals("false"))
+                throw new AlfrescoRuntimeException("Invalid parameter " + propertyName + " for action/condition " + name);
-                {
-                    typeQName = DataTypeDefinition.BOOLEAN;
-                }
-                else
-                {
-                    typeQName = DataTypeDefinition.TEXT;
-                }
-            }
-
-            Serializable value = null;
-
-            if (typeQName.equals(DataTypeDefinition.QNAME))
-            {
-                value = QName.createQName(propertyValue.toString(), namespaceService);
-            }
-
-            if (typeQName.equals(DataTypeDefinition.ANY))
-            {
-                try
-                {
-                    value = dateFormate.parse(propertyValue.toString());
-                }
-                catch (ParseException e)
-                {
-                    try
-                    {
-                        value = Long.valueOf(propertyValue.toString());
-                    }
-                    catch (NumberFormatException e1)
-                    {
-                        if (propertyValue instanceof JSONArray)
-                        {
-                            value = new ArrayList<String>();
-                            JSONArray array = (JSONArray) propertyValue;
-                            for (int j = 0; j < array.length(); j++)
-                            {
-                                ((List<String>) value).add(array.getString(j));
-                            }
-                        }
-                    }
-                }
-            }
-
-            if (value == null)
-            {
-                // convert to correct repository type
-                value = (Serializable) DefaultTypeConverter.INSTANCE.convert(dictionaryService.getDataType(typeQName), propertyValue);
             }
+            QName typeQName = paramDef.getType();
 
+            // Convert the property value
+            Serializable value = convertValue(typeQName, propertyValue);
             parameterValues.put(propertyName, value);
         }
 
         return parameterValues;
     }
+    
+    private Serializable convertValue(QName typeQName, Object propertyValue) throws JSONException
+    {        
+        Serializable value = null;
+        
+        DataTypeDefinition typeDef = dictionaryService.getDataType(typeQName);
+        if (typeDef == null)
+        {
+            throw new AlfrescoRuntimeException("Action property type definition " + typeQName.toPrefixString() + " is unknown.");
+        }
+        
+        if (propertyValue instanceof JSONArray)
+        {
+            // Convert property type to java class
+            Class<?> javaClass = null;
+            
+            String javaClassName = typeDef.getJavaClassName();
+            try
+            {
+                javaClass = Class.forName(javaClassName);
+            }
+            catch (ClassNotFoundException e)
+            {
+                throw new DictionaryException("Java class " + javaClassName + " of property type " + typeDef.getName() + " is invalid", e);
+            }
+            
+            int length = ((JSONArray)propertyValue).length();
+            List<Serializable> list = new ArrayList<Serializable>(length);
+            for (int i = 0; i < length; i++)
+            {
+                list.add(convertValue(typeQName, ((JSONArray)propertyValue).get(i)));
+            }
+            value = (Serializable)list;
+        }
+        else
+        {
+            if (typeQName.equals(DataTypeDefinition.QNAME) == true && 
+                typeQName.toString().contains(":") == true)
+            {
+                 value = QName.createQName(propertyValue.toString(), namespaceService);
+            }
+            else
+            {
+                value = (Serializable)DefaultTypeConverter.INSTANCE.convert(dictionaryService.getDataType(typeQName), propertyValue);
+            }
+        }
+        
+        return value;
+    }
 }

source/java/org/alfresco/repo/web/scripts/rule/ActionQueuePost.java

@@ -81,16 +81,10 @@ public class ActionQueuePost extends AbstractRuleWebScript
                 model.put(STATUS, STATUS_SUCCESS);
             }
 
-            try
+            // Execute action
-            {
+            actionService.executeAction(action, actionedUponNode, true, async);
-                actionService.executeAction(action, actionedUponNode, true, async);
-            }
-            catch (Throwable e)
-            {
-                model.put(STATUS, STATUS_FAIL);
-                model.put("exception", e);
-            }
 
+            // Prepair model
             model.put("actionedUponNode", actionedUponNode.toString());
             model.put("action", json);
         }

source/java/org/alfresco/repo/web/scripts/rule/RuleServiceTest.java

@@ -35,6 +35,7 @@ import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.rule.Rule;
 import org.alfresco.service.cmr.rule.RuleService;
 import org.alfresco.service.namespace.QName;
+import org.apache.commons.digester.Rules;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -161,7 +162,12 @@ public class RuleServiceTest extends BaseWebScriptTest
 
     private JSONObject createRule(NodeRef ruleOwnerNodeRef) throws Exception
     {
-        JSONObject jsonRule = buildTestRule();
+        return createRule(ruleOwnerNodeRef, "test_rule");
+    }
+    
+    private JSONObject createRule(NodeRef ruleOwnerNodeRef, String title) throws Exception
+    {
+        JSONObject jsonRule = buildTestRule(title);
 
         Response response = sendRequest(new PostRequest(formatRulesUrl(ruleOwnerNodeRef, false), jsonRule.toString(), "application/json"), 200);
 
@@ -216,8 +222,6 @@ public class RuleServiceTest extends BaseWebScriptTest
         assertEquals(jsonAction.getString("description"), "this is description for composite-action");
         assertEquals(jsonAction.getString("title"), "test_title");
 
-        assertTrue(jsonAction.has("parameterValues"));
-
         assertTrue(jsonAction.getBoolean("executeAsync"));
 
         assertTrue(jsonAction.has("actions"));
@@ -689,6 +693,61 @@ public class RuleServiceTest extends BaseWebScriptTest
         // no more rules present 
         assertEquals(0, ruleService.getRules(testNodeRef).size());
     }
+    
+    public void testRuleReorder() throws Exception
+    {
+        assertEquals(0, ruleService.getRules(testNodeRef).size());
+        
+        // Create 3 rules
+        NodeRef rule1 = createRuleNodeRef(testNodeRef, "Rule 1");
+        NodeRef rule2 = createRuleNodeRef(testNodeRef, "Rule 2");
+        NodeRef rule3 = createRuleNodeRef(testNodeRef, "Rule 3");
+        
+        List<Rule> rules = ruleService.getRules(testNodeRef);
+        assertEquals(3, rules.size());
+        assertEquals("Rule 1", rules.get(0).getTitle());
+        assertEquals("Rule 2", rules.get(1).getTitle());
+        assertEquals("Rule 3", rules.get(2).getTitle());
+        
+        JSONObject action = new JSONObject();
+        action.put("actionDefinitionName", "reorder-rules");
+        action.put("actionedUponNode", testNodeRef.toString());
+        
+        JSONObject params = new JSONObject();
+        JSONArray orderArray = new JSONArray();
+        orderArray.put(rules.get(2).getNodeRef().toString());
+        orderArray.put(rules.get(1).getNodeRef().toString());
+        orderArray.put(rules.get(0).getNodeRef().toString());
+        params.put("rules", orderArray);
+        action.put("parameterValues", params);
+        
+        String url = formateQueueActionUrl(false);
+
+        // execute before response (should be successful)
+        Response successResponse = sendRequest(new PostRequest(url, action.toString(), "application/json"), 200);
+        JSONObject successResult = new JSONObject(successResponse.getContentAsString());
+        assertNotNull(successResult);
+        assertTrue(successResult.has("data"));
+        JSONObject successData = successResult.getJSONObject("data");
+        assertTrue(successData.has("status"));
+        assertEquals("success", successData.getString("status"));
+        assertTrue(successData.has("actionedUponNode"));
+        assertFalse(successData.has("exception"));
+        assertTrue(successData.has("action"));
+        
+        rules = ruleService.getRules(testNodeRef);
+        assertEquals(3, rules.size());
+        assertEquals("Rule 3", rules.get(0).getTitle());
+        assertEquals("Rule 2", rules.get(1).getTitle());
+        assertEquals("Rule 1", rules.get(2).getTitle());
+    }
+    
+    private NodeRef createRuleNodeRef(NodeRef folder, String title) throws Exception
+    {
+        JSONObject jsonRule = createRule(folder, title);
+        String id = jsonRule.getJSONObject("data").getString("id");
+        return new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, id);
+    }
 
     private JSONObject buildCopyAction(NodeRef destination) throws JSONException
     {
@@ -713,10 +772,15 @@ public class RuleServiceTest extends BaseWebScriptTest
     }
 
     private JSONObject buildTestRule() throws JSONException
+    {
+        return buildTestRule("test_rule");
+    }
+    
+    private JSONObject buildTestRule(String title) throws JSONException
     {
         JSONObject result = new JSONObject();
 
-        result.put("title", "test_rule");
+        result.put("title", title);
         result.put("description", "this is description for test_rule");
 
         JSONArray ruleType = new JSONArray();
@@ -743,10 +807,10 @@ public class RuleServiceTest extends BaseWebScriptTest
         result.put("description", "this is description for " + actionName);
         result.put("title", "test_title");
 
-        JSONObject parameterValues = new JSONObject();
+        //JSONObject parameterValues = new JSONObject();
-        parameterValues.put("test_name", "test_value");
+        //parameterValues.put("test_name", "test_value");
 
-        result.put("parameterValues", parameterValues);
+        //result.put("parameterValues", parameterValues);
 
         result.put("executeAsync", addActions);
 
@@ -780,10 +844,10 @@ public class RuleServiceTest extends BaseWebScriptTest
         result.put("conditionDefinitionName", conditionName);
         result.put("invertCondition", false);
 
-        JSONObject parameterValues = new JSONObject();
+        //JSONObject parameterValues = new JSONObject();
-        parameterValues.put("test_name", "test_value");
+        //parameterValues.put("test_name", "test_value");
 
-        result.put("parameterValues", parameterValues);
+        //result.put("parameterValues", parameterValues);
 
         return result;
     }