Merged 5.2.N (5.2.1) to HEAD (5.2)

130871 jvonka: REPO-1027: V1 REST API - fix error handling (add site member)
   - fix error code (should be 400 not 500) & add -ve api tests
   - ACE-5458


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@132196 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/impl/PeopleImpl.java

@@ -1,28 +1,28 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Remote API
+ * Alfresco Remote API
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.rest.api.impl;
 
 import java.io.Serializable;
@@ -39,6 +39,7 @@ import org.alfresco.rest.api.Sites;
 import org.alfresco.rest.api.model.Company;
 import org.alfresco.rest.api.model.Person;
 import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException;
+import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
 import org.alfresco.service.cmr.repository.AssociationRef;
 import org.alfresco.service.cmr.repository.ContentData;
 import org.alfresco.service.cmr.repository.ContentReader;
@@ -124,6 +125,11 @@ public class PeopleImpl implements People
 
 	public String validatePerson(String personId, boolean validateIsCurrentUser)
 	{
+		if(personId == null)
+		{
+			throw new InvalidArgumentException("personId is null.");
+		}
+        
     	if(personId.equalsIgnoreCase(DEFAULT_USER))
     	{
     		personId = AuthenticationUtil.getFullyAuthenticatedUser();

source/test-java/org/alfresco/rest/api/tests/AbstractBaseApiTest.java

@@ -537,7 +537,7 @@ public abstract class AbstractBaseApiTest extends EnterpriseTestApi
     protected SiteMember addSiteMember(String siteId, String userId, final SiteRole siteRole) throws Exception
     {
         SiteMember siteMember = new SiteMember(userId, siteRole.name());
-        HttpResponse response = publicApiClient.post(getScope(), "sites", siteId, "members", null, siteMember.postJSON().toString());
+        HttpResponse response = publicApiClient.post(getScope(), "sites", siteId, "members", null, siteMember.toJSON().toString());
         checkStatus(201, response.getStatusCode());
         return SiteMember.parseSiteMember(siteMember.getSiteId(), (JSONObject)response.getJsonResponse().get("entry"));
     }

source/test-java/org/alfresco/rest/api/tests/TestSiteMembers.java

@@ -61,6 +61,7 @@ import org.junit.Test;
 public class TestSiteMembers extends EnterpriseTestApi
 {
 	// TODO set create member for a user who is a member of the site (not the creator)
+	// TODO split into more manageable test methods
 	@Test
 	public void testSiteMembers() throws Exception
 	{
@@ -382,6 +383,30 @@ public class TestSiteMembers extends EnterpriseTestApi
 			{
 				assertEquals(e.getMessage(), HttpStatus.SC_NOT_FOUND, e.getHttpResponse().getStatusCode());
 			}
+            
+			// missing person id
+			try
+			{
+				publicApiClient.setRequestContext(new RequestContext(network1.getId(), person2.getId()));
+				sitesProxy.createSiteMember(site.getSiteId(), new SiteMember(null, SiteRole.SiteContributor.toString()));
+				fail("");
+			}
+			catch(PublicApiException e)
+			{
+				assertEquals(HttpStatus.SC_BAD_REQUEST, e.getHttpResponse().getStatusCode());
+			}
+
+			// missing role
+			try
+			{
+				publicApiClient.setRequestContext(new RequestContext(network1.getId(), person2.getId()));
+				sitesProxy.createSiteMember(site.getSiteId(), new SiteMember(person1.getId(), null));
+				fail("");
+			}
+			catch(PublicApiException e)
+			{
+				assertEquals(HttpStatus.SC_BAD_REQUEST, e.getHttpResponse().getStatusCode());
+			}
 	
 			// check site membership in GET
 			List<SiteMember> expectedSiteMembers = site.getMembers();

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

@@ -913,7 +913,7 @@ public class PublicApiClient
 
 		public SiteMember createSiteMember(String siteId, SiteMember siteMember) throws PublicApiException
 		{
-			HttpResponse response = create("sites", siteId, "members", null, siteMember.postJSON().toString(), "Failed to create site member");
+			HttpResponse response = create("sites", siteId, "members", null, siteMember.toJSON().toString(), "Failed to create site member");
 			SiteMember retSiteMember = SiteMember.parseSiteMember(siteMember.getSiteId(), (JSONObject)response.getJsonResponse().get("entry"));
 			return retSiteMember;
 		}

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

@@ -1,28 +1,28 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Remote API
+ * Alfresco Remote API
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.rest.api.tests.client.data;
 
 import static org.junit.Assert.assertNotNull;
@@ -165,22 +165,17 @@ public class SiteMember implements Serializable, ExpectedComparison, Comparable<
 	public JSONObject toJSON()
 	{
 		JSONObject entry = new JSONObject();
-		entry.put("id", getMemberId());
+        
-		if(getRole() != null)
+		if (getMemberId() != null)
 		{
-			entry.put("role", getRole());
+            entry.put("id", getMemberId());
 		}
-
+        
-		return entry;
+		if (getRole() != null) 
-	}
+		{
-	
+            entry.put("role", getRole());
-	@SuppressWarnings("unchecked")
+		}
-	public JSONObject postJSON()
+        
-	{
-		JSONObject entry = new JSONObject();
-		entry.put("id", getMemberId());
-		entry.put("role", getRole());
-
 		return entry;
 	}