Mailing-List: contact commits-help@sling.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@sling.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r927532 - in /sling/trunk:
 bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/
 bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/
 bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/cont...
Date: Thu, 25 Mar 2010 18:34:31 -0000
To: commits@sling.apache.org
From: enorman@apache.org
Message-Id: <20100325183431.7040B23889E2@eris.apache.org>

Author: enorman
Date: Thu Mar 25 18:34:30 2010
New Revision: 927532

URL: http://svn.apache.org/viewvc?rev=927532&view=rev
Log:
SLING-1457 SLING-1458 - enable support for specifying the position of an ACE (within the ACL) when it is added/updated

Modified:
    sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/AccessControlUtil.java
    sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentCreator.java
    sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/DefaultContentCreator.java
    sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/readers/JsonReader.java
    sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/JsonReaderTest.java
    sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/readers/XmlReaderTest.java
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/GetAclServlet.java
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java
    sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java
    sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/RemoveAcesTest.java

Modified: sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/AccessControlUtil.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/AccessControlUtil.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/AccessControlUtil.java (original)
+++ sling/trunk/bundles/jcr/base/src/main/java/org/apache/sling/jcr/base/util/AccessControlUtil.java Thu Mar 25 18:34:30 2010
@@ -19,6 +19,7 @@
 package org.apache.sling.jcr.base.util;
 
 import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.UserManager;
@@ -211,6 +212,36 @@ public class AccessControlUtil {
     	Class[] types = new Class[] {Principal.class, Privilege[].class, boolean.class, Map.class};
 		return safeInvokeRepoMethod(acl, METHOD_JACKRABBIT_ACL_ADD_ENTRY, Boolean.class, args, types);
     }
