Fixed YangStore exception thrown at startup

Fixed a bug in yang store service that caused cache to be updated at illegal state

Change-Id: I567bb245af6577a53a52d003a14b74c9edc5850d
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

diff --git a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java
index 2ead596d05f7be50610b574177be44a0555e466c..90b94ef120c8d2e05473e76db4db7e7dd325c46a 100644 (file)
--- a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java
+++ b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTracker.java
@@ -18,6 +18,8 @@ import com.google.common.collect.Sets;
 import org.opendaylight.controller.config.yang.store.api.YangStoreException;
 import org.opendaylight.controller.config.yang.store.api.YangStoreService;
 import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot;
+import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
+import org.opendaylight.yangtools.yang.model.api.Module;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
@@ -34,6 +36,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -75,7 +78,7 @@ public class ExtenderYangTracker extends BundleTracker<Object> implements YangSt
     }
 
     @Override
-    public synchronized Object addingBundle(Bundle bundle, BundleEvent event) {
+    public Object addingBundle(Bundle bundle, BundleEvent event) {
 
         // Ignore system bundle:
         // system bundle might have config-api on classpath &&
@@ -107,37 +110,40 @@ public class ExtenderYangTracker extends BundleTracker<Object> implements YangSt
                 proposedNewState.putAll(bundle, addedURLs);
 
                 Preconditions.checkArgument(addedURLs.size() > 0, "No change can occur when no URLs are changed");
-                boolean success;
-                String failureReason = null;
+
                 try(YangStoreSnapshotImpl snapshot = createSnapshot(mbeParser, proposedNewState)) {
-                    updateCache(snapshot);
-                    success = true;
+                    onSnapshotSuccess(proposedNewState, snapshot);
                 } catch(YangStoreException e) {
-                    failureReason = e.toString();
-                    success = false;
-                }
-                if (success){
-                    // consistent state
-                    // merge into
-                    consistentBundlesToYangURLs.clear();
-                    consistentBundlesToYangURLs.putAll(proposedNewState);
-                    inconsistentBundlesToYangURLs.clear();
-
-                    logger.info("Yang store updated to new consistent state containing {} yang files", consistentBundlesToYangURLs.size());
-                    logger.trace("Yang store updated to new consistent state containing {}", consistentBundlesToYangURLs);
-                } else {
-                    // inconsistent state
-                    logger.debug("Yang store is falling back on last consistent state containing {}, inconsistent yang files {}, reason {}",
-                            consistentBundlesToYangURLs, inconsistentBundlesToYangURLs, failureReason);
-                    logger.warn("Yang store is falling back on last consistent state containing {} files, inconsistent yang files size is {}, reason {}",
-                            consistentBundlesToYangURLs.size(), inconsistentBundlesToYangURLs.size(), failureReason);
-                    inconsistentBundlesToYangURLs.putAll(bundle, addedURLs);
+                    onSnapshotFailure(bundle, addedURLs, e);
                 }
             }
         }
         return bundle;
     }
 
+    private void onSnapshotFailure(Bundle bundle, List<URL> addedURLs, Exception failureReason) {
+        // inconsistent state
+        inconsistentBundlesToYangURLs.putAll(bundle, addedURLs);
+
+        logger.debug("Yang store is falling back on last consistent state containing {}, inconsistent yang files {}, reason {}",
+                consistentBundlesToYangURLs, inconsistentBundlesToYangURLs, failureReason);
+        logger.warn("Yang store is falling back on last consistent state containing {} files, inconsistent yang files size is {}, reason {}",
+                consistentBundlesToYangURLs.size(), inconsistentBundlesToYangURLs.size(), failureReason.toString());
+    }
+
+    private void onSnapshotSuccess(Multimap<Bundle, URL> proposedNewState, YangStoreSnapshotImpl snapshot) {
+        // consistent state
+        // merge into
+        consistentBundlesToYangURLs.clear();
+        consistentBundlesToYangURLs.putAll(proposedNewState);
+        inconsistentBundlesToYangURLs.clear();
+
+        updateCache(snapshot);
+
+        logger.info("Yang store updated to new consistent state containing {} yang files", consistentBundlesToYangURLs.size());
+        logger.debug("Yang store updated to new consistent state containing {}", consistentBundlesToYangURLs);
+    }
+
     private void updateCache(YangStoreSnapshotImpl snapshot) {
         cache.cacheYangStore(consistentBundlesToYangURLs, snapshot);
     }
