From 2b1c7382d19c95816062ceb7852e45102b3c464b Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Wed, 11 Jan 2017 10:34:24 +0000 Subject: [PATCH] Merged 5.2.0 (5.2.0) to HEAD (5.2) 133845 rmunteanu: REPO-1746: Merge fixes for 5.2 GA issues to 5.2.0 branch Merged 5.2.N (5.2.1) to 5.2.0 (5.2.0) 133290 jvonka: REPO-1646: V1 REST API - cannot unset optional fields (eg. when updating person / site details ...) - part 1 (Update Site) git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@134184 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org/alfresco/rest/api/impl/SitesImpl.java | 6 +-- .../org/alfresco/rest/api/model/Site.java | 54 +++++++++++++------ .../alfresco/rest/api/model/SiteUpdate.java | 26 +++++++-- .../rest/api/sites/SiteEntityResource.java | 32 +++++++---- .../alfresco/rest/api/tests/TestSites.java | 13 +++-- 5 files changed, 97 insertions(+), 34 deletions(-) diff --git a/source/java/org/alfresco/rest/api/impl/SitesImpl.java b/source/java/org/alfresco/rest/api/impl/SitesImpl.java index 111714d536..727efd64f2 100644 --- a/source/java/org/alfresco/rest/api/impl/SitesImpl.java +++ b/source/java/org/alfresco/rest/api/impl/SitesImpl.java @@ -1179,15 +1179,15 @@ public class SitesImpl implements Sites } // Bind any provided values to the site info, allowing for "partial" updates. - if (update.getTitle() != null) + if (update.wasSet(Site.TITLE)) { siteInfo.setTitle(update.getTitle()); } - if (update.getDescription() != null) + if (update.wasSet(Site.DESCRIPTION)) { siteInfo.setDescription(update.getDescription()); } - if (update.getVisibility() != null) + if (update.wasSet(Site.VISIBILITY)) { siteInfo.setVisibility(update.getVisibility()); } diff --git a/source/java/org/alfresco/rest/api/model/Site.java b/source/java/org/alfresco/rest/api/model/Site.java index 2046562dac..83dd539e4b 100644 --- a/source/java/org/alfresco/rest/api/model/Site.java +++ b/source/java/org/alfresco/rest/api/model/Site.java @@ -29,6 +29,9 @@ import org.alfresco.rest.framework.resource.UniqueId; import org.alfresco.service.cmr.site.SiteInfo; import org.alfresco.service.cmr.site.SiteVisibility; +import java.util.HashMap; +import java.util.Map; + /** * Represents a site. * @@ -37,8 +40,6 @@ import org.alfresco.service.cmr.site.SiteVisibility; */ public class Site implements Comparable { - public static final String ROLE = "role"; - protected String id; // site id (aka short name) protected String guid; // site nodeId protected String title; @@ -47,23 +48,33 @@ public class Site implements Comparable protected String preset; protected String role; + private Map setFields = new HashMap<>(7); + + public static final String ID = "id"; + public static final String GUID = "guid"; + public static final String TITLE = "title"; + public static final String DESCRIPTION = "description"; + public static final String VISIBILITY = "visibility"; + public static final String PRESET = "preset"; + public static final String ROLE = "role"; + public Site() { } - public Site(SiteInfo siteInfo, String role) + public Site(SiteInfo siteInfo, String role) { - if (siteInfo == null) + if (siteInfo == null) { throw new IllegalArgumentException("Must provide siteInfo"); } - this.id = siteInfo.getShortName(); - this.guid = siteInfo.getNodeRef().getId(); - this.title = siteInfo.getTitle(); - this.description = siteInfo.getDescription(); - this.visibility = siteInfo.getVisibility(); - this.preset = siteInfo.getSitePreset(); - this.role = role; + setId(siteInfo.getShortName()); + setGuid(siteInfo.getNodeRef().getId()); + setTitle(siteInfo.getTitle()); + setDescription(siteInfo.getDescription()); + setVisibility(siteInfo.getVisibility()); + setPreset(siteInfo.getSitePreset()); + setRole(role); } @UniqueId @@ -75,6 +86,7 @@ public class Site implements Comparable public void setId(String id) { this.id = id; + setFields.put(ID, true); } public String getGuid() @@ -85,6 +97,7 @@ public class Site implements Comparable public void setGuid(String guid) { this.guid = guid; + setFields.put(GUID, true); } public String getTitle() @@ -95,6 +108,7 @@ public class Site implements Comparable public void setTitle(String title) { this.title = title; + setFields.put(TITLE, true); } public String getDescription() @@ -105,6 +119,7 @@ public class Site implements Comparable public void setDescription(String description) { this.description = description; + setFields.put(DESCRIPTION, true); } public SiteVisibility getVisibility() @@ -115,16 +130,18 @@ public class Site implements Comparable public void setVisibility(SiteVisibility visibility) { this.visibility = visibility; + setFields.put(VISIBILITY, true); } public String getPreset() { return preset; } - + public void setPreset(String preset) { this.preset = preset; + setFields.put(PRESET, true); } public String getRole() @@ -135,6 +152,13 @@ public class Site implements Comparable public void setRole(String role) { this.role = role; + setFields.put(ROLE, true); + } + + public boolean wasSet(String fieldName) + { + Boolean b = setFields.get(fieldName); + return (b != null ? b : false); } @Override @@ -144,17 +168,17 @@ public class Site implements Comparable { return true; } - + if (obj == null) { return false; } - + if (getClass() != obj.getClass()) { return false; } - + Site other = (Site) obj; return id.equals(other.id); } diff --git a/source/java/org/alfresco/rest/api/model/SiteUpdate.java b/source/java/org/alfresco/rest/api/model/SiteUpdate.java index a06515fa6e..1d5a48e6d6 100644 --- a/source/java/org/alfresco/rest/api/model/SiteUpdate.java +++ b/source/java/org/alfresco/rest/api/model/SiteUpdate.java @@ -28,6 +28,8 @@ package org.alfresco.rest.api.model; import org.alfresco.service.cmr.site.SiteVisibility; import java.io.Serializable; +import java.util.HashMap; +import java.util.Map; /** * Class representing a site update API operation. @@ -42,12 +44,21 @@ public class SiteUpdate implements Serializable private String description; private SiteVisibility visibility; + private Map setFields = new HashMap<>(3); + + public static final String TITLE = "title"; + public static final String DESCRIPTION = "description"; + public static final String VISIBILITY = "visibility"; + + public SiteUpdate() + { + } public SiteUpdate(String title, String description, SiteVisibility visibility) { - this.title = title; - this.description = description; - this.visibility = visibility; + setTitle(title); + setDescription(description); + setVisibility(visibility); } public String getTitle() @@ -58,6 +69,7 @@ public class SiteUpdate implements Serializable public void setTitle(String title) { this.title = title; + setFields.put(TITLE, true); } public String getDescription() @@ -68,6 +80,7 @@ public class SiteUpdate implements Serializable public void setDescription(String description) { this.description = description; + setFields.put(DESCRIPTION, true); } public SiteVisibility getVisibility() @@ -78,6 +91,13 @@ public class SiteUpdate implements Serializable public void setVisibility(SiteVisibility visibility) { this.visibility = visibility; + setFields.put(VISIBILITY, true); + } + + public boolean wasSet(String fieldName) + { + Boolean b = setFields.get(fieldName); + return (b != null ? b : false); } @Override diff --git a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java index ea82313d64..accd3d40ae 100644 --- a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java +++ b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java @@ -132,35 +132,49 @@ public class SiteEntityResource implements EntityResourceAction.Read, @Override @WebApiDescription(title="Update site", description="Update the Share site") public Site update(String siteId, Site site, Parameters parameters) + { + return sites.updateSite(siteId, convert(site), parameters); + } + + protected SiteUpdate convert(Site site) { // Until REPO-110 is solved, we need to explicitly test for the presence of fields // on the Site object that aren't valid SiteUpdate fields. Once REPO-110 is solved, // the update method will take a SiteUpdate as a parameter rather than a Site // and only the correct fields will be exposed. Any attempt to access illegal fields // should then result in the framework returning a 400 automatically. - if (site.getId() != null) + if (site.wasSet(Site.ID)) { throw new InvalidArgumentException("Site update does not support field: id"); } - if (site.getGuid() != null) + if (site.wasSet(Site.GUID)) { throw new InvalidArgumentException("Site update does not support field: guid"); } - if (site.getRole() != null) + if (site.wasSet(Site.ROLE)) { throw new InvalidArgumentException("Site update does not support field: role"); } - if (site.getPreset() != null) + if (site.wasSet(Site.PRESET)) { throw new InvalidArgumentException("Site update does not support field: preset"); } // Bind valid fields to a SiteUpdate instance. - final String title = site.getTitle(); - final String description = site.getDescription(); - final SiteVisibility visibility = site.getVisibility(); - SiteUpdate update = new SiteUpdate(title, description, visibility); + SiteUpdate siteUpdate = new SiteUpdate(); + if (site.wasSet(Site.TITLE)) + { + siteUpdate.setTitle(site.getTitle()); + } + if (site.wasSet(Site.DESCRIPTION)) + { + siteUpdate.setDescription(site.getDescription()); + } + if (site.wasSet(Site.VISIBILITY)) + { + siteUpdate.setVisibility(site.getVisibility()); + } - return sites.updateSite(siteId, update, parameters); + return siteUpdate; } } diff --git a/source/test-java/org/alfresco/rest/api/tests/TestSites.java b/source/test-java/org/alfresco/rest/api/tests/TestSites.java index 3f5ee5418b..27bacd5aa6 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestSites.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestSites.java @@ -30,7 +30,6 @@ import static org.junit.Assert.fail; import java.util.*; -import org.alfresco.repo.model.filefolder.HiddenAspect; import org.alfresco.repo.tenant.TenantUtil; import org.alfresco.repo.tenant.TenantUtil.TenantRunAsWork; import org.alfresco.rest.api.model.SiteUpdate; @@ -45,10 +44,10 @@ import org.alfresco.rest.api.tests.client.RequestContext; import org.alfresco.rest.api.tests.client.data.*; import org.alfresco.service.cmr.site.SiteVisibility; import org.alfresco.util.GUID; -import org.apache.commons.collections.map.HashedMap; import org.apache.commons.httpclient.HttpStatus; import org.json.simple.JSONArray; import org.json.simple.JSONObject; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -541,9 +540,9 @@ public class TestSites extends EnterpriseTestApi } // +ve test: remove the description. - // This is a workaround for being unable to eplicitly null the field (see REPO-1409) + // This is a workaround for being unable to explicitly null the field (see REPO-1409) { - Site site = sitesProxy.createSite(new SiteImpl("NoDescriptionTest", SiteVisibility.PUBLIC.toString()), 201); + Site site = sitesProxy.createSite(new SiteImpl(null, "NoDescriptionTest", "Initial description", SiteVisibility.PUBLIC.toString()), 201); Site updatedSite = sitesProxy.updateSite( site.getSiteId(), @@ -566,10 +565,16 @@ public class TestSites extends EnterpriseTestApi // Check the return from updateSite(...) expectedUpdate.expected(updatedSite); + // TODO improve expected (and/or AssertUtil) and affected test cases + Assert.assertNull(updatedSite.getDescription()); + // Double check a fresh retrieval matches. Site fresh = sitesProxy.getSite(site.getSiteId(), 200); expectedUpdate.expected(fresh); + // TODO improve expected (and/or AssertUtil) and affected test cases + Assert.assertNull(fresh.getDescription()); + removeSiteQuietly(site.getSiteId()); }