From d37da7d1c5bd24b679fccabd9d6e54beb5c854c4 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Sat, 14 Mar 2015 00:40:45 +0000 Subject: QPID-6449: [Java Broker] Change REST interfaces to return 422 status code from create/update requests when provided attribute values are invalid or required attributes are missing git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1666625 13f79535-47bb-0310-9956-ffa450edef68 --- .../server/model/AbstractConfiguredObject.java | 8 +++---- .../qpid/server/model/port/AbstractPort.java | 3 ++- .../PrincipalDatabaseAuthenticationManager.java | 28 +++++----------------- .../qpid/server/model/adapter/PortFactoryTest.java | 3 ++- .../plugin/servlet/rest/RestServlet.java | 20 +++++++++++++--- .../rest/AuthenticationProviderRestTest.java | 5 ++-- .../apache/qpid/systest/rest/BindingRestTest.java | 3 ++- .../apache/qpid/systest/rest/BrokerRestTest.java | 4 +++- .../qpid/systest/rest/GroupProviderRestTest.java | 23 ++++++++++++------ .../qpid/systest/rest/HttpManagementRestTest.java | 6 +++-- .../org/apache/qpid/systest/rest/PortRestTest.java | 15 +++++++----- .../qpid/systest/rest/VirtualHostRestTest.java | 6 +++-- .../qpid/systest/rest/acl/BrokerACLTest.java | 3 ++- .../qpid/systest/rest/acl/ExchangeRestACLTest.java | 3 ++- 14 files changed, 76 insertions(+), 54 deletions(-) (limited to 'qpid/java') diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java index abdd79dc82..1485b3df5c 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java @@ -2615,18 +2615,18 @@ public abstract class AbstractConfiguredObject> im } } - protected final static class DuplicateIdException extends IllegalArgumentException + public final static class DuplicateIdException extends IllegalArgumentException { - public DuplicateIdException(final ConfiguredObject child) + private DuplicateIdException(final ConfiguredObject child) { super("Child of type " + child.getClass().getSimpleName() + " already exists with id of " + child.getId()); } } - protected final static class DuplicateNameException extends IllegalArgumentException + public final static class DuplicateNameException extends IllegalArgumentException { private final String _name; - public DuplicateNameException(final ConfiguredObject child) + private DuplicateNameException(final ConfiguredObject child) { super("Child of type " + child.getClass().getSimpleName() + " already exists with name of " + child.getName()); _name = child.getName(); diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java index 44c8869308..d08d06ab7b 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/port/AbstractPort.java @@ -30,6 +30,7 @@ import java.util.Set; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; +import org.apache.qpid.server.model.IntegrityViolationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -326,7 +327,7 @@ abstract public class AbstractPort> extends AbstractCo intersection.retainAll(getProtocols()); if(!intersection.isEmpty()) { - throw new IllegalConfigurationException("Port for protocols " + intersection + " already exists. Only one management port per protocol can be created."); + throw new IntegrityViolationException("Port for protocols " + intersection + " already exists. Only one management port per protocol can be created."); } } } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java index 19efee888f..243e9add66 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/PrincipalDatabaseAuthenticationManager.java @@ -342,31 +342,15 @@ public abstract class PrincipalDatabaseAuthenticationManager> providerDetails = getRestTestHelper().getJsonAsList("authenticationprovider/" + providerName); assertNotNull("Providers details cannot be null", providerDetails); diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BindingRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BindingRestTest.java index 368bc90d3d..b767c8be0f 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BindingRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BindingRestTest.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet; import org.apache.qpid.server.model.Binding; public class BindingRestTest extends QpidRestTestCase @@ -126,6 +127,6 @@ public class BindingRestTest extends QpidRestTestCase attributes.put(Binding.ARGUMENTS, "blah"); responseCode = getRestTestHelper().submitRequest(bindingUrl, "PUT", attributes); - assertEquals("Update should be unsupported", 409, responseCode); + assertEquals("Update should be unsupported", RestServlet.SC_UNPROCESSABLE_ENTITY, responseCode); } } diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BrokerRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BrokerRestTest.java index 4023830a8d..6c06e18faf 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BrokerRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/BrokerRestTest.java @@ -33,6 +33,7 @@ import javax.jms.Session; import javax.jms.TextMessage; import org.apache.qpid.common.QpidProperties; +import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.BrokerModel; import org.apache.qpid.server.model.ConfiguredObject; @@ -144,7 +145,8 @@ public class BrokerRestTest extends QpidRestTestCase Map brokerAttributes = getValidBrokerAttributes(); brokerAttributes.put(entry.getKey(), entry.getValue()); int response = getRestTestHelper().submitRequest("broker", "PUT", brokerAttributes); - assertEquals("Unexpected update response for invalid attribute " + entry.getKey() + "=" + entry.getValue(), 409, response); + assertEquals("Unexpected update response for invalid attribute " + entry.getKey() + "=" + entry.getValue(), + RestServlet.SC_UNPROCESSABLE_ENTITY, response); } } diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/GroupProviderRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/GroupProviderRestTest.java index 4f1c1ad7a7..cf024d598f 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/GroupProviderRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/GroupProviderRestTest.java @@ -20,6 +20,8 @@ */ package org.apache.qpid.systest.rest; +import static org.apache.qpid.server.management.plugin.servlet.rest.RestServlet.SC_UNPROCESSABLE_ENTITY; + import java.io.File; import java.io.FileOutputStream; import java.util.HashMap; @@ -177,7 +179,7 @@ public class GroupProviderRestTest extends QpidRestTestCase attributes.put(GroupProvider.TYPE, FileBasedGroupProviderImpl.GROUP_FILE_PROVIDER_TYPE); int responseCode = getRestTestHelper().submitRequest("groupprovider/" + providerName, "PUT", attributes); - assertEquals("Group provider was created", 409, responseCode); + assertEquals("Group provider was created", SC_UNPROCESSABLE_ENTITY, responseCode); } public void testCreateNewFileGroupProviderFromNonExistingGroupFile() throws Exception @@ -228,7 +230,7 @@ public class GroupProviderRestTest extends QpidRestTestCase attributes.put(GroupProvider.NAME, providerName + 2); responseCode = getRestTestHelper().submitRequest("groupprovider/" + providerName + 2, "PUT", attributes); - assertEquals("Group provider for the same group file was created", 409, responseCode); + assertEquals("Group provider for the same group file was created", SC_UNPROCESSABLE_ENTITY, responseCode); } finally { @@ -275,13 +277,20 @@ public class GroupProviderRestTest extends QpidRestTestCase attributes.put(FileBasedGroupProvider.PATH, groupFile.getAbsolutePath()); int responseCode = getRestTestHelper().submitRequest("groupprovider/" + providerName, "PUT", attributes); - assertEquals("Expected to fail because we can have only one password provider", 201, responseCode); + assertEquals("Password provider should be created successfully", 201, responseCode); - File newGroupFile = new File(TMP_FOLDER + File.separator + getTestName() + File.separator + "groups"); - attributes.put(FileBasedGroupProvider.PATH, newGroupFile.getAbsolutePath()); + File newGroupFile = TestFileUtils.createTempFile(this, ".groups"); + try + { + attributes.put(FileBasedGroupProvider.PATH, newGroupFile.getAbsolutePath()); - responseCode = getRestTestHelper().submitRequest("groupprovider/" + providerName, "PUT", attributes); - assertEquals("Expected to fail because we can have only one password provider", 409, responseCode); + responseCode = getRestTestHelper().submitRequest("groupprovider/" + providerName, "PUT", attributes); + assertEquals("Changing of group file is unsupported at the moment", SC_UNPROCESSABLE_ENTITY, responseCode); + } + finally + { + newGroupFile.delete(); + } } finally { diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java index abafb7fcaf..dfa7bd1168 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/HttpManagementRestTest.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.Map; import org.apache.qpid.server.management.plugin.HttpManagement; +import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet; import org.apache.qpid.test.utils.TestBrokerConfiguration; public class HttpManagementRestTest extends QpidRestTestCase @@ -80,12 +81,13 @@ public class HttpManagementRestTest extends QpidRestTestCase Map attributes = new HashMap(); attributes.put(invalidAttribute.getKey(), invalidAttribute.getValue()); int response = getRestTestHelper().submitRequest("plugin/" + TestBrokerConfiguration.ENTRY_NAME_HTTP_MANAGEMENT, "PUT", attributes); - assertEquals("Update should fail for attribute " + invalidAttribute.getKey() + " with value " + invalidAttribute.getValue() , 409, response); + assertEquals("Update should fail for attribute " + invalidAttribute.getKey() + " with value " + invalidAttribute.getValue(), + RestServlet.SC_UNPROCESSABLE_ENTITY, response); } Map attributes = new HashMap(); attributes.put(HttpManagement.TIME_OUT, -1l); int response = getRestTestHelper().submitRequest("plugin/" + TestBrokerConfiguration.ENTRY_NAME_HTTP_MANAGEMENT, "PUT", attributes); - assertEquals("Update should fail for invalid session timeout", 409, response); + assertEquals("Update should fail for invalid session timeout", RestServlet.SC_UNPROCESSABLE_ENTITY, response); } } diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/PortRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/PortRestTest.java index 4394d55294..46832e7b97 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/PortRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/PortRestTest.java @@ -20,6 +20,8 @@ */ package org.apache.qpid.systest.rest; +import static org.apache.qpid.server.management.plugin.servlet.rest.RestServlet.SC_UNPROCESSABLE_ENTITY; + import java.net.ServerSocket; import java.util.Arrays; import java.util.Collection; @@ -233,7 +235,7 @@ public class PortRestTest extends QpidRestTestCase attributes.put(Port.TRANSPORTS, Collections.singleton(Transport.SSL)); int responseCode = getRestTestHelper().submitRequest("port/" + portName, "PUT", attributes); - assertEquals("Creation of SSL port without keystore should fail", 409, responseCode); + assertEquals("Creation of SSL port without keystore should fail", SC_UNPROCESSABLE_ENTITY, responseCode); } public void testUpdateWantNeedClientAuth() throws Exception @@ -270,7 +272,8 @@ public class PortRestTest extends QpidRestTestCase attributes.put(Port.TRANSPORTS, Collections.singleton(Transport.TCP)); responseCode = getRestTestHelper().submitRequest("port/" + portName, "PUT", attributes); - assertEquals("Should not be able to change transport to TCP without reseting of attributes for need/want client auth", 409, responseCode); + assertEquals("Should not be able to change transport to TCP without reseting of attributes for need/want client auth", + SC_UNPROCESSABLE_ENTITY, responseCode); attributes = new HashMap(); attributes.put(Port.NAME, portName); @@ -298,13 +301,13 @@ public class PortRestTest extends QpidRestTestCase attributes.put(Port.NAME, portName); attributes.put(Port.NEED_CLIENT_AUTH, true); int responseCode = getRestTestHelper().submitRequest("port/" + portName, "PUT", attributes); - assertEquals("Unexpected response when trying to set 'needClientAuth' on non-SSL port", 409, responseCode); + assertEquals("Unexpected response when trying to set 'needClientAuth' on non-SSL port", SC_UNPROCESSABLE_ENTITY, responseCode); attributes = new HashMap(); attributes.put(Port.NAME, portName); attributes.put(Port.WANT_CLIENT_AUTH, true); responseCode = getRestTestHelper().submitRequest("port/" + portName, "PUT", attributes); - assertEquals("Unexpected response when trying to set 'wantClientAuth' on non-SSL port", 409, responseCode); + assertEquals("Unexpected response when trying to set 'wantClientAuth' on non-SSL port", SC_UNPROCESSABLE_ENTITY, responseCode); } public void testUpdatePortAuthenticationProvider() throws Exception @@ -314,7 +317,7 @@ public class PortRestTest extends QpidRestTestCase attributes.put(Port.NAME, portName); attributes.put(Port.AUTHENTICATION_PROVIDER, "non-existing"); int responseCode = getRestTestHelper().submitRequest("port/" + portName, "PUT", attributes); - assertEquals("Unexpected response when trying to change auth provider to non-existing one", 409, responseCode); + assertEquals("Unexpected response when trying to change auth provider to non-existing one", SC_UNPROCESSABLE_ENTITY, responseCode); attributes = new HashMap(); attributes.put(Port.NAME, portName); @@ -357,7 +360,7 @@ public class PortRestTest extends QpidRestTestCase attributes.put(Port.AUTHENTICATION_PROVIDER, TestBrokerConfiguration.ENTRY_NAME_AUTHENTICATION_PROVIDER); int responseCode = getRestTestHelper().submitRequest("port/" + newPortName, "PUT", attributes); - assertEquals("Unexpected response code for port creation", 409, responseCode); + assertEquals("Unexpected response code for port creation", SC_UNPROCESSABLE_ENTITY, responseCode); List> ports = getRestTestHelper().getJsonAsList("port/" + getRestTestHelper().encodeAsUTF(newPortName)); assertTrue("Port should not be created", ports.isEmpty()); diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostRestTest.java index 69d8f0f4f7..7a2078dd88 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostRestTest.java @@ -20,6 +20,8 @@ */ package org.apache.qpid.systest.rest; +import static org.apache.qpid.server.management.plugin.servlet.rest.RestServlet.SC_UNPROCESSABLE_ENTITY; + import java.io.IOException; import java.util.Collections; import java.util.HashMap; @@ -388,7 +390,7 @@ public class VirtualHostRestTest extends QpidRestTestCase { String queueName = getTestQueueName() + "-sorted"; int responseCode = tryCreateQueue(queueName, "sorted", null); - assertEquals("Unexpected response code", HttpServletResponse.SC_CONFLICT, responseCode); + assertEquals("Unexpected response code", SC_UNPROCESSABLE_ENTITY, responseCode); Map hostDetails = getRestTestHelper().getJsonAsSingletonList("virtualhost/test"); @@ -433,7 +435,7 @@ public class VirtualHostRestTest extends QpidRestTestCase { String queueName = getTestQueueName(); int responseCode = tryCreateQueue(queueName, "unsupported", null); - assertEquals("Unexpected response code", HttpServletResponse.SC_CONFLICT, responseCode); + assertEquals("Unexpected response code", SC_UNPROCESSABLE_ENTITY, responseCode); Map hostDetails = getRestTestHelper().getJsonAsSingletonList("virtualhost/test"); diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/BrokerACLTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/BrokerACLTest.java index 1b958f728f..7f1dd1f91a 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/BrokerACLTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/BrokerACLTest.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet; import org.codehaus.jackson.JsonGenerationException; import org.codehaus.jackson.map.JsonMappingException; import org.apache.qpid.server.management.plugin.HttpManagement; @@ -663,7 +664,7 @@ public class BrokerACLTest extends QpidRestTestCase attributes.put(GroupProvider.TYPE, FileBasedGroupProviderImpl.GROUP_FILE_PROVIDER_TYPE); attributes.put(FileBasedGroupProvider.PATH, "/path/to/file"); responseCode = getRestTestHelper().submitRequest("groupprovider/" + groupProviderName, "PUT", attributes); - assertEquals("Setting of group provider attributes should be allowed but not supported", 409, responseCode); + assertEquals("Setting of group provider attributes should be allowed but not supported", RestServlet.SC_UNPROCESSABLE_ENTITY, responseCode); } public void testSetGroupProviderAttributesDenied() throws Exception diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/ExchangeRestACLTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/ExchangeRestACLTest.java index b0c66cb3af..b158bbdec1 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/ExchangeRestACLTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/acl/ExchangeRestACLTest.java @@ -21,6 +21,7 @@ package org.apache.qpid.systest.rest.acl; import org.apache.qpid.server.management.plugin.HttpManagement; +import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet; import org.apache.qpid.server.model.Binding; import org.apache.qpid.server.model.Exchange; import org.apache.qpid.server.model.Plugin; @@ -149,7 +150,7 @@ public class ExchangeRestACLTest extends QpidRestTestCase attributes.put(Exchange.ALTERNATE_EXCHANGE, "my-alternate-exchange"); responseCode = getRestTestHelper().submitRequest(_exchangeUrl, "PUT", attributes); - assertEquals("Setting of exchange attribites should be allowed but it is currently unsupported", 409, responseCode); + assertEquals("Exchange 'my-alternate-exchange' does not exist", RestServlet.SC_UNPROCESSABLE_ENTITY, responseCode); } public void testSetExchangeAttributesDenied() throws Exception -- cgit v1.2.1