Merged 5.2.0 (5.2.0) to HEAD (5.2)

132872 mward: Merged mward/5.2.n-custpeopleprops (5.2.1) to 5.2.N (5.2.1)
      132803 mward: REPO-1395: add further tests and tweak functionality.


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

source/java/org/alfresco/rest/api/Nodes.java

@@ -287,9 +287,10 @@ public interface Nodes
      * by the API JSON response for get nodes, get person etc.
      * 
      * @param nodeAspects
+     * @param excludedAspects
      * @return
      */
-    List<String> mapFromNodeAspects(Set<QName> nodeAspects);
+    List<String> mapFromNodeAspects(Set<QName> nodeAspects, List<QName> excludedAspects);
 
     /**
      * Add aspects to the specified NodeRef. Aspects that appear in the exclusions list
@@ -301,6 +302,16 @@ public interface Nodes
      */
     void addCustomAspects(NodeRef nodeRef, List<String> aspectNames, List<QName> exclusions);
 
+    /**
+     * Update aspects for the specified NodeRef. An empty list will result in
+     * aspects being <strong>removed</strong>.
+     * 
+     * @param nodeRef
+     * @param aspectNames
+     * @param exclusions
+     */
+    void updateCustomAspects(NodeRef nodeRef, List<String> aspectNames, List<QName> exclusions);
+    
     /**
      * API Constants - query parameters, etc
      */

source/java/org/alfresco/rest/api/impl/NodesImpl.java

@@ -895,7 +895,7 @@ public class NodesImpl implements Nodes
         if (includeParam.contains(PARAM_INCLUDE_ASPECTNAMES))
         {
             aspects = nodeService.getAspects(nodeRef);
-            node.setAspectNames(mapFromNodeAspects(aspects));
+            node.setAspectNames(mapFromNodeAspects(aspects, EXCLUDED_ASPECTS));
         }
 
         if (includeParam.contains(PARAM_INCLUDE_ISLINK))
@@ -1135,7 +1135,7 @@ public class NodesImpl implements Nodes
         else
         {
             // return selected properties
-            selectedProperties = createQNames(selectParam);
+            selectedProperties = createQNames(selectParam, excludedProps);
         }
 
         Map<String, Object> props = null;
@@ -1164,13 +1164,13 @@ public class NodesImpl implements Nodes
         return props;
     }
 