+
+    /**
+     * Replaces existing access control entries in the ACL for the specified
+     * <code>principal</code> and <code>resourcePath</code>. Any existing granted
+     * or denied privileges which do not conflict with the specified privileges
+     * are maintained. Where conflicts exist, existing privileges are dropped.
+     * The end result will be at most two ACEs for the principal: one for grants
+     * and one for denies. Aggregate privileges are disaggregated before checking
+     * for conflicts.
+     * @param session
+     * @param resourcePath
+     * @param principal
+     * @param grantedPrivilegeNames
+     * @param deniedPrivilegeNames
+     * @param removedPrivilegeNames privileges which, if they exist, should be
+     * removed for this principal and resource
+     * @throws RepositoryException
+     * @deprecated use @link {@link #replaceAccessControlEntry(Session, String, Principal, String[], String[], String[], String)} instead.
+     */
+    public static void replaceAccessControlEntry(Session session, String resourcePath, Principal principal, 
+			String[] grantedPrivilegeNames, String[] deniedPrivilegeNames, String[] removedPrivilegeNames)
+    		throws RepositoryException {
+    	replaceAccessControlEntry(session, 
+    			resourcePath, 
+    			principal, 
+    			grantedPrivilegeNames, 
+    			deniedPrivilegeNames, 
+    			removedPrivilegeNames, 
+    			null);
+    }    
     
     /**
      * Replaces existing access control entries in the ACL for the specified
@@ -227,10 +258,21 @@ public class AccessControlUtil {
      * @param deniedPrivilegeNames
      * @param removedPrivilegeNames privileges which, if they exist, should be
      * removed for this principal and resource
+     * @param order where the access control entry should go in the list.  
+     *         Value should be one of these:
+     *         <table>
+     *          <tr><td>null</td><td>If the ACE for the principal doesn't exist add at the end, otherwise leave the ACE at it's current position.</td></tr>
+     * 			<tr><td>first</td><td>Place the target ACE as the first amongst its siblings</td></tr>
+	 *			<tr><td>last</td><td>Place the target ACE as the last amongst its siblings</td></tr>
+	 * 			<tr><td>before xyz</td><td>Place the target ACE immediately before the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>after xyz</td><td>Place the target ACE immediately after the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>numeric</td><td>Place the target ACE at the specified numeric index</td></tr>
+	 *         </table>
      * @throws RepositoryException
      */
     public static void replaceAccessControlEntry(Session session, String resourcePath, Principal principal, 
-    			String[] grantedPrivilegeNames, String[] deniedPrivilegeNames, String[] removedPrivilegeNames)
+    			String[] grantedPrivilegeNames, String[] deniedPrivilegeNames, String[] removedPrivilegeNames,
+    			String order)
         		throws RepositoryException {
     	AccessControlManager accessControlManager = getAccessControlManager(session);
     	Set<String> specifiedPrivilegeNames = new HashSet<String>();
@@ -270,11 +312,17 @@ public class AccessControlUtil {
       
     	// Combine all existing ACEs for the target principal.
     	AccessControlEntry[] accessControlEntries = acl.getAccessControlEntries();
-    	for (AccessControlEntry ace : accessControlEntries) {
+    	for (int i=0; i < accessControlEntries.length; i++) {
+    		AccessControlEntry ace = accessControlEntries[i];
     		if (principal.equals(ace.getPrincipal())) {
     			if (log.isDebugEnabled()) {
     				log.debug("Found Existing ACE for principal {} on resource {}", new Object[] {principal.getName(), resourcePath});
     			}
+    			if (order == null || order.length() == 0) {
+    				//order not specified, so keep track of the original ACE position.
+    				order = String.valueOf(i);
+    			}
+    			
     			boolean isAllow = isAllow(ace);
     			Privilege[] privileges = ace.getPrivileges();
     			if (log.isDebugEnabled()) {
@@ -329,6 +377,10 @@ public class AccessControlUtil {
    			addEntry(acl, principal, deniedPrivilegeList.toArray(new Privilege[deniedPrivilegeList.size()]), false);
    		}
 
+   		
+   		//order the ACL
+   		reorderAccessControlEntries(acl, principal, order);
+   		
     	accessControlManager.setPolicy(resourcePath, acl);
     	if (log.isDebugEnabled()) {
     		List<String> oldGrantedNames = new ArrayList<String>(oldGrants.size());
@@ -443,4 +495,128 @@ public class AccessControlUtil {
 		}
 		return disaggregatedPrivilegeNames;
 	}
+
+	/**
+	 * Move the ACE(s) for the specified principal to the position specified by the 'order'
+	 * parameter. 
+	 * 
+	 * @param acl the acl of the node containing the ACE to position
+	 * @param principal the user or group of the ACE to position
+     * @param order where the access control entry should go in the list.  
+     *         Value should be one of these:
+     *         <table>
+     * 			<tr><td>first</td><td>Place the target ACE as the first amongst its siblings</td></tr>
+	 *			<tr><td>last</td><td>Place the target ACE as the last amongst its siblings</td></tr>
+	 * 			<tr><td>before xyz</td><td>Place the target ACE immediately before the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>after xyz</td><td>Place the target ACE immediately after the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>numeric</td><td>Place the target ACE at the specified index</td></tr>
+	 *         </table>
+	 * @throws RepositoryException 
+	 * @throws UnsupportedRepositoryOperationException 
+	 * @throws AccessControlException 
+	 */
+	private static void reorderAccessControlEntries(AccessControlList acl, 
+														Principal principal, 
+														String order) 
+							throws RepositoryException {
+		if (order == null || order.length() == 0) {
+			return; //nothing to do
+		}
+		if (acl instanceof JackrabbitAccessControlList) {
+			JackrabbitAccessControlList jacl = (JackrabbitAccessControlList)acl;
+			
+			AccessControlEntry[] accessControlEntries = jacl.getAccessControlEntries();
+			if (accessControlEntries.length <= 1) {
+				return; //only one ACE, so nothing to reorder.
+			}
+
+			AccessControlEntry beforeEntry = null;
+			if ("first".equals(order)) {
+				beforeEntry = accessControlEntries[0];
+			} else if ("last".equals(order)) {
+				beforeEntry = null;
+			} else if (order.startsWith("before ")) {
+				String beforePrincipalName = order.substring(7);
+				
+				//find the index of the ACE of the 'before' principal
+				for (int i=0; i < accessControlEntries.length; i++) {
+					if (beforePrincipalName.equals(accessControlEntries[i].getPrincipal().getName())) {
+						//found it!
+						beforeEntry = accessControlEntries[i];
+						break;
+					} 
+				}
+				
+				if (beforeEntry == null) {
+					//didn't find an ACE that matched the 'before' principal
+					throw new IllegalArgumentException("No ACE was found for the specified principal: " + beforePrincipalName);
+				}
+			} else if (order.startsWith("after ")) {
+				String afterPrincipalName = order.substring(6);
+				
+				//find the index of the ACE of the 'after' principal
+				for (int i = accessControlEntries.length - 1; i >= 0; i--) {
+					if (afterPrincipalName.equals(accessControlEntries[i].getPrincipal().getName())) {
+						//found it!
+						
+						// the 'before' ACE is the next one after the 'after' ACE
+						if (i >= accessControlEntries.length - 1) {
+							//the after is the last one in the list
+							beforeEntry = null;
+						} else {
+							beforeEntry = accessControlEntries[i + 1];
+						}
+						break;
+					} 
+				}
+				
+				if (beforeEntry == null) {
+					//didn't find an ACE that matched the 'after' principal
+					throw new IllegalArgumentException("No ACE was found for the specified principal: " + afterPrincipalName);
+				}
+			} else {
+				try {
+					int index = Integer.parseInt(order);
+					if (index > accessControlEntries.length) {
+						//invalid index
+						throw new IndexOutOfBoundsException("Index value is too large: " + index);
+					}
+					
+					if (index == 0) {
+						beforeEntry = accessControlEntries[0];
+					} else {
+						//the index value is the index of the principal.  A principal may have more
+						// than one ACEs (deny + grant), so we need to compensate.
+						Set<Principal> processedPrincipals = new HashSet<Principal>();
+						for (int i = 0; i < accessControlEntries.length; i++) {
+							Principal principal2 = accessControlEntries[i].getPrincipal();
+							if (processedPrincipals.size() == index &&
+									!processedPrincipals.contains(principal2)) {
+								//we are now at the correct position in the list
+								beforeEntry = accessControlEntries[i];
+								break;
+							}
+
+							processedPrincipals.add(principal2);
+						}					
+					}
+				} catch (NumberFormatException nfe) {
+					//not a number.
+					throw new IllegalArgumentException("Illegal value for the order parameter: " + order);
+				}
+			}
+			
+			//now loop through the entries to move the affected ACEs to the specified
+			// position.
+			for (int i = accessControlEntries.length - 1; i >= 0; i--) {
+				AccessControlEntry ace = accessControlEntries[i];
+				if (principal.equals(ace.getPrincipal())) {
+					//this ACE is for the specified principal.
+					jacl.orderBefore(ace, beforeEntry);
+				}
+			}
+		} else {
+			throw new IllegalArgumentException("The acl must be an instance of JackrabbitAccessControlList");
+		}
+	}
 }

Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentCreator.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentCreator.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentCreator.java (original)
+++ sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentCreator.java Thu Mar 25 18:34:30 2010
@@ -162,8 +162,17 @@ public interface ContentCreator {
      * @param principal the user or group id for the ACE
      * @param grantedPrivileges the set of privileges to grant the principal
      * @param deniedPrivileges the set of privileges to deny the principal (for users only)
+     * @param order specifies the position of the ACE in the containing ACL. (may be null)
+     *         Value should be one of these:
+     *         <table>
+     * 			<tr><td>first</td><td>Place the target ACE as the first amongst its siblings</td></tr>
+	 *			<tr><td>last</td><td>Place the target ACE as the last amongst its siblings</td></tr>
+	 * 			<tr><td>before xyz</td><td>Place the target ACE immediately before the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>after xyz</td><td>Place the target ACE immediately after the sibling whose name is xyz</td></tr>
+	 * 			<tr><td>numeric</td><td>Place the target ACE at the specified index</td></tr>
+	 *         </table>
      * @throws RepositoryException
      */
-    void createAce(String principal, String [] grantedPrivileges, String [] deniedPrivileges )
+    void createAce(String principal, String [] grantedPrivileges, String [] deniedPrivileges, String order )
     throws RepositoryException;
 }

Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/DefaultContentCreator.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/DefaultContentCreator.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/DefaultContentCreator.java (original)
+++ sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/DefaultContentCreator.java Thu Mar 25 18:34:30 2010
@@ -799,7 +799,8 @@ public class DefaultContentCreator imple
 	 * @see org.apache.sling.jcr.contentloader.internal.ContentCreator#createAce(java.lang.String, java.lang.String, java.lang.String[], java.lang.String[])
 	 */
 	public void createAce(String principalId,
-			String[] grantedPrivilegeNames, String[] deniedPrivilegeNames)
+			String[] grantedPrivilegeNames, String[] deniedPrivilegeNames,
+			String order)
 			throws RepositoryException {
 		final Node parentNode = this.parentNodeStack.peek();
 		Session session = parentNode.getSession();
@@ -812,7 +813,7 @@ public class DefaultContentCreator imple
 
 		if ((grantedPrivilegeNames != null) || (deniedPrivilegeNames != null)) {
 			AccessControlUtil.replaceAccessControlEntry(session, resourcePath, principal,
-					grantedPrivilegeNames, deniedPrivilegeNames, null);
+					grantedPrivilegeNames, deniedPrivilegeNames, null, order);
 		}
 	}
 

Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/readers/JsonReader.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/readers/JsonReader.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/readers/JsonReader.java (original)
+++ sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/readers/JsonReader.java Thu Mar 25 18:34:30 2010
@@ -427,8 +427,10 @@ public class JsonReader implements Conte
 			}
 		}
 
+		String order = ace.optString("order", null);
+		
 		//do the work.
-		contentCreator.createAce(principalID, grantedPrivileges, deniedPrivileges);
+		contentCreator.createAce(principalID, grantedPrivileges, deniedPrivileges, order);
     }
 
 }

