From 6a5f88763f9df5ed16db22c29af448f51bc56b42 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Thu, 3 Nov 2016 13:28:23 +0000 Subject: [PATCH] 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 --- .../alfresco/rest/api/impl/PeopleImpl.java | 56 +++++++------- .../rest/api/tests/AbstractBaseApiTest.java | 2 +- .../rest/api/tests/TestSiteMembers.java | 25 +++++++ .../api/tests/client/PublicApiClient.java | 2 +- .../api/tests/client/data/SiteMember.java | 73 +++++++++---------- 5 files changed, 92 insertions(+), 66 deletions(-) diff --git a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java index a54ec294af..4b68640e68 100644 --- a/source/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/source/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -1,28 +1,28 @@ -/* - * #%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% - */ +/* + * #%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.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(); diff --git a/source/test-java/org/alfresco/rest/api/tests/AbstractBaseApiTest.java b/source/test-java/org/alfresco/rest/api/tests/AbstractBaseApiTest.java index 6564003f9f..e53a160432 100644 --- a/source/test-java/org/alfresco/rest/api/tests/AbstractBaseApiTest.java +++ b/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")); } diff --git a/source/test-java/org/alfresco/rest/api/tests/TestSiteMembers.java b/source/test-java/org/alfresco/rest/api/tests/TestSiteMembers.java index 84c2904cd3..082cbe1468 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestSiteMembers.java +++ b/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 expectedSiteMembers = site.getMembers(); 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 2f8d3d8130..fbdc32256a 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 @@ -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; } diff --git a/source/test-java/org/alfresco/rest/api/tests/client/data/SiteMember.java b/source/test-java/org/alfresco/rest/api/tests/client/data/SiteMember.java index 37c97f96b7..57ed044a5a 100644 --- a/source/test-java/org/alfresco/rest/api/tests/client/data/SiteMember.java +++ b/source/test-java/org/alfresco/rest/api/tests/client/data/SiteMember.java @@ -1,28 +1,28 @@ -/* - * #%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% - */ +/* + * #%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.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; - } - - @SuppressWarnings("unchecked") - public JSONObject postJSON() - { - JSONObject entry = new JSONObject(); - entry.put("id", getMemberId()); - entry.put("role", getRole()); - + + if (getRole() != null) + { + entry.put("role", getRole()); + } + return entry; }