FEKLIX-2288 Fix handling of the component ID: The ID is now only generated and assigned to a component while the component is enabled. To still access components without an assigned ID, ScrService gets a new method getComponent(String name) to access components by their name.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@981772 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/pom.xml b/scr/pom.xml
index 2df9bee..41cf1c0 100644
--- a/scr/pom.xml
+++ b/scr/pom.xml
@@ -158,7 +158,7 @@
org.apache.felix.scr.impl.Activator
</Bundle-Activator>
<Export-Package>
- org.apache.felix.scr;version=1.4,
+ org.apache.felix.scr;version=1.5,
org.osgi.service.component
</Export-Package>
<Private-Package>
diff --git a/scr/src/main/java/org/apache/felix/scr/Component.java b/scr/src/main/java/org/apache/felix/scr/Component.java
index 894d9eb..302b89f 100644
--- a/scr/src/main/java/org/apache/felix/scr/Component.java
+++ b/scr/src/main/java/org/apache/felix/scr/Component.java
@@ -84,9 +84,11 @@
static final int STATE_REGISTERED = 32;
/**
- * The Component is a Component Factory ready to manage Component instances
- * from configuration data received from the Configuration Admin Service
- * (value is 64).
+ * The Component is a Component Factory ready to create Component instances
+ * with the <code>ComponentFactory.newInstance(Dictionary)</code> method
+ * or (if enabled with the <code>ds.factory.enabled</code> configuration) to
+ * manage Component instances from configuration data received from the
+ * Configuration Admin Service (value is 64).
*/
static final int STATE_FACTORY = 64;
@@ -133,7 +135,9 @@
/**
* Returns the component ID of this component. This ID is managed by the
- * SCR.
+ * SCR. If the component is not currently enabled the ID might not be
+ * assigned to the component (yet) and this method will return -1 in this
+ * case.
*/
long getId();
diff --git a/scr/src/main/java/org/apache/felix/scr/ScrService.java b/scr/src/main/java/org/apache/felix/scr/ScrService.java
index 1605e55..5897a22 100644
--- a/scr/src/main/java/org/apache/felix/scr/ScrService.java
+++ b/scr/src/main/java/org/apache/felix/scr/ScrService.java
@@ -55,6 +55,24 @@
/**
+ * Returns the components whose <code>component.name</code> matches the
+ * given <code>componentName</code> or <code>null</code> if no component
+ * with the given name is currently managed.
+ * <p>
+ * If the component name refers to a component factory component or a
+ * component configured with multiple factory configurations this method
+ * returns multiple component instances.
+ *
+ * @param componentName The name of the component to return
+ *
+ * @return The indicated components or <code>null</code> if no such
+ * component exists.
+ * @since 1.5 (Apache Felix Declarative Services 1.4.2)
+ */
+ Component[] getComponents( String componentName );
+
+
+ /**
* Reuturns an array of all components managed by this SCR instance on
* behalf of the given bundle. The components are returned in ascending
* order of their component.id. If there are no components managed by the
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java b/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
index 731a6f8..685f7c6 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
@@ -23,7 +23,6 @@
import java.util.Dictionary;
import java.util.HashMap;
import java.util.Hashtable;
-import java.util.List;
import java.util.Map;
import org.apache.felix.scr.Component;
@@ -119,47 +118,58 @@
public Component[] getComponents()
{
- synchronized ( m_componentsById )
+ Object[] holders = getComponentHolders();
+ ArrayList list = new ArrayList();
+ for ( int i = 0; i < holders.length; i++ )
{
- if ( m_componentsById.isEmpty() )
+ if ( holders[i] instanceof ComponentHolder )
{
- return null;
+ Component[] components = ( ( ComponentHolder ) holders[i] ).getComponents();
+ for ( int j = 0; j < components.length; j++ )
+ {
+ list.add( components[j] );
+ }
}
-
- return ( org.apache.felix.scr.Component[] ) m_componentsById.values().toArray(
- new Component[m_componentsById.size()] );
}
+
+ // nothing to return
+ if ( list.isEmpty() )
+ {
+ return null;
+ }
+
+ return ( Component[] ) list.toArray( new Component[list.size()] );
}
public Component[] getComponents( Bundle bundle )
{
- Component[] all = getComponents();
- if ( all == null || all.length == 0 )
+ Object[] holders = getComponentHolders();
+ ArrayList list = new ArrayList();
+ for ( int i = 0; i < holders.length; i++ )
{
- return null;
- }
-
- // compare the bundle by its id
- long bundleId = bundle.getBundleId();
-
- // scan the components for the the desired components
- List perBundle = new ArrayList();
- for ( int i = 0; i < all.length; i++ )
- {
- if ( all[i].getBundle().getBundleId() == bundleId )
+ if ( holders[i] instanceof ComponentHolder )
{
- perBundle.add( all[i] );
+ ComponentHolder holder = ( ComponentHolder ) holders[i];
+ BundleComponentActivator activator = holder.getActivator();
+ if ( activator != null && activator.getBundleContext().getBundle() == bundle )
+ {
+ Component[] components = holder.getComponents();
+ for ( int j = 0; j < components.length; j++ )
+ {
+ list.add( components[j] );
+ }
+ }
}
}
// nothing to return
- if ( perBundle.isEmpty() )
+ if ( list.isEmpty() )
{
return null;
}
- return ( org.apache.felix.scr.Component[] ) perBundle.toArray( new Component[perBundle.size()] );
+ return ( Component[] ) list.toArray( new Component[list.size()] );
}
@@ -172,6 +182,18 @@
}
+ public Component[] getComponents( String componentName )
+ {
+ final ComponentHolder holder = getComponentHolder( componentName );
+ if ( holder != null )
+ {
+ return holder.getComponents();
+ }
+
+ return null;
+ }
+
+
//---------- ComponentManager registration by component Id
/**
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
index c0ac7ca..859f610 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
@@ -21,6 +21,7 @@
import java.util.Dictionary;
+import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -69,6 +70,10 @@
*/
void configurationUpdated( String pid, Dictionary props );
+ /**
+ * Returns all <code>Component</code> instances held by this holder.
+ */
+ Component[] getComponents();
/**
* Enables all components of this holder.
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolder.java
index a4e0bf7..2c44470 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolder.java
@@ -24,6 +24,7 @@
import java.util.Iterator;
import java.util.Map;
+import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -239,6 +240,14 @@
}
+ public Component[] getComponents()
+ {
+ Component[] components = getComponentManagers( false );
+ return ( components != null ) ? components : new Component[]
+ { m_singleComponent };
+ }
+
+
public void enableComponents()
{
final ImmediateComponentManager[] cms = getComponentManagers( false );
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/UnconfiguredComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/UnconfiguredComponentHolder.java
index 36b086d..0b53cd8 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/UnconfiguredComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/UnconfiguredComponentHolder.java
@@ -21,6 +21,7 @@
import java.util.Dictionary;
+import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -55,6 +56,13 @@
}
+ public Component[] getComponents()
+ {
+ return new Component[]
+ { m_component };
+ }
+
+
public void enableComponents()
{
m_component.enable();
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
index 588d035..4c6e2d8 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
@@ -80,9 +80,7 @@
{
m_activator = activator;
m_componentMetadata = metadata;
-
- // for some testing, the activator may be null
- m_componentId = ( activator != null ) ? activator.registerComponentId( this ) : -1;
+ m_componentId = -1;
m_state = Disabled.getInstance();
m_dependencyManagers = loadDependencyManagers( metadata );
@@ -116,6 +114,32 @@
}
+ //---------- Component ID management
+
+ void registerComponentId()
+ {
+ final BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ this.m_componentId = activator.registerComponentId( this );
+ }
+ }
+
+
+ void unregisterComponentId()
+ {
+ if ( this.m_componentId >= 0 )
+ {
+ final BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ activator.unregisterComponentId( this );
+ }
+ this.m_componentId = -1;
+ }
+ }
+
+
//---------- Asynchronous frontend to state change methods ----------------
/**
* Enables this component and - if satisfied - also activates it. If
@@ -869,7 +893,7 @@
void enable( AbstractComponentManager acm )
{
acm.changeState( Enabling.getInstance() );
-
+ acm.registerComponentId();
try
{
acm.enableDependencyManagers();
@@ -881,6 +905,7 @@
// one of the reference target filters is invalid, fail
acm.log( LogService.LOG_ERROR, "Failed enabling Component", ise );
acm.disableDependencyManagers();
+ acm.unregisterComponentId();
acm.changeState( Disabled.getInstance() );
}
}
@@ -994,6 +1019,9 @@
// dispose and recreate dependency managers
acm.disableDependencyManagers();
+ // reset the component id now (a disabled component has none)
+ acm.unregisterComponentId();
+
// we are now disabled, ready for re-enablement or complete destroyal
acm.changeState( Disabled.getInstance() );
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
index 8c51285..c9b17bc 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
@@ -24,6 +24,7 @@
import java.util.IdentityHashMap;
import java.util.Map;
+import org.apache.felix.scr.Component;
import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.config.ComponentHolder;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
@@ -320,6 +321,26 @@
}
+ public Component[] getComponents()
+ {
+ ImmediateComponentManager[] instances = getComponentManagers( m_componentInstances );
+ ImmediateComponentManager[] services = getComponentManagers( m_configuredServices );
+ int size = instances.length + services.length;
+
+ if ( size > 0 )
+ {
+ Component[] result = new Component[size + 1];
+ result[0] = this;
+ System.arraycopy( instances, 0, result, 1, instances.length );
+ System.arraycopy( services, 0, result, instances.length + 1, services.length );
+ return result;
+ }
+
+ return new Component[]
+ { this };
+ }
+
+
/**
* A component factory component holder enables the held components by
* enabling itself.
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
index 68ec203..d7ea67b 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
@@ -300,6 +300,20 @@
}
+ void registerComponentId()
+ {
+ super.registerComponentId();
+ this.m_properties = null;
+ }
+
+
+ void unregisterComponentId()
+ {
+ super.unregisterComponentId();
+ this.m_properties = null;
+ }
+
+
/**
* Returns the (private copy) of the Component properties to be used
* for the ComponentContext as well as eventual service registration.
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentConfigurationTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentConfigurationTest.java
index be076ff..9da95d4 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentConfigurationTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentConfigurationTest.java
@@ -261,14 +261,36 @@
final Component[] twoConfigs = findComponentsByName( factoryPid );
TestCase.assertNotNull( twoConfigs );
TestCase.assertEquals( 2, twoConfigs.length );
- TestCase.assertEquals( Component.STATE_ACTIVE, twoConfigs[0].getState() );
- TestCase.assertEquals( Component.STATE_DISABLED, twoConfigs[1].getState() );
+
+ // find the active and inactive configs, fail if none
+ int activeConfig;
+ int inactiveConfig;
+ if ( twoConfigs[0].getState() == Component.STATE_ACTIVE )
+ {
+ // [0] is active, [1] expected disabled
+ activeConfig = 0;
+ inactiveConfig = 1;
+ }
+ else if ( twoConfigs[1].getState() == Component.STATE_ACTIVE )
+ {
+ // [1] is active, [0] expected disabled
+ activeConfig = 1;
+ inactiveConfig = 0;
+ }
+ else
+ {
+ TestCase.fail( "One of two components expected active" );
+ return; // eases the compiler...
+ }
+
+ TestCase.assertEquals( Component.STATE_ACTIVE, twoConfigs[activeConfig].getState() );
+ TestCase.assertEquals( Component.STATE_DISABLED, twoConfigs[inactiveConfig].getState() );
TestCase.assertEquals( 1, SimpleComponent.INSTANCES.size() );
- TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[0].getId() ) );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[1].getId() ) );
+ TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[activeConfig].getId() ) );
+ TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[inactiveConfig].getId() ) );
// enable second component
- twoConfigs[1].enable();
+ twoConfigs[inactiveConfig].enable();
delay();
// ensure both components active
@@ -288,8 +310,7 @@
TestCase.assertEquals( 1, oneConfig.length );
TestCase.assertEquals( Component.STATE_ACTIVE, oneConfig[0].getState() );
TestCase.assertEquals( 1, SimpleComponent.INSTANCES.size() );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[0].getId() ) );
- TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[1].getId() ) );
+ TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( oneConfig[0].getId() ) );
// delete second configuration
deleteConfig( pid1 );
@@ -301,8 +322,6 @@
TestCase.assertEquals( 1, configsDeleted.length );
TestCase.assertEquals( Component.STATE_UNSATISFIED, configsDeleted[0].getState() );
TestCase.assertEquals( 0, SimpleComponent.INSTANCES.size() );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[0].getId() ) );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[1].getId() ) );
}
@Test
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentDisposeTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentDisposeTest.java
index 14ca69d..ae0adf5 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentDisposeTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentDisposeTest.java
@@ -76,14 +76,36 @@
final Component[] twoConfigs = findComponentsByName( factoryPid );
TestCase.assertNotNull( twoConfigs );
TestCase.assertEquals( 2, twoConfigs.length );
- TestCase.assertEquals( Component.STATE_ACTIVE, twoConfigs[0].getState() );
- TestCase.assertEquals( Component.STATE_DISABLED, twoConfigs[1].getState() );
+
+ // find the active and inactive configs, fail if none
+ int activeConfig;
+ int inactiveConfig;
+ if ( twoConfigs[0].getState() == Component.STATE_ACTIVE )
+ {
+ // [0] is active, [1] expected disabled
+ activeConfig = 0;
+ inactiveConfig = 1;
+ }
+ else if ( twoConfigs[1].getState() == Component.STATE_ACTIVE )
+ {
+ // [1] is active, [0] expected disabled
+ activeConfig = 1;
+ inactiveConfig = 0;
+ }
+ else
+ {
+ TestCase.fail( "One of two components expected active" );
+ return; // eases the compiler...
+ }
+
+ TestCase.assertEquals( Component.STATE_ACTIVE, twoConfigs[activeConfig].getState() );
+ TestCase.assertEquals( Component.STATE_DISABLED, twoConfigs[inactiveConfig].getState() );
TestCase.assertEquals( 1, SimpleComponent.INSTANCES.size() );
- TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[0].getId() ) );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[1].getId() ) );
+ TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[activeConfig].getId() ) );
+ TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( twoConfigs[inactiveConfig].getId() ) );
// enable second component
- twoConfigs[1].enable();
+ twoConfigs[inactiveConfig].enable();
delay();
// ensure both components active
@@ -106,8 +128,7 @@
TestCase.assertEquals( 1, oneConfig.length );
TestCase.assertEquals( Component.STATE_ACTIVE, oneConfig[0].getState() );
TestCase.assertEquals( 1, SimpleComponent.INSTANCES.size() );
- TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( twoConfigs[0].getId() ) );
- TestCase.assertFalse( SimpleComponent.INSTANCES.containsKey( anInstance.m_id ) );
+ TestCase.assertTrue( SimpleComponent.INSTANCES.containsKey( oneConfig[0].getId() ) );
final SimpleComponent instance = SimpleComponent.INSTANCES.values().iterator().next();
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
index 4126d23..1f59883 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
@@ -29,10 +29,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Field;
-import java.util.ArrayList;
import java.util.Dictionary;
import java.util.Hashtable;
-import java.util.List;
import junit.framework.TestCase;
import org.apache.felix.scr.Component;
@@ -162,16 +160,10 @@
protected Component findComponentByName( String name )
{
- Component[] components = getComponents();
- if ( components != null )
+ Component[] components = findComponentsByName( name );
+ if ( components != null && components.length > 0 )
{
- for ( Component component : components )
- {
- if ( name.equals( component.getName() ) )
- {
- return component;
- }
- }
+ return components[0];
}
return null;
@@ -180,22 +172,10 @@
protected Component[] findComponentsByName( String name )
{
- List<Component> cList = new ArrayList<Component>();
- Component[] components = getComponents();
- if ( components != null )
+ ScrService scr = ( ScrService ) scrTracker.getService();
+ if ( scr != null )
{
- for ( Component component : components )
- {
- if ( name.equals( component.getName() ) )
- {
- cList.add( component );
- }
- }
- }
-
- if ( !cList.isEmpty() )
- {
- return cList.toArray( new Component[cList.size()] );
+ return scr.getComponents( name );
}
return null;