Modified: sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/JsonReaderTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/JsonReaderTest.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/JsonReaderTest.java (original)
+++ sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/JsonReaderTest.java Thu Mar 25 18:34:30 2010
@@ -276,16 +276,17 @@ public class JsonReaderTest {
     			"  {" +
     			"    \"principal\" : \"groupname2\"," +
     			"    \"granted\" : [\"jcr:read\"]," +
-    			"    \"denied\" : [\"jcr:write\"]" +
+    			"    \"denied\" : [\"jcr:write\"]," +
+    			"    \"order\" : \"first\"" +
     			"  }" +
     			"]" +
     			"}";
         this.mockery.checking(new Expectations() {{
         	allowing(creator).createNode(null, null, null); inSequence(mySequence);
 
-            allowing(creator).createAce("username1",new String[]{"jcr:read","jcr:write"},new String[]{}); inSequence(mySequence);
-            allowing(creator).createAce("groupname1",new String[]{"jcr:read","jcr:write"},null); inSequence(mySequence);
-            allowing(creator).createAce("groupname2",new String[]{"jcr:read"},new String[]{"jcr:write"}); inSequence(mySequence);
+            allowing(creator).createAce("username1",new String[]{"jcr:read","jcr:write"},new String[]{}, null); inSequence(mySequence);
+            allowing(creator).createAce("groupname1",new String[]{"jcr:read","jcr:write"},null, null); inSequence(mySequence);
+            allowing(creator).createAce("groupname2",new String[]{"jcr:read"},new String[]{"jcr:write"}, "first"); inSequence(mySequence);
             allowing(creator).finishNode(); inSequence(mySequence);
         }});
         this.parse(json);

Modified: sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/readers/XmlReaderTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/readers/XmlReaderTest.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/readers/XmlReaderTest.java (original)
+++ sling/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/internal/readers/XmlReaderTest.java Thu Mar 25 18:34:30 2010
@@ -145,7 +145,8 @@ public class XmlReaderTest extends TestC
         }
 
 		public void createAce(String principal,
-				String[] grantedPrivileges, String[] deniedPrivileges)
+				String[] grantedPrivileges, String[] deniedPrivileges,
+				String order)
 				throws RepositoryException {
 		}
 

