From d37da7d1c5bd24b679fccabd9d6e54beb5c854c4 Mon Sep 17 00:00:00 2001
From: Alex Rudyy <orudyy@apache.org>
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 ++-
 4 files changed, 14 insertions(+), 28 deletions(-)

(limited to 'qpid/java/broker-core')

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<X extends ConfiguredObject<X>> 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<X extends AbstractPort<X>> 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<T extends Principal
             String username = (String) attributes.get("name");
             String password = (String) attributes.get("password");
             Principal p = new UsernamePrincipal(username);
-            if (_userMap.containsKey(p))
-            {
-                throw new IllegalArgumentException("User '" + username + "' already exists");
-            }
-
+            PrincipalAdapter principalAdapter = new PrincipalAdapter(p);
+            principalAdapter.create(); // for a duplicate user DuplicateNameException should be thrown
             boolean created = getPrincipalDatabase().createPrincipal(p, password.toCharArray());
-            if(created)
-            {
-                p = getPrincipalDatabase().getUser(username);
-
-                PrincipalAdapter principalAdapter = new PrincipalAdapter(p);
-                principalAdapter.create();
-                _userMap.put(p, principalAdapter);
-            }
-
-            if(created)
-            {
-                return (C) _userMap.get(p);
-            }
-            else
+            if (!created)
             {
-                LOGGER.info("Failed to create user " + username + ". User already exists?");
-                return null;
-
+                throw new IllegalArgumentException("User '" + username + "' was not added into principal database");
             }
+            _userMap.put(p, principalAdapter);
+            return (C)principalAdapter;
         }
 
         return super.addChild(childClass, attributes, otherParents);
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
index b763568ab5..5dbd082152 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/PortFactoryTest.java
@@ -41,6 +41,7 @@ import org.apache.qpid.server.model.BrokerModel;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ConfiguredObjectFactory;
 import org.apache.qpid.server.model.ConfiguredObjectFactoryImpl;
+import org.apache.qpid.server.model.IntegrityViolationException;
 import org.apache.qpid.server.model.KeyStore;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.Protocol;
@@ -374,7 +375,7 @@ public class PortFactoryTest extends QpidTestCase
             _port = _factory.create(Port.class, attributes, _broker);
             fail("RMI port creation should fail as another one already exist");
         }
-        catch(IllegalConfigurationException e)
+        catch(IntegrityViolationException e)
         {
             // pass
         }
-- 
cgit v1.2.1