From 3a7a946e952545d34966a5569839b631df92e448 Mon Sep 17 00:00:00 2001 From: Robert Godfrey Date: Sun, 12 Apr 2015 15:03:34 +0000 Subject: QPID-5818 : [Java Broker] creating children from within the configuration thread leads to deadlock as the configuration thread blocks waiting for a task which cannot be executed because it needs the config thread. Instead use asynchronous child creation. git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1673014 13f79535-47bb-0310-9956-ffa450edef68 --- .../server/model/AbstractConfiguredObject.java | 148 +++++++++++--- .../model/AbstractConfiguredObjectTypeFactory.java | 15 +- .../apache/qpid/server/model/ConfiguredObject.java | 3 + .../qpid/server/model/adapter/BrokerAdapter.java | 213 ++++++--------------- .../server/model/adapter/ConnectionAdapter.java | 2 +- .../model/adapter/FileBasedGroupProviderImpl.java | 8 +- .../apache/qpid/server/queue/AbstractQueue.java | 6 +- .../manager/AbstractAuthenticationManager.java | 16 +- ...odelPasswordManagingAuthenticationProvider.java | 9 +- .../PrincipalDatabaseAuthenticationManager.java | 6 +- .../qpid/server/security/group/GroupImpl.java | 6 +- .../server/security/group/GroupProviderImpl.java | 7 +- .../server/virtualhost/AbstractVirtualHost.java | 58 +++--- .../AbstractStandardVirtualHostNode.java | 6 +- .../auth/manager/MD5AuthenticationManagerTest.java | 5 +- .../ManagedAuthenticationManagerTestBase.java | 9 +- .../qpid/systest/rest/VirtualHostNodeRestTest.java | 23 +++ 17 files changed, 286 insertions(+), 254 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 852f512ca6..731cfd0895 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 @@ -46,6 +46,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -1651,8 +1652,8 @@ public abstract class AbstractConfiguredObject> im @Override public String toString() { - return getClass().getSimpleName() + "[name=" + getName() + ", categoryClass=" + getCategoryClass() + ", type=" - + getType() + ", id=" + getId() + "]"; + return AbstractConfiguredObject.this.getClass().getSimpleName() + "[name=" + getName() + ", categoryClass=" + getCategoryClass() + ", type=" + + getType() + ", id=" + getId() + ", attributes=" + getAttributes() + "]"; } }; } @@ -1662,27 +1663,49 @@ public abstract class AbstractConfiguredObject> im public C createChild(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { - return _taskExecutor.run(new Task() { + return doSync(createChildAsync(childClass, attributes, otherParents)); + } + @SuppressWarnings("unchecked") + @Override + public ListenableFuture createChildAsync(final Class childClass, final Map attributes, + final ConfiguredObject... otherParents) + { + return doOnConfigThread(new Callable>() + { @Override - public C execute() + public ListenableFuture call() throws Exception { authoriseCreateChild(childClass, attributes, otherParents); - C child = addChild(childClass, attributes, otherParents); - if (child != null) - { - childAdded(child); - } - return child; + return doAfter(addChildAsync(childClass, attributes, otherParents), + new CallableWithArgument, C>() + { + + @Override + public ListenableFuture call(final C child) throws Exception + { + if (child != null) + { + childAdded(child); + } + return Futures.immediateFuture(child); + } + }); } }); } + protected C addChild(Class childClass, Map attributes, ConfiguredObject... otherParents) { throw new UnsupportedOperationException(); } + protected ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) + { + throw new UnsupportedOperationException(); + } + private void registerChild(final C child) { synchronized(_children) @@ -1861,18 +1884,18 @@ public abstract class AbstractConfiguredObject> im doSync(setAttributesAsync(attributes)); } - protected final ChainedListenableFuture doAfter(ListenableFuture first, final Runnable second) + protected final ChainedListenableFuture doAfter(ListenableFuture first, final Runnable second) { return doAfter(getTaskExecutor().getExecutor(), first, second); } - protected static final ChainedListenableFuture doAfter(Executor executor, ListenableFuture first, final Runnable second) + protected static ChainedListenableFuture doAfter(Executor executor, ListenableFuture first, final Runnable second) { - final ChainedSettableFuture returnVal = new ChainedSettableFuture(executor); - Futures.addCallback(first, new FutureCallback() + final ChainedSettableFuture returnVal = new ChainedSettableFuture(executor); + Futures.addCallback(first, new FutureCallback() { @Override - public void onSuccess(final Void result) + public void onSuccess(final V result) { try { @@ -1895,13 +1918,19 @@ public abstract class AbstractConfiguredObject> im return returnVal; } - public static interface ChainedListenableFuture extends ListenableFuture + public interface CallableWithArgument + { + V call(A argument) throws Exception; + } + + public static interface ChainedListenableFuture extends ListenableFuture { - ChainedListenableFuture then(Runnable r); - ChainedListenableFuture then(Callable> r); + ChainedListenableFuture then(Runnable r); + ChainedListenableFuture then(Callable> r); + ChainedListenableFuture then(CallableWithArgument,V> r); } - public static class ChainedSettableFuture extends AbstractFuture implements ChainedListenableFuture + public static class ChainedSettableFuture extends AbstractFuture implements ChainedListenableFuture { private final Executor _exector; @@ -1911,7 +1940,7 @@ public abstract class AbstractConfiguredObject> im } @Override - public boolean set(Void value) + public boolean set(V value) { return super.set(value); } @@ -1923,40 +1952,96 @@ public abstract class AbstractConfiguredObject> im } @Override - public ChainedListenableFuture then(final Runnable r) + public ChainedListenableFuture then(final Runnable r) { return doAfter(_exector, this, r); } @Override - public ChainedListenableFuture then(final Callable> r) + public ChainedListenableFuture then(final Callable> r) { return doAfter(_exector, this,r); } + + @Override + public ChainedListenableFuture then(final CallableWithArgument,V> r) + { + return doAfter(_exector, this, r); + } } - protected final ChainedListenableFuture doAfter(ListenableFuture first, final Callable> second) + protected final ChainedListenableFuture doAfter(ListenableFuture first, final Callable> second) { return doAfter(getTaskExecutor().getExecutor(), first, second); } - protected static final ChainedListenableFuture doAfter(final Executor executor, ListenableFuture first, final Callable> second) + protected final ChainedListenableFuture doAfter(ListenableFuture first, final CallableWithArgument,A> second) { - final ChainedSettableFuture returnVal = new ChainedSettableFuture(executor); - Futures.addCallback(first, new FutureCallback() + return doAfter(getTaskExecutor().getExecutor(), first, second); + } + + + protected static ChainedListenableFuture doAfter(final Executor executor, ListenableFuture first, final Callable> second) + { + final ChainedSettableFuture returnVal = new ChainedSettableFuture(executor); + Futures.addCallback(first, new FutureCallback() { @Override - public void onSuccess(final Void result) + public void onSuccess(final V result) { try { - final ListenableFuture future = second.call(); - Futures.addCallback(future, new FutureCallback() + final ListenableFuture future = second.call(); + Futures.addCallback(future, new FutureCallback() { @Override - public void onSuccess(final Void result) + public void onSuccess(final V result) { - returnVal.set(null); + returnVal.set(result); + } + + @Override + public void onFailure(final Throwable t) + { + returnVal.setException(t); + } + }, executor); + + } + catch(Throwable e) + { + returnVal.setException(e); + } + } + + @Override + public void onFailure(final Throwable t) + { + returnVal.setException(t); + } + }, executor); + + return returnVal; + } + + + protected static ChainedListenableFuture doAfter(final Executor executor, ListenableFuture first, final CallableWithArgument,A> second) + { + final ChainedSettableFuture returnVal = new ChainedSettableFuture<>(executor); + Futures.addCallback(first, new FutureCallback() + { + @Override + public void onSuccess(final A result) + { + try + { + final ListenableFuture future = second.call(result); + Futures.addCallback(future, new FutureCallback() + { + @Override + public void onSuccess(final V result) + { + returnVal.set(result); } @Override @@ -1983,6 +2068,7 @@ public abstract class AbstractConfiguredObject> im return returnVal; } + @Override public ListenableFuture setAttributesAsync(final Map attributes) throws IllegalStateException, AccessControlException, IllegalArgumentException { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObjectTypeFactory.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObjectTypeFactory.java index f97d2dfe14..d1a044f9dd 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObjectTypeFactory.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObjectTypeFactory.java @@ -23,6 +23,8 @@ package org.apache.qpid.server.model; import java.util.HashMap; import java.util.Map; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; @@ -72,14 +74,21 @@ abstract public class AbstractConfiguredObjectTypeFactory returnVal = SettableFuture.create(); final X instance = createInstance(attributes, parents); final ListenableFuture createFuture = instance.createAsync(); - createFuture.addListener(new Runnable() + Futures.addCallback(createFuture, new FutureCallback() { @Override - public void run() + public void onSuccess(final Void result) { returnVal.set(instance); } - }, MoreExecutors.sameThreadExecutor()); + + @Override + public void onFailure(final Throwable t) + { + returnVal.setException(t); + } + },MoreExecutors.sameThreadExecutor()); + return returnVal; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java index b058ec95b8..bbe7916883 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObject.java @@ -236,6 +236,9 @@ public interface ConfiguredObject> C createChild(Class childClass, Map attributes, ConfiguredObject... otherParents); + ListenableFuture createChildAsync(Class childClass, + Map attributes, + ConfiguredObject... otherParents); void setAttributes(Map attributes) throws IllegalStateException, AccessControlException, IllegalArgumentException; ListenableFuture setAttributesAsync(Map attributes) throws IllegalStateException, AccessControlException, IllegalArgumentException; diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java index 09e911d627..3fc6e13e87 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java @@ -22,6 +22,7 @@ package org.apache.qpid.server.model.adapter; import java.security.AccessControlException; import java.security.PrivilegedAction; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -42,7 +43,6 @@ import org.slf4j.LoggerFactory; import org.apache.qpid.common.QpidProperties; import org.apache.qpid.server.BrokerOptions; import org.apache.qpid.server.configuration.IllegalConfigurationException; -import org.apache.qpid.server.configuration.updater.Task; import org.apache.qpid.server.logging.EventLogger; import org.apache.qpid.server.logging.LogRecorder; import org.apache.qpid.server.logging.messages.BrokerMessages; @@ -288,7 +288,8 @@ public class BrokerAdapter extends AbstractConfiguredObject imple } } - final boolean brokerShutdownOnErroredChild = getContextValue(Boolean.class, BROKER_FAIL_STARTUP_WITH_ERRORED_CHILD); + final boolean brokerShutdownOnErroredChild = getContextValue(Boolean.class, + BROKER_FAIL_STARTUP_WITH_ERRORED_CHILD); if (!_parent.isManagementMode() && brokerShutdownOnErroredChild && hasBrokerAnyErroredChildren) { throw new IllegalStateException(String.format("Broker context variable %s is set and the broker has %s children", @@ -497,24 +498,32 @@ public class BrokerAdapter extends AbstractConfiguredObject imple return children; } - private VirtualHostNode createVirtualHostNode(Map attributes) + private ListenableFuture createVirtualHostNodeAsync(Map attributes) throws AccessControlException, IllegalArgumentException { - final VirtualHostNode virtualHostNode = getObjectFactory().create(VirtualHostNode.class,attributes, this); - - // permission has already been granted to create the virtual host - // disable further access check on other operations, e.g. create exchange - Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedAction() - { - @Override - public Object run() - { - virtualHostNode.start(); - return null; - } - }); - return virtualHostNode; + return doAfter(getObjectFactory().createAsync(VirtualHostNode.class, attributes, this), + new CallableWithArgument, VirtualHostNode>() + { + @Override + public ListenableFuture call(final VirtualHostNode virtualHostNode) + throws Exception + { + // permission has already been granted to create the virtual host + // disable further access check on other operations, e.g. create exchange + Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), + new PrivilegedAction() + { + @Override + public Object run() + { + virtualHostNode.start(); + return null; + } + }); + return Futures.immediateFuture(virtualHostNode); + } + }); } @Override @@ -543,140 +552,63 @@ public class BrokerAdapter extends AbstractConfiguredObject imple @SuppressWarnings("unchecked") @Override - public C addChild(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) + public ListenableFuture addChildAsync(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { - return runTask(new Task() + if (childClass == VirtualHostNode.class) { - @Override - public C execute() - { - if (childClass == VirtualHostNode.class) - { - return (C) createVirtualHostNode(attributes); - } - else if (childClass == Port.class) - { - return (C) createPort(attributes); - } - else if (childClass == AccessControlProvider.class) - { - return (C) createAccessControlProvider(attributes); - } - else if (childClass == AuthenticationProvider.class) - { - return (C) createAuthenticationProvider(attributes); - } - else if (childClass == KeyStore.class) - { - return (C) createKeyStore(attributes); - } - else if (childClass == TrustStore.class) - { - return (C) createTrustStore(attributes); - } - else if (childClass == GroupProvider.class) - { - return (C) createGroupProvider(attributes); - } - else - { - return createChild(childClass, attributes); - } - } - }); - - } - - /** - * Called when adding a new port via the management interface - */ - private Port createPort(Map attributes) - { - Port port = createChild(Port.class, attributes); - addPort(port); - return port; - } - - private void addPort(final Port port) - { - port.addChangeListener(this); - - } - - private AccessControlProvider createAccessControlProvider(final Map attributes) - { - final Collection> currentProviders = getAccessControlProviders(); - if(currentProviders != null && !currentProviders.isEmpty()) + return (ListenableFuture) createVirtualHostNodeAsync(attributes); + } + else if (Arrays.asList(Port.class, + AccessControlProvider.class, + AuthenticationProvider.class, + KeyStore.class, + TrustStore.class, + GroupProvider.class).contains(childClass)) { - throw new IllegalConfigurationException("Cannot add a second AccessControlProvider"); + return createAndAddChangeListener(childClass, attributes); + } + else + { + return getObjectFactory().createAsync(childClass, attributes, this); } - AccessControlProvider accessControlProvider = (AccessControlProvider) createChild(AccessControlProvider.class, attributes); - accessControlProvider.addChangeListener(this); - return accessControlProvider; } - private boolean deleteAccessControlProvider(AccessControlProvider accessControlProvider) + private ListenableFuture createAndAddChangeListener(Class clazz, Map attributes) { - accessControlProvider.removeChangeListener(this); - - return true; + return addChangeListener(getObjectFactory().createAsync(clazz, attributes, this)); } - private AuthenticationProvider createAuthenticationProvider(final Map attributes) + private ListenableFuture addChangeListener(ListenableFuture child) { - return runTask(new Task() + return doAfter(child, new CallableWithArgument, V>() { @Override - public AuthenticationProvider execute() + public ListenableFuture call(final V child) throws Exception { - AuthenticationProvider authenticationProvider = createChild(AuthenticationProvider.class, attributes); - addAuthenticationProvider(authenticationProvider); - - return authenticationProvider; + child.addChangeListener(BrokerAdapter.this); + return Futures.immediateFuture(child); } }); } - private X createChild(Class clazz, Map attributes) + private AccessControlProvider createAccessControlProvider(final Map attributes) { - if(!attributes.containsKey(ConfiguredObject.ID)) - { - attributes = new HashMap(attributes); - attributes.put(ConfiguredObject.ID, UUID.randomUUID()); - } - final X instance = (X) getObjectFactory().create(clazz,attributes, this); - return instance; - } - - /** - * @throws IllegalConfigurationException if an AuthenticationProvider with the same name already exists - */ - private void addAuthenticationProvider(AuthenticationProvider authenticationProvider) - { - authenticationProvider.addChangeListener(this); - } + AccessControlProvider accessControlProvider = (AccessControlProvider) (AccessControlProvider) getObjectFactory() + .create(AccessControlProvider.class, attributes, this); + accessControlProvider.addChangeListener(this); - private GroupProvider createGroupProvider(final Map attributes) - { - return runTask(new Task>() - { - @Override - public GroupProvider execute() - { - GroupProvider groupProvider = createChild(GroupProvider.class, attributes); - addGroupProvider(groupProvider); + return accessControlProvider; - return groupProvider; - } - }); } - private void addGroupProvider(GroupProvider groupProvider) + private boolean deleteAccessControlProvider(AccessControlProvider accessControlProvider) { - groupProvider.addChangeListener(this); + accessControlProvider.removeChangeListener(this); + + return true; } private boolean deleteGroupProvider(GroupProvider groupProvider) @@ -685,38 +617,12 @@ public class BrokerAdapter extends AbstractConfiguredObject imple return true; } - private KeyStore createKeyStore(Map attributes) - { - - KeyStore keyStore = createChild(KeyStore.class, attributes); - - addKeyStore(keyStore); - return keyStore; - } - - private TrustStore createTrustStore(Map attributes) - { - TrustStore trustStore = createChild(TrustStore.class, attributes); - addTrustStore(trustStore); - return trustStore; - } - - private void addKeyStore(KeyStore keyStore) - { - keyStore.addChangeListener(this); - } - private boolean deleteKeyStore(KeyStore keyStore) { keyStore.removeChangeListener(this); return true; } - private void addTrustStore(TrustStore trustStore) - { - trustStore.addChangeListener(this); - } - private boolean deleteTrustStore(TrustStore trustStore) { trustStore.removeChangeListener(this); @@ -740,11 +646,6 @@ public class BrokerAdapter extends AbstractConfiguredObject imple return true; } - private void addVirtualHostNode(VirtualHostNode virtualHostNode) - { - virtualHostNode.addChangeListener(this); - } - private boolean deleteVirtualHostNode(final VirtualHostNode virtualHostNode) throws AccessControlException, IllegalStateException { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java index 0b7be2c28a..2cca7cdc84 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/ConnectionAdapter.java @@ -231,7 +231,7 @@ public final class ConnectionAdapter extends AbstractConfiguredObject C addChild(Class childClass, Map attributes, ConfiguredObject... otherParents) + public ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { if(childClass == Session.class) { diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java index 7c34adbaa0..07c17c5c92 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileBasedGroupProviderImpl.java @@ -211,7 +211,7 @@ public class FileBasedGroupProviderImpl } @Override - public C addChild(Class childClass, + public ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { if (childClass == Group.class) @@ -231,7 +231,7 @@ public class FileBasedGroupProviderImpl attrMap.put(Group.NAME, groupName); GroupAdapter groupAdapter = new GroupAdapter(attrMap); groupAdapter.create(); - return (C) groupAdapter; + return Futures.immediateFuture((C) groupAdapter); } @@ -433,7 +433,7 @@ public class FileBasedGroupProviderImpl @Override - public C addChild(Class childClass, + public ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { @@ -448,7 +448,7 @@ public class FileBasedGroupProviderImpl attrMap.put(GroupMember.NAME, memberName); GroupMemberAdapter groupMemberAdapter = new GroupMemberAdapter(attrMap); groupMemberAdapter.create(); - return (C) groupMemberAdapter; + return Futures.immediateFuture((C) groupMemberAdapter); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java index 568fb546b5..8b977da478 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java @@ -3003,7 +3003,7 @@ public abstract class AbstractQueue> } @Override - protected C addChild(final Class childClass, + protected ListenableFuture addChildAsync(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { @@ -3016,12 +3016,12 @@ public abstract class AbstractQueue> { if(binding.getExchange() == otherParents[0] && binding.getName().equals(bindingKey)) { - return (C) binding; + return Futures.immediateFuture((C) binding); } } return null; } - return super.addChild(childClass, attributes, otherParents); + return super.addChildAsync(childClass, attributes, otherParents); } @Override diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractAuthenticationManager.java index 8f53eec4f2..6e9d9b7cd1 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractAuthenticationManager.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractAuthenticationManager.java @@ -141,15 +141,21 @@ public abstract class AbstractAuthenticationManager C addChild(Class childClass, Map attributes, ConfiguredObject... otherParents) + public ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { if(childClass == PreferencesProvider.class) { attributes = new HashMap<>(attributes); - PreferencesProvider pp = getObjectFactory().create(PreferencesProvider.class, attributes, this); - - _preferencesProvider = pp; - return (C)pp; + return doAfter(getObjectFactory().createAsync(PreferencesProvider.class, attributes, this), + new CallableWithArgument, PreferencesProvider>() + { + @Override + public ListenableFuture call(final PreferencesProvider preferencesProvider) throws Exception + { + _preferencesProvider = preferencesProvider; + return Futures.immediateFuture((C)preferencesProvider); + } + }); } throw new IllegalArgumentException("Cannot create child of class " + childClass.getSimpleName()); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ConfigModelPasswordManagingAuthenticationProvider.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ConfigModelPasswordManagingAuthenticationProvider.java index 50a2a36130..65e6128d71 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ConfigModelPasswordManagingAuthenticationProvider.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/ConfigModelPasswordManagingAuthenticationProvider.java @@ -32,6 +32,9 @@ import javax.security.auth.login.AccountNotFoundException; import javax.security.sasl.SaslException; import javax.security.sasl.SaslServer; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; + import org.apache.qpid.server.configuration.updater.Task; import org.apache.qpid.server.configuration.updater.VoidTaskWithException; import org.apache.qpid.server.model.Broker; @@ -189,7 +192,7 @@ public abstract class ConfigModelPasswordManagingAuthenticationProvider C addChild(final Class childClass, + public ListenableFuture addChildAsync(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { @@ -203,9 +206,9 @@ public abstract class ConfigModelPasswordManagingAuthenticationProvider C addChild(Class childClass, + public ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { @@ -351,10 +351,10 @@ public abstract class PrincipalDatabaseAuthenticationManager implements Gr } @Override - protected C addChild(final Class childClass, + protected ListenableFuture addChildAsync(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { if(childClass == GroupMember.class) { - return (C) getObjectFactory().create(childClass, attributes, this); + return getObjectFactory().createAsync(childClass, attributes, this); } else { - return super.addChild(childClass, attributes, otherParents); + return super.addChildAsync(childClass, attributes, otherParents); } } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/GroupProviderImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/GroupProviderImpl.java index 7dc032cc90..be3e405c82 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/GroupProviderImpl.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/group/GroupProviderImpl.java @@ -75,19 +75,18 @@ public class GroupProviderImpl extends AbstractConfiguredObject C addChild(final Class childClass, + protected ListenableFuture addChildAsync(final Class childClass, final Map attributes, final ConfiguredObject... otherParents) { if(childClass == Group.class) { - C child = (C) getObjectFactory().create(childClass, attributes, this); + return getObjectFactory().createAsync(childClass, attributes, this); - return child; } else { - return super.addChild(childClass, attributes, otherParents); + return super.addChildAsync(childClass, attributes, otherParents); } } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java index ccbee865fb..e74da76d4e 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java @@ -484,17 +484,17 @@ public abstract class AbstractVirtualHost> exte } @Override - protected C addChild(Class childClass, Map attributes, ConfiguredObject... otherParents) + protected ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { checkVHostStateIsActive(); if(childClass == Exchange.class) { - return (C) addExchange(attributes); + return (ListenableFuture) addExchangeAsync(attributes); } else if(childClass == Queue.class) { - return (C) addQueue(attributes); + return (ListenableFuture) addQueueAsync(attributes); } else if(childClass == VirtualHostAlias.class) @@ -693,7 +693,7 @@ public abstract class AbstractVirtualHost> exte return (AMQQueue )createChild(Queue.class, attributes); } - private AMQQueue addQueue(Map attributes) throws QueueExistsException + private ListenableFuture> addQueueAsync(Map attributes) throws QueueExistsException { if (shouldCreateDLQ(attributes)) { @@ -704,7 +704,7 @@ public abstract class AbstractVirtualHost> exte attributes = new LinkedHashMap(attributes); attributes.put(Queue.ALTERNATE_EXCHANGE, altExchangeName); } - return addQueueWithoutDLQ(attributes); + return Futures.immediateFuture(addQueueWithoutDLQ(attributes)); } private AMQQueue addQueueWithoutDLQ(Map attributes) throws QueueExistsException @@ -778,34 +778,34 @@ public abstract class AbstractVirtualHost> exte } - private ExchangeImpl addExchange(Map attributes) - throws ExchangeExistsException, ReservedExchangeNameException, - NoFactoryForTypeException - { - try - { - return (ExchangeImpl) getObjectFactory().create(Exchange.class, attributes, this); - } - catch (DuplicateNameException e) - { - throw new ExchangeExistsException(String.format("Exchange with name '%s' already exists", e.getName()), getExchange(e.getName())); - } - - } - private ListenableFuture addExchangeAsync(Map attributes) throws ExchangeExistsException, ReservedExchangeNameException, NoFactoryForTypeException { - try - { - ListenableFuture result = getObjectFactory().createAsync(Exchange.class, attributes, this); - return result; - } - catch (DuplicateNameException e) - { - throw new ExchangeExistsException(getExchange(e.getName())); - } + final SettableFuture returnVal = SettableFuture.create(); + Futures.addCallback(getObjectFactory().createAsync(Exchange.class, attributes, this), + new FutureCallback() + { + @Override + public void onSuccess(final Exchange result) + { + returnVal.set((ExchangeImpl) result); + } + + @Override + public void onFailure(final Throwable t) + { + if(t instanceof DuplicateNameException) + { + returnVal.setException(new ExchangeExistsException(getExchange(((DuplicateNameException)t).getName()))); + } + else + { + returnVal.setException(t); + } + } + }); + return returnVal; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java index 7bd1b14e42..018945b147 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractStandardVirtualHostNode.java @@ -60,14 +60,14 @@ public abstract class AbstractStandardVirtualHostNode C addChild(Class childClass, Map attributes, + protected ListenableFuture addChildAsync(Class childClass, Map attributes, ConfiguredObject... otherParents) { if(childClass == VirtualHost.class) { - return (C) getObjectFactory().create(VirtualHost.class, attributes, this); + return getObjectFactory().createAsync(childClass, attributes, this); } - return super.addChild(childClass, attributes, otherParents); + return super.addChildAsync(childClass, attributes, otherParents); } @Override diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/MD5AuthenticationManagerTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/MD5AuthenticationManagerTest.java index 25540dcb92..5281986325 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/MD5AuthenticationManagerTest.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/MD5AuthenticationManagerTest.java @@ -23,6 +23,7 @@ package org.apache.qpid.server.security.auth.manager; import javax.security.sasl.SaslServer; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ExecutionException; import org.apache.qpid.server.model.User; import org.apache.qpid.server.security.auth.AuthenticationResult; @@ -89,13 +90,13 @@ public class MD5AuthenticationManagerTest extends ManagedAuthenticationManagerTe return getAuthManager().authenticate(ss, response); } - private User createUser(String userName, String userPassword) + private User createUser(String userName, String userPassword) throws ExecutionException, InterruptedException { final Map childAttrs = new HashMap(); childAttrs.put(User.NAME, userName); childAttrs.put(User.PASSWORD, userPassword); - User user = getAuthManager().addChild(User.class, childAttrs); + User user = getAuthManager().addChildAsync(User.class, childAttrs).get(); assertNotNull("User should be created but addChild returned null", user); assertEquals(userName, user.getName()); return user; diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ManagedAuthenticationManagerTestBase.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ManagedAuthenticationManagerTestBase.java index 65cf7ad9d9..b23feb9605 100644 --- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ManagedAuthenticationManagerTestBase.java +++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/ManagedAuthenticationManagerTestBase.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; import javax.security.auth.login.AccountNotFoundException; import javax.security.sasl.SaslException; @@ -120,7 +121,7 @@ abstract class ManagedAuthenticationManagerTestBase extends QpidTestCase } - public void testAddChildAndThenDelete() + public void testAddChildAndThenDelete() throws ExecutionException, InterruptedException { // No children should be present before the test starts assertEquals("No users should be present before the test starts", 0, _authManager.getChildren(User.class).size()); @@ -130,7 +131,7 @@ abstract class ManagedAuthenticationManagerTestBase extends QpidTestCase childAttrs.put(User.NAME, getTestName()); childAttrs.put(User.PASSWORD, "password"); - User user = _authManager.addChild(User.class, childAttrs); + User user = _authManager.addChildAsync(User.class, childAttrs).get(); assertNotNull("User should be created but addChild returned null", user); assertEquals(getTestName(), user.getName()); if(!isPlain()) @@ -159,7 +160,7 @@ abstract class ManagedAuthenticationManagerTestBase extends QpidTestCase } - public void testCreateUser() + public void testCreateUser() throws ExecutionException, InterruptedException { assertEquals("No users should be present before the test starts", 0, _authManager.getChildren(User.class).size()); assertTrue(_authManager.createUser(getTestName(), "password", Collections.emptyMap())); @@ -178,7 +179,7 @@ abstract class ManagedAuthenticationManagerTestBase extends QpidTestCase childAttrs.put(User.PASSWORD, "password"); try { - user = _authManager.addChild(User.class, childAttrs); + user = _authManager.addChildAsync(User.class, childAttrs).get(); fail("Should not be able to create a second user with the same name"); } catch(IllegalArgumentException e) diff --git a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostNodeRestTest.java b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostNodeRestTest.java index 083187b8db..6efdeace9e 100644 --- a/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostNodeRestTest.java +++ b/qpid/java/systests/src/test/java/org/apache/qpid/systest/rest/VirtualHostNodeRestTest.java @@ -63,6 +63,29 @@ public class VirtualHostNodeRestTest extends QpidRestTestCase assertFalse("Store should not exist after deletion", storePathAsFile.exists()); } + public void testCreateVirtualHostNodeWithVirtualHost() throws Exception + { + String nodeName = "virtualhostnode-" + getTestName(); + + Map nodeData = new HashMap(); + nodeData.put(VirtualHostNode.NAME, nodeName); + nodeData.put(VirtualHostNode.TYPE, getTestProfileVirtualHostNodeType()); + + nodeData.put("virtualHostInitialConfiguration", "{ \"type\" : \"DERBY\" }"); + + getRestTestHelper().submitRequest("virtualhostnode/" + nodeName, + "PUT", + nodeData, + HttpServletResponse.SC_CREATED); + + + Map virtualhostNode = getRestTestHelper().getJsonAsSingletonList("virtualhostnode/" + nodeName); + Asserts.assertVirtualHostNode(nodeName, virtualhostNode); + + Map virtualhost = getRestTestHelper().getJsonAsSingletonList("virtualhost/" + nodeName + "/" + nodeName); + Asserts.assertVirtualHost(nodeName, virtualhost); + } + public void testCreateVirtualHostNodeWithDefaultStorePath() throws Exception { String virtualhostNodeType = getTestProfileVirtualHostNodeType(); -- cgit v1.2.1