@@ -153,6 +159,7 @@ public class ExtenderYangTracker extends BundleTracker<Object> implements YangSt
      */
     @Override
     public synchronized void removedBundle(Bundle bundle, BundleEvent event, Object object) {
+        logger.debug("Removed bundle {} {} {}", bundle, event, object);
         inconsistentBundlesToYangURLs.removeAll(bundle);
         consistentBundlesToYangURLs.removeAll(bundle);
     }
@@ -162,9 +169,10 @@ public class ExtenderYangTracker extends BundleTracker<Object> implements YangSt
             throws YangStoreException {
         Optional<YangStoreSnapshot> yangStoreOpt = cache.getSnapshotIfPossible(consistentBundlesToYangURLs);
         if (yangStoreOpt.isPresent()) {
-            logger.trace("Returning cached yang store {}", yangStoreOpt.get());
+            logger.debug("Returning cached yang store {}", yangStoreOpt.get());
             return yangStoreOpt.get();
         }
+
         YangStoreSnapshotImpl snapshot = createSnapshot(mbeParser, consistentBundlesToYangURLs);
         updateCache(snapshot);
         return snapshot;
@@ -205,18 +213,21 @@ public class ExtenderYangTracker extends BundleTracker<Object> implements YangSt
 }
 
 class YangStoreCache {
+
     @GuardedBy("this")
-    private Set<URL> cachedUrls = Collections.emptySet();
+    private Set<URL> cachedUrls = null;
     @GuardedBy("this")
-    private Optional<YangStoreSnapshotImpl> cachedYangStoreSnapshot = Optional.absent();
+    private Optional<YangStoreSnapshot> cachedYangStoreSnapshot = getInitialSnapshot();
 
     synchronized Optional<YangStoreSnapshot> getSnapshotIfPossible(Multimap<Bundle, URL> bundlesToYangURLs) {
         Set<URL> urls = setFromMultimapValues(bundlesToYangURLs);
-        if (cachedUrls != null && cachedUrls.equals(urls)) {
+
+        if (cachedUrls==null || cachedUrls.equals(urls)) {
             Preconditions.checkState(cachedYangStoreSnapshot.isPresent());
             YangStoreSnapshot freshSnapshot = new YangStoreSnapshotImpl(cachedYangStoreSnapshot.get());
             return Optional.of(freshSnapshot);
         }
+
         return Optional.absent();
     }
 
@@ -228,7 +239,7 @@ class YangStoreCache {
     }
 
     synchronized void cacheYangStore(Multimap<Bundle, URL> urls,
-                        YangStoreSnapshotImpl yangStoreSnapshot) {
+                        YangStoreSnapshot yangStoreSnapshot) {
         this.cachedUrls = setFromMultimapValues(urls);
         this.cachedYangStoreSnapshot = Optional.of(yangStoreSnapshot);
     }
@@ -240,4 +251,28 @@ class YangStoreCache {
             cachedYangStoreSnapshot = Optional.absent();
         }
     }
+
+    private Optional<YangStoreSnapshot> getInitialSnapshot() {
+        YangStoreSnapshot initialSnapshot = new YangStoreSnapshot() {
+            @Override
+            public Map<String, Map<String, ModuleMXBeanEntry>> getModuleMXBeanEntryMap() {
+                return Collections.emptyMap();
+            }
+
+            @Override
+            public Map<String, Map.Entry<Module, String>> getModuleMap() {
+                return Collections.emptyMap();
+            }
+
+            @Override
+            public int countModuleMXBeanEntries() {
+                return 0;
+            }
+
+            @Override
+            public void close() {
+            }
+        };
+        return Optional.of(initialSnapshot);
+    }
 }

