glimmerveen commented on a change in pull request #19:
URL: https://github.com/apache/felix-dev/pull/19#discussion_r412444967
##########
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:
Perhaps it is related to current approach of effectively aggregating
multiple references into a single ServiceListener. As this PR aims to remove
this aggregation, perhaps this is something that can be further simplified?
----------------------------------------------------------------
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]
