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

9 files changed, 45 insertions, 23 deletions

diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/AuthenticationProviderRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/AuthenticationProviderRestTest.java
index dd93a917c0..5cc7e2d4d9 100644
--- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/AuthenticationProviderRestTest.java
+++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/AuthenticationProviderRestTest.java
@@ -27,6 +27,7 @@ import java.util.Map;
 import java.util.UUID;
 
 import org.apache.qpid.server.BrokerOptions;
+import org.apache.qpid.server.management.plugin.servlet.rest.RestServlet;
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.BrokerModel;
 import org.apache.qpid.server.model.ConfiguredObject;
@@ -103,7 +104,7 @@ public class AuthenticationProviderRestTest extends QpidRestTestCase
         attributes.put(AuthenticationProvider.ID, UUID.randomUUID());
 
         responseCode = getRestTestHelper().submitRequest("authenticationprovider/" + providerName, "PUT", attributes);
-        assertEquals("Update with new ID should fail", 409, responseCode);
+        assertEquals("Update with new ID should fail", RestServlet.SC_UNPROCESSABLE_ENTITY, responseCode);
     }
 
     public void testDeleteOfUsedAuthenticationProviderFails() throws Exception
@@ -128,7 +129,7 @@ public class AuthenticationProviderRestTest extends QpidRestTestCase
         assertEquals("Unexpected response code for port creation", 201, responseCode);
 
         responseCode = getRestTestHelper().submitRequest("authenticationprovider/" + providerName , "DELETE");
-        assertEquals("Unexpected response code for provider deletion", 409, responseCode);
+        assertEquals("Unexpected response code for deletion of provider in use", 409, responseCode);
 
         List<Map<String, Object>> 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<String, Object> 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<String, Object> attributes = new HashMap<String, Object>();
             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<String, Object> attributes = new HashMap<String, Object>();
         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<String, Object>();
         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<String, Object>();
         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<String, Object>();
         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<Map<String,Object>> 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<String, Object> 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<String, Object> 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