Modified: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/GetAclServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/GetAclServlet.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/GetAclServlet.java (original)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/GetAclServlet.java Thu Mar 25 18:34:30 2010
@@ -18,8 +18,10 @@ package org.apache.sling.jcr.jackrabbit.
 
 import java.io.IOException;
 import java.security.Principal;
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Map.Entry;
@@ -41,6 +43,7 @@ import org.apache.sling.api.SlingHttpSer
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceNotFoundException;
 import org.apache.sling.api.servlets.SlingAllMethodsServlet;
+import org.apache.sling.commons.json.JSONArray;
 import org.apache.sling.commons.json.JSONObject;
 import org.apache.sling.jcr.base.util.AccessControlUtil;
 import org.slf4j.Logger;
@@ -191,13 +194,15 @@ public class GetAclServlet extends Sling
         	response.setContentType("application/json");
         	response.setCharacterEncoding("UTF-8");
 
-        	JSONObject jsonObj = new JSONObject();
+        	List<JSONObject> aclList = new ArrayList<JSONObject>();
         	Set<Entry<String, Map<String, Set<String>>>> entrySet = aclMap.entrySet();
         	for (Entry<String, Map<String, Set<String>>> entry : entrySet) {
         		String principalName = entry.getKey();
         		Map<String, Set<String>> value = entry.getValue();
-        		
-        		JSONObject aceObject = new JSONObject();
+
+            	JSONObject aceObject = new JSONObject();
+            	aceObject.put("principal", principalName);
+
         		Set<String> grantedSet = value.get("granted");
         		if (grantedSet != null) {
             		aceObject.put("granted", grantedSet);
@@ -208,12 +213,12 @@ public class GetAclServlet extends Sling
         			aceObject.put("denied", deniedSet);
         		}
 
-        		jsonObj.put(principalName, aceObject);
+        		aclList.add(aceObject);
 			}
-        	
+        	JSONArray jsonAclArray = new JSONArray(aclList);
 
             // do the dump
-        	jsonObj.write(response.getWriter());
+        	jsonAclArray.write(response.getWriter());
         } catch (AccessDeniedException ade) {
         	response.sendError(HttpServletResponse.SC_NOT_FOUND);
         } catch (ResourceNotFoundException rnfe) {

Modified: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java (original)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ModifyAceServlet.java Thu Mar 25 18:34:30 2010
@@ -140,12 +140,15 @@ public class ModifyAceServlet extends Ab
 			}
 		}
 
+		String order = request.getParameter("order");
+		
 		// Make the actual changes.
 		try {
 			AccessControlUtil.replaceAccessControlEntry(session, resourcePath, principal,
 					grantedPrivilegeNames.toArray(new String[grantedPrivilegeNames.size()]),
 					deniedPrivilegeNames.toArray(new String[deniedPrivilegeNames.size()]),
-					removedPrivilegeNames.toArray(new String[removedPrivilegeNames.size()]));
+					removedPrivilegeNames.toArray(new String[removedPrivilegeNames.size()]),
+					order);
 			if (session.hasPendingChanges()) {
 				session.save();
 			}

