You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by cz...@apache.org on 2018/01/24 14:11:46 UTC

svn commit: r1822106 - in /felix/trunk/configadmin/src: main/java/org/apache/felix/cm/impl/ test/java/org/apache/felix/cm/impl/

Author: cziegeler
Date: Wed Jan 24 14:11:46 2018
New Revision: 1822106

URL: http://svn.apache.org/viewvc?rev=1822106&view=rev
Log:
FELIX-5778 : Refactor factory configuration handling

Removed:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/Factory.java
Modified:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxy.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxyTest.java
    felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationManagerTest.java

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxy.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxy.java?rev=1822106&r1=1822105&r2=1822106&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxy.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxy.java Wed Jan 24 14:11:46 2018
@@ -22,7 +22,11 @@ package org.apache.felix.cm.impl;
 import java.io.IOException;
 import java.util.Dictionary;
 import java.util.Enumeration;
-import java.util.Hashtable;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.Vector;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
@@ -31,6 +35,7 @@ import java.util.concurrent.locks.Reentr
 import org.apache.felix.cm.NotCachablePersistenceManager;
 import org.apache.felix.cm.PersistenceManager;
 import org.osgi.framework.Constants;
+import org.osgi.service.cm.ConfigurationAdmin;
 
 
 /**
@@ -41,13 +46,14 @@ import org.osgi.framework.Constants;
  */
 class CachingPersistenceManagerProxy implements PersistenceManager
 {
-    /** the actual PersistenceManager */
+
+    /** The actual PersistenceManager */
     private final PersistenceManager pm;
 
-    /** cached dictionaries */
-    private final Hashtable<String, CaseInsensitiveDictionary> cache;
+    /** Cached dictionaries */
+    private final Map<String, CaseInsensitiveDictionary> cache = new HashMap<String, CaseInsensitiveDictionary>();
 
-    /** protecting lock */
+    /** Protecting lock */
     private final ReadWriteLock globalLock = new ReentrantReadWriteLock();
 
     /**
@@ -55,8 +61,10 @@ class CachingPersistenceManagerProxy imp
      * and the cache is complete with respect to the contents of the underlying
      * persistence manager.
      */
-    private boolean fullyLoaded;
+    private volatile boolean fullyLoaded;
 
+    /** Factory configuration cache. */
+    private final Map<String, Set<String>> factoryConfigCache = new HashMap<String, Set<String>>();
 
     /**
      * Creates a new caching layer for the given actual {@link PersistenceManager}.
@@ -65,7 +73,6 @@ class CachingPersistenceManagerProxy imp
     public CachingPersistenceManagerProxy( final PersistenceManager pm )
     {
         this.pm = pm;
-        this.cache = new Hashtable<String, CaseInsensitiveDictionary>();
     }
 
     public boolean isNotCachablePersistenceManager() {
@@ -83,13 +90,29 @@ class CachingPersistenceManagerProxy imp
      * manager.
      */
     @Override
-    public void delete( String pid ) throws IOException
+    public void delete( final String pid ) throws IOException
     {
         Lock lock = globalLock.writeLock();
         try
         {
             lock.lock();
-            cache.remove( pid );
+            final Dictionary props = cache.remove( pid );
+            if ( props != null )
+            {
+                final String factoryPid = (String)props.get(ConfigurationAdmin.SERVICE_FACTORYPID);
+                if ( factoryPid != null )
+                {
+                    final Set<String> factoryPids = this.factoryConfigCache.get(factoryPid);
+                    if ( factoryPids != null )
+                    {
+                        factoryPids.remove(pid);
+                        if ( factoryPids.isEmpty() )
+                        {
+                            this.factoryConfigCache.remove(factoryPid);
+                        }
+                    }
+                }
+            }
             pm.delete(pid);
         }
         finally
@@ -105,7 +128,7 @@ class CachingPersistenceManagerProxy imp
      * persistence manager is asked.
      */
     @Override
-    public boolean exists( String pid )
+    public boolean exists( final String pid )
     {
         Lock lock = globalLock.readLock();
         try
@@ -123,7 +146,7 @@ class CachingPersistenceManagerProxy imp
     /**
      * Returns an <code>Enumeration</code> of <code>Dictionary</code> objects
      * representing the configurations stored in the underlying persistence
-     * managers. The dictionaries returned are garanteed to contain the
+     * managers. The dictionaries returned are guaranteed to contain the
      * <code>service.pid</code> property.
      * <p>
      * Note, that each call to this method will return new dictionary objects.
@@ -136,6 +159,26 @@ class CachingPersistenceManagerProxy imp
         return getDictionaries( null );
     }
 
+    private final void cache(final Dictionary props)
+    {
+        final String pid = (String) props.get( Constants.SERVICE_PID );
+        if ( pid != null && !cache.containsKey(pid) )
+        {
+            cache.put( pid, copy( props ) );
+            final String factoryPid = (String)props.get(ConfigurationAdmin.SERVICE_FACTORYPID);
+            if ( factoryPid != null )
+            {
+                Set<String> factoryPids = this.factoryConfigCache.get(factoryPid);
+                if ( factoryPids == null )
+                {
+                    factoryPids = new HashSet<String>();
+                    this.factoryConfigCache.put(factoryPid, factoryPids);
+                }
+                factoryPids.add(pid);
+            }
+        }
+    }
+
     public Enumeration getDictionaries( SimpleFilter filter ) throws IOException
     {
         Lock lock = globalLock.readLock();
@@ -160,20 +203,7 @@ class CachingPersistenceManagerProxy imp
                     while ( fromPm.hasMoreElements() )
                     {
                         Dictionary next = (Dictionary) fromPm.nextElement();
-                        String pid = (String) next.get( Constants.SERVICE_PID );
-                        if ( pid != null )
-                        {
-                            cache.put( pid, copy( next ) );
-                        }
-                        else
-                        {
-                            pid = (String) next.get( Factory.FACTORY_PID );
-                            if ( pid != null )
-                            {
-                                pid = Factory.factoryPidToIdentifier( pid );
-                                cache.put( pid, copy( next ) );
-                            }
-                        }
+                        this.cache(next);
                     }
                     this.fullyLoaded = true;
                 }
@@ -199,7 +229,7 @@ class CachingPersistenceManagerProxy imp
 
     /**
      * Returns the dictionary for the given PID or <code>null</code> if no
-     * such dictionary is stored by the underyling persistence manager. This
+     * such dictionary is stored by the underlying persistence manager. This
      * method caches the returned dictionary for future use after retrieving
      * if from the persistence manager.
      * <p>
@@ -208,7 +238,7 @@ class CachingPersistenceManagerProxy imp
      * has no influence on the dictionaries stored in the cache.
      */
     @Override
-    public Dictionary load( String pid ) throws IOException
+    public Dictionary load( final String pid ) throws IOException
     {
         Lock lock = globalLock.readLock();
         try
@@ -223,11 +253,15 @@ class CachingPersistenceManagerProxy imp
                 loaded = cache.get( pid );
                 if ( loaded == null )
                 {
-                    loaded = pm.load( pid );
-                    cache.put( pid, copy( loaded ) );
+                    loaded = pm.load(pid);
+                    if ( loaded != null )
+                    {
+                        this.cache(loaded);
+                    }
                 }
+
             }
-            return copy( loaded );
+            return loaded == null ? null : copy( loaded );
         }
         finally
         {
@@ -246,14 +280,15 @@ class CachingPersistenceManagerProxy imp
      * the cached data.
      */
     @Override
-    public void store( String pid, Dictionary properties ) throws IOException
+    public void store( final String pid, final Dictionary properties ) throws IOException
     {
-        Lock lock = globalLock.writeLock();
+        final Lock lock = globalLock.writeLock();
         try
         {
             lock.lock();
             pm.store( pid, properties );
-            cache.put( pid, copy( properties ) );
+            this.cache.remove(pid);
+            this.cache(properties);
         }
         finally
         {
@@ -261,6 +296,46 @@ class CachingPersistenceManagerProxy imp
         }
     }
 
+    public void getFactoryConfigurationPids(final List<String> targetedFactoryPids, final Set<String> pids)
+    throws IOException
+    {
+        Lock lock = globalLock.readLock();
+        try
+        {
+            lock.lock();
+            if ( !this.fullyLoaded )
+            {
+                lock.unlock();
+                lock = globalLock.writeLock();
+                lock.lock();
+                if ( !this.fullyLoaded )
+                {
+                    final Enumeration fromPm = pm.getDictionaries();
+                    while ( fromPm.hasMoreElements() )
+                    {
+                        Dictionary next = (Dictionary) fromPm.nextElement();
+                        this.cache(next);
+                    }
+                    this.fullyLoaded = true;
+                }
+                lock.unlock();
+                lock = globalLock.readLock();
+                lock.lock();
+            }
+            for(final String targetFactoryPid : targetedFactoryPids)
+            {
+                final Set<String> cachedPids = this.factoryConfigCache.get(targetFactoryPid);
+                if ( cachedPids != null )
+                {
+                    pids.addAll(cachedPids);
+                }
+            }
+        }
+        finally
+        {
+            lock.unlock();
+        }
+    }
 
     /**
      * Creates and returns a copy of the given dictionary. This method simply

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java?rev=1822106&r1=1822105&r2=1822106&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java Wed Jan 24 14:11:46 2018
@@ -383,12 +383,6 @@ public class ConfigurationImpl extends C
             // finally assign the configuration for use
             configure( newProperties );
 
-            // if this is a factory configuration, update the factory with
-            // do this only after configuring with current properties such
-            // that a concurrently registered ManagedServiceFactory service
-            // does not receive a new/unusable configuration
-            updateFactory();
-
             // update the service and fire an CM_UPDATED event
             getConfigurationManager().updated( this, true );
         }
@@ -466,39 +460,6 @@ public class ConfigurationImpl extends C
     }
 
 
-    /**
-     * Makes sure the configuration is added to the {@link Factory} (and
-     * the factory be stored if updated) if this is a factory
-     * configuration.
-     *
-     * @throws IOException If an error occurrs storing the {@link Factory}
-     */
-    private void updateFactory() throws IOException {
-        String factoryPid = getFactoryPidString();
-        if ( factoryPid != null )
-        {
-            Factory factory = getConfigurationManager().getOrCreateFactory( factoryPid );
-            synchronized (factory) {
-                if ( factory.addPID( getPidString() ) )
-                {
-                    // only write back if the pid was not already registered
-                    // with the factory
-                    try
-                    {
-                        factory.store();
-                    }
-                    catch ( IOException ioe )
-                    {
-                        getConfigurationManager().log( LogService.LOG_ERROR,
-                            "Failure storing factory {0} with new configuration {1}", new Object[]
-                                { factoryPid, getPidString(), ioe } );
-                    }
-                }
-            }
-        }
-    }
-
-
     @Override
     void store() throws IOException
     {

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java?rev=1822106&r1=1822105&r2=1822106&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java Wed Jan 24 14:11:46 2018
@@ -27,11 +27,13 @@ import java.util.Arrays;
 import java.util.Dictionary;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Hashtable;
-import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Random;
+import java.util.Set;
 
 import org.apache.felix.cm.PersistenceManager;
 import org.apache.felix.cm.file.FilePersistenceManager;
@@ -168,12 +170,9 @@ public class ConfigurationManager implem
     // persistenceManagers were last got
     private int pmtCount;
 
-    // the cache of Factory instances mapped by their factory PID
-    private final HashMap<String, Factory> factories = new HashMap<String, Factory>();
-
     // the cache of Configuration instances mapped by their PID
     // have this always set to prevent NPE on bundle shutdown
-    private final HashMap<String, ConfigurationImpl> configurations = new HashMap<String, ConfigurationImpl>();
+    private final Map<String, ConfigurationImpl> configurations = new HashMap<String, ConfigurationImpl>();
 
     /**
      * The map of dynamic configuration bindings. This maps the
@@ -277,7 +276,7 @@ public class ConfigurationManager implem
 
         // create and register configuration admin - start after PM tracker ...
         ConfigurationAdminFactory caf = new ConfigurationAdminFactory( this );
-        Hashtable props = new Hashtable();
+        Hashtable<String, Object> props = new Hashtable<String, Object>();
         props.put( Constants.SERVICE_PID, "org.apache.felix.cm.ConfigurationAdmin" );
         props.put( Constants.SERVICE_DESCRIPTION, "Configuration Admin Service Specification 1.5 Implementation" );
         props.put( Constants.SERVICE_VENDOR, "Apache Software Foundation" );
@@ -366,12 +365,6 @@ public class ConfigurationManager implem
             configurations.clear();
         }
 
-        // just ensure the factory cache is empty
-        synchronized ( factories )
-        {
-            factories.clear();
-        }
-
         this.bundleContext = null;
     }
 
@@ -436,33 +429,6 @@ public class ConfigurationManager implem
     }
 
 
-    Factory getCachedFactory( String factoryPid )
-    {
-        synchronized ( factories )
-        {
-            return factories.get( factoryPid );
-        }
-    }
-
-
-    Factory[] getCachedFactories()
-    {
-        synchronized ( factories )
-        {
-            return factories.values().toArray( new Factory[factories.size()] );
-        }
-    }
-
-
-    void cacheFactory( Factory factory )
-    {
-        synchronized ( factories )
-        {
-            factories.put( factory.getFactoryPidString(), factory );
-        }
-    }
-
-
     // ---------- ConfigurationAdminImpl support
 
     void setDynamicBundleLocation( final String pid, final String location )
@@ -520,8 +486,7 @@ public class ConfigurationManager implem
         if ( serviceBundle != null )
         {
             // list of targeted PIDs to check
-            // (StringBuffer for pre-1.5 API compatibility)
-            final StringBuffer targetedPid = new StringBuffer( rawPid );
+            final StringBuilder targetedPid = new StringBuilder( rawPid );
             int i = 3;
             String[] names = new String[4];
             names[i--] = targetedPid.toString();
@@ -574,7 +539,7 @@ public class ConfigurationManager implem
      *
      * @param pid The PID for which to return the configuration
      * @return The configuration or <code>null</code> if non exists
-     * @throws IOException If an error occurrs reading from a persistence
+     * @throws IOException If an error occurs reading from a persistence
      *      manager.
      */
     ConfigurationImpl getConfiguration( String pid ) throws IOException
@@ -995,122 +960,39 @@ public class ConfigurationManager implem
      * Configuration Admin 1.5 specification for targeted PIDs (Section
      * 104.3.2)
      *
-     * @param rawFactoryPid The raw factory PID without any targetting.
+     * @param rawFactoryPid The raw factory PID without any targeting.
      * @param target The <code>ServiceReference</code> of the service to
      *      be supplied with targeted configuration.
      * @return A list of {@link Factory} instances as listed above. This
      *      list will always at least include an instance for the
      *      <code>rawFactoryPid</code>. Other instances are only included
      *      if existing.
-     * @throws IOException If an error occurrs reading any of the
+     * @throws IOException If an error occurs reading any of the
      *      {@link Factory} instances from persistence
      */
-    List<Factory> getTargetedFactories( final String rawFactoryPid, final ServiceReference target ) throws IOException
+    List<String> getTargetedFactories( final String rawFactoryPid, final ServiceReference target ) throws IOException
     {
-        LinkedList<Factory> factories = new LinkedList<Factory>();
+        List<String> factories = new LinkedList<String>();
 
         final Bundle serviceBundle = target.getBundle();
         if ( serviceBundle != null )
         {
-            // for pre-1.5 API compatibility
-            final StringBuffer targetedPid = new StringBuffer( rawFactoryPid );
-            factories.add( getOrCreateFactory( targetedPid.toString() ) );
+            final StringBuilder targetedPid = new StringBuilder( rawFactoryPid );
+            factories.add( targetedPid.toString() );
 
             targetedPid.append( '|' ).append( serviceBundle.getSymbolicName() );
-            Factory f = getFactory( targetedPid.toString() );
-            if ( f != null )
-            {
-                factories.add( 0, f );
-            }
+            factories.add( 0, targetedPid.toString() );
 
             targetedPid.append( '|' ).append( TargetedPID.getBundleVersion( serviceBundle ) );
-            f = getFactory( targetedPid.toString() );
-            if ( f != null )
-            {
-                factories.add( 0, f );
-            }
+            factories.add( 0, targetedPid.toString() );
 
             targetedPid.append( '|' ).append( serviceBundle.getLocation() );
-            f = getFactory( targetedPid.toString() );
-            if ( f != null )
-            {
-                factories.add( 0, f );
-            }
+            factories.add( 0, targetedPid.toString() );
         }
 
         return factories;
     }
 
-
-    /**
-     * Gets the factory with the exact identifier from the cached or from
-     * the persistence managers. If no factory exists already one is
-     * created and cached.
-     *
-     * @param factoryPid The PID of the {@link Factory} to return
-     * @return The existing or newly created {@link Factory}
-     * @throws IOException If an error occurrs reading the factory from
-     *      a {@link PersistenceManager}
-     */
-    Factory getOrCreateFactory( String factoryPid ) throws IOException
-    {
-        Factory factory = getFactory( factoryPid );
-        if ( factory != null )
-        {
-            return factory;
-        }
-
-        return createFactory( factoryPid );
-    }
-
-
-    /**
-     * Gets the factory with the exact identifier from the cached or from
-     * the persistence managers. If no factory exists <code>null</code>
-     * is returned.
-     *
-     * @param factoryPid The PID of the {@link Factory} to return
-     * @return The existing {@link Factory} or <code>null</code>
-     * @throws IOException If an error occurrs reading the factory from
-     *      a {@link PersistenceManager}
-     */
-    Factory getFactory( String factoryPid ) throws IOException
-    {
-        // check for cached factory
-        Factory factory = getCachedFactory( factoryPid );
-        if ( factory != null )
-        {
-            return factory;
-        }
-
-        // try to load factory from persistence
-        PersistenceManager[] pmList = getPersistenceManagers();
-        for ( int i = 0; i < pmList.length; i++ )
-        {
-            if ( Factory.exists( pmList[i], factoryPid ) )
-            {
-                factory = Factory.load( this, pmList[i], factoryPid );
-                cacheFactory( factory );
-                return factory;
-            }
-        }
-
-        // no existing factory
-        return null;
-    }
-
-
-    /**
-     * Creates a new factory with the given <code>factoryPid</code>.
-     */
-    Factory createFactory( String factoryPid )
-    {
-        Factory factory = new Factory( this, getPersistenceManagers()[0], factoryPid );
-        cacheFactory( factory );
-        return factory;
-    }
-
-
     /**
      * Calls the registered configuration plugins on the given configuration
      * properties from the given configuration object.
@@ -1121,7 +1003,7 @@ public class ConfigurationManager implem
      * configurations) or the PID of the configuration (for non-factory
      * configurations).
      *
-     * @param props The configuraiton properties run through the registered
+     * @param props The configuration properties run through the registered
      *          ConfigurationPlugin services. This must not be
      *          <code>null</code>.
      * @param sr The service reference of the managed service (factory) which
@@ -1225,7 +1107,7 @@ public class ConfigurationManager implem
         randomBytes[8] &= 0x3f; /* clear variant */
         randomBytes[8] |= 0x80; /* set to IETF variant */
 
-        StringBuffer buf = new StringBuffer( factoryPid.length() + 1 + 36 );
+        StringBuilder buf = new StringBuilder( factoryPid.length() + 1 + 36 );
 
         // prefix the new pid with the factory pid
         buf.append( factoryPid ).append( "." );
@@ -1516,75 +1398,53 @@ public class ConfigurationManager implem
             for ( String factoryPid : this.factoryPids )
             {
 
-                List<Factory> factories = null;
                 try
                 {
-                    factories = getTargetedFactories( factoryPid, sr );
-                    for ( Factory factory : factories )
+                    final List<String> targetedFactoryPids = getTargetedFactories( factoryPid, sr );
+                    final Set<String> pids = new HashSet<String>();
+                    for(final CachingPersistenceManagerProxy pm : getPersistenceManagers())
+                    {
+                        pm.getFactoryConfigurationPids(targetedFactoryPids, pids);
+                    }
+                    for ( final String pid : pids )
                     {
-                        synchronized (factory) {
-                            for ( Iterator pi = factory.getPIDs().iterator(); pi.hasNext(); )
-                            {
-                                final String pid = ( String ) pi.next();
-                                ConfigurationImpl cfg;
-                                try
-                                {
-                                    cfg = getConfiguration( pid );
-                                }
-                                catch ( IOException ioe )
-                                {
-                                    log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
-                                        { pid, ioe } );
-                                    continue;
-                                }
-
-                                // sanity check on the configuration
-                                if ( cfg == null )
-                                {
-                                    log( LogService.LOG_ERROR,
-                                        "Configuration {0} referred to by factory {1} does not exist", new Object[]
-                                            { pid, factoryPid } );
-                                    factory.removePID( pid );
-                                    factory.storeSilently();
-                                    continue;
-                                }
-                                else if ( cfg.isNew() )
-                                {
-                                    // Configuration has just been created but not yet updated
-                                    // we currently just ignore it and have the update mechanism
-                                    // provide the configuration to the ManagedServiceFactory
-                                    // As of FELIX-612 (not storing new factory configurations)
-                                    // this should not happen. We keep this for added stability
-                                    // but raise the logging level to error.
-                                    log( LogService.LOG_ERROR, "Ignoring new configuration pid={0}", new Object[]
-                                        { pid } );
-                                    continue;
-                                }
-
-                                /*
-                                 * this code would catch targeted factory PIDs;
-                                 * since this is not expected any way, we can
-                                 * leave this out
-                                 */
-                                /*
-                                else if ( !factoryPid.equals( cfg.getFactoryPid() ) )
-                                {
-                                    log( LogService.LOG_ERROR,
-                                        "Configuration {0} referred to by factory {1} seems to belong to factory {2}",
-                                        new Object[]
-                                            { pid, factoryPid, cfg.getFactoryPid() } );
-                                    factory.removePID( pid );
-                                    factory.storeSilently();
-                                    continue;
-                                }
-                                */
+                        ConfigurationImpl cfg;
+                        try
+                        {
+                            cfg = getConfiguration( pid );
+                        }
+                        catch ( IOException ioe )
+                        {
+                            log( LogService.LOG_ERROR, "Error loading configuration for {0}", new Object[]
+                                { pid, ioe } );
+                            continue;
+                        }
 
-                                provide( factoryPid, cfg );
-                            }
+                        // sanity check on the configuration
+                        if ( cfg == null )
+                        {
+                            log( LogService.LOG_ERROR,
+                                "Configuration {0} referred to by factory {1} does not exist", new Object[]
+                                    { pid, factoryPid } );
+                            continue;
                         }
+                        else if ( cfg.isNew() )
+                        {
+                            // Configuration has just been created but not yet updated
+                            // we currently just ignore it and have the update mechanism
+                            // provide the configuration to the ManagedServiceFactory
+                            // As of FELIX-612 (not storing new factory configurations)
+                            // this should not happen. We keep this for added stability
+                            // but raise the logging level to error.
+                            log( LogService.LOG_ERROR, "Ignoring new configuration pid={0}", new Object[]
+                                { pid } );
+                            continue;
+                        }
+
+                        provide( factoryPid, cfg );
                     }
                 }
-                catch ( IOException ioe )
+                catch ( final IOException ioe )
                 {
                     log( LogService.LOG_ERROR, "Cannot get factory mapping for factory PID {0}", new Object[]
                         { factoryPid, ioe } );
@@ -1878,26 +1738,6 @@ public class ConfigurationManager implem
                     }
                 }
             }
-
-            final TargetedPID factoryPid = config.getFactoryPid();
-            if ( factoryPid != null )
-            {
-                // remove the pid from the factory
-                final String pid = config.getPidString();
-                try
-                {
-                    Factory factory = getOrCreateFactory( factoryPid.toString() );
-                    synchronized (factory) {
-                        factory.removePID( pid );
-                        factory.store();
-                    }
-                }
-                catch ( IOException ioe )
-                {
-                    log( LogService.LOG_ERROR, "Failed removing {0} from the factory {1}", new Object[]
-                        { pid, factoryPid, ioe } );
-                }
-            }
         }
 
         @Override

Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxyTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxyTest.java?rev=1822106&r1=1822105&r2=1822106&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxyTest.java (original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/CachingPersistenceManagerProxyTest.java Wed Jan 24 14:11:46 2018
@@ -27,7 +27,6 @@ import java.util.List;
 import org.apache.felix.cm.MockNotCachablePersistenceManager;
 import org.apache.felix.cm.MockPersistenceManager;
 import org.apache.felix.cm.PersistenceManager;
-
 import org.osgi.framework.Constants;
 
 import junit.framework.TestCase;
@@ -95,7 +94,7 @@ public class CachingPersistenceManagerPr
 
         dictionaries = cpm.getDictionaries( filter );
         list = Collections.list( dictionaries );
-        assertEquals(0, list.size());
+        assertEquals(1, list.size());
     }
 
 }
\ No newline at end of file

Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationManagerTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationManagerTest.java?rev=1822106&r1=1822105&r2=1822106&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationManagerTest.java (original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/impl/ConfigurationManagerTest.java Wed Jan 24 14:11:46 2018
@@ -139,7 +139,7 @@ public class ConfigurationManagerTest ex
         dictionary.put("property1", "valueNotCached");
         pid = "testDefaultPersistenceManager";
         dictionary.put( Constants.SERVICE_PID, pid );
-        pm.store( pid, dictionary );
+        persistenceManagers[0].store( pid, dictionary );
 
         conf = configMgr.listConfigurations(new ConfigurationAdminImpl(configMgr, null), null);
         assertEquals(1, conf.length);
@@ -407,13 +407,13 @@ public class ConfigurationManagerTest ex
         final ConfigurationImpl c3 = configMgr.createFactoryConfiguration(factoryPid, null);
         c3.update(props);
 
-        assertEquals(4, pm.getStored().size());
+        assertEquals(3, pm.getStored().size());
 
         c1.delete();
-        assertEquals(3, pm.getStored().size());
+        assertEquals(2, pm.getStored().size());
 
         c2.delete();
-        assertEquals(2, pm.getStored().size());
+        assertEquals(1, pm.getStored().size());
 
         c3.delete();
         assertEquals(0, pm.getStored().size());