diff --git a/source/java/org/alfresco/rest/api/Nodes.java b/source/java/org/alfresco/rest/api/Nodes.java index f11814a9a2..257685269f 100644 --- a/source/java/org/alfresco/rest/api/Nodes.java +++ b/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 mapFromNodeAspects(Set nodeAspects); + List mapFromNodeAspects(Set nodeAspects, List excludedAspects); /** * Add aspects to the specified NodeRef. Aspects that appear in the exclusions list @@ -300,6 +301,16 @@ public interface Nodes * @param exclusions */ void addCustomAspects(NodeRef nodeRef, List aspectNames, List exclusions); + + /** + * Update aspects for the specified NodeRef. An empty list will result in + * aspects being removed. + * + * @param nodeRef + * @param aspectNames + * @param exclusions + */ + void updateCustomAspects(NodeRef nodeRef, List aspectNames, List exclusions); /** * API Constants - query parameters, etc diff --git a/source/java/org/alfresco/rest/api/impl/NodesImpl.java b/source/java/org/alfresco/rest/api/impl/NodesImpl.java index 18a2dc873a..ae05652ddd 100644 --- a/source/java/org/alfresco/rest/api/impl/NodesImpl.java +++ b/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 props = null; @@ -1164,13 +1164,13 @@ public class NodesImpl implements Nodes return props; } - public List mapFromNodeAspects(Set nodeAspects) + public List mapFromNodeAspects(Set nodeAspects, List excludedAspects) { List 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 aspectNames, List exclusions) + public void addCustomAspects(NodeRef nodeRef, List aspectNames, List excludedAspects) { if (aspectNames == null) { @@ -1771,7 +1771,7 @@ public class NodesImpl implements Nodes Set 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 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 aspectNames, List 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,45 +2292,8 @@ 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) { String targetParentId = parentNodeRef.getId(); @@ -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 createQNames(List qnameStrList) + protected List createQNames(List qnameStrList, List 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); } diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index ce3dbba74c..457112a4c8 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/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 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); } diff --git a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java b/source/test-java/org/alfresco/rest/api/tests/TestPeople.java index 6ea081dde6..774094bf2b 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestPeople.java +++ b/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,7 +102,14 @@ public class TestPeople extends EnterpriseTestApi public void tearDown() { // Restore authentication to pre-test state. - AuthenticationUtil.popAuthentication(); + try + { + AuthenticationUtil.popAuthentication(); + } + catch(EmptyStackException e) + { + // Nothing to do. + } } private TestNetwork createNetwork(String networkPrefix) @@ -574,51 +582,142 @@ 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 props = new HashMap<>(); - props.put("cm:title", "Initial title"); - person.setProperties(props); - person = people.create(person); - assertEquals("Initial title", person.getProperties().get("cm:title")); - assertTrue(person.getAspectNames().contains("cm:titled")); - - // Update property - person.getProperties().put("cm:title", "Updated title"); + // 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); - // 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); - assertEquals("Updated title", person.getProperties().get("cm:title")); - assertTrue(person.getAspectNames().contains("cm:titled")); + // 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")); + } - // Remove property - person.getProperties().put("cm:title", null); + // Simple update of properties + { + Person person = createTestUpdatePerson(); + person = people.update(person.getId(), qjson("{`properties`: {`cm:title`: `Updated title`}}"), 200); + + // Property updated + assertEquals("Updated title", person.getProperties().get("cm:title")); + // Aspect untouched + assertTrue(person.getAspectNames().contains("cm:titled")); + } + + // Update with zero aspects - clear them all, except for protected items. + { + Person person = createTestUpdatePerson(); + person = people.update(person.getId(), qjson("{`aspectNames`: []}"), 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 aspectNames = person.getAspectNames(); + assertTrue(aspectNames.contains("cm:auditable")); + + // 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 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); - // 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 aspectNames = person.getAspectNames(); + assertTrue(aspectNames.contains("cm:auditable")); + + // Newly added aspects + assertTrue(aspectNames.contains("cm:dublincore")); + assertTrue(aspectNames.contains("cm:summarizable")); + } - person.setUserName(null); - person = people.update(personId, person); - - 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")); + // 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 backticks instead. + *

+ * Double and single quotes may still be used as normal, if required. + * + * @param raw The untreated JSON string to munge + * @return JSON String with backticks replaced with double quotes. + */ + private String qjson(String raw) + { + return raw.replace("`", "\""); } public static class PersonJSONSerializer implements JSONAble diff --git a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java b/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java index 51c9846970..c025012b6b 100644 --- a/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java +++ b/source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java @@ -1086,11 +1086,16 @@ 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; } - + public Person create(Person person) throws PublicApiException { return create(person, 201);