Modified: sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java (original)
+++ sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/ModifyAceTest.java Thu Mar 25 18:34:30 2010
@@ -37,6 +37,7 @@ import org.apache.sling.commons.json.JSO
 public class ModifyAceTest extends AbstractAccessManagerTest {
 
 	String testUserId = null;
+	String testUserId2 = null;
 	String testGroupId = null;
 	String testFolderUrl = null;
 	
@@ -65,6 +66,12 @@ public class ModifyAceTest extends Abstr
 			List<NameValuePair> postParams = new ArrayList<NameValuePair>();
 			assertAuthenticatedPostStatus(creds, postUrl, HttpServletResponse.SC_OK, postParams, null);
 		}
+		if (testUserId2 != null) {
+			//remove the test user if it exists.
+			String postUrl = HTTP_BASE_URL + "/system/userManager/user/" + testUserId2 + ".delete.html";
+			List<NameValuePair> postParams = new ArrayList<NameValuePair>();
+			assertAuthenticatedPostStatus(creds, postUrl, HttpServletResponse.SC_OK, postParams, null);
+		}
 	}
 
 	public void testModifyAceForUser() throws IOException, JSONException {
@@ -89,19 +96,21 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
 		
-		JSONObject aceObject = new JSONObject(aceString); 
+		JSONObject aceObject = jsonArray.optJSONObject(0);
 		assertNotNull(aceObject);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		String principalString = aceObject.optString("principal");
+		assertEquals(testUserId, principalString);
+		
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(1, grantedArray.length());
 		assertEquals("jcr:read", grantedArray.getString(0));
 
-		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		JSONArray deniedArray = aceObject.optJSONArray("denied");
 		assertNotNull(deniedArray);
 		assertEquals(1, deniedArray.length());
 		assertEquals("jcr:write", deniedArray.getString(0));
@@ -129,19 +138,21 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testGroupId);
-		assertNotNull(aceString);
-
-		JSONObject aceObject = new JSONObject(aceString);
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
+		
+		JSONObject aceObject = jsonArray.optJSONObject(0);
 		assertNotNull(aceObject);
+
+		String principalString = aceObject.optString("principal");
+		assertEquals(testGroupId, principalString);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(1, grantedArray.length());
 		assertEquals("jcr:read", grantedArray.getString(0));
 
-		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		JSONArray deniedArray = aceObject.optJSONArray("denied");
 		assertNotNull(deniedArray);
 		assertEquals("jcr:write", deniedArray.getString(0));
 	}
@@ -173,14 +184,16 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
 		
-		JSONObject aceObject = new JSONObject(aceString); 
+		JSONObject aceObject = jsonArray.optJSONObject(0);
 		assertNotNull(aceObject);
+
+		String principalString = aceObject.optString("principal");
+		assertEquals(testUserId, principalString);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(3, grantedArray.length());
 		Set<String> grantedPrivilegeNames = new HashSet<String>();
@@ -191,7 +204,7 @@ public class ModifyAceTest extends Abstr
 		assertTrue(grantedPrivilegeNames.contains("jcr:readAccessControl"));
 		assertTrue(grantedPrivilegeNames.contains("jcr:addChildNodes"));
 
-		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		JSONArray deniedArray = aceObject.optJSONArray("denied");
 		assertNotNull(deniedArray);
 		assertEquals(2, deniedArray.length());
 		Set<String> deniedPrivilegeNames = new HashSet<String>();
@@ -219,16 +232,17 @@ public class ModifyAceTest extends Abstr
 		
 		//fetch the JSON for the acl to verify the settings.
 		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
-		
 		assertNotNull(json2);
-		JSONObject jsonObj2 = new JSONObject(json2);
-		String aceString2 = jsonObj2.getString(testUserId);
-		assertNotNull(aceString2);
+		JSONArray jsonArray2 = new JSONArray(json2);
+		assertEquals(1, jsonArray2.length());
 		
-		JSONObject aceObject2 = new JSONObject(aceString2); 
+		JSONObject aceObject2 = jsonArray2.optJSONObject(0);
 		assertNotNull(aceObject2);
+
+		String principalString2 = aceObject2.optString("principal");
+		assertEquals(testUserId, principalString2);
 		
-		JSONArray grantedArray2 = aceObject2.getJSONArray("granted");
+		JSONArray grantedArray2 = aceObject2.optJSONArray("granted");
 		assertNotNull(grantedArray2);
 		assertEquals(3, grantedArray2.length());
 		Set<String> grantedPrivilegeNames2 = new HashSet<String>();
@@ -239,7 +253,7 @@ public class ModifyAceTest extends Abstr
 		assertTrue(grantedPrivilegeNames2.contains("jcr:addChildNodes"));
 		assertTrue(grantedPrivilegeNames2.contains("jcr:modifyProperties"));
 
-		JSONArray deniedArray2 = aceObject2.getJSONArray("denied");
+		JSONArray deniedArray2 = aceObject2.optJSONArray("denied");
 		assertNotNull(deniedArray2);
 		assertEquals(2, deniedArray2.length());
 		Set<String> deniedPrivilegeNames2 = new HashSet<String>();
