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

@@ -44,7 +44,7 @@ public interface Sites
     CollectionWithPagingInfo<SiteMember> 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/<personId>/sites/<siteId>
@@ -71,4 +71,5 @@ public interface Sites
     String getSiteRole(String siteId, String personId);
 
     String PARAM_PERMANENT = "permanent";
+	String PARAM_SKIP_ADDTOFAVORITES = "skipAddToFavorites";
 }

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,9 +101,17 @@ 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;
@@ -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();
+            // generate a site id from title (similar to Share create site dialog)
-            siteId = siteId.replace(" ","-");
+            siteId = siteTitle.
-            siteId = siteId.replaceAll("[^A-Za-z0-9\\-]","");
+                    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");

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<Site>,
@@ -98,6 +99,7 @@ public class SiteEntityResource implements EntityResourceAction.Read<Site>,
     }
 
     /**
+     * Create the given site.
      *
      * @param entity
      * @param parameters
@@ -113,7 +115,7 @@ public class SiteEntityResource implements EntityResourceAction.Read<Site>,
         }
 
         List<Site> result = new ArrayList<>(1);
-        result.add(sites.createSite(entity.get(0)));
+        result.add(sites.createSite(entity.get(0), parameters));
         return result;
     }
 }

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(' ', '-');
+			String expectedSiteId = "my-site-123";
-			Site siteExp = new SiteImpl(null, siteId, ret.getGuid(), siteTitle, null,  SiteVisibility.PRIVATE.toString(), null, SiteRole.SiteManager);
+			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));
@@ -217,6 +152,62 @@ public class TestSites extends EnterpriseTestApi
             }
         }
 
+		// -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

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;
@@ -678,22 +677,18 @@ public class PublicApiClient
 		}
 
         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);
-		        
+                checkStatus(errorMessage, expectedStatus, response);
-		        if (HttpServletResponse.SC_OK != response.getStatusCode())
-		        {
-		            String msg = errorMessage + ": \n" +
-		                    "   Response: " + response;
-		            throw new PublicApiException(msg, response);
-		        }
-		        else
-		        {
                 return response;
 			}
-			}
 			catch(IOException e)
 			{
 		        throw new PublicApiException(e);
@@ -729,45 +724,37 @@ public class PublicApiClient
 		}
 
         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, int expectedStatus) throws PublicApiException
 		{
 	        try
 	        {
 		        HttpResponse response = post("public", entityCollectionName, entityId, relationCollectionName, relationId, body);
-
+                checkStatus(errorMessage, expectedStatus, response);
-		        if (HttpServletResponse.SC_CREATED != response.getStatusCode())
-		        {
-		            String msg = errorMessage + ": \n" +
-		                    "   Response: " + response;
-		            throw new PublicApiException(msg, response);
-		        }
-		        else
-		        {
                 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, int expectedStatus) throws PublicApiException
 		{
 	        try
 	        {
 		        HttpResponse response = delete("public", entityCollectionName, entityId, relationCollectionName, relationId);
-
+                checkStatus(errorMessage, expectedStatus, response);
-		        if (HttpServletResponse.SC_NO_CONTENT != response.getStatusCode())
-		        {
-		            String msg = errorMessage + ": \n" +
-		                    "   Response: " + response;
-		            throw new PublicApiException(msg, response);
-		        }
-		        else
-		        {
                 return response;
 			}
-			}
 			catch(IOException e)
 			{
 		        throw new PublicApiException(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<T>
@@ -818,19 +816,41 @@ public class PublicApiClient
 
         public Site getSite(String siteId) throws PublicApiException
         {
-			HttpResponse response = getSingle("sites", siteId, null, null, "Failed to get site " + siteId);
+            return getSite(siteId, 200);
+        }
+		
+		public Site getSite(String siteId, int expectedStatus) throws PublicApiException
+		{
+			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
         {
-			HttpResponse response = create("sites", null, null, null, site.toJSON().toString(), "Failed to create site");
+            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 "+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<SiteContainer> getSiteContainers(String siteId, Map<String, String> params) throws PublicApiException

source/test-java/org/alfresco/rest/api/tests/client/data/SiteImpl.java

@@ -218,6 +218,11 @@ public class SiteImpl implements Serializable, Site, Comparable<SiteImpl>, 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");