From 33913d16da7b2e0fd0515c773c26e0072212f17a Mon Sep 17 00:00:00 2001 From: Tomas Olvecky Date: Wed, 19 Feb 2014 15:43:41 +0100 Subject: [PATCH] Fix some sonar bugs in config-api and config-manager. Change-Id: I417bb4bdd649606d871382e7bf561d35daafc11b Signed-off-by: Tomas Olvecky --- .../config/api/IdentityAttributeRef.java | 15 +++-- .../controller/config/api/JmxAttribute.java | 12 ++-- .../config/api/ModuleIdentifier.java | 24 ++++---- .../config/api/ValidationException.java | 21 ++++--- .../config/api/jmx/CommitStatus.java | 55 ++++++++++--------- .../config/api/jmx/ObjectNameUtil.java | 28 ++++++---- .../constants/ConfigRegistryConstants.java | 7 ++- .../manager/impl/ConfigRegistryImpl.java | 45 ++++++++------- .../impl/ConfigTransactionControllerImpl.java | 25 +++++---- .../impl/ModuleInternalTransactionalInfo.java | 18 +++--- .../impl/ServiceReferenceRegistryImpl.java | 16 +++--- .../manager/impl/TransactionIdentifier.java | 9 ++- .../manager/impl/TransactionStatus.java | 9 ++- .../DependencyResolverImpl.java | 19 +++---- .../dynamicmbean/AbstractDynamicWrapper.java | 48 ++++++++-------- .../impl/dynamicmbean/AnnotationsHelper.java | 6 +- .../impl/dynamicmbean/AttributeHolder.java | 24 ++++---- ...ierarchicalConfigMBeanFactoriesHolder.java | 14 ++--- .../impl/jmx/InternalJMXRegistrator.java | 45 ++++++++------- .../manager/impl/jmx/ServiceReference.java | 16 ++++-- .../impl/jmx/TransactionJMXRegistrator.java | 14 ++--- ...eContextBackedModuleFactoriesResolver.java | 22 +++----- .../impl/osgi/ConfigManagerActivator.java | 6 +- .../plugin/ftl/FtlFilePersister.java | 2 +- .../ModuleMXBeanEntryBuilder.java | 9 +-- .../attribute/TOAttribute.java | 40 +++++++------- 26 files changed, 294 insertions(+), 255 deletions(-) diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/IdentityAttributeRef.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/IdentityAttributeRef.java index 73737593cf..5ad6e0da8d 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/IdentityAttributeRef.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/IdentityAttributeRef.java @@ -19,8 +19,9 @@ public final class IdentityAttributeRef { @ConstructorProperties(QNAME_ATTR_NAME) public IdentityAttributeRef(String qNameOfIdentity) { - if (qNameOfIdentity == null) + if (qNameOfIdentity == null) { throw new NullPointerException("Parameter " + QNAME_ATTR_NAME + " is null"); + } this.qNameOfIdentity = qNameOfIdentity; } @@ -46,12 +47,18 @@ public final class IdentityAttributeRef { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof IdentityAttributeRef)) return false; + if (this == o) { + return true; + } + if (!(o instanceof IdentityAttributeRef)) { + return false; + } IdentityAttributeRef that = (IdentityAttributeRef) o; - if (!qNameOfIdentity.equals(that.qNameOfIdentity)) return false; + if (!qNameOfIdentity.equals(that.qNameOfIdentity)) { + return false; + } return true; } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttribute.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttribute.java index 96deb051ef..244f22f58e 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttribute.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/JmxAttribute.java @@ -15,8 +15,9 @@ public class JmxAttribute { private final String attributeName; public JmxAttribute(String attributeName) { - if (attributeName == null) + if (attributeName == null) { throw new NullPointerException("Parameter 'attributeName' is null"); + } this.attributeName = attributeName; } @@ -26,16 +27,19 @@ public class JmxAttribute { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } JmxAttribute that = (JmxAttribute) o; if (attributeName != null ? !attributeName.equals(that.attributeName) - : that.attributeName != null) + : that.attributeName != null) { return false; + } return true; } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleIdentifier.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleIdentifier.java index c654f5fe0a..7baaf9f6e4 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleIdentifier.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ModuleIdentifier.java @@ -14,12 +14,12 @@ public class ModuleIdentifier implements Identifier { private final String factoryName, instanceName; public ModuleIdentifier(String factoryName, String instanceName) { - if (factoryName == null) - throw new IllegalArgumentException( - "Parameter 'factoryName' is null"); - if (instanceName == null) - throw new IllegalArgumentException( - "Parameter 'instanceName' is null"); + if (factoryName == null) { + throw new IllegalArgumentException("Parameter 'factoryName' is null"); + } + if (instanceName == null) { + throw new IllegalArgumentException("Parameter 'instanceName' is null"); + } this.factoryName = factoryName; this.instanceName = instanceName; } @@ -34,17 +34,21 @@ public class ModuleIdentifier implements Identifier { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } ModuleIdentifier that = (ModuleIdentifier) o; - if (!factoryName.equals(that.factoryName)) + if (!factoryName.equals(that.factoryName)) { return false; - if (!instanceName.equals(that.instanceName)) + } + if (!instanceName.equals(that.instanceName)) { return false; + } return true; } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ValidationException.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ValidationException.java index f27d123995..a93d97c7ee 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ValidationException.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ValidationException.java @@ -118,23 +118,30 @@ public class ValidationException extends RuntimeException { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } ExceptionMessageWithStackTrace other = (ExceptionMessageWithStackTrace) obj; if (message == null) { - if (other.message != null) + if (other.message != null) { return false; - } else if (!message.equals(other.message)) + } + } else if (!message.equals(other.message)) { return false; + } if (stackTrace == null) { - if (other.stackTrace != null) + if (other.stackTrace != null) { return false; - } else if (!stackTrace.equals(other.stackTrace)) + } + } else if (!stackTrace.equals(other.stackTrace)) { return false; + } return true; } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/CommitStatus.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/CommitStatus.java index bd53bd4e58..4f9ea7ac6b 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/CommitStatus.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/CommitStatus.java @@ -7,32 +7,27 @@ */ package org.opendaylight.controller.config.api.jmx; +import javax.annotation.concurrent.Immutable; +import javax.management.ObjectName; import java.beans.ConstructorProperties; import java.util.Collections; import java.util.List; -import javax.annotation.concurrent.Immutable; -import javax.management.ObjectName; - @Immutable public class CommitStatus { private final List newInstances, reusedInstances, recreatedInstances; /** - * - * @param newInstances - * newly created instances - * @param reusedInstances - * reused instances - * @param recreatedInstances - * recreated instances + * @param newInstances newly created instances + * @param reusedInstances reused instances + * @param recreatedInstances recreated instances */ - @ConstructorProperties({ "newInstances", "reusedInstances", - "recreatedInstances" }) + @ConstructorProperties({"newInstances", "reusedInstances", + "recreatedInstances"}) public CommitStatus(List newInstances, - List reusedInstances, - List recreatedInstances) { + List reusedInstances, + List recreatedInstances) { this.newInstances = Collections.unmodifiableList(newInstances); this.reusedInstances = Collections.unmodifiableList(reusedInstances); this.recreatedInstances = Collections @@ -40,7 +35,6 @@ public class CommitStatus { } /** - * * @return list of objectNames representing newly created instances */ public List getNewInstances() { @@ -48,7 +42,6 @@ public class CommitStatus { } /** - * * @return list of objectNames representing reused instances */ public List getReusedInstances() { @@ -56,7 +49,6 @@ public class CommitStatus { } /** - * * @return list of objectNames representing recreated instances */ public List getRecreatedInstances() { @@ -72,7 +64,7 @@ public class CommitStatus { result = prime * result + ((recreatedInstances == null) ? 0 : recreatedInstances - .hashCode()); + .hashCode()); result = prime * result + ((reusedInstances == null) ? 0 : reusedInstances.hashCode()); return result; @@ -80,28 +72,37 @@ public class CommitStatus { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } CommitStatus other = (CommitStatus) obj; if (newInstances == null) { - if (other.newInstances != null) + if (other.newInstances != null) { return false; - } else if (!newInstances.equals(other.newInstances)) + } + } else if (!newInstances.equals(other.newInstances)) { return false; + } if (recreatedInstances == null) { - if (other.recreatedInstances != null) + if (other.recreatedInstances != null) { return false; - } else if (!recreatedInstances.equals(other.recreatedInstances)) + } + } else if (!recreatedInstances.equals(other.recreatedInstances)) { return false; + } if (reusedInstances == null) { - if (other.reusedInstances != null) + if (other.reusedInstances != null) { return false; - } else if (!reusedInstances.equals(other.reusedInstances)) + } + } else if (!reusedInstances.equals(other.reusedInstances)) { return false; + } return true; } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java index 3baa1039e0..d60e608617 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/ObjectNameUtil.java @@ -11,6 +11,7 @@ import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.jmx.constants.ConfigRegistryConstants; import javax.annotation.concurrent.ThreadSafe; +import javax.management.MalformedObjectNameException; import javax.management.ObjectName; import java.util.Arrays; import java.util.HashMap; @@ -46,8 +47,8 @@ public class ObjectNameUtil { public static ObjectName createON(String on) { try { return new ObjectName(on); - } catch (Exception e) { - throw new RuntimeException(e); + } catch (MalformedObjectNameException e) { + throw new IllegalArgumentException(e); } } @@ -63,8 +64,8 @@ public class ObjectNameUtil { Hashtable table = new Hashtable<>(attribs); try { return new ObjectName(domain, table); - } catch (Exception e) { - throw new RuntimeException(e); + } catch (MalformedObjectNameException e) { + throw new IllegalArgumentException(e); } } @@ -116,8 +117,7 @@ public class ObjectNameUtil { public static String getServiceQName(ObjectName objectName) { checkType(objectName, TYPE_SERVICE_REFERENCE); String quoted = objectName.getKeyProperty(SERVICE_QNAME_KEY); - String result = unquoteAndUnescape(objectName, quoted); - return result; + return unquoteAndUnescape(objectName, quoted); } // ObjectName supports quotation and ignores tokens like =, but fails to ignore ? sign. @@ -292,8 +292,8 @@ public class ObjectNameUtil { } } - public static void checkTypeOneOf(ObjectName objectName, String ... types) { - for(String type: types) { + public static void checkTypeOneOf(ObjectName objectName, String... types) { + for (String type : types) { if (type.equals(objectName.getKeyProperty(TYPE_KEY))) { return; } @@ -304,10 +304,12 @@ public class ObjectNameUtil { public static ObjectName createModulePattern(String moduleName, String instanceName) { - if (moduleName == null) + if (moduleName == null) { moduleName = "*"; - if (instanceName == null) + } + if (instanceName == null) { instanceName = "*"; + } // do not return object names containing transaction name ObjectName namePattern = ObjectNameUtil .createON(ObjectNameUtil.ON_DOMAIN + ":" @@ -343,13 +345,15 @@ public class ObjectNameUtil { String expectedType) { checkType(objectName, expectedType); String factoryName = getFactoryName(objectName); - if (factoryName == null) + if (factoryName == null) { throw new IllegalArgumentException( "ObjectName does not contain module name"); + } String instanceName = getInstanceName(objectName); - if (instanceName == null) + if (instanceName == null) { throw new IllegalArgumentException( "ObjectName does not contain instance name"); + } return new ModuleIdentifier(factoryName, instanceName); } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/constants/ConfigRegistryConstants.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/constants/ConfigRegistryConstants.java index 81a29bf7b3..1d9563bf4e 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/constants/ConfigRegistryConstants.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/jmx/constants/ConfigRegistryConstants.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.api.jmx.constants; +import javax.management.MalformedObjectNameException; import javax.management.ObjectName; public class ConfigRegistryConstants { @@ -19,7 +20,7 @@ public class ConfigRegistryConstants { public static final ObjectName OBJECT_NAME = createONWithDomainAndType(TYPE_CONFIG_REGISTRY); - public static String GET_AVAILABLE_MODULE_NAMES_ATTRIBUT_NAME = "AvailableModuleNames"; + public static final String GET_AVAILABLE_MODULE_NAMES_ATTRIBUT_NAME = "AvailableModuleNames"; public static ObjectName createONWithDomainAndType(String type) { return createON(ON_DOMAIN, TYPE_KEY, type); @@ -28,8 +29,8 @@ public class ConfigRegistryConstants { public static ObjectName createON(String name, String key, String value) { try { return new ObjectName(name, key, value); - } catch (Exception e) { - throw new RuntimeException(e); + } catch (MalformedObjectNameException e) { + throw new IllegalArgumentException(e); } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java index 39682fa6b4..8f6a4654b2 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java @@ -105,27 +105,27 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe private List lastListOfFactories = Collections.emptyList(); @GuardedBy("this") // switched in every 2ndPC - private CloseableServiceReferenceReadableRegistry readableSRRegistry = ServiceReferenceRegistryImpl.createInitialSRLookupRegistry(); + private CloseableServiceReferenceReadableRegistry readableSRRegistry = ServiceReferenceRegistryImpl.createInitialSRLookupRegistry(); // constructor public ConfigRegistryImpl(ModuleFactoriesResolver resolver, - MBeanServer configMBeanServer, CodecRegistry codecRegistry) { + MBeanServer configMBeanServer, CodecRegistry codecRegistry) { this(resolver, configMBeanServer, new BaseJMXRegistrator(configMBeanServer), codecRegistry); } // constructor public ConfigRegistryImpl(ModuleFactoriesResolver resolver, - MBeanServer configMBeanServer, - BaseJMXRegistrator baseJMXRegistrator, CodecRegistry codecRegistry) { + MBeanServer configMBeanServer, + BaseJMXRegistrator baseJMXRegistrator, CodecRegistry codecRegistry) { this.resolver = resolver; - this.beanToOsgiServiceManager = new BeanToOsgiServiceManager(); + beanToOsgiServiceManager = new BeanToOsgiServiceManager(); this.configMBeanServer = configMBeanServer; this.baseJMXRegistrator = baseJMXRegistrator; this.codecRegistry = codecRegistry; - this.registryMBeanServer = MBeanServerFactory + registryMBeanServer = MBeanServerFactory .createMBeanServer("ConfigRegistry" + configMBeanServer.getDefaultDomain()); - this.transactionsMBeanServer = MBeanServerFactory + transactionsMBeanServer = MBeanServerFactory .createMBeanServer("ConfigTransactions" + configMBeanServer.getDefaultDomain()); } @@ -181,6 +181,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe * {@inheritDoc} */ @Override + @SuppressWarnings("PMD.AvoidCatchingThrowable") public synchronized CommitStatus commitConfig(ObjectName transactionControllerON) throws ConflictingVersionException, ValidationException { final String transactionName = ObjectNameUtil @@ -211,7 +212,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe return secondPhaseCommit( configTransactionController, commitInfo); } catch (Throwable t) { // some libs throw Errors: e.g. - // javax.xml.ws.spi.FactoryFinder$ConfigurationError + // javax.xml.ws.spi.FactoryFinder$ConfigurationError isHealthy = false; logger.error("Configuration Transaction failed on 2PC, server is unhealthy", t); if (t instanceof RuntimeException) { @@ -219,7 +220,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe } else if (t instanceof Error) { throw (Error) t; } else { - throw new RuntimeException(t); + throw new IllegalStateException(t); } } } @@ -232,7 +233,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe for (DestroyedModule toBeDestroyed : commitInfo .getDestroyedFromPreviousTransactions()) { toBeDestroyed.close(); // closes instance (which should close - // runtime jmx registrator), + // runtime jmx registrator), // also closes osgi registration and ModuleJMXRegistrator // registration currentConfig.remove(toBeDestroyed.getIdentifier()); @@ -278,9 +279,10 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe for (ModuleIdentifier moduleIdentifier : orderedModuleIdentifiers) { ModuleInternalTransactionalInfo entry = commitInfo.getCommitted() .get(moduleIdentifier); - if (entry == null) + if (entry == null) { throw new NullPointerException("Module not found " + moduleIdentifier); + } Module module = entry.getModule(); ObjectName primaryReadOnlyON = ObjectNameUtil .createReadOnlyModuleON(moduleIdentifier); @@ -342,7 +344,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // register to OSGi if (osgiRegistration == null) { ModuleFactory moduleFactory = entry.getModuleFactory(); - if(moduleFactory != null) { + if (moduleFactory != null) { BundleContext bc = configTransactionController. getModuleFactoryBundleContext(moduleFactory.getImplementationName()); osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(), @@ -369,8 +371,8 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe version = configTransactionController.getVersion(); // switch readable Service Reference Registry - this.readableSRRegistry.close(); - this.readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( + readableSRRegistry.close(); + readableSRRegistry = ServiceReferenceRegistryImpl.createSRReadableRegistry( configTransactionController.getWritableRegistry(), this, baseJMXRegistrator); return new CommitStatus(newInstances, reusedInstances, @@ -485,7 +487,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe */ @Override public Set lookupConfigBeans(String moduleName, - String instanceName) { + String instanceName) { ObjectName namePattern = ObjectNameUtil.createModulePattern(moduleName, instanceName); return baseJMXRegistrator.queryNames(namePattern, null); @@ -504,11 +506,13 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe */ @Override public Set lookupRuntimeBeans(String moduleName, - String instanceName) { - if (moduleName == null) + String instanceName) { + if (moduleName == null) { moduleName = "*"; - if (instanceName == null) + } + if (instanceName == null) { instanceName = "*"; + } ObjectName namePattern = ObjectNameUtil.createRuntimeBeanPattern( moduleName, instanceName); return baseJMXRegistrator.queryNames(namePattern, null); @@ -646,7 +650,6 @@ class TransactionsHolder { * {@link ConfigTransactionControllerInternal} instances, because platform * MBeanServer transforms mbeans into another representation. Map is cleaned * every time current transactions are requested. - * */ @GuardedBy("ConfigRegistryImpl.this") private final Map transactions = new HashMap<>(); @@ -655,7 +658,7 @@ class TransactionsHolder { * Can only be called from within synchronized method. */ public void add(String transactionName, - ConfigTransactionControllerInternal transactionController) { + ConfigTransactionControllerInternal transactionController) { Object oldValue = transactions.put(transactionName, transactionController); if (oldValue != null) { @@ -674,7 +677,7 @@ class TransactionsHolder { public Map getCurrentTransactions() { // first, remove closed transaction for (Iterator> it = transactions - .entrySet().iterator(); it.hasNext();) { + .entrySet().iterator(); it.hasNext(); ) { Entry entry = it .next(); if (entry.getValue().isClosed()) { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java index 0ec6969802..f7afded51f 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java @@ -53,7 +53,7 @@ import static java.lang.String.format; class ConfigTransactionControllerImpl implements ConfigTransactionControllerInternal, ConfigTransactionControllerImplMXBean, - Identifiable{ + Identifiable { private static final Logger logger = LoggerFactory.getLogger(ConfigTransactionControllerImpl.class); private final ConfigTransactionLookupRegistry txLookupRegistry; @@ -124,12 +124,12 @@ class ConfigTransactionControllerImpl implements List toBeAdded = new ArrayList<>(); List toBeRemoved = new ArrayList<>(); - for(ModuleFactory moduleFactory: factoriesHolder.getModuleFactories()) { - if (oldSet.contains(moduleFactory) == false){ + for (ModuleFactory moduleFactory : factoriesHolder.getModuleFactories()) { + if (oldSet.contains(moduleFactory) == false) { toBeAdded.add(moduleFactory); } } - for(ModuleFactory moduleFactory: lastListOfFactories){ + for (ModuleFactory moduleFactory : lastListOfFactories) { if (newSet.contains(moduleFactory) == false) { toBeRemoved.add(moduleFactory); } @@ -151,7 +151,7 @@ class ConfigTransactionControllerImpl implements } // remove modules belonging to removed factories - for(ModuleFactory removedFactory: toBeRemoved){ + for (ModuleFactory removedFactory : toBeRemoved) { List modulesOfRemovedFactory = dependencyResolverManager.findAllByFactory(removedFactory); for (ModuleIdentifier name : modulesOfRemovedFactory) { destroyModule(name); @@ -189,7 +189,7 @@ class ConfigTransactionControllerImpl implements @Override public synchronized ObjectName createModule(String factoryName, - String instanceName) throws InstanceAlreadyExistsException { + String instanceName) throws InstanceAlreadyExistsException { transactionStatus.checkNotCommitStarted(); transactionStatus.checkNotAborted(); @@ -213,11 +213,11 @@ class ConfigTransactionControllerImpl implements throws InstanceAlreadyExistsException { logger.debug("Adding module {} to transaction {}", moduleIdentifier, this); - if (moduleIdentifier.equals(module.getIdentifier())==false) { + if (moduleIdentifier.equals(module.getIdentifier()) == false) { throw new IllegalStateException("Incorrect name reported by module. Expected " - + moduleIdentifier + ", got " + module.getIdentifier()); + + moduleIdentifier + ", got " + module.getIdentifier()); } - if (dependencyResolver.getIdentifier().equals(moduleIdentifier) == false ) { + if (dependencyResolver.getIdentifier().equals(moduleIdentifier) == false) { throw new IllegalStateException("Incorrect name reported by dependency resolver. Expected " + moduleIdentifier + ", got " + dependencyResolver.getIdentifier()); } @@ -271,7 +271,7 @@ class ConfigTransactionControllerImpl implements // first remove refNames, it checks for objectname existence try { writableSRRegistry.removeServiceReferences( - ObjectNameUtil.createTransactionModuleON(getTransactionName(),moduleIdentifier)); + ObjectNameUtil.createTransactionModuleON(getTransactionName(), moduleIdentifier)); } catch (InstanceNotFoundException e) { logger.error("Possible code error: cannot find {} in {}", moduleIdentifier, writableSRRegistry); throw new IllegalStateException("Possible code error: cannot find " + moduleIdentifier, e); @@ -294,8 +294,9 @@ class ConfigTransactionControllerImpl implements @Override public synchronized void validateConfig() throws ValidationException { - if (configBeanModificationDisabled.get()) + if (configBeanModificationDisabled.get()) { throw new IllegalStateException("Cannot start validation"); + } configBeanModificationDisabled.set(true); try { validate_noLocks(); @@ -383,7 +384,7 @@ class ConfigTransactionControllerImpl implements logger.error("Commit failed on {} in transaction {}", name, getTransactionIdentifier(), e); internalAbort(); - throw new RuntimeException( + throw new IllegalStateException( format("Error - getInstance() failed for %s in transaction %s", name, getTransactionIdentifier()), e); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java index 0a4ceacb43..f598433433 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java @@ -7,16 +7,15 @@ */ package org.opendaylight.controller.config.manager.impl; -import javax.annotation.Nullable; - import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.manager.impl.dynamicmbean.DynamicReadableWrapper; -import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator - .TransactionModuleJMXRegistration; +import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator.TransactionModuleJMXRegistration; import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; import org.opendaylight.yangtools.concepts.Identifiable; +import javax.annotation.Nullable; + public class ModuleInternalTransactionalInfo implements Identifiable { private final ModuleIdentifier name; private final Module module; @@ -27,10 +26,10 @@ public class ModuleInternalTransactionalInfo implements Identifiable> factoryNamesToQNames = new HashMap<>(); + Map> modifiableFactoryNamesToQNames = new HashMap<>(); Set allAnnotations = new HashSet<>(); Set allQNames = new HashSet<>(); for (Entry entry : factories.entrySet()) { @@ -210,27 +210,27 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } allAnnotations.addAll(siAnnotations); allQNames.addAll(qNames); - factoryNamesToQNames.put(entry.getKey(), Collections.unmodifiableSet(qNames)); + modifiableFactoryNamesToQNames.put(entry.getKey(), Collections.unmodifiableSet(qNames)); } - this.factoryNamesToQNames = Collections.unmodifiableMap(factoryNamesToQNames); + this.factoryNamesToQNames = Collections.unmodifiableMap(modifiableFactoryNamesToQNames); this.allQNames = Collections.unmodifiableSet(allQNames); // fill namespacesToAnnotations - Map> namespacesToAnnotations = + Map> modifiableNamespacesToAnnotations = new HashMap<>(); for (ServiceInterfaceAnnotation sia : allAnnotations) { - Map ofNamespace = namespacesToAnnotations.get(sia.namespace()); + Map ofNamespace = modifiableNamespacesToAnnotations.get(sia.namespace()); if (ofNamespace == null) { ofNamespace = new HashMap<>(); - namespacesToAnnotations.put(sia.namespace(), ofNamespace); + modifiableNamespacesToAnnotations.put(sia.namespace(), ofNamespace); } if (ofNamespace.containsKey(sia.localName())) { logger.error("Cannot construct namespacesToAnnotations map, conflict between local names in {}, offending local name: {}, map so far {}", - sia.namespace(), sia.localName(), namespacesToAnnotations); + sia.namespace(), sia.localName(), modifiableNamespacesToAnnotations); throw new IllegalArgumentException("Conflict between local names in " + sia.namespace() + " : " + sia.localName()); } ofNamespace.put(sia.localName(), sia); } - this.namespacesToAnnotations = Collections.unmodifiableMap(namespacesToAnnotations); + this.namespacesToAnnotations = Collections.unmodifiableMap(modifiableNamespacesToAnnotations); // copy refNames logger.trace("factoryNamesToQNames:{}", this.factoryNamesToQNames); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionIdentifier.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionIdentifier.java index 28bd613648..15c69bf25f 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionIdentifier.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionIdentifier.java @@ -28,15 +28,18 @@ public class TransactionIdentifier implements Identifier { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } TransactionIdentifier that = (TransactionIdentifier) o; - if (name != null ? !name.equals(that.name) : that.name != null) + if (name != null ? !name.equals(that.name) : that.name != null) { return false; + } return true; } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionStatus.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionStatus.java index a25062074b..f86d6b1d81 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionStatus.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/TransactionStatus.java @@ -48,18 +48,21 @@ public class TransactionStatus { } public synchronized void checkNotCommitStarted() { - if (secondPhaseCommitStarted == true) + if (secondPhaseCommitStarted == true) { throw new IllegalStateException("Commit was triggered"); + } } public synchronized void checkCommitStarted() { - if (secondPhaseCommitStarted == false) + if (secondPhaseCommitStarted == false) { throw new IllegalStateException("Commit was not triggered"); + } } public synchronized void checkNotAborted() { - if (aborted == true) + if (aborted == true) { throw new IllegalStateException("Configuration was aborted"); + } } public synchronized void checkNotCommitted() { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java index ec9678fd2d..e3057a1179 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverImpl.java @@ -39,7 +39,7 @@ import static java.lang.String.format; * during validation. Tracks dependencies for ordering purposes. */ final class DependencyResolverImpl implements DependencyResolver, - Comparable { + Comparable { private static final Logger logger = LoggerFactory.getLogger(DependencyResolverImpl.class); private final ModulesHolder modulesHolder; @@ -74,15 +74,15 @@ final class DependencyResolverImpl implements DependencyResolver, throw new NullPointerException( "Parameter 'expectedServiceInterface' is null"); } - if (jmxAttribute == null) + if (jmxAttribute == null) { throw new NullPointerException("Parameter 'jmxAttribute' is null"); + } JmxAttributeValidationException.checkNotNull(dependentReadOnlyON, "is null, expected dependency implementing " + expectedServiceInterface, jmxAttribute); - // check that objectName belongs to this transaction - this should be // stripped // in DynamicWritableWrapper @@ -135,7 +135,7 @@ final class DependencyResolverImpl implements DependencyResolver, //TODO: check for cycles @Override public T resolveInstance(Class expectedType, ObjectName dependentReadOnlyON, - JmxAttribute jmxAttribute) { + JmxAttribute jmxAttribute) { if (expectedType == null || dependentReadOnlyON == null || jmxAttribute == null) { throw new IllegalArgumentException(format( "Null parameters not allowed, got %s %s %s", expectedType, @@ -161,8 +161,7 @@ final class DependencyResolverImpl implements DependencyResolver, throw new JmxAttributeValidationException(message, jmxAttribute); } try { - T result = expectedType.cast(instance); - return result; + return expectedType.cast(instance); } catch (ClassCastException e) { String message = format( "Instance cannot be cast to expected type. Instance class is %s , " @@ -178,7 +177,7 @@ final class DependencyResolverImpl implements DependencyResolver, IdentityCodec identityCodec = codecRegistry.getIdentityCodec(); Class deserialized = identityCodec.deserialize(qName); if (deserialized == null) { - throw new RuntimeException("Unable to retrieve identity class for " + qName + ", null response from " + throw new IllegalStateException("Unable to retrieve identity class for " + qName + ", null response from " + codecRegistry); } if (expectedBaseClass.isAssignableFrom(deserialized)) { @@ -194,7 +193,7 @@ final class DependencyResolverImpl implements DependencyResolver, public void validateIdentity(IdentityAttributeRef identityRef, Class expectedBaseClass, JmxAttribute jmxAttribute) { try { resolveIdentity(identityRef, expectedBaseClass); - } catch(Exception e) { + } catch (Exception e) { throw JmxAttributeValidationException.wrap(e, jmxAttribute); } } @@ -224,8 +223,8 @@ final class DependencyResolverImpl implements DependencyResolver, } private static int getMaxDepth(DependencyResolverImpl impl, - DependencyResolverManager manager, - LinkedHashSet chainForDetectingCycles) { + DependencyResolverManager manager, + LinkedHashSet chainForDetectingCycles) { int maxDepth = 0; LinkedHashSet chainForDetectingCycles2 = new LinkedHashSet<>( chainForDetectingCycles); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java index 6f0d1b2682..7e48af1caa 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AbstractDynamicWrapper.java @@ -55,7 +55,6 @@ import static java.lang.String.format; * requests (getAttribute, setAttribute, invoke) into the actual instance, but * provides additional functionality - namely it disallows setting attribute on * a read only wrapper. - * */ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { private static final Logger logger = LoggerFactory @@ -71,9 +70,9 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { protected final MBeanServer internalServer; public AbstractDynamicWrapper(Module module, boolean writable, - ModuleIdentifier moduleIdentifier, - ObjectName thisWrapperObjectName, MBeanOperationInfo[] dOperations, - MBeanServer internalServer, MBeanServer configMBeanServer) { + ModuleIdentifier moduleIdentifier, + ObjectName thisWrapperObjectName, MBeanOperationInfo[] dOperations, + MBeanServer internalServer, MBeanServer configMBeanServer) { this.writable = writable; this.module = module; @@ -98,10 +97,10 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { * case unregister the module and remove listener. */ private final NotificationListener registerActualModule(Module module, - final ObjectName thisWrapperObjectName, - final ObjectName objectNameInternal, - final MBeanServer internalServer, - final MBeanServer configMBeanServer) { + final ObjectName thisWrapperObjectName, + final ObjectName objectNameInternal, + final MBeanServer internalServer, + final MBeanServer configMBeanServer) { try { internalServer.registerMBean(module, objectNameInternal); @@ -116,7 +115,7 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { public void handleNotification(Notification n, Object handback) { if (n instanceof MBeanServerNotification && n.getType() - .equals(MBeanServerNotification.UNREGISTRATION_NOTIFICATION)) { + .equals(MBeanServerNotification.UNREGISTRATION_NOTIFICATION)) { if (((MBeanServerNotification) n).getMBeanName().equals( thisWrapperObjectName)) { try { @@ -142,8 +141,8 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } private static MBeanInfo generateMBeanInfo(String className, Module module, - Map attributeHolderMap, - MBeanOperationInfo[] dOperations, Set> jmxInterfaces) { + Map attributeHolderMap, + MBeanOperationInfo[] dOperations, Set> jmxInterfaces) { String dDescription = findDescription(module.getClass(), jmxInterfaces); MBeanConstructorInfo[] dConstructors = new MBeanConstructorInfo[0]; @@ -170,9 +169,9 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { // inspect all exported interfaces ending with MXBean, extract getters & // setters into attribute holder private static Map buildMBeanInfo(Module module, - boolean writable, ModuleIdentifier moduleIdentifier, - Set> jmxInterfaces, MBeanServer internalServer, - ObjectName internalObjectName) { + boolean writable, ModuleIdentifier moduleIdentifier, + Set> jmxInterfaces, MBeanServer internalServer, + ObjectName internalObjectName) { // internal variables for describing MBean elements Set methods = new HashSet<>(); @@ -217,7 +216,7 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } AttributeHolder attributeHolder = new AttributeHolder( attribName, module, attributeMap.get(attribName) - .getType(), writable, ifc, description); + .getType(), writable, ifc, description); attributeHolderMap.put(attribName, attributeHolder); } } @@ -257,7 +256,7 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } - if(isDependencyListAttr(attributeName, obj)) { + if (isDependencyListAttr(attributeName, obj)) { obj = fixDependencyListAttribute(obj); } @@ -265,14 +264,16 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } private Object fixDependencyListAttribute(Object attribute) { - if(attribute.getClass().isArray() == false) + if (attribute.getClass().isArray() == false) { throw new IllegalArgumentException("Unexpected attribute type, should be an array, but was " + attribute.getClass()); + } for (int i = 0; i < Array.getLength(attribute); i++) { Object on = Array.get(attribute, i); - if(on instanceof ObjectName == false) + if (on instanceof ObjectName == false) { throw new IllegalArgumentException("Unexpected attribute type, should be an ObjectName, but was " + on.getClass()); + } on = fixObjectName((ObjectName) on); Array.set(attribute, i, on); @@ -282,8 +283,9 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } private boolean isDependencyListAttr(String attributeName, Object attribute) { - if (attributeHolderMap.containsKey(attributeName) == false) + if (attributeHolderMap.containsKey(attributeName) == false) { return false; + } AttributeHolder attributeHolder = attributeHolderMap.get(attributeName); @@ -294,15 +296,17 @@ abstract class AbstractDynamicWrapper implements DynamicMBeanModuleWrapper { } protected ObjectName fixObjectName(ObjectName on) { - if (!ObjectNameUtil.ON_DOMAIN.equals(on.getDomain())) + if (!ObjectNameUtil.ON_DOMAIN.equals(on.getDomain())) { throw new IllegalArgumentException("Wrong domain, expected " + ObjectNameUtil.ON_DOMAIN + " setter on " + on); + } // if on contains transaction name, remove it String transactionName = ObjectNameUtil.getTransactionName(on); - if (transactionName != null) + if (transactionName != null) { return ObjectNameUtil.withoutTransactionName(on); - else + } else { return on; + } } @Override diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AnnotationsHelper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AnnotationsHelper.java index 64664f7980..f3e1b4e705 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AnnotationsHelper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AnnotationsHelper.java @@ -7,15 +7,15 @@ */ package org.opendaylight.controller.config.manager.impl.dynamicmbean; +import org.opendaylight.controller.config.api.annotations.Description; + import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; import java.util.Set; -import org.opendaylight.controller.config.api.annotations.Description; - -class AnnotationsHelper { +public class AnnotationsHelper { /** * Look for annotation specified by annotationType on method. First observe diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AttributeHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AttributeHolder.java index 9dd6a2269e..044f7a9ada 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AttributeHolder.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dynamicmbean/AttributeHolder.java @@ -40,9 +40,9 @@ class AttributeHolder { } public AttributeHolder(String name, Object object, String returnType, - boolean writable, - @Nullable RequireInterface requireInterfaceAnnotation, - String description) { + boolean writable, + @Nullable RequireInterface requireInterfaceAnnotation, + String description) { if (name == null) { throw new NullPointerException(); } @@ -65,7 +65,7 @@ class AttributeHolder { /** * @return annotation if setter sets ObjectName or ObjectName[], and is - * annotated. Return null otherwise. + * annotated. Return null otherwise. */ RequireInterface getRequireInterfaceOrNull() { return requireInterfaceAnnotation; @@ -98,7 +98,7 @@ class AttributeHolder { * @param setter * @param jmxInterfaces * @return empty string if no annotation is found, or list of descriptions - * separated by newline + * separated by newline */ static String findDescription(Method setter, Set> jmxInterfaces) { List descriptions = AnnotationsHelper @@ -112,21 +112,21 @@ class AttributeHolder { * * @param setter * @param inspectedInterfaces - * @throws IllegalStateException - * if more than one value is specified by found annotations - * @throws IllegalArgumentException - * if set of exported interfaces contains non interface type * @return null if no annotation is found, otherwise return the annotation + * @throws IllegalStateException if more than one value is specified by found annotations + * @throws IllegalArgumentException if set of exported interfaces contains non interface type */ static RequireInterface findRequireInterfaceAnnotation(final Method setter, - Set> inspectedInterfaces) { + Set> inspectedInterfaces) { // only allow setX(ObjectName y) or setX(ObjectName[] y) or setX(List y) to continue - if (setter.getParameterTypes().length > 1) + if (setter.getParameterTypes().length > 1) { return null; - if(PERMITTED_PARAMETER_TYPES_FOR_DEPENDENCY_SETTER.contains(setter.getParameterTypes()[0]) == false) + } + if (PERMITTED_PARAMETER_TYPES_FOR_DEPENDENCY_SETTER.contains(setter.getParameterTypes()[0]) == false) { return null; + } List foundRequireInterfaces = AnnotationsHelper .findMethodAnnotationInSuperClassesAndIfcs(setter, RequireInterface.class, inspectedInterfaces); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java index 16f7cf024a..adc8168af5 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java @@ -10,25 +10,21 @@ package org.opendaylight.controller.config.manager.impl.factoriesresolver; import org.opendaylight.controller.config.spi.ModuleFactory; import org.osgi.framework.BundleContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.util.Map.Entry; -import java.util.Set; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Collections; +import java.util.Map.Entry; +import java.util.Set; import java.util.TreeSet; -import java.util.Collection; -import java.util.ArrayList; /** * Hold sorted ConfigMBeanFactories by their module names. Check that module * names are globally unique. */ public class HierarchicalConfigMBeanFactoriesHolder { - private static final Logger logger = LoggerFactory - .getLogger(HierarchicalConfigMBeanFactoriesHolder.class); private final Map> moduleNamesToConfigBeanFactories; private final Set moduleNames; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java index 5d771560a5..98f0908dc7 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/InternalJMXRegistrator.java @@ -7,22 +7,24 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; -import java.io.Closeable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.concurrent.GuardedBy; import javax.management.InstanceAlreadyExistsException; +import javax.management.InstanceNotFoundException; import javax.management.JMX; +import javax.management.MBeanRegistrationException; import javax.management.MBeanServer; +import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; import javax.management.QueryExp; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.io.Closeable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; public class InternalJMXRegistrator implements Closeable { private static final Logger logger = LoggerFactory @@ -38,7 +40,7 @@ public class InternalJMXRegistrator implements Closeable { private final ObjectName on; InternalJMXRegistration(InternalJMXRegistrator internalJMXRegistrator, - ObjectName on) { + ObjectName on) { this.internalJMXRegistrator = internalJMXRegistrator; this.on = on; } @@ -54,13 +56,11 @@ public class InternalJMXRegistrator implements Closeable { private final List children = new ArrayList<>(); public synchronized InternalJMXRegistration registerMBean(Object object, - ObjectName on) throws InstanceAlreadyExistsException { + ObjectName on) throws InstanceAlreadyExistsException { try { configMBeanServer.registerMBean(object, on); - } catch (InstanceAlreadyExistsException e) { - throw e; - } catch (Exception e) { - throw new RuntimeException(e); + } catch (MBeanRegistrationException | NotCompliantMBeanException e) { + throw new IllegalStateException(e); } registeredObjectNames.add(on); return new InternalJMXRegistration(this, on); @@ -69,14 +69,13 @@ public class InternalJMXRegistrator implements Closeable { private synchronized void unregisterMBean(ObjectName on) { // first check that on was registered using this instance boolean removed = registeredObjectNames.remove(on); - if (!removed) - throw new IllegalStateException( - "Cannot unregister - ObjectName not found in 'registeredObjectNames': " - + on); + if (!removed) { + throw new IllegalStateException("Cannot unregister - ObjectName not found in 'registeredObjectNames': " + on); + } try { configMBeanServer.unregisterMBean(on); - } catch (Exception e) { - throw new RuntimeException(e); + } catch (InstanceNotFoundException | MBeanRegistrationException e) { + throw new IllegalStateException(e); } } @@ -112,7 +111,7 @@ public class InternalJMXRegistrator implements Closeable { } public T newMBeanProxy(ObjectName objectName, Class interfaceClass, - boolean notificationBroadcaster) { + boolean notificationBroadcaster) { return JMX.newMBeanProxy(configMBeanServer, objectName, interfaceClass, notificationBroadcaster); } @@ -123,7 +122,7 @@ public class InternalJMXRegistrator implements Closeable { } public T newMXBeanProxy(ObjectName objectName, Class interfaceClass, - boolean notificationBroadcaster) { + boolean notificationBroadcaster) { return JMX.newMXBeanProxy(configMBeanServer, objectName, interfaceClass, notificationBroadcaster); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReference.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReference.java index 849f75234f..cd74ddf756 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReference.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/ServiceReference.java @@ -25,13 +25,21 @@ public class ServiceReference { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } ServiceReference that = (ServiceReference) o; - if (!refName.equals(that.refName)) return false; - if (!serviceInterfaceName.equals(that.serviceInterfaceName)) return false; + if (!refName.equals(that.refName)) { + return false; + } + if (!serviceInterfaceName.equals(that.serviceInterfaceName)) { + return false; + } return true; } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java index b371c3f170..6fd2a2fc65 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/jmx/TransactionJMXRegistrator.java @@ -7,15 +7,14 @@ */ package org.opendaylight.controller.config.manager.impl.jmx; -import java.io.Closeable; -import java.util.Set; +import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; +import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; import javax.management.InstanceAlreadyExistsException; import javax.management.ObjectName; import javax.management.QueryExp; - -import org.opendaylight.controller.config.api.jmx.ObjectNameUtil; -import org.opendaylight.controller.config.manager.impl.jmx.InternalJMXRegistrator.InternalJMXRegistration; +import java.io.Closeable; +import java.util.Set; /** * Contains constraints on passed {@link ObjectName} parameters. Only allow (un) @@ -26,7 +25,7 @@ public class TransactionJMXRegistrator implements Closeable { private final String transactionName; TransactionJMXRegistrator(InternalJMXRegistrator internalJMXRegistrator, - String transactionName) { + String transactionName) { this.childJMXRegistrator = internalJMXRegistrator.createChild(); this.transactionName = transactionName; } @@ -46,10 +45,11 @@ public class TransactionJMXRegistrator implements Closeable { public TransactionJMXRegistration registerMBean(Object object, ObjectName on) throws InstanceAlreadyExistsException { - if (!transactionName.equals(ObjectNameUtil.getTransactionName(on))) + if (!transactionName.equals(ObjectNameUtil.getTransactionName(on))) { throw new IllegalArgumentException( "Transaction name mismatch between expected " + transactionName + " " + "and " + on); + } ObjectNameUtil.checkType(on, ObjectNameUtil.TYPE_CONFIG_TRANSACTION); return new TransactionJMXRegistration( childJMXRegistrator.registerMBean(object, on)); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java index 2a533ab9a3..1e94e5e9c0 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java @@ -53,7 +53,7 @@ public class BundleContextBackedModuleFactoriesResolver implements if(factory == null) { throw new NullPointerException("ServiceReference of class" + serviceReference.getClass() + "not found."); } - StringBuffer errors = new StringBuffer(); + String moduleName = factory.getImplementationName(); if (moduleName == null || moduleName.isEmpty()) { throw new IllegalStateException( @@ -63,23 +63,17 @@ public class BundleContextBackedModuleFactoriesResolver implements throw new NullPointerException("Bundle context of " + factory + " ModuleFactory not found."); } logger.debug("Reading factory {} {}", moduleName, factory); - String error = null; + Map.Entry conflicting = result.get(moduleName); if (conflicting != null) { - error = String - .format("Module name is not unique. Found two conflicting factories with same name '%s': " + - "\n\t%s\n\t%s\n", moduleName, conflicting.getKey(), factory); - - } - - if (error == null) { + String error = String + .format("Module name is not unique. Found two conflicting factories with same name '%s': '%s' '%s'", + moduleName, conflicting.getKey(), factory); + logger.error(error); + throw new IllegalArgumentException(error); + } else { result.put(moduleName, new AbstractMap.SimpleImmutableEntry<>(factory, serviceReference.getBundle().getBundleContext())); - } else { - errors.append(error); - } - if (errors.length() > 0) { - throw new IllegalArgumentException(errors.toString()); } } return result; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java index d464cb9006..02cc5ea1e4 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java @@ -41,7 +41,7 @@ public class ConfigManagerActivator implements BundleActivator { private RuntimeGeneratedMappingServiceActivator mappingServiceActivator; @Override - public void start(BundleContext context) throws Exception { + public void start(BundleContext context) { // track bundles containing YangModuleInfo ModuleInfoBundleTracker moduleInfoBundleTracker = new ModuleInfoBundleTracker(); @@ -72,7 +72,7 @@ public class ConfigManagerActivator implements BundleActivator { try { configRegistryJMXRegistrator.registerToJMX(configRegistry); } catch (InstanceAlreadyExistsException e) { - throw new RuntimeException("Config Registry was already registered to JMX", e); + throw new IllegalStateException("Config Registry was already registered to JMX", e); } ServiceTracker serviceTracker = new ServiceTracker<>(context, ModuleFactory.class, @@ -81,7 +81,7 @@ public class ConfigManagerActivator implements BundleActivator { } @Override - public void stop(BundleContext context) throws Exception { + public void stop(BundleContext context) { try { configRegistry.close(); } catch (Exception e) { diff --git a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/FtlFilePersister.java b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/FtlFilePersister.java index d6d3893beb..37d5e6bd3f 100644 --- a/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/FtlFilePersister.java +++ b/opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/FtlFilePersister.java @@ -53,7 +53,7 @@ public class FtlFilePersister { ftlFile.getFtlTempleteLocation()); try { template.process(ftlFile, writer); - } catch (Throwable e) { + } catch (Exception e) { throw new IllegalStateException( "Template error while generating " + ftlFile, e); } diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java index 4a9d5512d3..676c8eca6e 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java @@ -195,16 +195,17 @@ final class ModuleMXBeanEntryBuilder { boolean providedServiceWasSet = false; for (UnknownSchemaNode unknownNode : id.getUnknownSchemaNodes()) { // TODO: test this - if (ConfigConstants.PROVIDED_SERVICE_EXTENSION_QNAME.equals(unknownNode.getNodeType())) { - // no op: 0 or more provided identities are allowed - } else if (ConfigConstants.JAVA_NAME_PREFIX_EXTENSION_QNAME.equals(unknownNode.getNodeType())) { + boolean unknownNodeIsProvidedServiceExtension = ConfigConstants.PROVIDED_SERVICE_EXTENSION_QNAME.equals(unknownNode.getNodeType()); + // true => no op: 0 or more provided identities are allowed + + if (ConfigConstants.JAVA_NAME_PREFIX_EXTENSION_QNAME.equals(unknownNode.getNodeType())) { // 0..1 allowed checkState( providedServiceWasSet == false, format("More than one language extension %s is not allowed here: %s", ConfigConstants.JAVA_NAME_PREFIX_EXTENSION_QNAME, id)); providedServiceWasSet = true; - } else { + } else if (unknownNodeIsProvidedServiceExtension == false) { throw new IllegalStateException("Unexpected language extension " + unknownNode.getNodeType()); } } diff --git a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/attribute/TOAttribute.java b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/attribute/TOAttribute.java index 84300cb81d..cbeb5c3b29 100644 --- a/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/attribute/TOAttribute.java +++ b/opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/attribute/TOAttribute.java @@ -25,7 +25,9 @@ import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; import javax.management.openmbean.CompositeType; import javax.management.openmbean.OpenDataException; import javax.management.openmbean.OpenType; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -208,22 +210,20 @@ public class TOAttribute extends AbstractAttribute implements TypedAttribute { @Override public CompositeType getOpenType() { - String description = getNullableDescription() == null ? getAttributeYangName() - : getNullableDescription(); - final String[] itemNames = new String[yangNameToAttributeMap.keySet() - .size()]; - String[] itemDescriptions = itemNames; - FunctionImpl functionImpl = new FunctionImpl(itemNames); + String description = getNullableDescription() == null ? getAttributeYangName() : getNullableDescription(); + + FunctionImpl functionImpl = new FunctionImpl(); Map jmxPropertiesToTypesMap = getJmxPropertiesToTypesMap(); OpenType[] itemTypes = Collections2.transform( jmxPropertiesToTypesMap.entrySet(), functionImpl).toArray( new OpenType[] {}); + String[] itemNames = functionImpl.getItemNames(); try { // TODO add package name to create fully qualified name for this // type CompositeType compositeType = new CompositeType( getUpperCaseCammelCase(), description, itemNames, - itemDescriptions, itemTypes); + itemNames, itemTypes); return compositeType; } catch (OpenDataException e) { throw new RuntimeException("Unable to create CompositeType for " @@ -235,20 +235,20 @@ public class TOAttribute extends AbstractAttribute implements TypedAttribute { return packageName; } - private static final class FunctionImpl implements - Function, OpenType> { - private final String[] itemNames; - int i = 0; +} - private FunctionImpl(String[] itemNames) { - this.itemNames = itemNames; - } +class FunctionImpl implements + Function, OpenType> { + private final List itemNames = new ArrayList<>(); - @Override - public OpenType apply(Entry input) { - AttributeIfc innerType = input.getValue(); - itemNames[i++] = input.getKey(); - return innerType.getOpenType(); - } + @Override + public OpenType apply(Entry input) { + AttributeIfc innerType = input.getValue(); + itemNames.add(input.getKey()); + return innerType.getOpenType(); + } + + public String[] getItemNames(){ + return itemNames.toArray(new String[itemNames.size()]); } } -- 2.36.6