Optimize NetconfMonitoringServiceImpl getSchemaForCapability

Update mappedModulesToRevisionToSchema only on capability change,
so getSchemaForCapability doesn't need to compute it every time
it's called.

Change-Id: I059085d45c988b58b1379f87c4a5162014b81624
Signed-off-by: Andrej Mak <andmak@cisco.com>

diff --git a/netconf/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java b/netconf/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
index f5249e4d12dcb62a563d40e454773f58fb99b2f8..27dbb67715c7e499b19f9796d4f6fb0a4572bedb 100644 (file)
--- a/netconf/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
+++ b/netconf/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
@@ -73,6 +73,7 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
     private final Set<NetconfManagementSession> sessions = new HashSet<>();
     private final NetconfOperationServiceFactory netconfOperationProvider;
     private final Map<Uri, Capability> capabilities = new HashMap<>();
+    private final Map<String, Map<String, String>> mappedModulesToRevisionToSchema = Maps.newHashMap();
 
     private final Set<MonitoringListener> listeners = Sets.newHashSet();
     private volatile BaseNotificationPublisherRegistration notificationPublisher;
@@ -117,33 +118,9 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
     @Override
     public synchronized String getSchemaForCapability(final String moduleName, final Optional<String> revision) {
 
-        // FIXME not effective at all
-
-        Map<String, Map<String, String>> mappedModulesToRevisionToSchema = Maps.newHashMap();
-
-        final Collection<Capability> caps = capabilities.values();
-
-        for (Capability cap : caps) {
-            if (!cap.getModuleName().isPresent()
-                    || !cap.getRevision().isPresent()
-                    || !cap.getCapabilitySchema().isPresent()){
-                continue;
-            }
-
-            final String currentModuleName = cap.getModuleName().get();
-            Map<String, String> revisionMap = mappedModulesToRevisionToSchema.get(currentModuleName);
-            if (revisionMap == null) {
-                revisionMap = Maps.newHashMap();
-                mappedModulesToRevisionToSchema.put(currentModuleName, revisionMap);
-            }
-
-            String currentRevision = cap.getRevision().get();
-            revisionMap.put(currentRevision, cap.getCapabilitySchema().get());
-        }
-
         Map<String, String> revisionMapRequest = mappedModulesToRevisionToSchema.get(moduleName);
         Preconditions.checkState(revisionMapRequest != null, "Capability for module %s not present, " + ""
-                + "available modules : %s", moduleName, Collections2.transform(caps, CAPABILITY_TO_URI));
+                + "available modules : %s", moduleName, Collections2.transform(capabilities.values(), CAPABILITY_TO_URI));
 
         if (revision.isPresent()) {
             String schema = revisionMapRequest.get(revision.get());
@@ -161,6 +138,42 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
         }
     }
 
