You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by fm...@apache.org on 2009/08/14 23:45:11 UTC
svn commit: r804387 - in /felix/trunk/configadmin/src:
main/java/org/apache/felix/cm/impl/ConfigurationManager.java
test/java/org/apache/felix/cm/integration/ConfigurationTest.java
Author: fmeschbe
Date: Fri Aug 14 21:45:10 2009
New Revision: 804387
URL: http://svn.apache.org/viewvc?rev=804387&view=rev
Log:
FELIX-1165 Ensure setting the service reference field for a ManagedService
configuration also during update.
Additional fixes:
* NPE prevention if bundle event is handled after bundle stop (due to timing)
* logging if on configuration update multiple ManagedService candidates exist
* integration test for service reference field setting
Modified:
felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationTest.java
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=804387&r1=804386&r2=804387&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 Fri Aug 14 21:45:10 2009
@@ -138,7 +138,8 @@
private Map factories;
// the cache of Configuration instances mapped by their PID
- private Map configurations;
+ // have this always set to prevent NPE on bundle shutdown
+ private final Map configurations = new HashMap();
// the maximum log level when no LogService is available
private int logLevel = CM_LOG_LEVEL_DEFAULT;
@@ -173,7 +174,6 @@
// set up some fields
this.bundleContext = bundleContext;
this.factories = new HashMap();
- this.configurations = new HashMap();
// configurationlistener support
configurationListenerTracker = new ServiceTracker( bundleContext, ConfigurationListener.class.getName(), null );
@@ -205,8 +205,7 @@
bundleContext.addBundleListener( this );
// get all persistence managers to begin with
- pmtCount = 1; // make sure to get the persistence managers at least
- // once
+ pmtCount = 1; // make sure to get the persistence managers at least once
persistenceManagerTracker = new ServiceTracker( bundleContext, PersistenceManager.class.getName(), null );
persistenceManagerTracker.open();
@@ -227,6 +226,9 @@
public void stop( BundleContext bundleContext )
{
+ // stop handling bundle events immediately
+ handleBundleEvents = false;
+
// immediately unregister the Configuration Admin before cleaning up
// clearing the field before actually unregistering the service
// prevents IllegalStateException in getServiceReference() if
@@ -246,7 +248,6 @@
// stop listening for events
bundleContext.removeBundleListener( this );
- handleBundleEvents = false;
if ( configurationListenerTracker != null )
{
@@ -274,8 +275,13 @@
logTracker.close();
}
+ // just ensure the configuration cache is cleared, not cleaning
+ synchronized ( configurations )
+ {
+ configurations.clear();
+ }
+
this.bundleContext = null;
- this.configurations = null;
}
@@ -942,8 +948,7 @@
}
// 104.3 Report an error in the log if more than one service
- // with
- // the same PID asks for the configuration
+ // with the same PID asks for the configuration
if ( cfg.getServiceReference() != null && !sr.equals( cfg.getServiceReference() ) )
{
log( LogService.LOG_ERROR, "Configuration for " + pid + " has already been used for service "
@@ -1189,17 +1194,31 @@
{
if ( config.getFactoryPid() == null )
{
- ServiceReference[] sr = bundleContext.getServiceReferences( ManagedService.class.getName(), "("
- + Constants.SERVICE_PID + "=" + config.getPid() + ")" );
- if ( sr != null && sr.length > 0 )
+ final ServiceReference[] srList = bundleContext.getServiceReferences( ManagedService.class
+ .getName(), "(" + Constants.SERVICE_PID + "=" + config.getPid() + ")" );
+ if ( srList != null && srList.length > 0 )
{
- ManagedService srv = ( ManagedService ) bundleContext.getService( sr[0] );
+ final ServiceReference sr = srList[0];
+ final ManagedService srv = ( ManagedService ) bundleContext.getService( sr );
+
+ // 104.3 Report an error in the log if more than one service
+ // with the same PID asks for the configuration
+ if ( srList.length > 1 )
+ {
+ for ( int i = 1; i < srList.length; i++ )
+ {
+ log( LogService.LOG_ERROR, "Configuration for " + config.getPid()
+ + " is used for service " + sr
+ + "following services will not receive configuration: " + srList[i], null );
+ }
+ }
+
try
{
// bind the configuration, fail if bound to another
// bundle !!
// check bundle location of configuration
- String bundleLocation = sr[0].getBundle().getLocation();
+ String bundleLocation = sr.getBundle().getLocation();
if ( config.getBundleLocation() == null )
{
// bind to the location of the service if
@@ -1215,8 +1234,14 @@
return;
}
+ // record the delivery of the configuration
+ if ( config.getServiceReference() == null )
+ {
+ config.setServiceReference( sr );
+ }
+
// prepare the configuration for the service (call plugins)
- Dictionary dictionary = callPlugins( config.getPid(), sr[0], config );
+ Dictionary dictionary = callPlugins( config.getPid(), sr, config );
// update the ManagedService with the properties
srv.updated( dictionary );
@@ -1224,23 +1249,24 @@
}
finally
{
- bundleContext.ungetService( sr[0] );
+ bundleContext.ungetService( sr );
}
}
}
else
{
- ServiceReference[] sr = bundleContext.getServiceReferences( ManagedServiceFactory.class.getName(),
- "(" + Constants.SERVICE_PID + "=" + config.getFactoryPid() + ")" );
- if ( sr != null && sr.length > 0 )
+ ServiceReference[] srList = bundleContext.getServiceReferences( ManagedServiceFactory.class
+ .getName(), "(" + Constants.SERVICE_PID + "=" + config.getFactoryPid() + ")" );
+ if ( srList != null && srList.length > 0 )
{
- ManagedServiceFactory srv = ( ManagedServiceFactory ) bundleContext.getService( sr[0] );
+ final ServiceReference sr = srList[0];
+ final ManagedServiceFactory srv = ( ManagedServiceFactory ) bundleContext.getService( sr );
try
{
// bind the configuration, fail if bound to another
// bundle !!
// check bundle location of configuration
- String bundleLocation = sr[0].getBundle().getLocation();
+ String bundleLocation = sr.getBundle().getLocation();
if ( config.getBundleLocation() == null )
{
// bind to the location of the service if
@@ -1259,7 +1285,7 @@
// prepare the configuration for the service (call plugins)
// call the plugins with cm.target set to the service's factory PID
// (clarification in Section 104.9.1 of Compendium 4.2)
- Dictionary dictionary = callPlugins( config.getFactoryPid(), sr[0], config );
+ Dictionary dictionary = callPlugins( config.getFactoryPid(), sr, config );
// update the ManagedServiceFactory with the properties
// only, if there is non-null configuration data
@@ -1271,7 +1297,7 @@
}
finally
{
- bundleContext.ungetService( sr[0] );
+ bundleContext.ungetService( sr );
}
}
}
@@ -1331,11 +1357,12 @@
{
if ( factoryPid == null )
{
- ServiceReference[] sr = bundleContext.getServiceReferences( ManagedService.class.getName(), "("
+ ServiceReference[] srList = bundleContext.getServiceReferences( ManagedService.class.getName(), "("
+ Constants.SERVICE_PID + "=" + pid + ")" );
- if ( sr != null && sr.length > 0 )
+ if ( srList != null && srList.length > 0 )
{
- ManagedService srv = ( ManagedService ) bundleContext.getService( sr[0] );
+ final ServiceReference sr = srList[0];
+ final ManagedService srv = ( ManagedService ) bundleContext.getService( sr );
try
{
srv.updated( null );
@@ -1343,7 +1370,7 @@
}
finally
{
- bundleContext.ungetService( sr[0] );
+ bundleContext.ungetService( sr );
}
}
}
@@ -1354,11 +1381,12 @@
factory.removePID( pid );
factory.store();
- ServiceReference[] sr = bundleContext.getServiceReferences( ManagedServiceFactory.class.getName(),
- "(" + Constants.SERVICE_PID + "=" + factoryPid + ")" );
- if ( sr != null && sr.length > 0 )
+ ServiceReference[] srList = bundleContext.getServiceReferences( ManagedServiceFactory.class
+ .getName(), "(" + Constants.SERVICE_PID + "=" + factoryPid + ")" );
+ if ( srList != null && srList.length > 0 )
{
- ManagedServiceFactory srv = ( ManagedServiceFactory ) bundleContext.getService( sr[0] );
+ final ServiceReference sr = srList[0];
+ final ManagedServiceFactory srv = ( ManagedServiceFactory ) bundleContext.getService( sr );
try
{
srv.deleted( pid );
@@ -1366,7 +1394,7 @@
}
finally
{
- bundleContext.ungetService( sr[0] );
+ bundleContext.ungetService( sr );
}
}
}
Modified: felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationTest.java?rev=804387&r1=804386&r2=804387&view=diff
==============================================================================
--- felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationTest.java (original)
+++ felix/trunk/configadmin/src/test/java/org/apache/felix/cm/integration/ConfigurationTest.java Fri Aug 14 21:45:10 2009
@@ -129,6 +129,10 @@
// give cm time for distribution
delay();
+ // assert activater has configuration
+ TestCase.assertNotNull( "Expect Properties after Service Registration", tester.props );
+ TestCase.assertEquals( "Expect a single update call", 1, tester.numUpdatedCalls );
+
// ensure a freshly retrieved object also has the location
final Configuration beforeStop = getConfiguration( pid );
TestCase.assertEquals( beforeStop.getBundleLocation(), bundle.getLocation() );
@@ -160,17 +164,100 @@
// ensure a freshly retrieved object also does not have the location
final Configuration atEnd = getConfiguration( pid );
TestCase.assertNull( atEnd.getBundleLocation() );
+
+ // remove the configuration for good
+ deleteConfig( pid );
+ }
+
+
+ @Test
+ public void test_start_bundle_configure_stop_start_bundle() throws IOException, BundleException
+ {
+ String pid = "test_start_bundle_configure_stop_start_bundle";
+
+ // start the bundle and assert this
+ bundle = installBundle( pid );
+ bundle.start();
+ final TestActivator tester = TestActivator.INSTANCE;
+ TestCase.assertNotNull( "Activator not started !!", tester );
+
+ // give cm time for distribution
+ delay();
+
+ // assert activater has no configuration
+ TestCase.assertNull( "Expect no Properties after Service Registration", tester.props );
+ TestCase.assertEquals( "Expect no update call", 1, tester.numUpdatedCalls );
+
+ // configure after ManagedServiceRegistration --> configure via update
+ configure( pid );
+ delay();
+
+ // assert activater has configuration
+ TestCase.assertNotNull( "Expect Properties after Service Registration", tester.props );
+ TestCase.assertEquals( "Expect a single update call", 2, tester.numUpdatedCalls );
+
+ // stop the bundle now
+ bundle.stop();
+
+ // assert INSTANCE is null
+ TestCase.assertNull( TestActivator.INSTANCE );
+
+ delay();
+
+ // start the bundle again (and check)
+ bundle.start();
+ final TestActivator tester2 = TestActivator.INSTANCE;
+ TestCase.assertNotNull( "Activator not started the second time!!", tester2 );
+ TestCase.assertNotSame( "Instances must not be the same", tester, tester2 );
+
+ // give cm time for distribution
+ delay();
+
+ // assert activater has configuration
+ TestCase.assertNotNull( "Expect Properties after Service Registration", tester2.props );
+ TestCase.assertEquals( "Expect a second update call", 1, tester2.numUpdatedCalls );
+
+ // cleanup
+ bundle.uninstall();
+ bundle = null;
+
+ // remove the configuration for good
+ deleteConfig( pid );
}
- private Bundle installBundle( final String pid ) throws BundleException {
+ /*
+ @Test
+ public void test_() throws BundleException
+ {
+ final int count = 2;
+ for (int i=0; i < count; i++) {
+ final Bundle bundle = installBundle( "dummy", FailureActivator.class );
+ bundle.start();
+ delay();
+ bundle.uninstall();
+ delay();
+ }
+ }
+ */
+
+
+ private Bundle installBundle( final String pid ) throws BundleException
+ {
+ return installBundle( pid, TestActivator.class );
+ }
+
+
+ private Bundle installBundle( final String pid, final Class<?> activatorClass )
+ throws BundleException
+ {
final InputStream bundleStream = new MyTinyBundle()
.prepare(
withBnd()
.set( Constants.BUNDLE_SYMBOLICNAME, "simpleconfiguration" )
.set( Constants.BUNDLE_VERSION, "0.0.11" )
.set( Constants.IMPORT_PACKAGE, "org.apache.felix.cm.integration.helper" )
- .set( Constants.BUNDLE_ACTIVATOR, "org.apache.felix.cm.integration.helper.TestActivator" )
+ .set( Constants.BUNDLE_ACTIVATOR, activatorClass.getName() )
.set( TestActivator.HEADER_PID, pid )
)
.build( TinyBundles.asStream() );