From 5e3eff411379c24e871b03a04421c39535103cdc Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Fri, 3 Jun 2016 14:00:09 +0000 Subject: [PATCH] Merged HEAD (5.2) to 5.2.N (5.2.1) 127531 jkaabimofrad: Merged API-STRIKES-BACK (5.2.0) to HEAD (5.2) 125513 jvonka: RA-779 / RA-780: Sites API - create site & delete site - follow-on improvemnts (generating site id, adding favorite with option to sip) - more tests (+ve & -ve) git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@127641 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- source/java/org/alfresco/rest/api/Sites.java | 3 +- .../org/alfresco/rest/api/impl/SitesImpl.java | 78 ++++++++-- .../rest/api/sites/SiteEntityResource.java | 4 +- .../alfresco/rest/api/tests/TestSites.java | 137 ++++++++---------- .../api/tests/client/PublicApiClient.java | 112 ++++++++------ .../rest/api/tests/client/data/SiteImpl.java | 5 + 6 files changed, 203 insertions(+), 136 deletions(-) diff --git a/source/java/org/alfresco/rest/api/Sites.java b/source/java/org/alfresco/rest/api/Sites.java index 9cc3bfee5a..bc1a157416 100644 --- a/source/java/org/alfresco/rest/api/Sites.java +++ b/source/java/org/alfresco/rest/api/Sites.java @@ -44,7 +44,7 @@ public interface Sites CollectionWithPagingInfo getSiteMembers(String siteShortName, Parameters parameters); Site getSite(String siteId); void deleteSite(String siteId, Parameters parameters); - Site createSite(Site site); + Site createSite(Site site, Parameters parameters); /** * people//sites/ @@ -71,4 +71,5 @@ public interface Sites String getSiteRole(String siteId, String personId); String PARAM_PERMANENT = "permanent"; + String PARAM_SKIP_ADDTOFAVORITES = "skipAddToFavorites"; } diff --git a/source/java/org/alfresco/rest/api/impl/SitesImpl.java b/source/java/org/alfresco/rest/api/impl/SitesImpl.java index 1f7e0f3424..8213adcbc9 100644 --- a/source/java/org/alfresco/rest/api/impl/SitesImpl.java +++ b/source/java/org/alfresco/rest/api/impl/SitesImpl.java @@ -50,6 +50,7 @@ import org.alfresco.repo.security.authority.UnknownAuthorityException; 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.rest.api.Nodes; import org.alfresco.rest.api.People; import org.alfresco.rest.api.Sites; @@ -100,10 +101,18 @@ import org.apache.commons.logging.LogFactory; */ public class SitesImpl implements Sites { + private static final Log logger = LogFactory.getLog(SitesImpl.class); + private static final String FAVOURITE_SITES_PREFIX = "org.alfresco.share.sites.favourites."; private static final int FAVOURITE_SITES_PREFIX_LENGTH = FAVOURITE_SITES_PREFIX.length(); - private static final Log logger = LogFactory.getLog(SitesImpl.class); + // based on Share create site + private static final int SITE_MAXLEN_ID = 72; + private static final int SITE_MAXLEN_TITLE = 256; + private static final int SITE_MAXLEN_DESCRIPTION = 512; + + private static final String SITE_ID_VALID_CHARS_PARTIAL_REGEX = "A-Za-z0-9\\-"; + protected Nodes nodes; protected People people; protected NodeService nodeService; @@ -255,8 +264,13 @@ public class SitesImpl implements Sites // site does not exist throw new EntityNotFoundException(siteId); } + return getSite(siteInfo, includeRole); + } + + private Site getSite(SiteInfo siteInfo, boolean includeRole) + { // set the site id to the short name (to deal with case sensitivity issues with using the siteId from the url) - siteId = siteInfo.getShortName(); + String siteId = siteInfo.getShortName(); String role = null; if(includeRole) { @@ -855,11 +869,6 @@ public class SitesImpl implements Sites siteService.deleteSite(siteId); } - // based on Share create site - private static final int SITE_MAXLEN_ID = 72; - private static final int SITE_MAXLEN_TITLE = 256; - private static final int SITE_MAXLEN_DESCRIPTION = 512; - /** * Create default/preset (Share) site - with DocLib container/component @@ -867,18 +876,46 @@ public class SitesImpl implements Sites * @param site * @return */ - public Site createSite(Site site) + public Site createSite(Site site, Parameters parameters) { + // note: if site id is null then will be generated from the site title site = validateSite(site); - String siteId = site.getId(); - siteService.createSite("sitePreset", siteId, site.getTitle(), site.getDescription(), site.getVisibility()); + SiteInfo siteInfo = null; + try + { + siteInfo = siteService.createSite("sitePreset", site.getId(), site.getTitle(), site.getDescription(), site.getVisibility()); + } + catch (SiteServiceException sse) + { + if (sse.getMsgId().equals("site_service.unable_to_create")) + { + throw new ConstraintViolatedException(sse.getMessage()); + } + else + { + throw sse; + } + } + + String siteId = siteInfo.getShortName(); + + // import default surf config importSite(siteId); // pre-create doclib siteService.createContainer(siteId, SiteService.DOCUMENT_LIBRARY, ContentModel.TYPE_FOLDER, null); - return getSite(siteId); + // default false (if not provided) + boolean skipAddToFavorites = Boolean.valueOf(parameters.getParameter(PARAM_SKIP_ADDTOFAVORITES)); + if (skipAddToFavorites == false) + { + NodeRef siteNodeRef = siteInfo.getNodeRef(); + String personId = AuthenticationUtil.getFullyAuthenticatedUser(); + favouritesService.addFavourite(personId, siteNodeRef); // ignore result + } + + return getSite(siteInfo, true); } private Site validateSite(Site site) @@ -903,13 +940,17 @@ public class SitesImpl implements Sites String siteId = site.getId(); if (siteId == null) { - siteId = siteTitle.trim(); - siteId = siteId.replace(" ","-"); - siteId = siteId.replaceAll("[^A-Za-z0-9\\-]",""); + // generate a site id from title (similar to Share create site dialog) + siteId = siteTitle. + trim(). // trim leading & trailing whitespace + replaceAll("[^"+SITE_ID_VALID_CHARS_PARTIAL_REGEX+" ]",""). // remove special characters (except spaces) + replaceAll(" +", " "). // collapse multiple spaces to single space + replace(" ","-"). // replaces spaces with dashs + toLowerCase(); // lowercase :-) } else { - if (! siteId.matches("[^A-Za-z0-9\\-]")) + if (! siteId.matches("[^"+SITE_ID_VALID_CHARS_PARTIAL_REGEX+"]")) { throw new InvalidArgumentException("Invalid site id - should consist of alphanumeric/dash characters"); } @@ -923,6 +964,13 @@ public class SitesImpl implements Sites site.setId(siteId); String siteDescription = site.getDescription(); + + if (siteDescription == null) + { + // workaround: to avoid Share error (eg. in My Sites dashlet / freemarker template) + site.setDescription(""); + } + if ((siteDescription != null) && (siteDescription.length() > SITE_MAXLEN_DESCRIPTION)) { throw new InvalidArgumentException("Site description exceeds max length of "+SITE_MAXLEN_DESCRIPTION+" characters"); diff --git a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java index 74c929e172..6e5bd05787 100644 --- a/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java +++ b/source/java/org/alfresco/rest/api/sites/SiteEntityResource.java @@ -44,6 +44,7 @@ import java.util.List; * * @author Gethin James * @author steveglover + * @author janv */ @EntityResource(name="sites", title = "Sites") public class SiteEntityResource implements EntityResourceAction.Read, @@ -98,6 +99,7 @@ public class SiteEntityResource implements EntityResourceAction.Read, } /** + * Create the given site. * * @param entity * @param parameters @@ -113,7 +115,7 @@ public class SiteEntityResource implements EntityResourceAction.Read, } List result = new ArrayList<>(1); - result.add(sites.createSite(entity.get(0))); + result.add(sites.createSite(entity.get(0), parameters)); return result; } } 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 a39a80661c..1d6c777b05 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestSites.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestSites.java @@ -87,73 +87,6 @@ public class TestSites extends EnterpriseTestApi { Sites sitesProxy = publicApiClient.sites(); - // make sure that a user can't see PRIVATE sites of which they are not a member by creating one and checking it's not in the results - try - { - publicApiClient.setRequestContext(new RequestContext(network1.getId(), personId)); - sitesProxy.getSite(site1.getSiteId()); - fail(); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode()); - } - - try - { - sitesProxy.create("sites", "site", null, null, null, "Unable to POST to a site"); - fail(); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, e.getHttpResponse().getStatusCode()); - } - - try - { - sitesProxy.remove("sites", null, null, null, "Unable to DELETE sites"); - fail(); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, e.getHttpResponse().getStatusCode()); - } - - // -ve - try to delete unknown site - try - { - sitesProxy.remove("sites", "dummy", null, null, "Unable to DELETE site - not found"); - fail(); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode()); - } - - // invalid site - try - { - publicApiClient.setRequestContext(new RequestContext(network1.getId(), personId)); - sitesProxy.getSite(GUID.generate()); - fail(""); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode()); - } - - // invalid auth - try - { - publicApiClient.setRequestContext(new RequestContext(network1.getId(), GUID.generate(), "password")); - sitesProxy.getSite(site1.getSiteId()); - fail(""); - } - catch(PublicApiException e) - { - assertEquals(HttpStatus.SC_UNAUTHORIZED, e.getHttpResponse().getStatusCode()); - } - // get a site { publicApiClient.setRequestContext(new RequestContext(network1.getId(), person1Id)); @@ -188,15 +121,17 @@ public class TestSites extends EnterpriseTestApi checkList(expectedSites.subList(skipCount, skipCount + paging.getExpectedPaging().getCount()), paging.getExpectedPaging(), resp); } + // test create and delete site { - String siteTitle = "my site 123"; + String siteTitle = "my site !*#$ 123"; - Site site = new SiteImpl("my site 123", SiteVisibility.PRIVATE.toString()); + Site site = new SiteImpl(siteTitle, SiteVisibility.PRIVATE.toString()); publicApiClient.setRequestContext(new RequestContext(network1.getId(), personId)); Site ret = sitesProxy.createSite(site); + String siteId = ret.getSiteId(); - String siteId = siteTitle.replace(' ', '-'); - Site siteExp = new SiteImpl(null, siteId, ret.getGuid(), siteTitle, null, SiteVisibility.PRIVATE.toString(), null, SiteRole.SiteManager); + String expectedSiteId = "my-site-123"; + Site siteExp = new SiteImpl(null, expectedSiteId, ret.getGuid(), siteTitle, null, SiteVisibility.PRIVATE.toString(), null, SiteRole.SiteManager); siteExp.expected(ret); publicApiClient.setRequestContext(new RequestContext(network1.getId(), personId)); @@ -216,8 +151,64 @@ public class TestSites extends EnterpriseTestApi assertEquals(HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode()); } } - - // Test Case cloud-1478 + + // -ve tests + { + // invalid auth + publicApiClient.setRequestContext(new RequestContext(network1.getId(), GUID.generate(), "password")); + sitesProxy.getSite(site1.getSiteId(), 401); + + publicApiClient.setRequestContext(new RequestContext(network1.getId(), personId)); + + // -ve - permission test + // make sure that a user can't see PRIVATE sites of which they are not a member by creating one and checking it's not in the results + sitesProxy.getSite(site1.getSiteId(), 404); + + // -ve - try to get unknown site + sitesProxy.getSite(GUID.generate(), 404); + + Site site = new SiteImpl("my site 123", "invalidsitevisibility"); + 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 already exists (409) + String siteTitle = "my site 456"; + 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(GUID.generate(), 404); + } + + // -ve - cannot call POST method on /sites/siteId + try + { + sitesProxy.create("sites", "site", null, null, null, "Unable to POST to a site"); + fail(); + } + catch(PublicApiException e) + { + assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, e.getHttpResponse().getStatusCode()); + } + + // -ve - cannot call DELETE method on /sites + try + { + sitesProxy.remove("sites", null, null, null, "Unable to DELETE sites"); + fail(); + } + catch(PublicApiException e) + { + assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, e.getHttpResponse().getStatusCode()); + } + + // Test Case cloud-1478 // Test Case cloud-1479 // user invited to network and user invited to site // user invited to network and user not invited to 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 66d04203c6..9425b058a4 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 @@ -63,7 +63,6 @@ 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.service.cmr.site.SiteVisibility; import org.apache.chemistry.opencmis.client.api.CmisObject; import org.apache.chemistry.opencmis.client.api.Document; import org.apache.chemistry.opencmis.client.api.FileableCmisObject; @@ -676,23 +675,19 @@ public class PublicApiClient throw new PublicApiException(e); } } - - public HttpResponse getSingle(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage) throws PublicApiException + + public HttpResponse getSingle(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage) throws PublicApiException + { + return getSingle(entityCollectionName, entityId, relationCollectionName, relationId, errorMessage, HttpServletResponse.SC_OK); + } + + public HttpResponse getSingle(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage, int expectedStatus) throws PublicApiException { try { HttpResponse response = get("public", entityCollectionName, entityId, relationCollectionName, relationId, null); - - 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) { @@ -727,46 +722,38 @@ public class PublicApiClient throw new PublicApiException(e); } } + + public HttpResponse create(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, String errorMessage) throws PublicApiException + { + return create(entityCollectionName, entityId, relationCollectionName, relationId, body, errorMessage, HttpServletResponse.SC_CREATED); + } - public HttpResponse create(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, String errorMessage) throws PublicApiException + public HttpResponse create(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String body, String errorMessage, int expectedStatus) throws PublicApiException { try { HttpResponse response = post("public", entityCollectionName, entityId, relationCollectionName, relationId, body); - - if (HttpServletResponse.SC_CREATED != 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) + catch (IOException e) { throw new PublicApiException(e); } } + + public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage) throws PublicApiException + { + return remove(entityCollectionName, entityId, relationCollectionName, relationId, errorMessage, HttpServletResponse.SC_GONE); + } - public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage) throws PublicApiException + public HttpResponse remove(String entityCollectionName, String entityId, String relationCollectionName, String relationId, String errorMessage, int expectedStatus) throws PublicApiException { try { HttpResponse response = delete("public", entityCollectionName, entityId, relationCollectionName, relationId); - - if (HttpServletResponse.SC_NO_CONTENT != 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) { @@ -783,6 +770,17 @@ public class PublicApiClient assertNotNull(source); return source; } + + public void checkStatus(String errorMessage, int expectedStatus, HttpResponse response) throws PublicApiException + { + int actualStatus = response.getStatusCode(); + if ((expectedStatus > 0) && (expectedStatus != actualStatus)) + { + String msg = "Status code " + actualStatus + " returned, but expected " + expectedStatus + ": \n"+ + errorMessage + ": \n" + " Response: " + response; + throw new PublicApiException(msg, response); + } + } } public static class ListResponse @@ -815,22 +813,44 @@ public class PublicApiClient HttpResponse response = getAll("sites", null, null, null, params, "Failed to get sites"); return SiteImpl.parseSites(response.getJsonResponse()); } + + public Site getSite(String siteId) throws PublicApiException + { + return getSite(siteId, 200); + } - public Site getSite(String siteId) throws PublicApiException + public Site getSite(String siteId, int expectedStatus) throws PublicApiException { - HttpResponse response = getSingle("sites", siteId, null, null, "Failed to get site " + siteId); - return SiteImpl.parseSite((JSONObject)response.getJsonResponse().get("entry")); + HttpResponse response = getSingle("sites", siteId, null, null, "Failed to get site " + siteId, expectedStatus); + if ((response != null) && (response.getJsonResponse() != null)) + { + return SiteImpl.parseSite((JSONObject)response.getJsonResponse().get("entry")); + } + else + { + return null; + } } - public Site createSite(Site site) throws PublicApiException + public Site createSite(Site site) throws PublicApiException + { + return createSite(site, 201); + } + + public Site createSite(Site site, int expectedStatus) throws PublicApiException { - HttpResponse response = create("sites", null, null, null, site.toJSON().toString(), "Failed to create site"); - return SiteImpl.parseSite((JSONObject)response.getJsonResponse().get("entry")); + HttpResponse response = create("sites", null, null, null, site.toJSON().toString(), "Failed to create site "+site.getTitle(), expectedStatus); + return SiteImpl.parseSite((JSONObject)response.getJsonResponse().get("entry")); } public void removeSite(String siteId) throws PublicApiException { - remove("sites", siteId, null, null, "Failed to remove site"); + removeSite(siteId, 204); + } + + public void removeSite(String siteId, int expectedStatus) throws PublicApiException + { + remove("sites", siteId, null, null, "Failed to remove site", expectedStatus); } public ListResponse getSiteContainers(String siteId, Map params) throws PublicApiException diff --git a/source/test-java/org/alfresco/rest/api/tests/client/data/SiteImpl.java b/source/test-java/org/alfresco/rest/api/tests/client/data/SiteImpl.java index 444a1bc749..fbef49962f 100644 --- a/source/test-java/org/alfresco/rest/api/tests/client/data/SiteImpl.java +++ b/source/test-java/org/alfresco/rest/api/tests/client/data/SiteImpl.java @@ -218,6 +218,11 @@ public class SiteImpl implements Serializable, Site, Comparable, Expec public static Site parseSite(JSONObject jsonObject) { + if (jsonObject == null) + { + return null; + } + String id = (String)jsonObject.get("id"); String guid = (String)jsonObject.get("guid"); String title = (String)jsonObject.get("title");