+    private synchronized void updateCapabilityToSchemaMap(final Set<Capability> added, final Set<Capability> removed) {
+        for (final Capability cap : added) {
+            if (!isValidModuleCapability(cap)){
+                continue;
+            }
+
+            final String currentModuleName = cap.getModuleName().get();
+            Map<String, String> revisionMap = mappedModulesToRevisionToSchema.get(currentModuleName);
+            if (revisionMap == null) {
+                revisionMap = Maps.newHashMap();
+                mappedModulesToRevisionToSchema.put(currentModuleName, revisionMap);
+            }
+
+            final String currentRevision = cap.getRevision().get();
+            revisionMap.put(currentRevision, cap.getCapabilitySchema().get());
+        }
+        for (final Capability cap : removed) {
+            if (!isValidModuleCapability(cap)){
+                continue;
+            }
+            final Map<String, String> revisionMap = mappedModulesToRevisionToSchema.get(cap.getModuleName().get());
+            if (revisionMap != null) {
+                revisionMap.remove(cap.getRevision().get());
+                if (revisionMap.isEmpty()) {
+                    mappedModulesToRevisionToSchema.remove(cap.getModuleName().get());
+                }
+            }
+        }
+    }
+
+    private boolean isValidModuleCapability(Capability cap) {
+        return cap.getModuleName().isPresent()
+                && cap.getRevision().isPresent()
+                && cap.getCapabilitySchema().isPresent();
+    }
+
     @Override
     public synchronized Capabilities getCapabilities() {
         return new CapabilitiesBuilder().setCapability(Lists.newArrayList(capabilities.keySet())).build();
@@ -250,6 +263,7 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
     public void onCapabilitiesChanged(Set<Capability> added, Set<Capability> removed) {
         onCapabilitiesAdded(added);
         onCapabilitiesRemoved(removed);
+        updateCapabilityToSchemaMap(added, removed);
         notifyListeners();
 
         // publish notification to notification collector about changed capabilities

diff --git a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/NetconfMonitoringServiceImplTest.java b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/NetconfMonitoringServiceImplTest.java
deleted file mode 100644 (file)
index b4ea374..0000000
--- a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/NetconfMonitoringServiceImplTest.java
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * Copyright (c) 2014 Cisco Systems, Inc. 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.netconf.impl;
-
-public class NetconfMonitoringServiceImplTest {
-
-    // TODO redo test
-}

diff --git a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImplTest.java b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImplTest.java
index 3181030fee5e29a1ce145e44fc0a2f19edf569e2..6f1c91ceeb08c5a60f073a238b99346fe21bc164 100644 (file)
--- a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImplTest.java
+++ b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImplTest.java
@@ -13,7 +13,7 @@ import static org.mockito.Mockito.doReturn;
 import com.google.common.base.Optional;
 import java.net.URI;
 import java.util.ArrayList;
-import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -21,6 +21,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -39,33 +40,41 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.mon
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.schemas.Schema;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.sessions.SessionBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.notifications.rev120206.NetconfCapabilityChange;
+import org.opendaylight.yangtools.yang.common.SimpleDateFormatUtil;
 import org.opendaylight.yangtools.yang.model.api.Module;
 
 public class NetconfMonitoringServiceImplTest {
 
     private static final String TEST_MODULE_CONTENT = "content";
+    private static final String TEST_MODULE_CONTENT2 = "content2";
     private static final String TEST_MODULE_REV = "1970-01-01";
+    private static final String TEST_MODULE_REV2 = "1970-01-02";
     private static final  Uri TEST_MODULE_NAMESPACE = new Uri("testModuleNamespace");
     private static final String TEST_MODULE_NAME = "testModule";
-    private static final Date TEST_MODULE_DATE;
+    private static Date TEST_MODULE_DATE;
+    private static Date TEST_MODULE_DATE2;
 
-    static {
-        Calendar calendar = Calendar.getInstance();
-        calendar.set(1970, Calendar.JANUARY, 1);
-        TEST_MODULE_DATE = calendar.getTime();
-    }
+    private YangModuleCapability moduleCapability1;
+    private YangModuleCapability moduleCapability2;
 
     private final Set<Capability> CAPABILITIES = new HashSet<>();
 
     private NetconfMonitoringServiceImpl monitoringService;
-
     @Mock
     private Module moduleMock;
     @Mock
+    private Module moduleMock2;
+    @Mock
     private NetconfOperationServiceFactory operationServiceFactoryMock;
     @Mock
     private NetconfManagementSession sessionMock;
 
+    @BeforeClass
+    public static void suiteSetUp() throws Exception {
+        TEST_MODULE_DATE = SimpleDateFormatUtil.getRevisionFormat().parse(TEST_MODULE_REV);
+        TEST_MODULE_DATE2= SimpleDateFormatUtil.getRevisionFormat().parse(TEST_MODULE_REV2);
+    }
+
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -75,14 +84,20 @@ public class NetconfMonitoringServiceImplTest {
         doReturn(new URI(TEST_MODULE_NAMESPACE.getValue())).when(moduleMock).getNamespace();
         doReturn(TEST_MODULE_NAME).when(moduleMock).getName();
         doReturn(TEST_MODULE_DATE).when(moduleMock).getRevision();
-        doReturn(TEST_MODULE_NAME).when(moduleMock).getName();
+        moduleCapability1 = new YangModuleCapability(moduleMock, TEST_MODULE_CONTENT);
+
+        CAPABILITIES.add(moduleCapability1);
+
+        doReturn(new URI(TEST_MODULE_NAMESPACE.getValue())).when(moduleMock2).getNamespace();
+        doReturn(TEST_MODULE_NAME).when(moduleMock2).getName();
+        doReturn(TEST_MODULE_DATE2).when(moduleMock2).getRevision();
+        moduleCapability2 = new YangModuleCapability(moduleMock2, TEST_MODULE_CONTENT2);
 
-        CAPABILITIES.add(new YangModuleCapability(moduleMock, TEST_MODULE_CONTENT));
         doReturn(CAPABILITIES).when(operationServiceFactoryMock).getCapabilities();
         doReturn(null).when(operationServiceFactoryMock).registerCapabilityListener(any(NetconfMonitoringServiceImpl.class));
 
         monitoringService = new NetconfMonitoringServiceImpl(operationServiceFactoryMock);
-        monitoringService.onCapabilitiesChanged(CAPABILITIES, new HashSet<Capability>());
+        monitoringService.onCapabilitiesChanged(CAPABILITIES, Collections.emptySet());
 
         doReturn(new SessionBuilder().build()).when(sessionMock).toManagementSession();
     }
@@ -115,9 +130,17 @@ public class NetconfMonitoringServiceImplTest {
 
     @Test
     public void testGetSchemaForCapability() throws Exception {
-        String schema = monitoringService.getSchemaForCapability(TEST_MODULE_NAME, Optional.of(TEST_MODULE_REV));
+        //test multiple revisions of the same capability
+        monitoringService.onCapabilitiesChanged(Collections.singleton(moduleCapability2), Collections.emptySet());
+        final String schema = monitoringService.getSchemaForCapability(TEST_MODULE_NAME, Optional.of(TEST_MODULE_REV));
         Assert.assertEquals(TEST_MODULE_CONTENT, schema);
-
+        final String schema2 = monitoringService.getSchemaForCapability(TEST_MODULE_NAME, Optional.of(TEST_MODULE_REV2));
+        Assert.assertEquals(TEST_MODULE_CONTENT2, schema2);
+        //remove one revision
+        monitoringService.onCapabilitiesChanged(Collections.emptySet(), Collections.singleton(moduleCapability1));
+        //only one revision present
+        final String schema3 = monitoringService.getSchemaForCapability(TEST_MODULE_NAME, Optional.absent());
+        Assert.assertEquals(TEST_MODULE_CONTENT2, schema3);
     }
 
     @Test