From 6125de2e1aa8779d7fd1e59f378ce0a2225d453f Mon Sep 17 00:00:00 2001 From: Robert Godfrey Date: Mon, 28 Apr 2014 16:32:02 +0000 Subject: QPID-5578 : Address Review comment from Keith Wall re: r1589912 git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1590699 13f79535-47bb-0310-9956-ffa450edef68 --- .../server/exchange/AMQUnknownExchangeType.java | 41 ---------- .../server/model/ConfiguredObjectFactoryImpl.java | 88 +++++++++++----------- .../model/NoFactoryForCategoryException.java | 37 +++++++++ .../server/model/NoFactoryForTypeException.java | 45 +++++++++++ .../server/virtualhost/AbstractVirtualHost.java | 8 +- .../qpid/server/virtualhost/VirtualHostImpl.java | 4 +- .../protocol/v0_10/ServerSessionDelegate.java | 4 +- .../v0_8/handler/ExchangeDeclareHandler.java | 4 +- .../server/jmx/mbeans/VirtualHostManagerMBean.java | 4 +- .../destination/AddressBasedDestinationTest.java | 25 ++++-- 10 files changed, 157 insertions(+), 103 deletions(-) delete mode 100644 qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AMQUnknownExchangeType.java create mode 100644 qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForCategoryException.java create mode 100644 qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForTypeException.java (limited to 'qpid/java') diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AMQUnknownExchangeType.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AMQUnknownExchangeType.java deleted file mode 100644 index d5daf22a52..0000000000 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/exchange/AMQUnknownExchangeType.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * - */ - -package org.apache.qpid.server.exchange; - -/** - * AMQUnknownExchangeType represents coding error where unknown exchange type requested from exchange factory. - * - *

- *
CRC Card
Responsibilities Collaborations - *
Represents unknown exchange type request. - *
- * - * @todo Represent coding error, where unknown exchange type is requested by passing a string parameter. Use a type safe - * enum for the exchange type, or replace with IllegalArgumentException. Should be runtime. - */ -public class AMQUnknownExchangeType extends RuntimeException -{ - public AMQUnknownExchangeType(String message, Throwable cause) - { - super(message, cause); - } -} diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectFactoryImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectFactoryImpl.java index 5ac25cce57..440a790fc8 100644 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectFactoryImpl.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredObjectFactoryImpl.java @@ -44,49 +44,42 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory public ConfiguredObjectFactoryImpl(Model model) { _model = model; - try + QpidServiceLoader serviceLoader = + new QpidServiceLoader(); + Iterable allFactories = + serviceLoader.instancesOf(ConfiguredObjectTypeFactory.class); + for (ConfiguredObjectTypeFactory factory : allFactories) { - QpidServiceLoader serviceLoader = - new QpidServiceLoader(); - Iterable allFactories = - serviceLoader.instancesOf(ConfiguredObjectTypeFactory.class); - for (ConfiguredObjectTypeFactory factory : allFactories) - { - final Class categoryClass = factory.getCategoryClass(); - final String categoryName = categoryClass.getSimpleName(); - - Map categoryFactories = _allFactories.get(categoryName); - if (categoryFactories == null) - { - categoryFactories = new HashMap(); - _allFactories.put(categoryName, categoryFactories); - _supportedTypes.put(categoryName, new ArrayList()); - ManagedObject annotation = categoryClass.getAnnotation(ManagedObject.class); - if (annotation != null && !"".equals(annotation.defaultType())) - { - _defaultTypes.put(categoryName, annotation.defaultType()); - } - else - { - _defaultTypes.put(categoryName, categoryName); - } + final Class categoryClass = factory.getCategoryClass(); + final String categoryName = categoryClass.getSimpleName(); - } - if (categoryFactories.put(factory.getType(), factory) != null) + Map categoryFactories = _allFactories.get(categoryName); + if (categoryFactories == null) + { + categoryFactories = new HashMap(); + _allFactories.put(categoryName, categoryFactories); + _supportedTypes.put(categoryName, new ArrayList()); + ManagedObject annotation = categoryClass.getAnnotation(ManagedObject.class); + if (annotation != null && !"".equals(annotation.defaultType())) { - throw new ServerScopedRuntimeException( - "Misconfiguration - there is more than one factory defined for class " + categoryName - + " with type " + factory.getType()); + _defaultTypes.put(categoryName, annotation.defaultType()); } - if (factory.getType() != null) + else { - _supportedTypes.get(categoryName).add(factory.getType()); + _defaultTypes.put(categoryName, categoryName); } + + } + if (categoryFactories.put(factory.getType(), factory) != null) + { + throw new ServerScopedRuntimeException( + "Misconfiguration - there is more than one factory defined for class " + categoryName + + " with type " + factory.getType()); + } + if (factory.getType() != null) + { + _supportedTypes.get(categoryName).add(factory.getType()); } - } - catch (RuntimeException | Error e) - { - e.printStackTrace(); } } @@ -103,7 +96,7 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory if(factory == null) { - throw new ServerScopedRuntimeException("No factory defined for ConfiguredObject of category " + category + " and type " + type); + throw new NoFactoryForTypeException(category, type); } return factory.recover(this, record, parents); @@ -115,10 +108,7 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory final ConfiguredObject... parents) { ConfiguredObjectTypeFactory factory = getConfiguredObjectTypeFactory(clazz, attributes); - if(factory == null) - { - throw new ServerScopedRuntimeException("No factory defined for ConfiguredObject of category " + clazz.getSimpleName() + " and attributes " + attributes); - } + return factory.create(this, attributes, parents); } @@ -130,7 +120,7 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory Map categoryFactories = _allFactories.get(category); if(categoryFactories == null) { - throw new ServerScopedRuntimeException("No factory defined for ConfiguredObject of category " + category); + throw new NoFactoryForCategoryException(category); } String type = (String) attributes.get(ConfiguredObject.TYPE); @@ -139,6 +129,10 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory if(type != null) { factory = getConfiguredObjectTypeFactory(category, type); + if(factory == null) + { + throw new NoFactoryForTypeException(category, type); + } } else { @@ -147,6 +141,10 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory { ManagedObject annotation = categoryClass.getAnnotation(ManagedObject.class); factory = getConfiguredObjectTypeFactory(category, annotation.defaultType()); + if(factory == null) + { + throw new NoFactoryForTypeException(category, annotation.defaultType()); + } } } return factory; @@ -159,12 +157,16 @@ public class ConfiguredObjectFactoryImpl implements ConfiguredObjectFactory Map categoryFactories = _allFactories.get(category); if(categoryFactories == null) { - throw new ServerScopedRuntimeException("No factory defined for ConfiguredObject of category " + category); + throw new NoFactoryForCategoryException(category); } ConfiguredObjectTypeFactory factory = categoryFactories.get(type); if(factory == null) { factory = categoryFactories.get(_defaultTypes.get(category)); + if(factory == null) + { + throw new NoFactoryForTypeException(category, _defaultTypes.get(category)); + } } return factory; } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForCategoryException.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForCategoryException.java new file mode 100644 index 0000000000..4e249ab5dc --- /dev/null +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForCategoryException.java @@ -0,0 +1,37 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.qpid.server.model; + +public class NoFactoryForCategoryException extends RuntimeException +{ + private final String _category; + + public NoFactoryForCategoryException(final String category) + { + super("Unknown category: " + category); + _category = category; + } + + public String getCategory() + { + return _category; + } +} diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForTypeException.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForTypeException.java new file mode 100644 index 0000000000..1ccf685495 --- /dev/null +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/NoFactoryForTypeException.java @@ -0,0 +1,45 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.qpid.server.model; + +public class NoFactoryForTypeException extends RuntimeException +{ + private final String _category; + private final String _type; + + public NoFactoryForTypeException(final String category, + final String type) + { + super("Unknown configured object type '"+type+"' of category '" + category +"'"); + _category = category; + _type = type; + } + + public String getCategory() + { + return _category; + } + + public String getType() + { + return _type; + } +} 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 dd97b0cba9..93006823d9 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 @@ -41,12 +41,12 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.security.auth.Subject; import org.apache.log4j.Logger; + import org.apache.qpid.exchange.ExchangeDefaults; import org.apache.qpid.server.configuration.BrokerProperties; import org.apache.qpid.server.configuration.IllegalConfigurationException; import org.apache.qpid.server.connection.ConnectionRegistry; import org.apache.qpid.server.connection.IConnectionRegistry; -import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.exchange.DefaultDestination; import org.apache.qpid.server.exchange.ExchangeImpl; import org.apache.qpid.server.logging.EventLogger; @@ -671,7 +671,7 @@ public abstract class AbstractVirtualHost> exte @Override public ExchangeImpl createExchange(Map attributes) throws ExchangeExistsException, ReservedExchangeNameException, - AMQUnknownExchangeType + NoFactoryForTypeException { checkVHostStateIsActive(); ExchangeImpl child = addExchange(attributes); @@ -682,7 +682,7 @@ public abstract class AbstractVirtualHost> exte private ExchangeImpl addExchange(Map attributes) throws ExchangeExistsException, ReservedExchangeNameException, - AMQUnknownExchangeType + NoFactoryForTypeException { try { @@ -1329,7 +1329,7 @@ public abstract class AbstractVirtualHost> exte // We're ok if the exchange already exists dlExchange = e.getExistingExchange(); } - catch (ReservedExchangeNameException | AMQUnknownExchangeType | UnknownConfiguredObjectException e) + catch (ReservedExchangeNameException | NoFactoryForTypeException | UnknownConfiguredObjectException e) { throw new ConnectionScopedRuntimeException("Attempt to create an alternate exchange for a queue failed",e); } diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java index 46351aa970..d3698495c3 100755 --- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java +++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java @@ -27,12 +27,12 @@ import java.util.concurrent.ScheduledFuture; import org.apache.qpid.common.Closeable; import org.apache.qpid.server.connection.IConnectionRegistry; -import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.exchange.ExchangeImpl; import org.apache.qpid.server.logging.EventLogger; import org.apache.qpid.server.logging.EventLoggerProvider; import org.apache.qpid.server.message.MessageDestination; import org.apache.qpid.server.message.MessageSource; +import org.apache.qpid.server.model.NoFactoryForTypeException; import org.apache.qpid.server.model.VirtualHost; import org.apache.qpid.server.protocol.LinkRegistry; import org.apache.qpid.server.queue.AMQQueue; @@ -66,7 +66,7 @@ public interface VirtualHostImpl< X extends VirtualHostImpl, Q extends AM E createExchange(Map attributes) throws ExchangeExistsException, ReservedExchangeNameException, - AMQUnknownExchangeType; + NoFactoryForTypeException; void removeExchange(E exchange, boolean force) throws ExchangeIsAlternateException, RequiredExchangeException; diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java index ff57e6a33c..db94249c64 100644 --- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java +++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java @@ -33,7 +33,6 @@ import java.util.UUID; import org.apache.log4j.Logger; import org.apache.qpid.server.consumer.ConsumerImpl; -import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.exchange.DirectExchange; import org.apache.qpid.server.exchange.ExchangeImpl; import org.apache.qpid.server.exchange.HeadersExchange; @@ -47,6 +46,7 @@ import org.apache.qpid.server.message.MessageReference; import org.apache.qpid.server.message.MessageSource; import org.apache.qpid.server.model.ExclusivityPolicy; import org.apache.qpid.server.model.LifetimePolicy; +import org.apache.qpid.server.model.NoFactoryForTypeException; import org.apache.qpid.server.model.Queue; import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.plugin.ExchangeType; @@ -755,7 +755,7 @@ public class ServerSessionDelegate extends SessionDelegate exception(session, method, ExecutionErrorCode.NOT_FOUND, "Unknown alternate exchange " + e.getName()); } - catch(AMQUnknownExchangeType e) + catch(NoFactoryForTypeException e) { exception(session, method, ExecutionErrorCode.NOT_FOUND, "Unknown Exchange Type: " + method.getType()); } diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java index efd50e5595..73b53c5e64 100644 --- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java +++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ExchangeDeclareHandler.java @@ -33,10 +33,10 @@ import org.apache.qpid.framing.AMQShortString; import org.apache.qpid.framing.ExchangeDeclareBody; import org.apache.qpid.framing.MethodRegistry; import org.apache.qpid.protocol.AMQConstant; -import org.apache.qpid.server.exchange.AMQUnknownExchangeType; import org.apache.qpid.server.exchange.DirectExchange; import org.apache.qpid.server.exchange.ExchangeImpl; import org.apache.qpid.server.model.LifetimePolicy; +import org.apache.qpid.server.model.NoFactoryForTypeException; import org.apache.qpid.server.model.UnknownConfiguredObjectException; import org.apache.qpid.server.protocol.v0_8.AMQChannel; import org.apache.qpid.server.protocol.v0_8.AMQProtocolSession; @@ -147,7 +147,7 @@ public class ExchangeDeclareHandler implements StateAwareMethodListener