@@ -275,14 +289,16 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
 		
-		JSONObject aceObject = new JSONObject(aceString); 
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
+		
+		JSONObject aceObject = jsonArray.optJSONObject(0); 
 		assertNotNull(aceObject);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		assertEquals(testUserId, aceObject.optString("principal"));
+		
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(1, grantedArray.length());
 		Set<String> grantedPrivilegeNames = new HashSet<String>();
@@ -291,7 +307,7 @@ public class ModifyAceTest extends Abstr
 		}
 		assertTrue(grantedPrivilegeNames.contains("jcr:read"));
 
-		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		JSONArray deniedArray = aceObject.optJSONArray("denied");
 		assertNotNull(deniedArray);
 		assertEquals(1, deniedArray.length());
 		Set<String> deniedPrivilegeNames = new HashSet<String>();
@@ -315,16 +331,17 @@ public class ModifyAceTest extends Abstr
 		
 		//fetch the JSON for the acl to verify the settings.
 		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
-		
 		assertNotNull(json2);
-		JSONObject jsonObj2 = new JSONObject(json2);
-		String aceString2 = jsonObj2.getString(testUserId);
-		assertNotNull(aceString2);
 		
-		JSONObject aceObject2 = new JSONObject(aceString2); 
+		JSONArray jsonArray2 = new JSONArray(json2);
+		assertEquals(1, jsonArray2.length());
+		
+		JSONObject aceObject2 = jsonArray2.optJSONObject(0); 
 		assertNotNull(aceObject2);
 		
-		JSONArray grantedArray2 = aceObject2.getJSONArray("granted");
+		assertEquals(testUserId, aceObject2.optString("principal"));
+		
+		JSONArray grantedArray2 = aceObject2.optJSONArray("granted");
 		assertNotNull(grantedArray2);
 		assertEquals(2, grantedArray2.length());
 		Set<String> grantedPrivilegeNames2 = new HashSet<String>();
@@ -334,7 +351,7 @@ public class ModifyAceTest extends Abstr
 		assertTrue(grantedPrivilegeNames2.contains("jcr:read"));
 		assertTrue(grantedPrivilegeNames2.contains("jcr:modifyProperties"));
 
-		JSONArray deniedArray2 = aceObject2.getJSONArray("denied");
+		JSONArray deniedArray2 = aceObject2.optJSONArray("denied");
 		assertNotNull(deniedArray2);
 		assertEquals(3, deniedArray2.length());
 		Set<String> deniedPrivilegeNames2 = new HashSet<String>();
@@ -372,13 +389,15 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
 		
-		JSONObject aceObject = new JSONObject(aceString); 
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
+		
+		JSONObject aceObject = jsonArray.optJSONObject(0); 
 		assertNotNull(aceObject);
 		
+		assertEquals(testUserId, aceObject.optString("principal"));
+		
 		JSONArray grantedArray = aceObject.getJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(1, grantedArray.length());
@@ -413,16 +432,17 @@ public class ModifyAceTest extends Abstr
 		
 		//fetch the JSON for the acl to verify the settings.
 		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
-		
 		assertNotNull(json2);
-		JSONObject jsonObj2 = new JSONObject(json2);
-		String aceString2 = jsonObj2.getString(testUserId);
-		assertNotNull(aceString2);
 		
-		JSONObject aceObject2 = new JSONObject(aceString2); 
+		JSONArray jsonArray2 = new JSONArray(json2);
+		assertEquals(1, jsonArray2.length());
+		
+		JSONObject aceObject2 = jsonArray2.optJSONObject(0); 
 		assertNotNull(aceObject2);
 		
-		JSONArray grantedArray2 = aceObject2.getJSONArray("granted");
+		assertEquals(testUserId, aceObject.optString("principal"));
+		
+		JSONArray grantedArray2 = aceObject2.optJSONArray("granted");
 		assertNotNull(grantedArray2);
 		assertEquals(1, grantedArray2.length());
 		Set<String> grantedPrivilegeNames2 = new HashSet<String>();
@@ -431,7 +451,7 @@ public class ModifyAceTest extends Abstr
 		}
 		assertTrue(grantedPrivilegeNames2.contains("jcr:read"));
 
-		JSONArray deniedArray2 = aceObject2.getJSONArray("denied");
+		JSONArray deniedArray2 = aceObject2.optJSONArray("denied");
 		assertNotNull(deniedArray2);
 		assertEquals(1, deniedArray2.length());
 		Set<String> deniedPrivilegeNames2 = new HashSet<String>();
@@ -465,14 +485,16 @@ public class ModifyAceTest extends Abstr
 
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
 		
