glimmerveen commented on a change in pull request #19:
URL: https://github.com/apache/felix-dev/pull/19#discussion_r412434960
##########
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:
You are right, this is not correct. I think it is missing the removal of
the ExtendedServiceListener from the ListenerInfo first, and only removing it
from the map if the ListenerInfo is empty.
----------------------------------------------------------------
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]
