From: Robert Varga Date: Tue, 5 Oct 2021 18:06:01 +0000 (+0200) Subject: Ditch blueprint from srm-impl X-Git-Tag: release/sulfur~10 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F55%2F97755%2F2;p=serviceutils.git Ditch blueprint from srm-impl We are using a dead plugin to generate half of wiring, with the rest hand-written. Convert to OSGi DS, noticing a private method, which really should be part of interface (in keeping with others). This also fixes a TOCTOU race in removeRecoverableListener(), which could lead to NPEs. Since we are now SCR-enabled, we can ditch the use of ODL blueprint extensions in shell, making everyone a bit happier. Change-Id: Ia9d4d0fc3f0bc430d3a106f53975ab7d6e1241d0 Signed-off-by: Robert Varga --- diff --git a/srm/api/pom.xml b/srm/api/pom.xml index 346a4c47..1973faa6 100644 --- a/srm/api/pom.xml +++ b/srm/api/pom.xml @@ -16,9 +16,7 @@ and is available at http://www.eclipse.org/legal/epl-v10.html ../../commons/binding-parent - org.opendaylight.serviceutils srm-api - 0.9.0-SNAPSHOT bundle diff --git a/srm/api/src/main/java/org/opendaylight/serviceutils/srm/ServiceRecoveryRegistry.java b/srm/api/src/main/java/org/opendaylight/serviceutils/srm/ServiceRecoveryRegistry.java index f5356a0c..470ce3d2 100644 --- a/srm/api/src/main/java/org/opendaylight/serviceutils/srm/ServiceRecoveryRegistry.java +++ b/srm/api/src/main/java/org/opendaylight/serviceutils/srm/ServiceRecoveryRegistry.java @@ -19,4 +19,6 @@ public interface ServiceRecoveryRegistry { void removeRecoverableListener(String serviceName, RecoverableListener recoverableListener); Queue getRecoverableListeners(String serviceName); + + ServiceRecoveryInterface getRegisteredServiceRecoveryHandler(String entityName); } \ No newline at end of file diff --git a/srm/impl/pom.xml b/srm/impl/pom.xml index f38f4f45..e9d84bac 100644 --- a/srm/impl/pom.xml +++ b/srm/impl/pom.xml @@ -17,9 +17,7 @@ and is available at http://www.eclipse.org/legal/epl-v10.html ../../commons/quality-parent - org.opendaylight.serviceutils srm-impl - 0.9.0-SNAPSHOT bundle @@ -51,19 +49,13 @@ and is available at http://www.eclipse.org/legal/epl-v10.html true - org.apache.aries.blueprint - blueprint-maven-plugin-annotation - provided + javax.annotation + javax.annotation-api true + + org.osgi + org.osgi.service.component.annotations + - - - - - org.apache.aries.blueprint - blueprint-maven-plugin - - - diff --git a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryListener.java b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryListener.java index 6d16a96a..2284509f 100644 --- a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryListener.java +++ b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryListener.java @@ -12,40 +12,68 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.serviceutils.srm.ServiceRecoveryRegistry; import org.opendaylight.serviceutils.tools.mdsal.listener.AbstractClusteredSyncDataTreeChangeListener; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.ops.rev180626.ServiceOps; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.ops.rev180626.service.ops.Services; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.ops.rev180626.service.ops.services.Operations; +import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.types.rev180626.EntityNameBase; +import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.types.rev180626.EntityTypeBase; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.RequireServiceComponentRuntime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Singleton -public class ServiceRecoveryListener extends AbstractClusteredSyncDataTreeChangeListener { - +@Component +@RequireServiceComponentRuntime +public final class ServiceRecoveryListener extends AbstractClusteredSyncDataTreeChangeListener { private static final Logger LOG = LoggerFactory.getLogger(ServiceRecoveryListener.class); - private final ServiceRecoveryManager serviceRecoveryManager; + private final ServiceRecoveryRegistry serviceRecoveryRegistry; @Inject - public ServiceRecoveryListener(DataBroker dataBroker, ServiceRecoveryManager serviceRecoveryManager) { + @Activate + public ServiceRecoveryListener(@Reference DataBroker dataBroker, + @Reference ServiceRecoveryRegistry serviceRecoveryRegistry) { super(dataBroker, LogicalDatastoreType.OPERATIONAL, InstanceIdentifier.create(ServiceOps.class) .child(Services.class).child(Operations.class)); - this.serviceRecoveryManager = serviceRecoveryManager; + this.serviceRecoveryRegistry = serviceRecoveryRegistry; } @Override + @Deprecated public void add(InstanceIdentifier instanceIdentifier, Operations operations) { LOG.info("Service Recovery operation triggered for service: {}", operations); - serviceRecoveryManager.recoverService(operations.getEntityType(), operations.getEntityName(), - operations.getEntityId()); + recoverService(operations.getEntityType(), operations.getEntityName(), operations.getEntityId()); + } + + /** + * Initiates recovery mechanism for a particular interface-manager entity. This method tries to check whether there + * is a registered handler for the incoming service recovery request within interface-manager and redirects the call + * to the respective handler if found. + * + * @param entityType The type of service recovery. eg :SERVICE or INSTANCE. + * @param entityName The type entity for which recovery has to be started. eg : INTERFACE or DPN. + * @param entityId The unique id to represent the entity to be recovered + */ + private void recoverService(Class entityType, + Class entityName, String entityId) { + String serviceRegistryKey = entityName.toString(); + serviceRecoveryRegistry.getRegisteredServiceRecoveryHandler(serviceRegistryKey).recoverService(entityId); } @Override + @Deprecated public void remove(InstanceIdentifier instanceIdentifier, Operations removedDataObject) { + // FIXME: this should be doing something, right? } @Override + @Deprecated public void update(InstanceIdentifier instanceIdentifier, Operations originalDataObject, Operations updatedDataObject) { add(instanceIdentifier, updatedDataObject); diff --git a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryManager.java b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryManager.java deleted file mode 100644 index c84edcc4..00000000 --- a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryManager.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (c) 2018 Ericsson India Global Services Pvt Ltd. and others. All rights reserved. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v1.0 which accompanies this distribution, - * and is available at http://www.eclipse.org/legal/epl-v10.html - */ -package org.opendaylight.serviceutils.srm.impl; - -import javax.inject.Inject; -import javax.inject.Singleton; -import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.types.rev180626.EntityNameBase; -import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.types.rev180626.EntityTypeBase; - -@Singleton -public final class ServiceRecoveryManager { - - private final ServiceRecoveryRegistryImpl serviceRecoveryRegistry; - - @Inject - public ServiceRecoveryManager(ServiceRecoveryRegistryImpl serviceRecoveryRegistry) { - this.serviceRecoveryRegistry = serviceRecoveryRegistry; - } - - private static String getServiceRegistryKey(Class entityName) { - return entityName.toString(); - } - - /** - * Initiates recovery mechanism for a particular interface-manager entity. - * This method tries to check whether there is a registered handler for the incoming - * service recovery request within interface-manager and redirects the call - * to the respective handler if found. - * @param entityType - * The type of service recovery. eg :SERVICE or INSTANCE. - * @param entityName - * The type entity for which recovery has to be started. eg : INTERFACE or DPN. - * @param entityId - * The unique id to represent the entity to be recovered - */ - public void recoverService(Class entityType, - Class entityName, String entityId) { - String serviceRegistryKey = getServiceRegistryKey(entityName); - serviceRecoveryRegistry.getRegisteredServiceRecoveryHandler(serviceRegistryKey).recoverService(entityId); - } -} diff --git a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryRegistryImpl.java b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryRegistryImpl.java index df9013f8..2fd95d6b 100644 --- a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryRegistryImpl.java +++ b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/ServiceRecoveryRegistryImpl.java @@ -11,18 +11,21 @@ import java.util.Map; import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; +import javax.inject.Inject; import javax.inject.Singleton; -import org.apache.aries.blueprint.annotation.service.Service; import org.opendaylight.serviceutils.srm.RecoverableListener; import org.opendaylight.serviceutils.srm.ServiceRecoveryInterface; import org.opendaylight.serviceutils.srm.ServiceRecoveryRegistry; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.RequireServiceComponentRuntime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Singleton -@Service(classes = ServiceRecoveryRegistry.class) -public class ServiceRecoveryRegistryImpl implements ServiceRecoveryRegistry { - +@Component(immediate = true) +@RequireServiceComponentRuntime +public final class ServiceRecoveryRegistryImpl implements ServiceRecoveryRegistry { private static final Logger LOG = LoggerFactory.getLogger(ServiceRecoveryRegistryImpl.class); private final Map serviceRecoveryRegistry = new ConcurrentHashMap<>(); @@ -30,9 +33,14 @@ public class ServiceRecoveryRegistryImpl implements ServiceRecoveryRegistry { private final Map> recoverableListenersMap = new ConcurrentHashMap<>(); + @Inject + @Activate + public ServiceRecoveryRegistryImpl() { + // Exposed for DI + } + @Override - public void registerServiceRecoveryRegistry(String entityName, - ServiceRecoveryInterface serviceRecoveryHandler) { + public void registerServiceRecoveryRegistry(String entityName, ServiceRecoveryInterface serviceRecoveryHandler) { serviceRecoveryRegistry.put(entityName, serviceRecoveryHandler); LOG.trace("Registered service recovery handler for {}", entityName); } @@ -46,8 +54,9 @@ public class ServiceRecoveryRegistryImpl implements ServiceRecoveryRegistry { @Override public void removeRecoverableListener(String serviceName, RecoverableListener recoverableListener) { - if (recoverableListenersMap.get(serviceName) != null) { - recoverableListenersMap.get(serviceName).remove(recoverableListener); + Queue queue = getRecoverableListeners(serviceName); + if (queue != null) { + queue.remove(recoverableListener); } } @@ -56,7 +65,8 @@ public class ServiceRecoveryRegistryImpl implements ServiceRecoveryRegistry { return recoverableListenersMap.get(serviceName); } - ServiceRecoveryInterface getRegisteredServiceRecoveryHandler(String entityName) { + @Override + public ServiceRecoveryInterface getRegisteredServiceRecoveryHandler(String entityName) { return serviceRecoveryRegistry.get(entityName); } } diff --git a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/SrmRpcProvider.java b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/SrmRpcProvider.java index e18a8fcc..0f0bc178 100644 --- a/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/SrmRpcProvider.java +++ b/srm/impl/src/main/java/org/opendaylight/serviceutils/srm/impl/SrmRpcProvider.java @@ -8,27 +8,48 @@ package org.opendaylight.serviceutils.srm.impl; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.mdsal.binding.api.DataBroker; +import org.opendaylight.mdsal.binding.api.RpcProviderService; import org.opendaylight.serviceutils.tools.rpc.FutureRpcResults; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.rpc.rev180626.OdlSrmRpcsService; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.rpc.rev180626.RecoverInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.rpc.rev180626.RecoverOutput; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.rpc.rev180626.ReinstallInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.rpc.rev180626.ReinstallOutput; +import org.opendaylight.yangtools.concepts.Registration; import org.opendaylight.yangtools.yang.common.RpcResult; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.RequireServiceComponentRuntime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Singleton -public class SrmRpcProvider implements OdlSrmRpcsService { +@Component(immediate = true, service = OdlSrmRpcsService.class) +@RequireServiceComponentRuntime +public final class SrmRpcProvider implements OdlSrmRpcsService, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(SrmRpcProvider.class); + private final DataBroker dataBroker; + private final Registration reg; @Inject - public SrmRpcProvider(DataBroker dataBroker) { + @Activate + public SrmRpcProvider(@Reference DataBroker dataBroker, @Reference RpcProviderService rpcProvider) { this.dataBroker = dataBroker; + reg = rpcProvider.registerRpcImplementation(OdlSrmRpcsService.class, this); + } + + @Override + @Deactivate + @PreDestroy + public void close() { + reg.close(); } @Override diff --git a/srm/impl/src/main/resources/OSGI-INF/blueprint/srm.xml b/srm/impl/src/main/resources/OSGI-INF/blueprint/srm.xml deleted file mode 100644 index c07d3343..00000000 --- a/srm/impl/src/main/resources/OSGI-INF/blueprint/srm.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - - - - diff --git a/srm/shell/pom.xml b/srm/shell/pom.xml index 947a2060..00daa8d2 100644 --- a/srm/shell/pom.xml +++ b/srm/shell/pom.xml @@ -16,9 +16,7 @@ and is available at http://www.eclipse.org/legal/epl-v10.html ../../commons/quality-parent - org.opendaylight.serviceutils srm-shell - 0.9.0-SNAPSHOT bundle diff --git a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/RecoverCommand.java b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/RecoverCommand.java index e7d54b82..4769d5c5 100644 --- a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/RecoverCommand.java +++ b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/RecoverCommand.java @@ -5,7 +5,6 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.serviceutils.srm.shell; import java.util.concurrent.Future; @@ -25,25 +24,22 @@ import org.slf4j.LoggerFactory; @Command(scope = "srm", name = "recover", description = "Recover service or instance") public class RecoverCommand extends OsgiCommandSupport { - private static final Logger LOG = LoggerFactory.getLogger(RecoverCommand.class); - private final OdlSrmRpcsService srmRpcService; - - public RecoverCommand(OdlSrmRpcsService srmRpcService) { - this.srmRpcService = srmRpcService; - } - @Argument(index = 0, name = "type", description = "EntityType, required", required = false, multiValued = false) String type; - @Argument(index = 1, name = "name", description = "EntityName, required", required = false, multiValued = false) String name; - @Argument(index = 2, name = "id", description = "EntityId, optional", required = false, multiValued = false) String id; + private final OdlSrmRpcsService srmRpcService; + + public RecoverCommand(OdlSrmRpcsService srmRpcService) { + this.srmRpcService = srmRpcService; + } @Override + @Deprecated protected @Nullable Object doExecute() throws Exception { RecoverInput input = getInput(); if (input == null) { @@ -92,5 +88,4 @@ public class RecoverCommand extends OsgiCommandSupport { } return inputBuilder.build(); } - } diff --git a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/ReinstallCommand.java b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/ReinstallCommand.java index 973733be..42f46c43 100644 --- a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/ReinstallCommand.java +++ b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/ReinstallCommand.java @@ -5,7 +5,6 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.serviceutils.srm.shell; import java.util.concurrent.Future; @@ -26,9 +25,12 @@ import org.slf4j.LoggerFactory; @Command(scope = "srm", name = "reinstall", description = "Reinstall service or instance") public class ReinstallCommand extends OsgiCommandSupport { - private static final Logger LOG = LoggerFactory.getLogger(ReinstallCommand.class); + @Argument(index = 0, name = "name", description = "EntityName of type service, required", + required = false, multiValued = false) + String name; + private final OdlSrmRpcsService srmRpcService; private final Class entityType = EntityTypeService.class; @@ -36,11 +38,8 @@ public class ReinstallCommand extends OsgiCommandSupport { this.srmRpcService = srmRpcService; } - @Argument(index = 0, name = "name", description = "EntityName of type service, required", - required = false, multiValued = false) - String name; - @Override + @Deprecated protected @Nullable Object doExecute() throws Exception { ReinstallInput input = getInput(); if (input == null) { @@ -77,5 +76,4 @@ public class ReinstallCommand extends OsgiCommandSupport { inputBuilder.setEntityName(entityName); return inputBuilder.build(); } - } diff --git a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/SrmDebugCommand.java b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/SrmDebugCommand.java index 52741e29..55814751 100644 --- a/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/SrmDebugCommand.java +++ b/srm/shell/src/main/java/org/opendaylight/serviceutils/srm/shell/SrmDebugCommand.java @@ -5,7 +5,6 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.serviceutils.srm.shell; import org.apache.karaf.shell.commands.Command; @@ -18,25 +17,20 @@ import org.opendaylight.mdsal.binding.api.WriteTransaction; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.yang.gen.v1.urn.opendaylight.serviceutils.srm.ops.rev180626.ServiceOps; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Command(scope = "srm", name = "debug", description = "SRM debug commands") public class SrmDebugCommand extends OsgiCommandSupport { - - private static final Logger LOG = LoggerFactory.getLogger(SrmDebugCommand.class); - + @Option(name = "-c", aliases = {"--clear-ops"}, description = "Clear operations DS", + required = true, multiValued = false) + boolean clearOps; private final DataBroker txDataBroker; public SrmDebugCommand(DataBroker dataBroker) { - this.txDataBroker = dataBroker; + txDataBroker = dataBroker; } - @Option(name = "-c", aliases = {"--clear-ops"}, description = "Clear operations DS", - required = true, multiValued = false) - boolean clearOps; - @Override + @Deprecated protected @Nullable Object doExecute() throws Exception { if (clearOps) { clearOpsDs(); diff --git a/srm/shell/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/srm/shell/src/main/resources/OSGI-INF/blueprint/blueprint.xml index c3da29b6..d1158555 100644 --- a/srm/shell/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/srm/shell/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -5,25 +5,21 @@ * and is available at http://www.eclipse.org/legal/epl-v10.html --> - - + - - + - + - +