-    public List<String> mapFromNodeAspects(Set<QName> nodeAspects)
+    public List<String> mapFromNodeAspects(Set<QName> nodeAspects, List<QName> excludedAspects)
     {
         List<String> aspectNames = new ArrayList<>(nodeAspects.size());
 
         for (QName aspectQName : nodeAspects)
         {
-            if ((! EXCLUDED_NS.contains(aspectQName.getNamespaceURI())) && (! EXCLUDED_ASPECTS.contains(aspectQName)))
+            if ((! EXCLUDED_NS.contains(aspectQName.getNamespaceURI())) && (! excludedAspects.contains(aspectQName)))
             {
                 aspectNames.add(aspectQName.toPrefixString(namespaceService));
             }
@@ -1761,7 +1761,7 @@ public class NodesImpl implements Nodes
         return newNode;
     }
 
-    public void addCustomAspects(NodeRef nodeRef, List<String> aspectNames, List<QName> exclusions)
+    public void addCustomAspects(NodeRef nodeRef, List<String> aspectNames, List<QName> excludedAspects)
     {
         if (aspectNames == null)
         {
@@ -1771,7 +1771,7 @@ public class NodesImpl implements Nodes
         Set<QName> aspectQNames = mapToNodeAspects(aspectNames);
         for (QName aspectQName : aspectQNames)
         {
-            if (exclusions.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
+            if (excludedAspects.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
             {
                 continue; // ignore
             }
@@ -2181,6 +2181,48 @@ public class NodesImpl implements Nodes
         }
 
         List<String> aspectNames = nodeInfo.getAspectNames();
+        updateCustomAspects(nodeRef, aspectNames, EXCLUDED_ASPECTS);
+
+        if (props.size() > 0)
+        {
+            validatePropValues(props);
+
+            try
+            {
+                // update node properties - note: null will unset the specified property
+                nodeService.addProperties(nodeRef, props);
+            }
+            catch (DuplicateChildNodeNameException dcne)
+            {
+                throw new ConstraintViolatedException(dcne.getMessage());
+            }
+        }
+        
+        return nodeRef;
+    }
+
+    @Override
+    public Node moveOrCopyNode(String sourceNodeId, String targetParentId, String name, Parameters parameters, boolean isCopy)
+    {
+        if ((sourceNodeId == null) || (sourceNodeId.isEmpty()))
+        {
+            throw new InvalidArgumentException("Missing sourceNodeId");
+        }
+
+        if ((targetParentId == null) || (targetParentId.isEmpty()))
+        {
+            throw new InvalidArgumentException("Missing targetParentId");
+        }
+
+        final NodeRef parentNodeRef = validateOrLookupNode(targetParentId, null);
+        final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null);
+
+        FileInfo fi = moveOrCopyImpl(sourceNodeRef, parentNodeRef, name, isCopy);
+        return getFolderOrDocument(fi.getNodeRef().getId(), parameters);
+    }
+    
+    public void updateCustomAspects(NodeRef nodeRef, List<String> aspectNames, List<QName> excludedAspects)
+    {
         if (aspectNames != null)
         {
             // update aspects - note: can be empty (eg. to remove existing aspects+properties) but not cm:auditable, sys:referencable, sys:localized
@@ -2194,7 +2236,7 @@ public class NodesImpl implements Nodes
 
             for (QName aspectQName : aspectQNames)
             {
-                if (EXCLUDED_NS.contains(aspectQName.getNamespaceURI()) || EXCLUDED_ASPECTS.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
+                if (EXCLUDED_NS.contains(aspectQName.getNamespaceURI()) || excludedAspects.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
                 {
                     continue; // ignore
                 }
@@ -2207,7 +2249,7 @@ public class NodesImpl implements Nodes
 
             for (QName existingAspect : existingAspects)
             {
-                if (EXCLUDED_NS.contains(existingAspect.getNamespaceURI()) || EXCLUDED_ASPECTS.contains(existingAspect) || existingAspect.equals(ContentModel.ASPECT_AUDITABLE))
+                if (EXCLUDED_NS.contains(existingAspect.getNamespaceURI()) || excludedAspects.contains(existingAspect) || existingAspect.equals(ContentModel.ASPECT_AUDITABLE))
                 {
                     continue; // ignore
                 }
@@ -2250,43 +2292,6 @@ public class NodesImpl implements Nodes
                 nodeService.addAspect(nodeRef, aQName, null);
             }
         }
-
-        if (props.size() > 0)
-        {
-            validatePropValues(props);
-
-            try
-            {
-                // update node properties - note: null will unset the specified property
-                nodeService.addProperties(nodeRef, props);
-            }
-            catch (DuplicateChildNodeNameException dcne)
-            {
-                throw new ConstraintViolatedException(dcne.getMessage());
-            }
-        }
-        
-        return nodeRef;
-    }
-
-    @Override
-    public Node moveOrCopyNode(String sourceNodeId, String targetParentId, String name, Parameters parameters, boolean isCopy)
-    {
-        if ((sourceNodeId == null) || (sourceNodeId.isEmpty()))
-        {
-            throw new InvalidArgumentException("Missing sourceNodeId");
-        }
-
-        if ((targetParentId == null) || (targetParentId.isEmpty()))
-        {
-            throw new InvalidArgumentException("Missing targetParentId");
-        }
-
-        final NodeRef parentNodeRef = validateOrLookupNode(targetParentId, null);
-        final NodeRef sourceNodeRef = validateOrLookupNode(sourceNodeId, null);
-
-        FileInfo fi = moveOrCopyImpl(sourceNodeRef, parentNodeRef, name, isCopy);
-        return getFolderOrDocument(fi.getNodeRef().getId(), parameters);
     }
     
     protected FileInfo moveOrCopyImpl(NodeRef nodeRef, NodeRef parentNodeRef, String name, boolean isCopy)
@@ -3018,9 +3023,10 @@ public class NodesImpl implements Nodes
      * Helper to create a QName from either a fully qualified or short-name QName string
      *
      * @param qnameStrList list of fully qualified or short-name QName string
+     * @param excludedProps
      * @return a list of {@code QName} objects
      */
-    protected List<QName> createQNames(List<String> qnameStrList)
+    protected List<QName> createQNames(List<String> qnameStrList, List<QName> excludedProps)
     {
         String PREFIX = PARAM_INCLUDE_PROPERTIES +"/";
 
@@ -3033,7 +3039,7 @@ public class NodesImpl implements Nodes
             }
 
             QName name = createQName(str);
-            if (!EXCLUDED_PROPS.contains(name))
+            if (!excludedProps.contains(name))
             {
                 result.add(name);
             }

source/java/org/alfresco/rest/api/impl/PeopleImpl.java

@@ -389,7 +389,7 @@ public class PeopleImpl implements People
             person.setProperties(custProps);
             // Expose aspect names
             Set<QName> aspects = nodeService.getAspects(personNode);
-            person.setAspectNames(nodes.mapFromNodeAspects(aspects));
+            person.setAspectNames(nodes.mapFromNodeAspects(aspects, EXCLUDED_ASPECTS));
             
             // get avatar information
             if (hasAvatar(personNode))
@@ -547,8 +547,8 @@ public class PeopleImpl implements People
 		
         personService.setPersonProperties(personIdToUpdate, properties, false);
 
-		// Add custom aspects
-		nodes.addCustomAspects(personNodeRef, person.getAspectNames(), EXCLUDED_ASPECTS);
+		// Update custom aspects
+		nodes.updateCustomAspects(personNodeRef, person.getAspectNames(), EXCLUDED_ASPECTS);
 		
         return getPerson(personId);
     }

source/test-java/org/alfresco/rest/api/tests/TestPeople.java

@@ -42,6 +42,8 @@ import org.alfresco.rest.api.tests.client.RequestContext;
 import org.alfresco.rest.api.tests.client.data.Company;
 import org.alfresco.rest.api.tests.client.data.JSONAble;
 import org.alfresco.rest.api.tests.client.data.Person;
+import org.alfresco.service.cmr.dictionary.CustomModelService;
+import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.security.PersonService;
@@ -51,7 +53,6 @@ import org.apache.commons.httpclient.HttpStatus;
 import org.json.simple.JSONObject;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class TestPeople extends EnterpriseTestApi
@@ -101,8 +102,15 @@ public class TestPeople extends EnterpriseTestApi
 	public void tearDown()
 	{
 		// Restore authentication to pre-test state.
+		try
+		{
 			AuthenticationUtil.popAuthentication();
 		}
+		catch(EmptyStackException e)
+		{
+			// Nothing to do.
+		}
+	}
 
     private TestNetwork createNetwork(String networkPrefix)
     {
@@ -574,52 +582,143 @@ public class TestPeople extends EnterpriseTestApi
 		assertEquals("This is a title", retProps.get(ContentModel.PROP_TITLE));
 	}
 
+	// Create a person for use in the testing of updating custom aspects/props
+	private Person createTestUpdatePerson() throws PublicApiException
+	{
+		Person person = new Person();
+		String personId = UUID.randomUUID().toString()+"@"+account1.getId();
+		person.setUserName(personId);
+		person.setFirstName("Joe");
+		person.setEmail(personId);
+		person.setEnabled(true);
+		person.setPassword("password123");
+		person.setDescription("This is a very short bio.");
+		person.setProperties(Collections.singletonMap("cm:title", "Initial title"));
+		person.setAspectNames(Collections.singletonList("cm:projectsummary"));
+
+		person = people.create(person);
+
+		AuthenticationUtil.setFullyAuthenticatedUser("admin@"+account1.getId());
+		NodeService nodeService = applicationContext.getBean("NodeService", NodeService.class);
+		PersonService personService = applicationContext.getBean("PersonService", PersonService.class);
+		NodeRef nodeRef = personService.getPerson(person.getId());
+		nodeService.addAspect(nodeRef, ContentModel.ASPECT_AUDITABLE, null);
+		
+		assertEquals("Initial title", person.getProperties().get("cm:title"));
+		assertTrue(person.getAspectNames().contains("cm:titled"));
+		assertTrue(person.getAspectNames().contains("cm:projectsummary"));
+		assertTrue(nodeService.hasAspect(nodeRef, ContentModel.ASPECT_AUDITABLE));
+		return person;
+	}
+	
 	@Test
 	public void testUpdatePerson_withCustomProps() throws Exception
 	{
 		publicApiClient.setRequestContext(new RequestContext(account1.getId(), account1Admin, "admin"));
-		Person person = new Person();
-		String personId = "jbloggs2@"+account1.getId();
-		person.setUserName(personId);
-		person.setFirstName("Joe");
-		person.setEmail("jbloggs2@"+account1.getId());
-		person.setEnabled(true);
-		person.setPassword("password123");
-		Map<String, Object> props = new HashMap<>();
-		props.put("cm:title", "Initial title");
-		person.setProperties(props);
 		
-		person = people.create(person);
+		// Add a property
+		{
+			Person person = createTestUpdatePerson();
+			assertNull(person.getProperties().get("cm:middleName"));
+			String json = qjson(
+					"{" +
+					"    `properties`: {" +
+					"        `cm:middleName`: `Bertrand`" +
+					"    }" +
+					"}"
+			);
+			person = people.update(person.getId(), json, 200);
+
+			// Property added
+			assertEquals("Bertrand", person.getProperties().get("cm:middleName"));
 			assertEquals("Initial title", person.getProperties().get("cm:title"));
+			// Aspect untouched
 			assertTrue(person.getAspectNames().contains("cm:titled"));
+		}
 		
-		// Update property
-		person.getProperties().put("cm:title", "Updated title");
+		// Simple update of properties
+		{
+			Person person = createTestUpdatePerson();
+			person = people.update(person.getId(), qjson("{`properties`: {`cm:title`: `Updated title`}}"), 200);
 			
-		// ID/UserName is not a valid field for update.
-		person.setUserName(null);
-		// TODO: We don't want to attempt to set ownable using the text available?! ...it won't work
-		person.getProperties().remove("cm:owner");
-		person.getAspectNames().clear();
-		person = people.update(personId, person);
+			// Property updated
 			assertEquals("Updated title", person.getProperties().get("cm:title"));
+			// Aspect untouched
 			assertTrue(person.getAspectNames().contains("cm:titled"));
+		}
 		
-		// Remove property
-		person.getProperties().put("cm:title", null);
+		// Update with zero aspects - clear them all, except for protected items.
+		{
+			Person person = createTestUpdatePerson();
+			person = people.update(person.getId(), qjson("{`aspectNames`: []}"), 200);
 			
-		// TODO: We don't want to attempt to set ownable using the text available?! ...it won't work
-		person.getProperties().remove("cm:owner");
-		person.getAspectNames().clear();
+			// Aspect should no longer be present.
+			assertFalse(person.getAspectNames().contains("cm:titled"));
+			assertFalse(person.getProperties().containsKey("cm:title"));
+			// Protected aspects should still be present.
+			List<String> aspectNames = person.getAspectNames();
+			assertTrue(aspectNames.contains("cm:auditable"));
 			
-		person.setUserName(null);
-		person = people.update(personId, person);
+			// Check for the protected (but filtered) sys:* properties
+			NodeService nodeService = applicationContext.getBean("NodeService", NodeService.class);
+			PersonService personService = applicationContext.getBean("PersonService", PersonService.class);
+			NodeRef nodeRef = personService.getPerson(person.getId());
+			Set<QName> aspects = nodeService.getAspects(nodeRef);
+			assertTrue(aspects.contains(ContentModel.ASPECT_REFERENCEABLE));
+			assertTrue(aspects.contains(ContentModel.ASPECT_LOCALIZED));
+		}
+		
+		// Set aspects - all except protected items will be replaced with those presented.
+		{
+			Person person = createTestUpdatePerson();
+			String json = qjson(
+					"{" +
+					"    `aspectNames`: [" +
+					"        `cm:dublincore`," +
+					"        `cm:summarizable`" +
+					"    ]" +
+					"}"
+			);
+			person = people.update(person.getId(), json, 200);
+
+			// Aspect should no longer be present.
+			assertFalse(person.getAspectNames().contains("cm:titled"));
+			assertFalse(person.getProperties().containsKey("cm:title"));
+			// Protected aspects should still be present.
+			List<String> aspectNames = person.getAspectNames();
+			assertTrue(aspectNames.contains("cm:auditable"));
+			
+			// Newly added aspects
+			assertTrue(aspectNames.contains("cm:dublincore"));
+			assertTrue(aspectNames.contains("cm:summarizable"));
+		}
+		
+		// Remove a property by setting it to null
+		{
+			Person person = createTestUpdatePerson();
+			person = people.update(person.getId(), qjson("{`properties`: {`cm:title`: null}}"), 200);
 			
 			assertFalse(person.getProperties().containsKey("cm:title"));
 			// The aspect will still be there, I don't think we can easily remove the aspect automatically
 			// just because the associated properties have all been removed.
 			assertTrue(person.getAspectNames().contains("cm:titled"));
 		}
+	}
+
+	/**
+	 * Simple helper to make JSON literals a little easier to read in test code,
+	 * by allowing values that would normally be quoted with double quotes, to be
+	 * quoted with <strong>backticks</strong> instead.
+	 * <p>
+	 * Double and single quotes may still be used as normal, if required.
+	 * 
+	 * @param raw    The untreated JSON string to munge
+	 * @return JSON String with <strong>backticks</strong> replaced with double quotes.   
+	 */
+	private String qjson(String raw)
+	{
+		return raw.replace("`", "\"");
+	}
 	
 	public static class PersonJSONSerializer implements JSONAble
 	{

source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java

@@ -1086,7 +1086,12 @@ public class PublicApiClient
 		
 		public Person update(String personId, Person person, int expectedStatus) throws PublicApiException
 		{
-			HttpResponse response = update("people", personId, null, null, person.toJSON(true).toString(), null, "Failed to update person", expectedStatus);
+			return update(personId, person.toJSON(true).toString(), expectedStatus);
+		}
+
+		public Person update(String personId, String json, int expectedStatus) throws PublicApiException
+		{
+			HttpResponse response = update("people", personId, null, null, json, null, "Failed to update person", expectedStatus);
 			Person retSite = Person.parsePerson((JSONObject)response.getJsonResponse().get("entry"));
 			return retSite;
 		}