glimmerveen commented on a change in pull request #19:
URL: https://github.com/apache/felix-dev/pull/19#discussion_r412716542
##########
File path:
scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
##########
@@ -98,133 +98,82 @@
private static class ListenerInfo implements ServiceListener
{
- private Map<Filter,
List<ExtendedServiceListener<ExtendedServiceEvent>>> filterMap = new
HashMap<>();
+ List<ExtendedServiceListener<ExtendedServiceEvent>> listeners = new
ArrayList<>();
@Override
public void serviceChanged(ServiceEvent event)
{
ServiceReference<?> ref = event.getServiceReference();
- ExtendedServiceEvent extEvent = null;
- ExtendedServiceEvent endMatchEvent = null;
- Map<Filter, List<ExtendedServiceListener<ExtendedServiceEvent>>>
filterMap;
+ ExtendedServiceEvent extEvent = new ExtendedServiceEvent(event);
+ List<ExtendedServiceListener<ExtendedServiceEvent>> listeners;
synchronized ( this )
{
- filterMap = this.filterMap;
+ listeners = this.listeners;
}
- for ( Map.Entry<Filter,
List<ExtendedServiceListener<ExtendedServiceEvent>>> entry :
filterMap.entrySet() )
+
+ for ( ExtendedServiceListener<ExtendedServiceEvent> forwardTo :
listeners)
{
- Filter filter = entry.getKey();
- if ( filter == null || filter.match( ref ) )
- {
- if ( extEvent == null )
- {
- extEvent = new ExtendedServiceEvent( event );
- }
- for ( ExtendedServiceListener<ExtendedServiceEvent>
forwardTo : entry.getValue() )
- {
- forwardTo.serviceChanged( extEvent );
- }
- }
- else if ( event.getType() == ServiceEvent.MODIFIED )
- {
- if ( endMatchEvent == null )
- {
- endMatchEvent = new ExtendedServiceEvent(
ServiceEvent.MODIFIED_ENDMATCH, ref );
- }
- for ( ExtendedServiceListener<ExtendedServiceEvent>
forwardTo : entry.getValue() )
- {
- forwardTo.serviceChanged( endMatchEvent );
- }
- }
+ forwardTo.serviceChanged( extEvent );
}
+
if ( extEvent != null )
{
extEvent.activateManagers();
}
- if ( endMatchEvent != null )
- {
- endMatchEvent.activateManagers();
- }
}
- public synchronized void add(Filter filter,
ExtendedServiceListener<ExtendedServiceEvent> listener)
+ public synchronized void
add(ExtendedServiceListener<ExtendedServiceEvent> listener)
{
- filterMap = new HashMap<>( filterMap );
- List<ExtendedServiceListener<ExtendedServiceEvent>> listeners =
filterMap.get( filter );
- if ( listeners == null )
- {
- listeners =
Collections.<ExtendedServiceListener<ExtendedServiceEvent>> singletonList(
listener );
- }
- else
- {
- listeners = new ArrayList<>( listeners );
- listeners.add( listener );
- }
- filterMap.put( filter, listeners );
+ listeners = new ArrayList<>(listeners);
+ listeners.add(listener);
}
- public synchronized boolean remove(Filter filter,
ExtendedServiceListener<ExtendedServiceEvent> listener)
+ public synchronized boolean
remove(ExtendedServiceListener<ExtendedServiceEvent> listener)
{
- List<ExtendedServiceListener<ExtendedServiceEvent>> listeners =
filterMap.get( filter );
- if ( listeners != null )
- {
- filterMap = new HashMap<>( filterMap );
- listeners = new ArrayList<>( listeners );
- listeners.remove( listener );
- if ( listeners.isEmpty() )
- {
- filterMap.remove( filter );
- }
- else
- {
- filterMap.put( filter, listeners );
- }
- }
- return filterMap.isEmpty();
+ listeners = new ArrayList<>(listeners);
+ listeners.remove(listener);
+ return listeners.isEmpty();
}
}
@Override
- public void addServiceListener(String classNameFilter, Filter eventFilter,
+ public void addServiceListener(String serviceFilterString,
ExtendedServiceListener<ExtendedServiceEvent> listener)
{
ListenerInfo listenerInfo;
synchronized ( listenerMap )
{
- logger.log( LogService.LOG_DEBUG, "classNameFilter: " +
classNameFilter + " event filter: " + eventFilter, null);
- listenerInfo = listenerMap.get( classNameFilter );
+ logger.log( LogService.LOG_DEBUG, "serviceFilterString: " +
serviceFilterString, null);
+ listenerInfo = listenerMap.get( serviceFilterString );
if ( listenerInfo == null )
{
listenerInfo = new ListenerInfo();
- listenerMap.put( classNameFilter, listenerInfo );
+ listenerMap.put( serviceFilterString, listenerInfo );
try
{
- m_context.addServiceListener( listenerInfo,
classNameFilter );
+ m_context.addServiceListener( listenerInfo,
serviceFilterString );
}
catch ( InvalidSyntaxException e )
{
throw (IllegalArgumentException) new
IllegalArgumentException(
"invalid class name filter" ).initCause( e );
}
}
+ listenerInfo.add(listener);
}
- listenerInfo.add( eventFilter, listener );
}
@Override
- public void removeServiceListener(String className, Filter filter,
+ public void removeServiceListener(String serviceFilterString,
ExtendedServiceListener<ExtendedServiceEvent> listener)
{
synchronized ( listenerMap )
{
- ListenerInfo listenerInfo = listenerMap.get( className );
+ ListenerInfo listenerInfo = listenerMap.get( serviceFilterString );
if ( listenerInfo != null )
{
- if ( listenerInfo.remove( filter, listener ) )
- {
- listenerMap.remove( className );
- m_context.removeServiceListener( listenerInfo );
- }
+ listenerMap.remove( serviceFilterString );
Review comment:
@djencks I believe my current changes reflect the behaviour you
suggested, and I have added a test to verify this.
The important change also here that supports my use-case is the fact that
the target filter is now also visible to the ListenerHook's, allowing such an
implementation to register a service that will match the target filter.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
