On May 28, 2009, at 10:37 AM, Vincent Massol wrote:

Note that what I proposed doesn't work since it's not possible to register a component with 2 roles (for the same component instance) in plexus AFAIK. Thus you'd need 2 components: one for the listener and another one for the main business logic. Then the pb arises of how to pass information from one to another...

Still looking for a simple solution to that (I have one but it's too complex). If you have any idea, shoot.

I spoke too fast. I think this solution could work: @Component DefaultMacroManager implements MacroManager { private Map macros; public Macro getMacro(...) {...} public void registerMacro(...) {...} ... } @Component("macro") MacroManagerListener implements ObservationListener { @Requirement MacroManager mm; void onEvent(Event event) { mm.registerMacro(...); ... } The only "downside" is that it forces MacroManager (in our example) to add a public method registerMacro which was not needed before. Is that a problem? Is there a better solution? Thanks -Vincent

On May 26, 2009, at 10:19 PM, Vincent Massol wrote:

...

Hi,

There's a pb with the current implementation: When we get the Observation manager implementation and list the registered listeners we only get the components that have been initialized *before* we get the Observation manager component. Thus if we want to be sure all listeners will receive events we need to ensure they're loaded first. This is hard to do.

I'm proposing instead to modify the observation module:

* Modify EventListener to be a @ComponentRole * Add List<Event> EventListener.getEvents() (the events that the listener handles) * Inject List<EventListener> in DefaultObservationManager. Thus when the Observation Manager is looked up, all listener components are created and injected. * Keep add/removeListener in ObservationManager so that listeners can be manually added/removed (for ex if a new listener component is added dynamically in the system) but with following signatures: - addListener(EventListener); - removeListener(EventListener);

WDYT?

Thanks -Vincent

Here's my latest proposal: /** * Components wanting to receive Observation {@link Event events} must implement this interface. * * @version $Id: EventListener.java 10164 2008-06-09 12:44:53Z sdumitriu $ */ public interface EventListener { /** * @return the listener's name. It's a free form text identifying this listener instance in a unique manner. * This name is used for some operations in {@link ObservationManager}. */ String getName(); /** * @return the list of events this listener is configured to receive. This listener will be automatically * registered with this list of events against the {@link ObservationManager}. For each matching event * the {@link #onEvent(Event, Object, Object)} method will be called. */ List<Event> getEvents(); /** * The {@link org.xwiki.observation.ObservationManager} calls this methods when an event matches the events for * which this listener is registered (see {@link #getEvents()}. * * @param event the event triggered. Can be used to differentiate different events if your Object supports several * events for example. * @param source the event source i.e. the object for which the event was triggered. For example this would be the * document Object if the event is a document update event. * @param data some additional and optional data passed that can be acted on. */ void onEvent(Event event, Object source, Object data); } And: /** * The main orchestrator for event notification. To receive events create a component implementing the * {@link EventListener} interface. Your component will be automatically registered when this Observation * Manager component is loaded. To send events to all registered listeners, call one of the * {@link #notify} methods. * * @version $Id: ObservationManager.java 20537 2009-05-26 20:41:25Z vmassol $ */ @ComponentRole public interface ObservationManager { /** * Manually add a listener. Components implementing the {@link EventListener} interfaces are only loaded * when the Observation Manager component is created. Thus if you need to add a new listener while the * system is running you'll need to call this method. * * @param event the event to register the listener against; acts as a template that filters out only specific events * the listener is interested in * @param eventListener the listener to register */ void addListener(EventListener eventListener); /** * Remove a listener from the list of registered listeners. The removed listener will no longer receive events. * * @param listenerName the name of the listener to remove (must match {@link EventListener#getName()} */ void removeListener(String listenerName); /** * Adds an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be added * (must match {@link EventListener#getName()} * @param event the event to add to the matching listener */ void addEvent(String listenerName, Event event); /** * Removes an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be removed * (must match {@link EventListener#getName()} * @param event the event to remove to the matching listener */ void removeEvent(String listenerName, Event event); /** * Remove a listener from a specific event. * * @param event the event to remove the listener from. * @param eventListener the listener to remove. */ void removeListener(Event event, EventListener eventListener); /** * Call the registered listeners matching the passed Event. * The definition of <em>source</em> and <em>data</em> is purely up to the communicating classes. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @param data the additional data related to the event (or <code>null</code>) */ void notify(Event event, Object source, Object data); /** * Convenience front-end where the additional data parameter is <code>null</code>. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @see #notify(org.xwiki.observation.event.Event, Object, Object) */ void notify(Event event, Object source); } This allows adding events after the listener is registered thus allowing Sergiu's use case in the Localization module. WDYT? Thanks -Vincent On May 28, 2009, at 1:59 PM, Sergiu Dumitriu wrote:

Sergiu Dumitriu wrote:

...

Vincent Massol wrote:

...

Hi,

There's a pb with the current implementation: When we get the Observation manager implementation and list the registered listeners we only get the components that have been initialized *before* we get the Observation manager component. Thus if we want to be sure all listeners will receive events we need to ensure they're loaded first. This is hard to do.

I'm proposing instead to modify the observation module:

* Modify EventListener to be a @ComponentRole

+1

...

* Add List<Event> EventListener.getEvents() (the events that the listener handles)

Hm, I'm not sure about this. A document watcher will dynamically add and remove events to the list of monitored resources. We could have this method to be called for the initial registration, as otherwise it's hard to do the initialization.

+0

Also, this requires that the ComponentManager is Initializable, otherwise it won't actually register anything.

...

...

* Inject List<EventListener> in DefaultObservationManager. Thus when the Observation Manager is looked up, all listener components are created and injected.

+1

...

* Keep add/removeListener in ObservationManager so that listeners can be manually added/removed (for ex if a new listener component is added dynamically in the system) but with following signatures: - addListener(EventListener); - removeListener(EventListener);

-1. See above, a component with a dynamic list of events will need to register/unregister just one in a long list. So we do need the existing signatures. Maybe add these signatures, but also keep the existing ones.

On May 29, 2009, at 12:02 PM, Marius Dumitru Florea wrote:

Hi Vincent,

Vincent Massol wrote:

...

Here's my latest proposal:

/** * Components wanting to receive Observation {@link Event events} must implement this interface. * * @version $Id: EventListener.java 10164 2008-06-09 12:44:53Z sdumitriu $ */ public interface EventListener { /** * @return the listener's name. It's a free form text identifying this listener instance in a unique manner. * This name is used for some operations in {@link ObservationManager}. */ String getName();

/** * @return the list of events this listener is configured to receive. This listener will be automatically * registered with this list of events against the {@link ObservationManager}. For each matching event * the {@link #onEvent(Event, Object, Object)} method will be called. */ List<Event> getEvents();

/** * The {@link org.xwiki.observation.ObservationManager} calls this methods when an event matches the events for * which this listener is registered (see {@link #getEvents()}. * * @param event the event triggered. Can be used to differentiate different events if your Object supports several * events for example. * @param source the event source i.e. the object for which the event was triggered. For example this would be the * document Object if the event is a document update event. * @param data some additional and optional data passed that can be acted on. */ void onEvent(Event event, Object source, Object data); }

And:

/** * The main orchestrator for event notification. To receive events create a component implementing the * {@link EventListener} interface. Your component will be automatically registered when this Observation * Manager component is loaded. To send events to all registered listeners, call one of the * {@link #notify} methods. * * @version $Id: ObservationManager.java 20537 2009-05-26 20:41:25Z vmassol $ */ @ComponentRole public interface ObservationManager { /** * Manually add a listener. Components implementing the {@link EventListener} interfaces are only loaded * when the Observation Manager component is created. Thus if you need to add a new listener while the * system is running you'll need to call this method. * * @param event the event to register the listener against; acts as a template that filters out only specific events * the listener is interested in * @param eventListener the listener to register */ void addListener(EventListener eventListener);

/** * Remove a listener from the list of registered listeners. The removed listener will no longer receive events. * * @param listenerName the name of the listener to remove (must match {@link EventListener#getName()} */ void removeListener(String listenerName);

/** * Adds an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be added * (must match {@link EventListener#getName()} * @param event the event to add to the matching listener */ void addEvent(String listenerName, Event event);

/** * Removes an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be removed * (must match {@link EventListener#getName()} * @param event the event to remove to the matching listener */ void removeEvent(String listenerName, Event event);

/** * Remove a listener from a specific event. * * @param event the event to remove the listener from. * @param eventListener the listener to remove. */ void removeListener(Event event, EventListener eventListener);

I don't understand why sometimes you use the name of the listener (a String) to identify it and some times a reference (EventListener). Why does the ObservationManager need a listener name? I'm asking because in the event systems I know (like the DOM event system) there's no need for such a name.

The instance is used to register it and the name is used to modify an existing instance since it would be cumbersome for the calling code to have to retrieve an instance just to add an Event to it. Note that this is more complex than it looks and it also prevents having cyclic dependencies between Listener components and main components (like MacroManager and MacroManagerListener for ex) since the listener will need to call the main component when an event happens and thus if the main component need to add an Event it would need to get a listener instance. -Vincent

...

/** * Call the registered listeners matching the passed Event. * The definition of <em>source</em> and <em>data</em> is purely up to the communicating classes. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @param data the additional data related to the event (or <code>null</code>) */ void notify(Event event, Object source, Object data);

/** * Convenience front-end where the additional data parameter is <code>null</code>. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @see #notify(org.xwiki.observation.event.Event, Object, Object) */ void notify(Event event, Object source); }

This allows adding events after the listener is registered thus allowing Sergiu's use case in the Localization module.

WDYT?

Thanks -Vincent

On May 28, 2009, at 1:59 PM, Sergiu Dumitriu wrote:

...

Sergiu Dumitriu wrote:

...

Vincent Massol wrote:

...

Hi,

There's a pb with the current implementation: When we get the Observation manager implementation and list the registered listeners we only get the components that have been initialized *before* we get the Observation manager component. Thus if we want to be sure all listeners will receive events we need to ensure they're loaded first. This is hard to do.

I'm proposing instead to modify the observation module:

* Modify EventListener to be a @ComponentRole +1

...

* Add List<Event> EventListener.getEvents() (the events that the listener handles) Hm, I'm not sure about this. A document watcher will dynamically add and remove events to the list of monitored resources. We could have this method to be called for the initial registration, as otherwise it's hard to do the initialization.

+0 Also, this requires that the ComponentManager is Initializable, otherwise it won't actually register anything.

...

...

* Inject List<EventListener> in DefaultObservationManager. Thus when the Observation Manager is looked up, all listener components are created and injected. +1

...

* Keep add/removeListener in ObservationManager so that listeners can be manually added/removed (for ex if a new listener component is added dynamically in the system) but with following signatures: - addListener(EventListener); - removeListener(EventListener); -1. See above, a component with a dynamic list of events will need to register/unregister just one in a long list. So we do need the existing signatures. Maybe add these signatures, but also keep the existing ones.

The following method should be removed from ObservationManager from my proposal (I was a leftover from the previous and current api): /** * Remove a listener from a specific event. * * @param event the event to remove the listener from. * @param eventListener the listener to remove. */ void removeListener(Event event, EventListener eventListener); Thanks -Vincent On May 29, 2009, at 12:23 PM, Vincent Massol wrote:

I also wanted to add a ObservationManager.getListener(String listenerName) API.

-Vincent

On May 29, 2009, at 12:13 PM, Vincent Massol wrote:

...

On May 29, 2009, at 12:02 PM, Marius Dumitru Florea wrote:

...

Hi Vincent,

Vincent Massol wrote:

...

Here's my latest proposal:

/** * Components wanting to receive Observation {@link Event events} must implement this interface. * * @version $Id: EventListener.java 10164 2008-06-09 12:44:53Z sdumitriu $ */ public interface EventListener { /** * @return the listener's name. It's a free form text identifying this listener instance in a unique manner. * This name is used for some operations in {@link ObservationManager}. */ String getName();

/** * @return the list of events this listener is configured to receive. This listener will be automatically * registered with this list of events against the {@link ObservationManager}. For each matching event * the {@link #onEvent(Event, Object, Object)} method will be called. */ List<Event> getEvents();

/** * The {@link org.xwiki.observation.ObservationManager} calls this methods when an event matches the events for * which this listener is registered (see {@link #getEvents()}. * * @param event the event triggered. Can be used to differentiate different events if your Object supports several * events for example. * @param source the event source i.e. the object for which the event was triggered. For example this would be the * document Object if the event is a document update event. * @param data some additional and optional data passed that can be acted on. */ void onEvent(Event event, Object source, Object data); }

And:

/** * The main orchestrator for event notification. To receive events create a component implementing the * {@link EventListener} interface. Your component will be automatically registered when this Observation * Manager component is loaded. To send events to all registered listeners, call one of the * {@link #notify} methods. * * @version $Id: ObservationManager.java 20537 2009-05-26 20:41:25Z vmassol $ */ @ComponentRole public interface ObservationManager { /** * Manually add a listener. Components implementing the {@link EventListener} interfaces are only loaded * when the Observation Manager component is created. Thus if you need to add a new listener while the * system is running you'll need to call this method. * * @param event the event to register the listener against; acts as a template that filters out only specific events * the listener is interested in * @param eventListener the listener to register */ void addListener(EventListener eventListener);

/** * Remove a listener from the list of registered listeners. The removed listener will no longer receive events. * * @param listenerName the name of the listener to remove (must match {@link EventListener#getName()} */ void removeListener(String listenerName);

/** * Adds an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be added * (must match {@link EventListener#getName()} * @param event the event to add to the matching listener */ void addEvent(String listenerName, Event event);

/** * Removes an Event to an already registered listener. * * @param listenerName the name of the listener to which the event must be removed * (must match {@link EventListener#getName()} * @param event the event to remove to the matching listener */ void removeEvent(String listenerName, Event event);

/** * Remove a listener from a specific event. * * @param event the event to remove the listener from. * @param eventListener the listener to remove. */ void removeListener(Event event, EventListener eventListener);

I don't understand why sometimes you use the name of the listener (a String) to identify it and some times a reference (EventListener). Why does the ObservationManager need a listener name? I'm asking because in the event systems I know (like the DOM event system) there's no need for such a name.

The instance is used to register it and the name is used to modify an existing instance since it would be cumbersome for the calling code to have to retrieve an instance just to add an Event to it.

Note that this is more complex than it looks and it also prevents having cyclic dependencies between Listener components and main components (like MacroManager and MacroManagerListener for ex) since the listener will need to call the main component when an event happens and thus if the main component need to add an Event it would need to get a listener instance.

-Vincent

...

...

/** * Call the registered listeners matching the passed Event. * The definition of <em>source</em> and <em>data</em> is purely up to the communicating classes. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @param data the additional data related to the event (or <code>null</code>) */ void notify(Event event, Object source, Object data);

/** * Convenience front-end where the additional data parameter is <code>null</code>. * * @param event the event to pass to the registered listeners * @param source the source of the event (or <code>null</code>) * @see #notify(org.xwiki.observation.event.Event, Object, Object) */ void notify(Event event, Object source); }

This allows adding events after the listener is registered thus allowing Sergiu's use case in the Localization module.

WDYT?

Thanks -Vincent

On May 28, 2009, at 1:59 PM, Sergiu Dumitriu wrote:

...

Sergiu Dumitriu wrote:

...

Vincent Massol wrote: > Hi, > > There's a pb with the current implementation: > When we get the Observation manager implementation and list the > registered listeners we only get the components that have been > initialized *before* we get the Observation manager component. > Thus if > we want to be sure all listeners will receive events we need to > ensure they're loaded first. This is hard to do. > > I'm proposing instead to modify the observation module: > > * Modify EventListener to be a @ComponentRole +1

> * Add List<Event> EventListener.getEvents() (the events that the > listener handles) Hm, I'm not sure about this. A document watcher will dynamically add and remove events to the list of monitored resources. We could have this method to be called for the initial registration, as otherwise it's hard to do the initialization.

+0 Also, this requires that the ComponentManager is Initializable, otherwise it won't actually register anything.

...

> * Inject List<EventListener> in DefaultObservationManager. > Thus when > the Observation Manager is looked up, all listener components > are > created and injected. +1

> * Keep add/removeListener in ObservationManager so that > listeners > can > be manually added/removed (for ex if a new listener component is > added dynamically in the system) but with following signatures: > - addListener(EventListener); > - removeListener(EventListener); -1. See above, a component with a dynamic list of events will need to register/unregister just one in a long list. So we do need the existing signatures. Maybe add these signatures, but also keep the existing ones.