diff --git a/source/java/org/alfresco/rest/api/Sites.java b/source/java/org/alfresco/rest/api/Sites.java index a9d507022d..db030416f8 100644 --- a/source/java/org/alfresco/rest/api/Sites.java +++ b/source/java/org/alfresco/rest/api/Sites.java @@ -26,11 +26,7 @@ package org.alfresco.rest.api; import org.alfresco.query.PagingResults; -import org.alfresco.rest.api.model.FavouriteSite; -import org.alfresco.rest.api.model.MemberOfSite; -import org.alfresco.rest.api.model.Site; -import org.alfresco.rest.api.model.SiteContainer; -import org.alfresco.rest.api.model.SiteMember; +import org.alfresco.rest.api.model.*; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; import org.alfresco.rest.framework.resource.parameters.Paging; import org.alfresco.rest.framework.resource.parameters.Parameters; @@ -45,7 +41,7 @@ public interface Sites Site getSite(String siteId); void deleteSite(String siteId, Parameters parameters); Site createSite(Site site, Parameters parameters); - Site updateSite(String siteId, Site site, Parameters parameters); + Site updateSite(String siteId, SiteUpdate site, Parameters parameters); /** * people//sites/ diff --git a/source/java/org/alfresco/rest/api/impl/SitesImpl.java b/source/java/org/alfresco/rest/api/impl/SitesImpl.java index 7a5137c1f0..49a70ad457 100644 --- a/source/java/org/alfresco/rest/api/impl/SitesImpl.java +++ b/source/java/org/alfresco/rest/api/impl/SitesImpl.java @@ -62,11 +62,7 @@ import org.alfresco.rest.antlr.WhereClauseParser; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.People; import org.alfresco.rest.api.Sites; -import org.alfresco.rest.api.model.FavouriteSite; -import org.alfresco.rest.api.model.MemberOfSite; -import org.alfresco.rest.api.model.Site; -import org.alfresco.rest.api.model.SiteContainer; -import org.alfresco.rest.api.model.SiteMember; +import org.alfresco.rest.api.model.*; import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; @@ -1141,13 +1137,14 @@ public class SitesImpl implements Sites } @Override - public Site updateSite(String siteId, Site update, Parameters parameters) + public Site updateSite(String siteId, SiteUpdate update, Parameters parameters) { if (logger.isDebugEnabled()) { logger.debug("Updating site, ID: "+siteId+", site data: "+update+", parameters: "+parameters); } + // Get the site by ID (aka short name) SiteInfo siteInfo = validateSite(siteId); if (siteInfo == null) { @@ -1155,16 +1152,19 @@ public class SitesImpl implements Sites throw new EntityNotFoundException(siteId); } - // Although this method will not update the site ID even if it is provided, we sanity - // check that no attempt is being made to alter it. - if (update.getId() != null && (!update.getId().equals(siteId))) + // Bind any provided values to the site info, allowing for "partial" updates. + if (update.getTitle() != null) { - throw new InvalidArgumentException("Site updates cannot change the site ID"); + siteInfo.setTitle(update.getTitle()); + } + if (update.getDescription() != null) + { + siteInfo.setDescription(update.getDescription()); + } + if (update.getVisibility() != null) + { + siteInfo.setVisibility(update.getVisibility()); } - - siteInfo.setTitle(update.getTitle()); - siteInfo.setDescription(update.getDescription()); - siteInfo.setVisibility(update.getVisibility()); // Validate the new details validateSite(new Site(siteInfo, null)); diff --git a/source/java/org/alfresco/rest/api/model/SiteUpdate.java b/source/java/org/alfresco/rest/api/model/SiteUpdate.java new file mode 100644 index 0000000000..a06515fa6e --- /dev/null +++ b/source/java/org/alfresco/rest/api/model/SiteUpdate.java @@ -0,0 +1,115 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2016 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.rest.api.model; + +import org.alfresco.service.cmr.site.SiteVisibility; + +import java.io.Serializable; + +/** + * Class representing a site update API operation. + * + * @author Matt Ward + * @since 5.2 + */ +public class SiteUpdate implements Serializable +{ + private static final long serialVersionUID = 1L; + private String title; + private String description; + private SiteVisibility visibility; + + + public SiteUpdate(String title, String description, SiteVisibility visibility) + { + this.title = title; + this.description = description; + this.visibility = visibility; + } + + public String getTitle() + { + return title; + } + + public void setTitle(String title) + { + this.title = title; + } + + public String getDescription() + { + return description; + } + + public void setDescription(String description) + { + this.description = description; + } + + public SiteVisibility getVisibility() + { + return visibility; + } + + public void setVisibility(SiteVisibility visibility) + { + this.visibility = visibility; + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + SiteUpdate that = (SiteUpdate) o; + + if (title != null ? !title.equals(that.title) : that.title != null) return false; + if (description != null ? !description.equals(that.description) : that.description != null) return false; + return visibility == that.visibility; + + } + + @Override + public int hashCode() + { + int result = title != null ? title.hashCode() : 0; + result = 31 * result + (description != null ? description.hashCode() : 0); + result = 31 * result + (visibility != null ? visibility.hashCode() : 0); + return result; + } + + @Override + public String toString() + { + return "SiteUpdate{" + + "title='" + title + '\'' + + ", description='" + description + '\'' + + ", visibility=" + visibility + + '}'; + } +} diff --git a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java index 089577d3fd..65052574b3 100644 --- a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java +++ b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java @@ -27,6 +27,7 @@ package org.alfresco.rest.api.sites; import org.alfresco.rest.api.Sites; import org.alfresco.rest.api.model.Site; +import org.alfresco.rest.api.model.SiteUpdate; import org.alfresco.rest.framework.WebApiDescription; import org.alfresco.rest.framework.WebApiParam; import org.alfresco.rest.framework.core.ResourceParameter; @@ -35,6 +36,7 @@ import org.alfresco.rest.framework.resource.EntityResource; import org.alfresco.rest.framework.resource.actions.interfaces.EntityResourceAction; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; import org.alfresco.rest.framework.resource.parameters.Parameters; +import org.alfresco.service.cmr.site.SiteVisibility; import org.alfresco.util.ParameterCheck; import org.springframework.beans.factory.InitializingBean; @@ -131,6 +133,12 @@ public class SiteEntityResource implements EntityResourceAction.Read, @WebApiDescription(title="Update site", description="Update the Share site") public Site update(String siteId, Site site, Parameters parameters) { - return sites.updateSite(siteId, site, parameters); + final String title = site.getTitle(); + final String description = site.getDescription(); + final SiteVisibility visibility = site.getVisibility(); + + SiteUpdate update = new SiteUpdate(title, description, visibility); + + return sites.updateSite(siteId, update, parameters); } } 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 64f93a5273..b978c441e3 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestSites.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestSites.java @@ -28,14 +28,12 @@ package org.alfresco.rest.api.tests; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +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; import org.alfresco.rest.api.tests.RepoService.TestNetwork; import org.alfresco.rest.api.tests.RepoService.TestSite; import org.alfresco.rest.api.tests.client.HttpResponse; @@ -44,10 +42,7 @@ import org.alfresco.rest.api.tests.client.PublicApiClient.Paging; import org.alfresco.rest.api.tests.client.PublicApiClient.Sites; import org.alfresco.rest.api.tests.client.PublicApiException; import org.alfresco.rest.api.tests.client.RequestContext; -import org.alfresco.rest.api.tests.client.data.Site; -import org.alfresco.rest.api.tests.client.data.SiteImpl; -import org.alfresco.rest.api.tests.client.data.SiteRole; -import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; +import org.alfresco.rest.api.tests.client.data.*; import org.alfresco.service.cmr.site.SiteVisibility; import org.alfresco.util.GUID; import org.apache.commons.httpclient.HttpStatus; @@ -501,29 +496,14 @@ public class TestSites extends EnterpriseTestApi Sites sitesProxy = publicApiClient.sites(); publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - final String initialTitle = "Initial Title"; - final String initialDescription = "This is the initial description for the site"; - - SiteImpl site = new SiteImpl(initialTitle, SiteVisibility.PRIVATE.toString()); - site.setRole(SiteRole.SiteManager); - site.setDescription(initialDescription); - - Site ret = sitesProxy.createSite(site, 201); - final String siteId = ret.getSiteId(); - ret = sitesProxy.getSite(siteId); - - final String initialSiteId = "initial-title"; - final Site siteExp = new SiteImpl(null, initialSiteId, ret.getGuid(), initialTitle, initialDescription, SiteVisibility.PRIVATE.toString(), null, null); - // Check that the retrieved details match the initially saved details. - siteExp.expected(ret); - - // Update the site details - final String updatedTitle = "Updated Title"; - final String updatedDescription = "This is an updated description for the site"; - final Site expectedUpdate = new SiteImpl(null, initialSiteId, ret.getGuid(), updatedTitle, updatedDescription, SiteVisibility.PUBLIC.toString(), null, SiteRole.SiteManager); // +ve test: simple happy path with successful update. { + SiteImpl siteToCreate = new SiteImpl("Initial Title", SiteVisibility.PRIVATE.toString()); + siteToCreate.setRole(SiteRole.SiteManager); + siteToCreate.setDescription("This is the initial description for the site"); + Site site = sitesProxy.createSite(siteToCreate, 201); + // Testing with the actual JSON reveals more than testing with rich POJOs. // It shows us exactly what the input is and is easy to compare to the Open API spec. String updateJSON = "{\n" + @@ -531,226 +511,211 @@ public class TestSites extends EnterpriseTestApi " \"description\": \"This is an updated description for the site\",\n" + " \"visibility\": \"PUBLIC\"\n" + "}"; - HttpResponse response = sitesProxy.update("sites", siteId, null, null, updateJSON, "Failed to update site " + siteId); + // Update the site details + HttpResponse response = sitesProxy.update("sites", site.getSiteId(), null, null, updateJSON, "Failed to update site " + site.getSiteId()); - final Site updatedSite = SiteImpl.parseSite((JSONObject) response.getJsonResponse().get("entry")); + Site updatedSite = SiteImpl.parseSite((JSONObject) response.getJsonResponse().get("entry")); + Site expectedUpdate = new SiteImpl(null, "initial-title", site.getGuid(), "Updated Title", "This is an updated description for the site", SiteVisibility.PUBLIC.toString(), null, SiteRole.SiteManager); + // Check the updated site is as expected. expectedUpdate.expected(updatedSite); // Double check by do a fresh get of the details, since the details returned by updateSite() // aren't necessarily retrieved from the DB. - final Site fresh = sitesProxy.getSite(siteId, 200); + Site fresh = sitesProxy.getSite(site.getSiteId(), 200); expectedUpdate.expected(fresh); + + removeSiteQuietly(site.getSiteId()); } - // -ve test: site title too long + // +ve test: remove the description. + // This is a workaround for being unable to eplicitly null the field (see REPO-1409) { - // Testing with the actual JSON reveals more than testing with rich POJOs. - // It shows us exactly what the input is and is easy to compare to the Open API spec. + Site site = sitesProxy.createSite(new SiteImpl("NoDescriptionTest", SiteVisibility.PUBLIC.toString()), 201); - final String longTitle = new String(new char[257]).replace('\0', 'a'); + Site updatedSite = sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate("title", "", SiteVisibility.PUBLIC), + 200); - SiteImpl badUpdate = new SiteImpl(null, siteId, null, longTitle, updatedDescription, null, null, null); - try - { - sitesProxy.updateSite(siteId, badUpdate); - fail("Expected an error response."); - } - catch (PublicApiException e) - { - assertEquals(400, e.getHttpResponse().getStatusCode()); - } + // Note the null value for description, as the framework + // treats empty strings and null as equivalent in responses, and does not return + // those fields (as per API guidelines) + Site expectedUpdate = new SiteImpl( + null, + "nodescriptiontest", + site.getGuid(), + "title", + null, + SiteVisibility.PUBLIC.toString(), + null, + SiteRole.SiteManager); + + // Check the return from updateSite(...) + expectedUpdate.expected(updatedSite); + + // Double check a fresh retrieval matches. + Site fresh = sitesProxy.getSite(site.getSiteId(), 200); + expectedUpdate.expected(fresh); + + removeSiteQuietly(site.getSiteId()); + } + + // +ve test: partial updates must be allowed + { + Site site = sitesProxy.createSite(new SiteImpl( + "partial-updates-test", + "title-v1", + "description-v1", + SiteVisibility.PUBLIC.toString()), + 201); + + // Check we can update just the title + Site updatedSite = sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate("title-v2", null, null), + 200); + + Site expectedUpdate = new SiteImpl( + null, + "partial-updates-test", + site.getGuid(), + "title-v2", + "description-v1", + SiteVisibility.PUBLIC.toString(), + null, + SiteRole.SiteManager); + + expectedUpdate.expected(updatedSite); + + // Check we can update just the description + updatedSite = sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate(null, "description-v2", null), + 200); + + expectedUpdate = new SiteImpl( + null, + "partial-updates-test", + site.getGuid(), + "title-v2", + "description-v2", + SiteVisibility.PUBLIC.toString(), + null, + SiteRole.SiteManager); + + expectedUpdate.expected(updatedSite); + + + // Check we can update just the visibility + updatedSite = sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate(null, null, SiteVisibility.PRIVATE), + 200); + + expectedUpdate = new SiteImpl( + null, + "partial-updates-test", + site.getGuid(), + "title-v2", + "description-v2", + SiteVisibility.PRIVATE.toString(), + null, + SiteRole.SiteManager); + + expectedUpdate.expected(updatedSite); + + removeSiteQuietly(site.getSiteId()); + } + + // -ve test: 400, bad requests + { + Site site = sitesProxy.createSite(new SiteImpl("Site Update Bad Request Tests", SiteVisibility.PRIVATE.toString()), 201); + + // long title + sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate(new String(new char[257]).replace('\0', 'a'), "description", SiteVisibility.PUBLIC), + 400); + + // long description + sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate("title", new String(new char[513]).replace('\0', 'a'), SiteVisibility.PUBLIC), + 400); + + // Invalid visibility + sitesProxy.update( + "sites", + site.getSiteId(), + null, + null, + "{\n" + + " \"title\": \"Updated Title\",\n" + + " \"description\": \"This is an updated description for the site\",\n" + + " \"visibility\": \"INVALID_VISIBILITY\"\n" + + "}", + null, + "Expected 400 response when updating "+site.getSiteId(), 400); // Details should not have changed. - final Site fresh = sitesProxy.getSite(siteId, 200); - expectedUpdate.expected(fresh); + Site fresh = sitesProxy.getSite(site.getSiteId(), 200); + site.expected(fresh); + + removeSiteQuietly(site.getSiteId()); } - removeSiteQuietly(initialSiteId); - } - - @Test - public void testUpdateSiteCannotChangeExistingSiteId() throws PublicApiException - { - Sites sitesProxy = publicApiClient.sites(); - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - - final String initialTitle = "Initial Title"; - final String initialDescription = "This is the initial description for the site"; - - SiteImpl site = new SiteImpl(initialTitle, SiteVisibility.PRIVATE.toString()); - site.setDescription(initialDescription); - - Site ret = sitesProxy.createSite(site, 201); - final String siteId = ret.getSiteId(); - ret = sitesProxy.getSite(siteId); - - final String initialSiteId = "initial-title"; - final Site siteExp = new SiteImpl(null, initialSiteId, ret.getGuid(), initialTitle, initialDescription, SiteVisibility.PRIVATE.toString(), null, null); - // Check that the retrieved details match the initially saved details. - siteExp.expected(ret); - - // Update the site details - final String updatedTitle = "Updated Title"; - final String updatedDescription = "This is an updated description for the site"; - final String updatedSiteId = "update-short-name"; - final Site expectedUpdate = new SiteImpl(null, updatedSiteId, ret.getGuid(), updatedTitle, updatedDescription, SiteVisibility.PUBLIC.toString(), null, null); - - try + // -ve test: 404 for non-existent site { - sitesProxy.updateSite(siteId, expectedUpdate); - fail("An HTTP error code should have been returned, but was not."); + // Attempt to update site details using an invalid site ID. + sitesProxy.updateSite( + UUID.randomUUID().toString(), // Non-existent site. + new SiteUpdate("Updated title", "Updated description", SiteVisibility.PUBLIC), + HttpStatus.SC_NOT_FOUND); } - catch(PublicApiException e) + + // -ve test: 401 authorization failure { - assertEquals(HttpStatus.SC_BAD_REQUEST, e.getHttpResponse().getStatusCode()); + Site site = sitesProxy.createSite(new SiteImpl("SiteUpdate401", SiteVisibility.PRIVATE.toString()), 201); + + // Invalid auth details + publicApiClient.setRequestContext(new RequestContext(network1.getId(), GUID.generate(), "password")); + + // Update the site details + sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate("Updated Title", "Updated description", SiteVisibility.PUBLIC), + HttpStatus.SC_UNAUTHORIZED); + + // Valid authentication again + publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); + + // Check the details haven't changed. + Site fresh = sitesProxy.getSite(site.getSiteId(), 200); + site.expected(fresh); + + removeSiteQuietly(site.getSiteId()); } - // Check the details haven't changed. - final Site fresh = sitesProxy.getSite(siteId, 200); - siteExp.expected(fresh); - - removeSiteQuietly(initialSiteId); - } - - @Test - public void testUpdateSite404ForNonExistentSiteId() throws PublicApiException - { - Sites sitesProxy = publicApiClient.sites(); - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - - final String initialTitle = "Initial Title"; - final String initialDescription = "This is the initial description for the site"; - - SiteImpl site = new SiteImpl(initialTitle, SiteVisibility.PRIVATE.toString()); - site.setDescription(initialDescription); - - Site ret = sitesProxy.createSite(site, 201); - final String siteId = ret.getSiteId(); - ret = sitesProxy.getSite(siteId); - - final String initialSiteId = "initial-title"; - final Site siteExp = new SiteImpl(null, initialSiteId, ret.getGuid(), initialTitle, initialDescription, SiteVisibility.PRIVATE.toString(), null, null); - // Check that the retrieved details match the initially saved details. - siteExp.expected(ret); - - // Update the site details - final String updatedTitle = "Updated Title"; - final String updatedDescription = "This is an updated description for the site"; - final Site expectedUpdate = new SiteImpl(null, initialSiteId, ret.getGuid(), updatedTitle, updatedDescription, SiteVisibility.PUBLIC.toString(), null, null); - - try + // -ve test: 403 for incorrect permissions { - sitesProxy.updateSite("i-do-not-exist", expectedUpdate); - fail("An HTTP error code should have been returned, but was not."); + Site site = sitesProxy.createSite(new SiteImpl("SiteUpdate403", SiteVisibility.PUBLIC.toString()), 201); + + // Update as person2 + publicApiClient.setRequestContext(new RequestContext(network1.getId(), person2Id)); + + // Update the site details + sitesProxy.updateSite( + site.getSiteId(), + new SiteUpdate("Updated Title", "Updated description", SiteVisibility.PUBLIC), + HttpStatus.SC_FORBIDDEN); + + // Check details haven't changed as original creator. + publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); + Site fresh = sitesProxy.getSite(site.getSiteId(), 200); + site.expected(fresh); + + removeSiteQuietly(site.getSiteId()); } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode()); - } - - // Check the details haven't changed. - final Site fresh = sitesProxy.getSite(siteId, 200); - siteExp.expected(fresh); - - removeSiteQuietly(initialSiteId); - } - - @Test - public void testUpdateSite401AuthFailure() throws PublicApiException - { - Sites sitesProxy = publicApiClient.sites(); - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - - final String initialTitle = "Initial Title"; - final String initialDescription = "This is the initial description for the site"; - - SiteImpl site = new SiteImpl(initialTitle, SiteVisibility.PUBLIC.toString()); - site.setDescription(initialDescription); - - Site ret = sitesProxy.createSite(site, 201); - final String siteId = ret.getSiteId(); - ret = sitesProxy.getSite(siteId); - - final String initialSiteId = "initial-title"; - final Site siteExp = new SiteImpl(null, initialSiteId, ret.getGuid(), initialTitle, initialDescription, SiteVisibility.PUBLIC.toString(), null, null); - // Check that the retrieved details match the initially saved details. - siteExp.expected(ret); - - // Update the site details - final String updatedTitle = "Updated Title"; - final String updatedDescription = "This is an updated description for the site"; - final Site expectedUpdate = new SiteImpl(null, initialSiteId, ret.getGuid(), updatedTitle, updatedDescription, SiteVisibility.PUBLIC.toString(), null, null); - - // Invalid auth - publicApiClient.setRequestContext(new RequestContext(network1.getId(), GUID.generate(), "password")); - - try - { - sitesProxy.updateSite(initialSiteId, expectedUpdate); - fail("An HTTP error code should have been returned, but was not."); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_UNAUTHORIZED, e.getHttpResponse().getStatusCode()); - } - - // Valid authentication again - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - - // Check the details haven't changed. - final Site fresh = sitesProxy.getSite(siteId, 200); - siteExp.expected(fresh); - - removeSiteQuietly(initialSiteId); - } - - @Test - public void testUpdateSite403WithIncorrectPermissions() throws PublicApiException - { - Sites sitesProxy = publicApiClient.sites(); - // Create the site as person1 but try to update as person2 - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - - final String initialTitle = "Initial Title"; - final String initialDescription = "This is the initial description for the site"; - - SiteImpl site = new SiteImpl(initialTitle, SiteVisibility.PUBLIC.toString()); - site.setDescription(initialDescription); - - Site ret = sitesProxy.createSite(site, 201); - final String siteId = ret.getSiteId(); - ret = sitesProxy.getSite(siteId); - - final String initialSiteId = "initial-title"; - final Site siteExp = new SiteImpl(null, initialSiteId, ret.getGuid(), initialTitle, initialDescription, SiteVisibility.PUBLIC.toString(), null, null); - // Check that the retrieved details match the initially saved details. - siteExp.expected(ret); - - // Update the site details - final String updatedTitle = "Updated Title"; - final String updatedDescription = "This is an updated description for the site"; - final Site expectedUpdate = new SiteImpl(null, initialSiteId, ret.getGuid(), updatedTitle, updatedDescription, SiteVisibility.PUBLIC.toString(), null, null); - - // Update as person2 - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person2Id)); - - try - { - sitesProxy.updateSite(siteId, expectedUpdate); - fail("An HTTP error code should have been returned, but was not."); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_FORBIDDEN, e.getHttpResponse().getStatusCode()); - } - - // Check the details haven't changed. - final Site fresh = sitesProxy.getSite(siteId, 200); - siteExp.expected(fresh); - - // Re-auth to remove site as original creator - publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); - removeSiteQuietly(initialSiteId); } private void removeSiteQuietly(String siteId) throws PublicApiException @@ -1082,4 +1047,56 @@ public class TestSites extends EnterpriseTestApi testListSitesWhereSiteVisibilityModerated(); testListSitesWhereSiteVisibilityInvalid(); } + + + public static class SiteUpdateJSONSerializer implements JSONAble + { + private SiteUpdate siteUpdate; + + public SiteUpdateJSONSerializer(SiteUpdate siteUpdate) + { + this.siteUpdate = siteUpdate; + } + + @SuppressWarnings("unchecked") + @Override + public JSONObject toJSON() + { + JSONObject json = new JSONObject(); + + if (siteUpdate.getTitle() != null) + { + json.put("title", siteUpdate.getTitle()); + } + if (siteUpdate.getDescription() != null) + { + json.put("description", siteUpdate.getDescription()); + } + if (siteUpdate.getVisibility() != null) + { + // JSONObject bug: if we don't toString() the visibility, it won't put quotes around it. + json.put("visibility", siteUpdate.getVisibility().toString()); + } + + return json; + } + + public static SiteUpdate parseSiteUpdate(JSONObject jsonObject) + { + String title = (String) jsonObject.get("title"); + String description = (String) jsonObject.get("description"); + SiteVisibility visibility = SiteVisibility.valueOf((String) jsonObject.get("visibility")); + + return new SiteUpdate(title, description, visibility); + } + } + + public static class SiteUpdateAssertions + { + public static void assertEquals(SiteUpdate expected, SiteUpdate actual) + { + // Could separate these out into discreet field assertions. + assertEquals(expected, actual); + } + } } 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 1eb6d2bc0b..7398d3e4f4 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 @@ -25,6 +25,7 @@ */ package org.alfresco.rest.api.tests.client; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import java.io.IOException; @@ -43,27 +44,11 @@ import javax.servlet.http.HttpServletResponse; import org.alfresco.cmis.client.impl.AlfrescoObjectFactoryImpl; import org.alfresco.opencmis.CMISDispatcherRegistry.Binding; +import org.alfresco.rest.api.model.SiteUpdate; +import org.alfresco.rest.api.tests.TestSites; import org.alfresco.rest.api.tests.client.PublicApiHttpClient.BinaryPayload; import org.alfresco.rest.api.tests.client.PublicApiHttpClient.RequestBuilder; -import org.alfresco.rest.api.tests.client.data.Activities; -import org.alfresco.rest.api.tests.client.data.Activity; -import org.alfresco.rest.api.tests.client.data.CMISNode; -import org.alfresco.rest.api.tests.client.data.Comment; -import org.alfresco.rest.api.tests.client.data.ContentData; -import org.alfresco.rest.api.tests.client.data.Favourite; -import org.alfresco.rest.api.tests.client.data.FavouriteSite; -import org.alfresco.rest.api.tests.client.data.FolderNode; -import org.alfresco.rest.api.tests.client.data.MemberOfSite; -import org.alfresco.rest.api.tests.client.data.NodeRating; -import org.alfresco.rest.api.tests.client.data.Person; -import org.alfresco.rest.api.tests.client.data.PersonNetwork; -import org.alfresco.rest.api.tests.client.data.Preference; -import org.alfresco.rest.api.tests.client.data.Site; -import org.alfresco.rest.api.tests.client.data.SiteContainer; -import org.alfresco.rest.api.tests.client.data.SiteImpl; -import org.alfresco.rest.api.tests.client.data.SiteMember; -import org.alfresco.rest.api.tests.client.data.SiteMembershipRequest; -import org.alfresco.rest.api.tests.client.data.Tag; +import org.alfresco.rest.api.tests.client.data.*; import org.apache.chemistry.opencmis.client.api.CmisObject; import org.apache.chemistry.opencmis.client.api.Document; import org.apache.chemistry.opencmis.client.api.FileableCmisObject; @@ -88,6 +73,7 @@ import org.apache.chemistry.opencmis.commons.enums.BindingType; import org.apache.chemistry.opencmis.commons.enums.UnfileObject; import org.apache.chemistry.opencmis.commons.enums.VersioningState; import org.apache.chemistry.opencmis.commons.impl.dataobjects.ContentStreamImpl; +import org.apache.commons.httpclient.HttpStatus; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.json.simple.JSONObject; @@ -698,25 +684,16 @@ public class PublicApiClient public HttpResponse update(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, String errorMessage) throws PublicApiException { - return update(entityCollectionName, entityId, relationCollectionName, relationId, body, errorMessage, null); + return update(entityCollectionName, entityId, relationCollectionName, relationId, body, null, errorMessage, 200); } - public HttpResponse update(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, String errorMessage, Map params) throws PublicApiException + public HttpResponse update(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, Map params, String errorMessage, int expectedStatus) throws PublicApiException { try { HttpResponse response = put("public", entityCollectionName, entityId, relationCollectionName, relationId, body, params); - - if (HttpServletResponse.SC_OK != response.getStatusCode()) - { - String msg = errorMessage + ": \n" + - " Response: " + response; - throw new PublicApiException(msg, response); - } - else - { - return response; - } + checkStatus(errorMessage, expectedStatus, response); + return response; } catch(IOException e) { @@ -859,10 +836,16 @@ public class PublicApiClient remove("sites", siteId, null, null, params, "Failed to remove site", expectedStatus); } - public Site updateSite(String siteId, Site update) throws PublicApiException + public Site updateSite(String siteId, SiteUpdate update, int expectedStatus) throws PublicApiException { - HttpResponse response = update("sites", siteId, null, null, update.toJSON().toString(), "Failed to update site " + update.getTitle()); - return SiteImpl.parseSite((JSONObject)response.getJsonResponse().get("entry")); + JSONAble jsonizer = new TestSites.SiteUpdateJSONSerializer(update); + HttpResponse response = update("sites", siteId, null, null, jsonizer.toJSON().toString(), null, "Failed to update site " + update.getTitle(), expectedStatus); + if (response.getJsonResponse() != null) + { + return SiteImpl.parseSite((JSONObject) response.getJsonResponse().get("entry")); + } + // No JSON response to parse. + return null; } public ListResponse getSiteContainers(String siteId, Map params) throws PublicApiException diff --git a/source/test-java/org/alfresco/rest/workflow/api/tests/TaskWorkflowApiTest.java b/source/test-java/org/alfresco/rest/workflow/api/tests/TaskWorkflowApiTest.java index 0f78aed998..569b4d238d 100644 --- a/source/test-java/org/alfresco/rest/workflow/api/tests/TaskWorkflowApiTest.java +++ b/source/test-java/org/alfresco/rest/workflow/api/tests/TaskWorkflowApiTest.java @@ -1182,8 +1182,9 @@ public class TaskWorkflowApiTest extends EnterpriseWorkflowTestApi null, null, "{\"state\":\"completed\",\"variables\":[{\"name\":\"wf_reviewOutcome\",\"value\":\"Approve\",\"scope\":\"local\"},{\"name\":\"bpm_comment\",\"value\":\"approved by me\",\"scope\":\"local\"}]}", - "Failed to update task", params); - assertEquals(200, response.getStatusCode()); + params, + "Failed to update task", + 200); HistoricTaskInstance historyTask = activitiProcessEngine.getHistoryService().createHistoricTaskInstanceQuery().taskId(task.getId()).includeProcessVariables().includeTaskLocalVariables().singleResult(); String outcome = (String) historyTask.getTaskLocalVariables().get("bpm_outcome"); assertEquals("Approve", outcome); diff --git a/source/test-java/org/alfresco/rest/workflow/api/tests/WorkflowApiClient.java b/source/test-java/org/alfresco/rest/workflow/api/tests/WorkflowApiClient.java index 24fab9467d..acac39d0aa 100644 --- a/source/test-java/org/alfresco/rest/workflow/api/tests/WorkflowApiClient.java +++ b/source/test-java/org/alfresco/rest/workflow/api/tests/WorkflowApiClient.java @@ -287,7 +287,7 @@ public class WorkflowApiClient extends PublicApiClient Map params = new HashMap(); params.put("select", selectedFieldsValue); - HttpResponse response = update("tasks", taskId, null, null, task.toJSONString(), "Failed to update task", params); + HttpResponse response = update("tasks", taskId, null, null, task.toJSONString(), params, "Failed to update task", 200); JSONObject entry = (JSONObject) response.getJsonResponse().get("entry"); return entry;