From aee533110c6ae9039bd0355067417fa6d4b5d2bf Mon Sep 17 00:00:00 2001 From: Jamal Kaabi-Mofrad Date: Thu, 2 Jun 2016 21:37:56 +0000 Subject: [PATCH] Merged API-STRIKES-BACK (5.2.0) to HEAD (5.2) 125708 jvonka: RA-779 / RA-780: Sites API - fix (create site + permanent delete + create site with same site id) - RA-967 git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@127561 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/public-rest-context.xml | 2 + .../org/alfresco/rest/api/impl/SitesImpl.java | 30 ++++++++- .../alfresco/rest/api/tests/TestSites.java | 64 +++++++++++++++---- .../api/tests/client/PublicApiClient.java | 18 ++++-- 4 files changed, 96 insertions(+), 18 deletions(-) diff --git a/config/alfresco/public-rest-context.xml b/config/alfresco/public-rest-context.xml index 0fb1592ef0..611057507d 100644 --- a/config/alfresco/public-rest-context.xml +++ b/config/alfresco/public-rest-context.xml @@ -652,6 +652,8 @@ + + diff --git a/source/java/org/alfresco/rest/api/impl/SitesImpl.java b/source/java/org/alfresco/rest/api/impl/SitesImpl.java index 90de278fd8..06e9ed3fe8 100644 --- a/source/java/org/alfresco/rest/api/impl/SitesImpl.java +++ b/source/java/org/alfresco/rest/api/impl/SitesImpl.java @@ -40,10 +40,12 @@ import org.alfresco.query.PagingRequest; import org.alfresco.query.PagingResults; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authority.UnknownAuthorityException; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.repo.site.SiteMembership; import org.alfresco.repo.site.SiteMembershipComparator; import org.alfresco.repo.site.SiteModel; import org.alfresco.repo.site.SiteServiceException; +import org.alfresco.repo.site.SiteServiceImpl; import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.People; import org.alfresco.rest.api.Sites; @@ -70,6 +72,8 @@ import org.alfresco.service.cmr.preference.PreferenceService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; +import org.alfresco.service.cmr.security.AccessStatus; +import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.cmr.site.SiteInfo; import org.alfresco.service.cmr.site.SiteService; import org.alfresco.service.cmr.site.SiteVisibility; @@ -115,6 +119,8 @@ public class SitesImpl implements Sites protected PreferenceService preferenceService; protected ImporterService importerService; protected SiteSurfConfig siteSurfConfig; + protected PermissionService permissionService; + protected SiteServiceImpl siteServiceImpl; public void setPreferenceService(PreferenceService preferenceService) { @@ -161,6 +167,17 @@ public class SitesImpl implements Sites this.siteSurfConfig = siteSurfConfig; } + public void setPermissionService(PermissionService permissionService) + { + this.permissionService = permissionService; + } + + public void setSiteServiceImpl(SiteServiceImpl siteServiceImpl) + { + this.siteServiceImpl = siteServiceImpl; + } + + public SiteInfo validateSite(NodeRef guid) { SiteInfo siteInfo = null; @@ -850,13 +867,24 @@ public class SitesImpl implements Sites } siteId = siteInfo.getShortName(); + NodeRef siteNodeRef = siteInfo.getNodeRef(); + + // belt-and-braces - double-check before purge/delete (rather than rollback) + if (permissionService.hasPermission(siteNodeRef, PermissionService.DELETE) != AccessStatus.ALLOWED) + { + throw new AccessDeniedException("Cannot delete site: "+siteId); + } + // default false (if not provided) boolean permanentDelete = Boolean.valueOf(parameters.getParameter(PARAM_PERMANENT)); if (permanentDelete == true) { // Set as temporary to delete node instead of archiving. - nodeService.addAspect(siteInfo.getNodeRef(), ContentModel.ASPECT_TEMPORARY, null); + nodeService.addAspect(siteNodeRef, ContentModel.ASPECT_TEMPORARY, null); + + // bypassing trashcan means that purge behaviour will not fire, so explicitly force cleanup here + siteServiceImpl.beforePurgeNode(siteNodeRef); } siteService.deleteSite(siteId); 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 af15230369..dedf9bbe6b 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestSites.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestSites.java @@ -22,8 +22,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; import org.alfresco.repo.tenant.TenantUtil; import org.alfresco.repo.tenant.TenantUtil.TenantRunAsWork; @@ -156,11 +158,28 @@ public class TestSites extends EnterpriseTestApi sitesProxy.removeSite(siteId); + // -ve test - ie. cannot get site after it has been deleted sitesProxy.getSite(siteId, 404); } - // -ve tests - { + // test create + permanent delete + create + { + + String siteId = "bbb"; + String siteTitle = "BBB site"; + + Site site = new SiteImpl(null, siteId, null, siteTitle, null, SiteVisibility.PUBLIC.toString(), null, null); + + sitesProxy.createSite(site); + + // permanent site delete (bypass trashcan/archive) + sitesProxy.removeSite(siteId, true, 204); + + sitesProxy.createSite(site); + } + + // -ve tests + { // invalid auth publicApiClient.setRequestContext(new RequestContext(network1.getId(), GUID.generate(), "password")); sitesProxy.getSite(site1.getSiteId(), 401); @@ -169,23 +188,23 @@ public class TestSites extends EnterpriseTestApi // -ve - cannot view or delete a private site sitesProxy.getSite(site1.getSiteId(), 404); - sitesProxy.removeSite(site1.getSiteId(), 404); + sitesProxy.removeSite(site1.getSiteId(), false, 404); // -ve - test cannot delete a public site (but can view it) sitesProxy.getSite(site2.getSiteId(), 200); - sitesProxy.removeSite(site2.getSiteId(), 403); + sitesProxy.removeSite(site2.getSiteId(), false, 403); // -ve - try to get unknown site sitesProxy.getSite(GUID.generate(), 404); SiteImpl site = new SiteImpl("my site 123", "invalidsitevisibility"); - sitesProxy.createSite(site, 400); + sitesProxy.createSite(site, 400); - site = new SiteImpl(null, "invalid site id", null, "my site 123", null, SiteVisibility.PRIVATE.toString(), null, null); - sitesProxy.createSite(site, 400); + site = new SiteImpl(null, "invalid site id", null, "my site 123", null, SiteVisibility.PRIVATE.toString(), null, null); + sitesProxy.createSite(site, 400); - site = new SiteImpl(null, "invalidsiteid*", null, "my site 123", null, SiteVisibility.PRIVATE.toString(), null, null); - sitesProxy.createSite(site, 400); + site = new SiteImpl(null, "invalidsiteid*", null, "my site 123", null, SiteVisibility.PRIVATE.toString(), null, null); + sitesProxy.createSite(site, 400); site = new SiteImpl(); site.setSiteId(new String(new char[72]).replace('\0', 'a')); @@ -223,9 +242,32 @@ public class TestSites extends EnterpriseTestApi site = new SiteImpl(siteTitle, SiteVisibility.PRIVATE.toString()); String siteId = sitesProxy.createSite(site, 201).getSiteId(); sitesProxy.createSite(site, 409); - sitesProxy.removeSite(siteId, 204); // cleanup + sitesProxy.removeSite(siteId); // cleanup - sitesProxy.removeSite(GUID.generate(), 404); + sitesProxy.removeSite(GUID.generate(), false, 404); + } + + // -ve - cannot create site with same site id as an existing site (even if it is in the trashcan/archive) + { + String siteId = "aaa"; + String siteTitle = "AAA site"; + + Site site = new SiteImpl(null, siteId, null, siteTitle, null, SiteVisibility.PUBLIC.toString(), null, null); + + String siteNodeId = sitesProxy.createSite(site).getGuid(); + + // -ve - duplicate site id + sitesProxy.createSite(site, 409); + + sitesProxy.removeSite(siteId); + + // -ve - duplicate site id (even if site is in trashcan) + sitesProxy.createSite(site, 409); + + // now purge the site + sitesProxy.remove("deleted-nodes", siteNodeId, null, null, "Cannot purge site"); + + sitesProxy.createSite(site); } { 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 66689386f2..7412445f03 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 @@ -27,6 +27,7 @@ import java.math.BigInteger; import java.text.ParseException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -737,14 +738,14 @@ public class PublicApiClient public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage) throws PublicApiException { - return remove(entityCollectionName, entityId, relationCollectionName, relationId, errorMessage, HttpServletResponse.SC_NO_CONTENT); + return remove(entityCollectionName, entityId, relationCollectionName, relationId, null, errorMessage, HttpServletResponse.SC_NO_CONTENT); } - public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage, int expectedStatus) throws PublicApiException + public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, Map params, String errorMessage, int expectedStatus) throws PublicApiException { try { - HttpResponse response = delete("public", entityCollectionName, entityId, relationCollectionName, relationId); + HttpResponse response = delete("public", 1, entityCollectionName, entityId, relationCollectionName, relationId, params); checkStatus(errorMessage, expectedStatus, response); return response; } @@ -838,12 +839,17 @@ public class PublicApiClient public void removeSite(String siteId) throws PublicApiException { - removeSite(siteId, 204); + removeSite(siteId, false, 204); } - public void removeSite(String siteId, int expectedStatus) throws PublicApiException + public void removeSite(String siteId, boolean permanent, int expectedStatus) throws PublicApiException { - remove("sites", siteId, null, null, "Failed to remove site", expectedStatus); + Map params = null; + if (permanent) + { + params = Collections.singletonMap("permanent", "true"); + } + remove("sites", siteId, null, null, params, "Failed to remove site", expectedStatus); } public ListResponse getSiteContainers(String siteId, Map params) throws PublicApiException