diff --git a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreSnapshotImpl.java b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreSnapshotImpl.java
index 7a5ca7debe8c3d4379e1c03812afcabac3ee1390..d5936c28aebfb8b8a9e204ced8e5416590601e5d 100644 (file)
--- a/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreSnapshotImpl.java
+++ b/opendaylight/config/yang-store-impl/src/main/java/org/opendaylight/controller/config/yang/store/impl/YangStoreSnapshotImpl.java
@@ -29,9 +29,9 @@ public class YangStoreSnapshotImpl implements YangStoreSnapshot {
         this.moduleMap = Collections.unmodifiableMap(moduleMap);
     }
 
-    public YangStoreSnapshotImpl(YangStoreSnapshotImpl yangStoreSnapshot) {
-        this.moduleMXBeanEntryMap = yangStoreSnapshot.moduleMXBeanEntryMap;
-        this.moduleMap = yangStoreSnapshot.moduleMap;
+    public YangStoreSnapshotImpl(YangStoreSnapshot yangStoreSnapshot) {
+        this.moduleMXBeanEntryMap = yangStoreSnapshot.getModuleMXBeanEntryMap();
+        this.moduleMap = yangStoreSnapshot.getModuleMap();
     }
 
     @Override

diff --git a/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java b/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java
index c4c523992f271391b33f6220b9f49fc28a4fc4ca..b4054c242cee5a938f8968d109adfe7b5f29acf3 100644 (file)
--- a/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java
+++ b/opendaylight/config/yang-store-impl/src/test/java/org/opendaylight/controller/config/yang/store/impl/ExtenderYangTrackerCustomizerTest.java
@@ -9,12 +9,14 @@ package org.opendaylight.controller.config.yang.store.impl;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.opendaylight.controller.config.yang.store.api.YangStoreException;
 import org.opendaylight.controller.config.yang.store.api.YangStoreSnapshot;
+import org.opendaylight.yangtools.yang.model.api.Module;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleListener;
@@ -25,6 +27,7 @@ import java.net.URL;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.Map;
 import java.util.regex.Pattern;
 
 import static org.junit.Assert.assertEquals;
@@ -48,8 +51,28 @@ public class ExtenderYangTrackerCustomizerTest {
     @Mock
     private BundleContext bundleContext;
 
+    private Map<String, Map.Entry<Module, String>> moduleMap = Maps.newHashMap();
+
     @Before
     public void setUp() throws YangStoreException {
+
+        moduleMap.put("1", new Map.Entry<Module, String>() {
+            @Override
+            public Module getKey() {
+                return mock(Module.class);
+            }
+
+            @Override
+            public String getValue() {
+                return "v";
+            }
+
+            @Override
+            public String setValue(String value) {
+                return "v";
+            }
+        });
+
         MockitoAnnotations.initMocks(this);
         doNothing().when(bundleContext).addBundleListener(any(BundleListener.class));
         doReturn(new Bundle[0]).when(bundleContext).getBundles();
@@ -59,7 +82,8 @@ public class ExtenderYangTrackerCustomizerTest {
         doReturn(22).when(yangStoreSnapshot).countModuleMXBeanEntries();
         doReturn("mock yang store").when(yangStoreSnapshot).toString();
         doNothing().when(yangStoreSnapshot).close();
-        doReturn(Collections.emptyMap()).when(yangStoreSnapshot).getModuleMap();
+        doReturn(moduleMap).when(yangStoreSnapshot).getModuleMap();
+        doReturn(Collections.emptyMap()).when(yangStoreSnapshot).getModuleMXBeanEntryMap();
     }
 
     @Test
@@ -75,7 +99,7 @@ public class ExtenderYangTrackerCustomizerTest {
 
         returnedStore = tested.getYangStoreSnapshot();
 
-        assertEquals(yangStoreSnapshot, returnedStore);
+        assertEquals(yangStoreSnapshot.getModuleMap(), returnedStore.getModuleMap());
 
         tested.removedBundle(bundle, null, null);
         tested.getYangStoreSnapshot();
@@ -87,7 +111,7 @@ public class ExtenderYangTrackerCustomizerTest {
             tested.getYangStoreSnapshot();
         }
 
-        verify(parser, times(7)).parseYangFiles(anyCollectionOf(InputStream.class));
+        verify(parser, times(5)).parseYangFiles(anyCollectionOf(InputStream.class));
 
         returnedStore = tested.getYangStoreSnapshot();