-		JSONObject aceObject = new JSONObject(aceString); 
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
+		
+		JSONObject aceObject = jsonArray.optJSONObject(0); 
 		assertNotNull(aceObject);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		assertEquals(testUserId, aceObject.optString("principal"));
+		
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals(1, grantedArray.length());
 		Set<String> grantedPrivilegeNames = new HashSet<String>();
@@ -498,16 +520,17 @@ public class ModifyAceTest extends Abstr
 		
 		//fetch the JSON for the acl to verify the settings.
 		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
-		
 		assertNotNull(json2);
-		JSONObject jsonObj2 = new JSONObject(json2);
-		String aceString2 = jsonObj2.getString(testUserId);
-		assertNotNull(aceString2);
+
+		JSONArray jsonArray2 = new JSONArray(json2);
+		assertEquals(1, jsonArray2.length());
 		
-		JSONObject aceObject2 = new JSONObject(aceString2); 
+		JSONObject aceObject2 = jsonArray2.optJSONObject(0); 
 		assertNotNull(aceObject2);
 		
-		JSONArray grantedArray2 = aceObject2.getJSONArray("granted");
+		assertEquals(testUserId, aceObject2.optString("principal"));
+		
+		JSONArray grantedArray2 = aceObject2.optJSONArray("granted");
 		assertNotNull(grantedArray2);
 		assertEquals(1, grantedArray2.length());
 		Set<String> grantedPrivilegeNames2 = new HashSet<String>();
@@ -516,7 +539,7 @@ public class ModifyAceTest extends Abstr
 		}
 		assertTrue(grantedPrivilegeNames2.contains("jcr:write"));
 
-		JSONArray deniedArray2 = aceObject2.getJSONArray("denied");
+		JSONArray deniedArray2 = aceObject2.optJSONArray("denied");
 		assertNotNull(deniedArray2);
 		assertEquals(1, deniedArray2.length());
 		Set<String> deniedPrivilegeNames2 = new HashSet<String>();
@@ -525,5 +548,216 @@ public class ModifyAceTest extends Abstr
 		}
 		assertTrue(deniedPrivilegeNames2.contains("jcr:nodeTypeManagement"));
 	}
+
+
+	
+	/**
+	 * Test to verify adding an ACE in the first position of 
+	 * the ACL
+	 */
+	public void testAddAceOrderByFirst() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "first");
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testGroupId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testUserId, jsonArray.getJSONObject(1).getString("principal"));
+	}	
+
+	/**
+	 * Test to verify adding an ACE at the end 
+	 * the ACL
+	 */
+	public void testAddAceOrderByLast() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "last");
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testUserId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testGroupId, jsonArray.getJSONObject(1).getString("principal"));
+	}	
+
+	/**
+	 * Test to verify adding an ACE before an existing ACE 
+	 * the ACL
+	 */
+	public void testAddAceOrderByBefore() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "before " + testUserId);
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testGroupId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testUserId, jsonArray.getJSONObject(1).getString("principal"));
+	}	
+
+	/**
+	 * Test to verify adding an ACE after an existing ACE 
+	 * the ACL
+	 */
+	public void testAddAceOrderByAfter() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "after " + testUserId);
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testUserId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testGroupId, jsonArray.getJSONObject(1).getString("principal"));
+	}	
+
+	/**
+	 * Test to verify adding an ACE at a specific index inside 
+	 * the ACL
+	 */
+	public void testAddAceOrderByNumeric() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "0");
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testGroupId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testUserId, jsonArray.getJSONObject(1).getString("principal"));
+		
+		//add another principal between the testGroupId and testUserId
+		testUserId2 = createTestUser();
+		addOrUpdateAce(testFolderUrl, testUserId2, true, "1");
+
+		String json2 = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json2);
+
+		JSONArray jsonArray2 = new JSONArray(json2);
+		assertEquals(3, jsonArray2.length());
+		
+		assertEquals(testGroupId, jsonArray2.getJSONObject(0).getString("principal"));
+		assertEquals(testUserId2, jsonArray2.getJSONObject(1).getString("principal"));		
+		assertEquals(testUserId, jsonArray2.getJSONObject(2).getString("principal"));		
+	}	
+
+	/**
+	 * Test to make sure modifying an existing ace without changing the order 
+	 * leaves the ACE in the same position in the ACL
+	 */
+	public void testUpdateAcePreservePosition() throws IOException, JSONException {
+		createAceOrderTestFolderWithOneAce();
+		
+		testGroupId = createTestGroup();
+
+		addOrUpdateAce(testFolderUrl, testGroupId, true, "first");
+
+		//update the ace to make sure the update does not change the ACE order
+		addOrUpdateAce(testFolderUrl, testGroupId, false, null);
+		
+		
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+		
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(2, jsonArray.length());
+		
+		assertEquals(testGroupId, jsonArray.getJSONObject(0).getString("principal"));
+		assertEquals(testUserId, jsonArray.getJSONObject(1).getString("principal"));
+	}	
+
+	
+	/**
+	 * Helper to create a test folder with a single ACE pre-created
+	 */
+	private void createAceOrderTestFolderWithOneAce() throws IOException, JSONException {
+		testUserId = createTestUser();
+		
+		testFolderUrl = createTestFolder();
+
+		addOrUpdateAce(testFolderUrl, testUserId, true, null);
+
+		//fetch the JSON for the acl to verify the settings.
+		String getUrl = testFolderUrl + ".acl.json";
+
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
+		assertNotNull(json);
+		JSONArray jsonArray = new JSONArray(json);
+		assertEquals(1, jsonArray.length());
+		
+		assertEquals(testUserId, jsonArray.getJSONObject(0).getString("principal"));
+	}
+	
+	/**
+	 * Helper to add or update an ace for testing
+	 */
+	private void addOrUpdateAce(String folderUrl, String principalId, boolean readGranted, String order) throws IOException, JSONException {
+        String postUrl = folderUrl + ".modifyAce.html";
+
+		//1. create an initial set of privileges
+		List<NameValuePair> postParams = new ArrayList<NameValuePair>();
+		postParams.add(new NameValuePair("principalId", principalId));
+		postParams.add(new NameValuePair("privilege@jcr:read", readGranted ? "granted" : "denied"));
+		postParams.add(new NameValuePair("privilege@jcr:write", "denied"));
+		if (order != null) {
+			postParams.add(new NameValuePair("order", order));
+		}
+		
+		Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+		assertAuthenticatedPostStatus(creds, postUrl, HttpServletResponse.SC_OK, postParams, null);
+	}
 	
 }

