Merge pull request #6 from bosschaert/SLING-9395-2

SLING-9395 Support bundle updates in api regions
diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java b/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
index 69b2f79..bdacb40 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/Activator.java
@@ -71,6 +71,10 @@
     @Override
     public synchronized void stop(BundleContext context) throws Exception {
         // All services automatically get unregistered by the framework.
+
+        if (configuration != null) {
+            configuration.storePersistedConfiguration(context);
+        }
     }
 
     private void createConfiguration() {
diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/RegionConfiguration.java b/src/main/java/org/apache/sling/feature/apiregions/impl/RegionConfiguration.java
index b63e54b..e375f2c 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/RegionConfiguration.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/RegionConfiguration.java
@@ -18,9 +18,14 @@
  */
 package org.apache.sling.feature.apiregions.impl;
 
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.AbstractMap;
@@ -39,12 +44,15 @@
 import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Version;
 
 class RegionConfiguration {
-
+    private static final String BUNDLE_LOCATION_TO_FEATURE_FILE = "bundleLocationToFeature.properties";
 
     volatile Map<Map.Entry<String, Version>, List<String>> bsnVerMap;
     volatile Map<String, Set<String>> bundleFeatureMap;
@@ -62,6 +70,13 @@
     private final Map<String, Set<String>> baseFeatureRegionMap;
     private final Map<String, Set<String>> baseRegionPackageMap;
 
+    // This field stores the association between bundle location and features.
+    // It is populated dynamically as bundles are getting resolved. If a feature
+    // cannot be found for a location, it is looked up through the other maps in
+    // this class.
+    private final ConcurrentMap<String, Set<String>> bundleLocationFeatureMap =
+            new ConcurrentHashMap<>();
+
     private final String toGlobalConfig;
 
     RegionConfiguration(Map<Entry<String, Version>, List<String>> bsnVerMap, Map<String, Set<String>> bundleFeatureMap,
@@ -128,9 +143,62 @@
             defaultRegions = Collections.emptySet();
         }
 
+        loadLocationToFeatureMap(context);
         updateConfiguration();
     }
 
+    private void loadLocationToFeatureMap(BundleContext context) {
+        File file = context.getBundle().getDataFile(BUNDLE_LOCATION_TO_FEATURE_FILE);
+
+        if (file != null && file.exists()) {
+            Properties p = new Properties();
+            try (InputStream is = new BufferedInputStream(new FileInputStream(file))) {
+                p.load(is);
+            } catch (IOException e) {
+                Activator.LOG.log(Level.WARNING, "Unable to load " + BUNDLE_LOCATION_TO_FEATURE_FILE, e);
+            }
+
+            for (String k : p.stringPropertyNames()) {
+                bundleLocationFeatureMap.put(k, stringToSetOfString(p.getProperty(k)));
+            }
+        }
+    }
+
+    void storePersistedConfiguration(BundleContext context) {
+        File file = context.getBundle().getDataFile(BUNDLE_LOCATION_TO_FEATURE_FILE);
+        if (file == null) {
+            Activator.LOG.warning("Cannot store " + BUNDLE_LOCATION_TO_FEATURE_FILE
+                    + " Persistence not supported by this framework.");
+            return;
+        }
+
+        Properties p = new Properties();
+        for (Map.Entry<String, Set<String>> entry : bundleLocationFeatureMap.entrySet()) {
+            p.setProperty(entry.getKey(),
+                    entry.getValue().stream().collect(Collectors.joining(",")));
+        }
+        if (p.size() > 0) {
+            try (OutputStream os = new BufferedOutputStream(new FileOutputStream(file))) {
+                p.store(os, "Bundle Location to Feature Map");
+            } catch (IOException e) {
+                Activator.LOG.log(Level.WARNING, "Unable to store " + BUNDLE_LOCATION_TO_FEATURE_FILE, e);
+            }
+        }
+    }
+
+    private Set<String> stringToSetOfString(String prop) {
+        Set<String> res = new HashSet<>();
+
+        for (String s : prop.split(",")) {
+            String ts = s.trim();
+            if (ts.length() > 0) {
+                res.add(ts);
+            }
+        }
+
+        return res;
+    }
+
     private synchronized void updateConfiguration() {
         final Map<Entry<String, Version>, List<String>> bvm = cloneMapOfLists(this.baseBsnVerMap);
         final Map<String, Set<String>> bfm = cloneMapOfSets(this.baseBundleFeatureMap);
@@ -339,6 +407,17 @@
         return bsnVerMap;
     }
 
+    /**
+     * Obtain a mutable concurrent map that contains an association between
+     * bundle location and features. This map is persisted in bundle private
+     * storage. Initially this map is empty but as more bundles get resolved
+     * this map is gradually filled in.
+     * @return The bundle location to features map.
+     */
+    public ConcurrentMap<String, Set<String>> getBundleLocationFeatureMap() {
+        return bundleLocationFeatureMap;
+    }
+
     public Map<String, Set<String>> getBundleFeatureMap() {
         return bundleFeatureMap;
     }
diff --git a/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java b/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
index 3a721c1..da2e562 100644
--- a/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
+++ b/src/main/java/org/apache/sling/feature/apiregions/impl/ResolverHookImpl.java
@@ -28,8 +28,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
 
 import org.osgi.framework.Bundle;
@@ -42,7 +40,6 @@
 
 class ResolverHookImpl implements ResolverHook {
 
-    final ConcurrentMap<String, Set<String>> bundleLocationFeatureMap = new ConcurrentHashMap<>();
     final RegionConfiguration configuration;
 
     ResolverHookImpl(RegionConfiguration cfg) {
@@ -245,8 +242,8 @@
     }
 
     Set<String> getFeaturesForBundle(Bundle bundle) {
-        return bundleLocationFeatureMap.computeIfAbsent(bundle.getLocation(),
-                l -> getFeaturesForBundleFromConfig(bundle));
+        return this.configuration.getBundleLocationFeatureMap()
+                .computeIfAbsent(bundle.getLocation(), l -> getFeaturesForBundleFromConfig(bundle));
     }
 
     private Set<String> getFeaturesForBundleFromConfig(Bundle bundle) {
diff --git a/src/test/java/org/apache/sling/feature/apiregions/impl/ActivatorTest.java b/src/test/java/org/apache/sling/feature/apiregions/impl/ActivatorTest.java
index df86a4e..10cd4a3 100644
--- a/src/test/java/org/apache/sling/feature/apiregions/impl/ActivatorTest.java
+++ b/src/test/java/org/apache/sling/feature/apiregions/impl/ActivatorTest.java
@@ -84,6 +84,7 @@
         expectedProps.put(REGION_PACKAGE_FILENAME, new File(r).toURI().toString());
 
         BundleContext bc = Mockito.mock(BundleContext.class);
+        Mockito.when(bc.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(bc.getProperty(Activator.REGIONS_PROPERTY_NAME)).thenReturn("*");
         Mockito.when(bc.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).
             thenReturn(i);
diff --git a/src/test/java/org/apache/sling/feature/apiregions/impl/RegionConfigurationTest.java b/src/test/java/org/apache/sling/feature/apiregions/impl/RegionConfigurationTest.java
index fdae296..9d2f4d5 100644
--- a/src/test/java/org/apache/sling/feature/apiregions/impl/RegionConfigurationTest.java
+++ b/src/test/java/org/apache/sling/feature/apiregions/impl/RegionConfigurationTest.java
@@ -44,9 +44,11 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentMap;
 
 import org.junit.Test;
 import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.Version;
 import org.osgi.framework.hooks.resolver.ResolverHook;
@@ -69,6 +71,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/idbsnver1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(f);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -88,6 +91,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/idbsnver1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(f);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -119,6 +123,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/bundles1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(f);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -140,6 +145,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/bundles1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(f);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -175,6 +181,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/features1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(f);
@@ -194,6 +201,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/features1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(f);
@@ -225,6 +233,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/regions1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -244,6 +253,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/regions1.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + FEATURE_REGION_FILENAME)).thenReturn(e);
@@ -273,6 +283,7 @@
         String e = getClass().getResource("/empty.properties").toURI().toString();
         String f = getClass().getResource("/regions2.properties").toURI().toString();
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(APIREGIONS_JOINGLOBAL)).thenReturn("obsolete,deprecated");
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).thenReturn(e);
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).thenReturn(e);
@@ -288,6 +299,7 @@
     @Test
     public void testBegin() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + IDBSNVER_FILENAME)).
             thenReturn(getClass().getResource("/idbsnver1.properties").toURI().toString());
         Mockito.when(ctx.getProperty(PROPERTIES_RESOURCE_PREFIX + BUNDLE_FEATURE_FILENAME)).
