Do not use NotificationListener in LISP components 37/107937/7
authorMatej Sramcik <matej.sramcik@pantheon.tech>
Thu, 21 Sep 2023 07:53:44 +0000 (09:53 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 15 Feb 2024 20:28:45 +0000 (21:28 +0100)
Migrated usage of of NotificationListener to Listener<?> migration
for LISP components. Also fixes MapServer not relinquishing its listener
registration.

JIRA: LISPMAP-184
Change-Id: I8f2d22a7c1bad8d7e3bf23f2cc1580e9b4639701
Signed-off-by: Matej Sramcik <matej.sramcik@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
integrationtest/src/test/java/org/opendaylight/lispflowmapping/integrationtest/MappingServiceIntegrationTest.java
mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/LispMappingService.java
mappingservice/implementation/src/main/java/org/opendaylight/lispflowmapping/implementation/lisp/MapServer.java
mappingservice/implementation/src/test/java/org/opendaylight/lispflowmapping/implementation/LispMappingServiceTest.java

index a03edb0c9c78f5bea643de21839c01e4124f021d..72ea2154ec7aee22f60fd5383e31b1e903418b37 100644 (file)
@@ -73,6 +73,7 @@ import org.opendaylight.lispflowmapping.lisp.util.LispAddressUtil;
 import org.opendaylight.lispflowmapping.lisp.util.MappingRecordUtil;
 import org.opendaylight.lispflowmapping.lisp.util.MaskUtil;
 import org.opendaylight.lispflowmapping.type.sbplugin.IConfigLispSouthboundPlugin;
+import org.opendaylight.mdsal.binding.api.NotificationService.Listener;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpPrefix;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
@@ -106,18 +107,11 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.inet.binary.types.rev160303.Ipv4AddressBinary;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.binary.address.types.rev160504.Ipv4PrefixBinaryAfi;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.binary.address.types.rev160504.augmented.lisp.address.address.Ipv4PrefixBinaryBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.AddMapping;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.GotMapNotify;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.GotMapReply;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapNotify;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRegister;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapReply;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRequest;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MappingKeepAlive;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MessageType;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.OdlLispProtoListener;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.RequestMapping;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrReplyMapping;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrRequestMapping;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.Eid;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.EidBuilder;
@@ -167,7 +161,6 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase {
     private byte[] mapRegisterPacketWithoutNotify;
     String lispBindAddress = "127.0.0.1";
     static final String ourAddress = "127.0.0.2";
-    private Rloc locatorEid;
     private DatagramSocket socket;
     private byte[] mapRegisterPacketWithAuthenticationAndMapNotify;
 
@@ -2900,36 +2893,22 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase {
 
     }
 
-    private class XtrRequestMappingListener implements OdlLispProtoListener {
+    private class XtrRequestMappingListener implements Listener<XtrRequestMapping> {
+        private final String eid;
+        private final MapRequest mapRequest;
 
-        @Override
-        public void onGotMapReply(GotMapReply notification) {
-        }
-
-        @Override
-        public void onAddMapping(AddMapping notification) {
-        }
-
-        @Override
-        public void onXtrReplyMapping(XtrReplyMapping notification) {
-        }
-
-        @Override
-        public void onRequestMapping(RequestMapping notification) {
+        private XtrRequestMappingListener(String eid, MapRequest mapRequest) {
+            this.eid = eid;
+            this.mapRequest = mapRequest;
         }
 
         @Override
-        public void onGotMapNotify(GotMapNotify notification) {
+        public void onNotification(XtrRequestMapping notification) {
+            assertEquals(((Ipv4Prefix) mapRequest.getEidItem().get(0).getEid().getAddress())
+                .getIpv4Prefix().getValue(), eid);
+            notificationCalled = true;
+            LOG.warn("notification arrived");
         }
-
-        @Override
-        public void onXtrRequestMapping(XtrRequestMapping notification) {
-        }
-
-        @Override
-        public void onMappingKeepAlive(MappingKeepAlive notification) {
-        }
-
     }
 
     public void testRecievingNonProxyOnXtrPort() throws SocketTimeoutException, SocketException, Throwable {
@@ -2950,17 +2929,10 @@ public class MappingServiceIntegrationTest extends AbstractMdsalTestBase {
         Rloc adLcaf = rb.build();
 
         final MapRequest mapRequest = createNonProxyMapRequest(eid, adLcaf);
-        ((LispMappingService) lms).getNotificationService().registerNotificationListener(
-                new XtrRequestMappingListener() {
-
-            @Override
-            public void onXtrRequestMapping(XtrRequestMapping notification) {
-                assertEquals(((Ipv4Prefix) mapRequest.getEidItem().get(0).getEid().getAddress())
-                        .getIpv4Prefix().getValue(), eid);
-                notificationCalled = true;
-                LOG.warn("notification arrived");
-            }
-        });
+        final var handler = new XtrRequestMappingListener(eid, mapRequest);
+
+        ((LispMappingService) lms).getNotificationService().registerListener(XtrRequestMapping.class, handler);
+
         sendMapRequest(mapRequest, port);
         for (int i = 0; i < MAX_NOTIFICATION_RETRYS; i++) {
             if (notificationCalled) {
index bec9825fe8cb02d4be545f640b6a75bf2d0b3802..968b2640881f6fe0138817d6d44d04dc99f2cae9 100644 (file)
@@ -8,9 +8,11 @@
 
 package org.opendaylight.lispflowmapping.implementation;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.List;
+import java.util.Set;
 import javax.annotation.PreDestroy;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -24,12 +26,12 @@ import org.opendaylight.lispflowmapping.interfaces.lisp.IFlowMapping;
 import org.opendaylight.lispflowmapping.interfaces.lisp.IMapNotifyHandler;
 import org.opendaylight.lispflowmapping.interfaces.lisp.IMapRequestResultHandler;
 import org.opendaylight.lispflowmapping.interfaces.lisp.IMapResolverAsync;
-import org.opendaylight.lispflowmapping.interfaces.lisp.IMapServerAsync;
 import org.opendaylight.lispflowmapping.interfaces.lisp.ISmrNotificationListener;
 import org.opendaylight.lispflowmapping.interfaces.mappingservice.IMappingService;
 import org.opendaylight.lispflowmapping.lisp.type.LispMessage;
 import org.opendaylight.lispflowmapping.lisp.util.LispAddressStringifier;
 import org.opendaylight.mdsal.binding.api.NotificationService;
+import org.opendaylight.mdsal.binding.api.NotificationService.CompositeListener;
 import org.opendaylight.mdsal.binding.api.RpcProviderService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
@@ -43,7 +45,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.Ma
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapReply;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MapRequest;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.MappingKeepAlive;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.OdlLispProtoListener;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.RequestMapping;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.XtrReplyMapping;
@@ -73,7 +74,7 @@ import org.slf4j.LoggerFactory;
 @Component(service = {IFlowMapping.class, IMapRequestResultHandler.class, IMapNotifyHandler.class},
         immediate = true, property = "type=default")
 public class LispMappingService implements IFlowMapping, IMapRequestResultHandler,
-        IMapNotifyHandler, OdlLispProtoListener, AutoCloseable, ClusterSingletonService {
+        IMapNotifyHandler, AutoCloseable, ClusterSingletonService {
     private static final String LISPFLOWMAPPING_ENTITY_NAME = "lispflowmapping";
     private static final ServiceGroupIdentifier SERVICE_GROUP_IDENTIFIER = ServiceGroupIdentifier.create(
             LISPFLOWMAPPING_ENTITY_NAME);
@@ -88,12 +89,11 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
     private ThreadLocal<Pair<MapRequest, TransportAddress>> tlsMapRequest = new ThreadLocal<>();
 
     private IMapResolverAsync mapResolver;
-    private IMapServerAsync mapServer;
+    private MapServer mapServer;
 
     private final IMappingService mapService;
     private final OdlLispSbService lispSB;
     private final ClusterSingletonServiceProvider clusterSingletonService;
-    private final RpcProviderService rpcProviderService;
     private final NotificationService notificationService;
     private final Registration rpcRegistration;
     private final Registration listenerRegistration;
@@ -108,11 +108,17 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
         this.mapService = mappingService;
         this.lispSB = odlLispService;
         this.clusterSingletonService = clusterSingletonService;
-        this.rpcProviderService = rpcProviderService;
         this.notificationService = notificationService;
 
         // initialize
-        listenerRegistration = notificationService.registerNotificationListener(this);
+        listenerRegistration = notificationService.registerCompositeListener(new CompositeListener(Set.of(
+                new CompositeListener.Component<>(AddMapping.class, this::onAddMapping),
+                new CompositeListener.Component<>(GotMapNotify.class, this::onGotMapNotify),
+                new CompositeListener.Component<>(RequestMapping.class, this::onRequestMapping),
+                new CompositeListener.Component<>(GotMapReply.class, this::onGotMapReply),
+                new CompositeListener.Component<>(XtrRequestMapping.class, this::onXtrRequestMapping),
+                new CompositeListener.Component<>(XtrReplyMapping.class, this::onXtrReplyMapping),
+                new CompositeListener.Component<>(MappingKeepAlive.class, this::onMappingKeepAlive))));
         rpcRegistration = rpcProviderService.registerRpcImplementation(OdlLispSbService.class, lispSB);
 
         mapResolver = new MapResolver(mapService, smr, elpPolicy, this);
@@ -189,8 +195,8 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
         getLispSB().sendMapNotify(smnib.build());
     }
 
-    @Override
-    public void onAddMapping(AddMapping mapRegisterNotification) {
+    @VisibleForTesting
+    void onAddMapping(AddMapping mapRegisterNotification) {
         Pair<MapNotify, List<TransportAddress>> result = handleMapRegister(mapRegisterNotification.getMapRegister());
         if (result != null && result.getLeft() != null) {
             MapNotify mapNotify = result.getLeft();
@@ -210,8 +216,8 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
         }
     }
 
-    @Override
-    public void onRequestMapping(RequestMapping mapRequestNotification) {
+    @VisibleForTesting
+    void onRequestMapping(RequestMapping mapRequestNotification) {
         MapReply mapReply = handleMapRequest(mapRequestNotification.getMapRequest());
         if (mapReply != null) {
             SendMapReplyInputBuilder smrib = new SendMapReplyInputBuilder();
@@ -223,28 +229,28 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
         }
     }
 
-    @Override
-    public void onGotMapReply(GotMapReply notification) {
+    @VisibleForTesting
+    void onGotMapReply(GotMapReply notification) {
         LOG.debug("Received GotMapReply notification, ignoring");
     }
 
-    @Override
-    public void onGotMapNotify(GotMapNotify notification) {
+    @VisibleForTesting
+    void onGotMapNotify(GotMapNotify notification) {
         LOG.debug("Received GotMapNotify notification, ignoring");
     }
 
-    @Override
-    public void onXtrRequestMapping(XtrRequestMapping notification) {
+    @VisibleForTesting
+    void onXtrRequestMapping(XtrRequestMapping notification) {
         LOG.debug("Received XtrRequestMapping notification, ignoring");
     }
 
-    @Override
-    public void onXtrReplyMapping(XtrReplyMapping notification) {
+    @VisibleForTesting
+    void onXtrReplyMapping(XtrReplyMapping notification) {
         LOG.debug("Received XtrReplyMapping notification, ignoring");
     }
 
-    @Override
-    public void onMappingKeepAlive(MappingKeepAlive notification) {
+    @VisibleForTesting
+    void onMappingKeepAlive(MappingKeepAlive notification) {
         final MapRegisterCacheMetadata cacheMetadata = notification.getMapRegisterCacheMetadata();
         for (EidLispAddress eidLispAddress : cacheMetadata.nonnullEidLispAddress().values()) {
             final Eid eid = eidLispAddress.getEid();
@@ -293,7 +299,10 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
     private void destroy() {
         LOG.info("LISP (RFC6830) Mapping Service is destroyed!");
         mapResolver = null;
-        mapServer = null;
+        if (mapServer != null) {
+            mapServer.close();
+            mapServer = null;
+        }
     }
 
     @Deactivate
@@ -324,4 +333,3 @@ public class LispMappingService implements IFlowMapping, IMapRequestResultHandle
         return SERVICE_GROUP_IDENTIFIER;
     }
 }
-
index 1735e73cdd7306e54a972b88fb149b5e4cf78a83..50efa073f83d62f02dd27b6a3b66bb2874891a83 100644 (file)
@@ -50,6 +50,7 @@ import org.opendaylight.lispflowmapping.lisp.util.MappingRecordUtil;
 import org.opendaylight.lispflowmapping.lisp.util.MaskUtil;
 import org.opendaylight.lispflowmapping.lisp.util.SourceDestKeyHelper;
 import org.opendaylight.mdsal.binding.api.NotificationService;
+import org.opendaylight.mdsal.binding.api.NotificationService.Listener;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.lisp.address.types.rev151105.lisp.address.Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.lisp.address.types.rev151105.lisp.address.address.SourceDestKey;
@@ -62,7 +63,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.Ma
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.SiteId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.Eid;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.container.EidBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.list.EidItem;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.eid.list.EidItemBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapnotifymessage.MapNotifyBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.mapping._record.container.MappingRecord;
@@ -75,32 +75,32 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.tr
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.lisp.proto.rev151105.transport.address.TransportAddressBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.MappingChanged;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.MappingOrigin;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.lfm.mappingservice.rev150906.OdlMappingserviceListener;
-import org.opendaylight.yangtools.concepts.ListenerRegistration;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class MapServer implements IMapServerAsync, OdlMappingserviceListener, ISmrNotificationListener {
-
+public class MapServer implements IMapServerAsync, ISmrNotificationListener, Listener<MappingChanged>, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(MapServer.class);
     private static final byte[] ALL_ZEROES_XTR_ID = new byte[] {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ,0};
+
+    private final SmrScheduler scheduler = new SmrScheduler();
     private final IMappingService mapService;
-    private boolean subscriptionService;
     private final IMapNotifyHandler notifyHandler;
-    private final NotificationService notificationService;
-    private ListenerRegistration<MapServer> mapServerListenerRegistration;
-    private final SmrScheduler scheduler;
+    private final Registration listenerRegistration;
+
+    private boolean subscriptionService;
 
     public MapServer(IMappingService mapService, boolean subscriptionService,
                      IMapNotifyHandler notifyHandler, NotificationService notificationService) {
         this.mapService = requireNonNull(mapService);
         this.subscriptionService = subscriptionService;
         this.notifyHandler = notifyHandler;
-        this.notificationService = notificationService;
-        if (notificationService != null) {
-            notificationService.registerNotificationListener(this);
-        }
-        scheduler = new SmrScheduler();
+        listenerRegistration = notificationService.registerListener(MappingChanged.class, this);
+    }
+
+    @Override
+    public void close() {
+        listenerRegistration.close();
     }
 
     @Override
@@ -203,7 +203,7 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS
     }
 
     @Override
-    public void onMappingChanged(MappingChanged notification) {
+    public void onNotification(MappingChanged notification) {
         if (subscriptionService) {
             Eid eid = notification.getEid();
             if (eid == null) {
@@ -411,7 +411,7 @@ public class MapServer implements IMapServerAsync, OdlMappingserviceListener, IS
                     // a given mapping.
                     if (executionCount <= ConfigIni.getInstance().getSmrRetryCount()) {
                         synchronized (mrb) {
-                            mrb.setEidItem(new ArrayList<EidItem>());
+                            mrb.setEidItem(new ArrayList<>());
                             mrb.getEidItem().add(new EidItemBuilder()
                                     .setEidItemId(LispAddressStringifier.getString(subscriber.getSrcEid()))
                                     .setEid(subscriber.getSrcEid()).build());
index 021da330b73fa0f4229bd4fe8e9d486d4a02ef44..f022e5096ded3a61ba23f518028f3acdc7de2056 100644 (file)
@@ -24,8 +24,8 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.opendaylight.lispflowmapping.implementation.lisp.MapServer;
 import org.opendaylight.lispflowmapping.interfaces.lisp.IMapResolverAsync;
-import org.opendaylight.lispflowmapping.interfaces.lisp.IMapServerAsync;
 import org.opendaylight.lispflowmapping.interfaces.mappingservice.IMappingService;
 import org.opendaylight.lispflowmapping.lisp.type.LispMessage;
 import org.opendaylight.lispflowmapping.lisp.util.LispAddressUtil;
@@ -73,7 +73,7 @@ import org.opendaylight.yangtools.yang.common.Uint16;
 @RunWith(MockitoJUnitRunner.class)
 public class LispMappingServiceTest {
     @Mock private static MapRegister mapRegisterMock;
-    @Mock(name = "mapServer") private static IMapServerAsync mapServerMock;
+    @Mock(name = "mapServer") private static MapServer mapServerMock;
     @Mock(name = "mapResolver") private static IMapResolverAsync mapResolverMock;
     @Mock(name = "tlsMapReply") private static ThreadLocal<MapReply> tlsMapReplyMock;
     @Mock(name = "tlsMapNotify") private static ThreadLocal<Pair<MapNotify, List<TransportAddress>>> tlsMapNotifyMock;