Modified: sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/RemoveAcesTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/RemoveAcesTest.java?rev=927532&r1=927531&r2=927532&view=diff
==============================================================================
--- sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/RemoveAcesTest.java (original)
+++ sling/trunk/launchpad/testing/src/test/java/org/apache/sling/launchpad/webapp/integrationtest/accessManager/RemoveAcesTest.java Thu Mar 25 18:34:30 2010
@@ -95,29 +95,36 @@ public class RemoveAcesTest extends Abst
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
 		
-		JSONObject jsonObj = new JSONObject(json);
-		String aceString = jsonObj.getString(testUserId);
-		assertNotNull(aceString);
-
-		JSONObject aceObject = new JSONObject(aceString);
+		JSONArray jsonArray = new JSONArray(json);
+		
+		if (addGroupAce) {
+			assertEquals(2, jsonArray.length());
+		} else {
+			assertEquals(1, jsonArray.length());
+		}
+		
+		JSONObject aceObject = jsonArray.optJSONObject(0);
 		assertNotNull(aceObject);
 		
-		JSONArray grantedArray = aceObject.getJSONArray("granted");
+		String principalString = aceObject.optString("principal");
+		assertEquals(testUserId, principalString);
+		
+		JSONArray grantedArray = aceObject.optJSONArray("granted");
 		assertNotNull(grantedArray);
 		assertEquals("jcr:read", grantedArray.getString(0));
 
-		JSONArray deniedArray = aceObject.getJSONArray("denied");
+		JSONArray deniedArray = aceObject.optJSONArray("denied");
 		assertNotNull(deniedArray);
 		assertEquals("jcr:write", deniedArray.getString(0));
 
 		if (addGroupAce) {
-			aceString = jsonObj.getString(testGroupId);
-			assertNotNull(aceString);
-
-			aceObject = new JSONObject(aceString);
+			aceObject = jsonArray.optJSONObject(1);
 			assertNotNull(aceObject);
+			
+			principalString = aceObject.optString("principal");
+			assertEquals(testGroupId, principalString);
 
-			grantedArray = aceObject.getJSONArray("granted");
+			grantedArray = aceObject.optJSONArray("granted");
 			assertNotNull(grantedArray);
 			assertEquals("jcr:read", grantedArray.getString(0));
 		}
@@ -142,8 +149,9 @@ public class RemoveAcesTest extends Abst
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
 
-		JSONObject jsonObj = new JSONObject(json);
-		assertTrue(jsonObj.isNull(testUserId));
+		JSONArray jsonArray = new JSONArray(json);
+		assertNotNull(jsonArray);
+		assertEquals(0, jsonArray.length());
 	}
 
 	//test removing multiple aces
@@ -164,8 +172,8 @@ public class RemoveAcesTest extends Abst
 		String json = getAuthenticatedContent(creds, getUrl, CONTENT_TYPE_JSON, null, HttpServletResponse.SC_OK);
 		assertNotNull(json);
 
-		JSONObject jsonObj = new JSONObject(json);
-		assertTrue(jsonObj.isNull(testUserId));
-		assertTrue(jsonObj.isNull(testGroupId));
+		JSONArray jsonArray = new JSONArray(json);
+		assertNotNull(jsonArray);
+		assertEquals(0, jsonArray.length());
 	}
 }