@@ -313,6 +325,7 @@
     @Test
     public void testURLs() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         String location = new File(getClass().getResource("/props1/idbsnver.properties").
                 getFile()).getParentFile().toURI().toString();
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).thenReturn(location);
@@ -327,6 +340,7 @@
     @Test
     public void testClassloaderURLs() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props1");
 
@@ -340,6 +354,7 @@
     @Test
     public void testOrderingOfRegionsInFeatures() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props2");
 
@@ -351,6 +366,7 @@
     @Test
     public void testUnModifiableMaps() throws Exception {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
             thenReturn("classloader://props1");
 
@@ -373,9 +389,39 @@
         testDefaultRegions(null, Collections.emptySet());
     }
 
+    @Test
+    public void testStoreLoadPersistedConfig() throws Exception {
+        File f = File.createTempFile("testStorePersistedConfig", ".tmp");
+
+        try {
+            Bundle bundle = Mockito.mock(Bundle.class);
+            Mockito.when(bundle.getDataFile("bundleLocationToFeature.properties"))
+                .thenReturn(f);
+
+            BundleContext ctx = Mockito.mock(BundleContext.class);
+            Mockito.when(ctx.getBundle()).thenReturn(bundle);
+            Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
+                thenReturn("classloader://props1");
+
+            RegionConfiguration cfg = new RegionConfiguration(ctx);
+
+            ConcurrentMap<String, Set<String>> m = cfg.getBundleLocationFeatureMap();
+            m.put("foo://bar", Collections.singleton("blah"));
+            m.put("foo://tar", new HashSet<>(Arrays.asList("a", "b", "c")));
+            cfg.storePersistedConfiguration(ctx);
+
+            RegionConfiguration cfg2 = new RegionConfiguration(ctx);
+            ConcurrentMap<String, Set<String>> m2 = cfg2.getBundleLocationFeatureMap();
+            assertEquals(m, m2);
+        } finally {
+            f.delete();
+        }
+    }
+
     private void testDefaultRegions(String defProp, Set<String> expected)
             throws IOException, URISyntaxException, NoSuchFieldException, IllegalAccessException {
         BundleContext ctx = Mockito.mock(BundleContext.class);
+        Mockito.when(ctx.getBundle()).thenReturn(Mockito.mock(Bundle.class));
         Mockito.when(ctx.getProperty(DEFAULT_REGIONS)).thenReturn(defProp);
         Mockito.when(ctx.getProperty(PROPERTIES_FILE_LOCATION)).
         thenReturn("classloader://props1");