[xwiki-devs] Activity Stream and Events refactoring
Hi devs ! I thought about adding some changes in Events and Activity Stream. First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it). Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on). And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment. WDYT ? Stefan
On 11/10/2010 09:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
+1. Note that this will also require some changes to the ActivityStreamImpl#onEvent method, since it always assumes that the "source" parameter holds a XWikiDocument, and the "data" parameter holds a XWikiContent, which is not true for many types of events. Since the implementation needs the context to work properly, for the moment the fastest solution is to discard all events that don't come with a context.
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
+1.
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
+0.5. Note that the identifier should be a real identifier, so I'd say that the best choice is to use an EntityReference where possible. ObjectReference for comments and annotations, AttachmentReference for attachments, DocumentReference for documents. -- Sergiu Dumitriu http://purl.org/net/sergiu/
Hi Stefan, On Nov 10, 2010, at 9:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
How do you filter events to process? If you don't how would you display for example view events or internal events such as component manager descriptor added/removed event, or the new event being added by Anca for the XAR import?
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
I don't agree with this, or I don't understand it. You get the Event object so you can easily get the simple class name by calling event.getClass().getSimpleClassName().
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class.
Again I don't agree. A notion of event identifier is not linked to a Filterable event IMO but to the Event class itself.
And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
I don't quite agree. If it's supposed to be an identifier it must be unique and leaving the unicity to the user will not guarantee it's unique. I'm not sure what you want to do: 1) Be able to differentiate 2 events. For what reason? 2) Be able to have some description of the events. Again for what reason? Now if you need extra data, this is already taken into account in the onEvent method: void onEvent(Event event, Object source, Object data); The data parameter is there to pass extra data. The way to use it is to create compound objects if you need to pass several data. However for easiness of use, I'd agree to transform the signature into: void onEvent(Event event, Object source, Object... data); All that said, we have a small issue in that the listener of events need to know what type of data get passed and thus emitter and listeners must be in sync. Thus a generic listener will always have a problem since it won't know the type of objects it receives (since they're different for each event). Thus for me, when you have a generic listener you also need an EventActionHandler that needs to be registered against the generic listener so that it know what to do with the extra data passed. How would you handle anonymous events in the Activity Stream (AS) listener? BTW what actions do you need to do with received events in the AS? Thanks -Vincent
On 11/10/2010 10:57 AM, Vincent Massol wrote: > Hi Stefan, > > On Nov 10, 2010, at 9:08 AM, Stefan abageru wrote: > >> Hi devs ! >> >> I thought about adding some changes in Events and Activity Stream. >> >> First of all, I want to remove all the listeners from Activity Stream >> and make this class listen to AllEvent, making it capable of cathcing >> all events that occur in the wiki (and this way, when adding a new >> event, you wouldn't need any modifications to this class in order to >> catch it). > How do you filter events to process? If you don't how would you display for example view events or internal events such as component manager descriptor added/removed event, or the new event being added by Anca for the XAR import? > There would ne no filtering since IMO ActivityStream records the activities happening in the wiki. Should there be some filtering ? >> Secondly, I would like to modify onEvent function in the class so that >> it uses the simpleClassName of the event occurred in order to log the >> event (the eventType would be this simpleClassName...for example >> DocumentSavedEvent, AnnotationAddedEvent and so on). > I don't agree with this, or I don't understand it. You get the Event object so you can easily get the simple class name by calling event.getClass().getSimpleClassName(). > Yes..right now the implementation does a test of instanceOf on the event received (there is an awful if then else) and selects the type of event from the class ActivityEventType static fields. By this modification we can remove that if then else and use classname instead of the strings from ActivityEventType. >> And thirdly i thought about adding one field in AbstractFilterableEvent >> called /identifier/ and make all events extend directly this class. > Again I don't agree. A notion of event identifier is not linked to a Filterable event IMO but to the Event class itself. > >> And >> that /identifier/ would be used in a different way by the events >> themselves. But it would be used to retrieve infos about that events. >> And so, when logging an event, we could use classname + identifier. >> For example >> AnnotationAddedEvent + "This is my annotation" . >> CommentAddedEvent + "This is my comment" or the number of the comment. > I don't quite agree. If it's supposed to be an identifier it must be unique and leaving the unicity to the user will not guarantee it's unique. I'm not sure what you want to do: > 1) Be able to differentiate 2 events. For what reason? > 2) Be able to have some description of the events. Again for what reason? > The identifier field..I see it as a way so identify and specify more data about the event itself. For example, i think it is important do differentiate 2 CommentAddedEvent and not jusrt specify that they occurred in a list of events and i would do this by this field. The advantage of this field would be that it would be known in a generic way and access to it would be easily done. There would be no need of the sync you said between the listener and the emmiter. I aslways know i will receive a class extending AbstractFilterableEvent, I can get the identifier field. Maybe move it to Event itself ? > Now if you need extra data, this is already taken into account in the onEvent method: > void onEvent(Event event, Object source, Object data); > > The data parameter is there to pass extra data. > The way to use it is to create compound objects if you need to pass several data. > > However for easiness of use, I'd agree to transform the signature into: > void onEvent(Event event, Object source, Object... data); > > All that said, we have a small issue in that the listener of events need to know what type of data get passed and thus emitter and listeners must be in sync. Thus a generic listener will always have a problem since it won't know the type of objects it receives (since they're different for each event). Thus for me, when you have a generic listener you also need an EventActionHandler that needs to be registered against the generic listener so that it know what to do with the extra data passed. > > How would you handle anonymous events in the Activity Stream (AS) listener? BTW what actions do you need to do with received events in the AS? About anonymous events. I do not know exactly what you mean by that. In AS i need to record them by doing a call to addDocumentActivityEvent(streamName, currentDoc, eventType, msgPrefix + eventType, params, context); eventType would the the simple class name and the identifier would go in the params list. Thanks, Stefan > Thanks > -Vincent > > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs
On Nov 10, 2010, at 10:12 AM, Stefan abageru wrote: > On 11/10/2010 10:57 AM, Vincent Massol wrote: >> Hi Stefan, >> >> On Nov 10, 2010, at 9:08 AM, Stefan abageru wrote: >> >>> Hi devs ! >>> >>> I thought about adding some changes in Events and Activity Stream. >>> >>> First of all, I want to remove all the listeners from Activity Stream >>> and make this class listen to AllEvent, making it capable of cathcing >>> all events that occur in the wiki (and this way, when adding a new >>> event, you wouldn't need any modifications to this class in order to >>> catch it). >> How do you filter events to process? If you don't how would you display for example view events or internal events such as component manager descriptor added/removed event, or the new event being added by Anca for the XAR import? >> > There would ne no filtering since IMO ActivityStream records the > activities happening in the wiki. Should there be some filtering ? There are some events that are internal events and I don't think they should be displayed. + I don't know how the display will scale for view Events for ex (which can be received about 1000 times per second). >>> Secondly, I would like to modify onEvent function in the class so that >>> it uses the simpleClassName of the event occurred in order to log the >>> event (the eventType would be this simpleClassName...for example >>> DocumentSavedEvent, AnnotationAddedEvent and so on). >> I don't agree with this, or I don't understand it. You get the Event object so you can easily get the simple class name by calling event.getClass().getSimpleClassName(). >> > Yes..right now the implementation does a test of instanceOf on the event > received (there is an awful if then else) and selects the type of event > from the class ActivityEventType static fields. By this modification we > can remove that if then else and use classname instead of the strings > from ActivityEventType. Why do you need to do an instanceof? >>> And thirdly i thought about adding one field in AbstractFilterableEvent >>> called /identifier/ and make all events extend directly this class. >> Again I don't agree. A notion of event identifier is not linked to a Filterable event IMO but to the Event class itself. >> >>> And >>> that /identifier/ would be used in a different way by the events >>> themselves. But it would be used to retrieve infos about that events. >>> And so, when logging an event, we could use classname + identifier. >>> For example >>> AnnotationAddedEvent + "This is my annotation" . >>> CommentAddedEvent + "This is my comment" or the number of the comment. >> I don't quite agree. If it's supposed to be an identifier it must be unique and leaving the unicity to the user will not guarantee it's unique. I'm not sure what you want to do: >> 1) Be able to differentiate 2 events. For what reason? >> 2) Be able to have some description of the events. Again for what reason? >> > The identifier field..I see it as a way so identify and specify more > data about the event itself. For example, i think it is important do > differentiate 2 CommentAddedEvent and not jusrt specify that they > occurred in a list of events and i would do this by this field. > The advantage of this field would be that it would be known in a generic > way and access to it would be easily done. There would be no need of the > sync you said between the listener and the emmiter. I aslways know i > will receive a class extending AbstractFilterableEvent, I can get the > identifier field. Maybe move it to Event itself ? How do you ensure unicity? And I don't think you're using this field in the correct way. You seem to be needing extra data about the event in order to perform some logging/display. The field to use for this is the data field. Having a data field in Event rather than in EventListener.onEvent() doesn't change anything for the sync... :) >> Now if you need extra data, this is already taken into account in the onEvent method: >> void onEvent(Event event, Object source, Object data); >> >> The data parameter is there to pass extra data. >> The way to use it is to create compound objects if you need to pass several data. >> >> However for easiness of use, I'd agree to transform the signature into: >> void onEvent(Event event, Object source, Object... data); >> >> All that said, we have a small issue in that the listener of events need to know what type of data get passed and thus emitter and listeners must be in sync. Thus a generic listener will always have a problem since it won't know the type of objects it receives (since they're different for each event). Thus for me, when you have a generic listener you also need an EventActionHandler that needs to be registered against the generic listener so that it know what to do with the extra data passed. >> >> How would you handle anonymous events in the Activity Stream (AS) listener? BTW what actions do you need to do with received events in the AS? > About anonymous events. I do not know exactly what you mean by that. > In AS i need to record them by doing a call to > addDocumentActivityEvent(streamName, currentDoc, eventType, msgPrefix + > eventType, params, context); Why only Document (Event can be about anything)? In general I don't like this proposal as it is because it doesn't address the real problem. It's moving it from onEvent() to a field in Event, adding even more complexity since there would be two ways of passing information. I'm -1 on it right now, especially since it breaks API without solving the issue. We need to address the sync issue. Thanks -Vincent > eventType would the the simple class name and the identifier would go in > the params list. > > Thanks, > Stefan >> Thanks >> -Vincent
On 11/10/2010 11:08 AM, Vincent Massol wrote: > > On Nov 10, 2010, at 10:12 AM, Stefan abageru wrote: > >> On 11/10/2010 10:57 AM, Vincent Massol wrote: >>> Hi Stefan, >>> >>> On Nov 10, 2010, at 9:08 AM, Stefan abageru wrote: >>> >>>> Hi devs ! >>>> >>>> I thought about adding some changes in Events and Activity Stream. >>>> >>>> First of all, I want to remove all the listeners from Activity Stream >>>> and make this class listen to AllEvent, making it capable of cathcing >>>> all events that occur in the wiki (and this way, when adding a new >>>> event, you wouldn't need any modifications to this class in order to >>>> catch it). >>> How do you filter events to process? If you don't how would you display for example view events or internal events such as component manager descriptor added/removed event, or the new event being added by Anca for the XAR import? >>> >> There would ne no filtering since IMO ActivityStream records the >> activities happening in the wiki. Should there be some filtering ? > > There are some events that are internal events and I don't think they should be displayed. > > + I don't know how the display will scale for view Events for ex (which can be received about 1000 times per second). > >>>> Secondly, I would like to modify onEvent function in the class so that >>>> it uses the simpleClassName of the event occurred in order to log the >>>> event (the eventType would be this simpleClassName...for example >>>> DocumentSavedEvent, AnnotationAddedEvent and so on). >>> I don't agree with this, or I don't understand it. You get the Event object so you can easily get the simple class name by calling event.getClass().getSimpleClassName(). >>> >> Yes..right now the implementation does a test of instanceOf on the event >> received (there is an awful if then else) and selects the type of event >> from the class ActivityEventType static fields. By this modification we >> can remove that if then else and use classname instead of the strings >> from ActivityEventType. > > Why do you need to do an instanceof? Vincent, you're talking about the exact same thing, there's no contradiction. The "instanceof" is the existing implementation. Ștefan's proposal is to use exactly what you said, event.getClass().getSimpleName(). The onEvent function is the event listener implementation inside ActivityStreamImpl, and "uses" means just calling getSimpleName() on the event. >>>> And thirdly i thought about adding one field in AbstractFilterableEvent >>>> called /identifier/ and make all events extend directly this class. >>> Again I don't agree. A notion of event identifier is not linked to a Filterable event IMO but to the Event class itself. >>> >>>> And >>>> that /identifier/ would be used in a different way by the events >>>> themselves. But it would be used to retrieve infos about that events. >>>> And so, when logging an event, we could use classname + identifier. >>>> For example >>>> AnnotationAddedEvent + "This is my annotation" . >>>> CommentAddedEvent + "This is my comment" or the number of the comment. >>> I don't quite agree. If it's supposed to be an identifier it must be unique and leaving the unicity to the user will not guarantee it's unique. I'm not sure what you want to do: >>> 1) Be able to differentiate 2 events. For what reason? >>> 2) Be able to have some description of the events. Again for what reason? >>> >> The identifier field..I see it as a way so identify and specify more >> data about the event itself. For example, i think it is important do >> differentiate 2 CommentAddedEvent and not jusrt specify that they >> occurred in a list of events and i would do this by this field. >> The advantage of this field would be that it would be known in a generic >> way and access to it would be easily done. There would be no need of the >> sync you said between the listener and the emmiter. I aslways know i >> will receive a class extending AbstractFilterableEvent, I can get the >> identifier field. Maybe move it to Event itself ? > > How do you ensure unicity? And I don't think you're using this field in the correct way. You seem to be needing extra data about the event in order to perform some logging/display. The field to use for this is the data field. Having a data field in Event rather than in EventListener.onEvent() doesn't change anything for the sync... :) > >>> Now if you need extra data, this is already taken into account in the onEvent method: >>> void onEvent(Event event, Object source, Object data); >>> >>> The data parameter is there to pass extra data. >>> The way to use it is to create compound objects if you need to pass several data. >>> >>> However for easiness of use, I'd agree to transform the signature into: >>> void onEvent(Event event, Object source, Object... data); >>> >>> All that said, we have a small issue in that the listener of events need to know what type of data get passed and thus emitter and listeners must be in sync. Thus a generic listener will always have a problem since it won't know the type of objects it receives (since they're different for each event). Thus for me, when you have a generic listener you also need an EventActionHandler that needs to be registered against the generic listener so that it know what to do with the extra data passed. >>> >>> How would you handle anonymous events in the Activity Stream (AS) listener? BTW what actions do you need to do with received events in the AS? >> About anonymous events. I do not know exactly what you mean by that. >> In AS i need to record them by doing a call to >> addDocumentActivityEvent(streamName, currentDoc, eventType, msgPrefix + >> eventType, params, context); > > Why only Document (Event can be about anything)? > > In general I don't like this proposal as it is because it doesn't address the real problem. It's moving it from onEvent() to a field in Event, adding even more complexity since there would be two ways of passing information. I'm -1 on it right now, especially since it breaks API without solving the issue. We need to address the sync issue. > > Thanks > -Vincent > >> eventType would the the simple class name and the identifier would go in >> the params list. -- Sergiu Dumitriu http://purl.org/net/sergiu/
On 11/10/2010 11:08 AM, Vincent Massol wrote: > > On Nov 10, 2010, at 10:12 AM, Stefan abageru wrote: > >> On 11/10/2010 10:57 AM, Vincent Massol wrote: >>> Hi Stefan, >>> >>> On Nov 10, 2010, at 9:08 AM, Stefan abageru wrote: >>> >>>> Hi devs ! >>>> >>>> I thought about adding some changes in Events and Activity Stream. >>>> >>>> First of all, I want to remove all the listeners from Activity Stream >>>> and make this class listen to AllEvent, making it capable of cathcing >>>> all events that occur in the wiki (and this way, when adding a new >>>> event, you wouldn't need any modifications to this class in order to >>>> catch it). >>> How do you filter events to process? If you don't how would you display for example view events or internal events such as component manager descriptor added/removed event, or the new event being added by Anca for the XAR import? >>> >> There would ne no filtering since IMO ActivityStream records the >> activities happening in the wiki. Should there be some filtering ? > > There are some events that are internal events and I don't think they should be displayed. > > + I don't know how the display will scale for view Events for ex (which can be received about 1000 times per second). > >>>> Secondly, I would like to modify onEvent function in the class so that >>>> it uses the simpleClassName of the event occurred in order to log the >>>> event (the eventType would be this simpleClassName...for example >>>> DocumentSavedEvent, AnnotationAddedEvent and so on). >>> I don't agree with this, or I don't understand it. You get the Event object so you can easily get the simple class name by calling event.getClass().getSimpleClassName(). >>> >> Yes..right now the implementation does a test of instanceOf on the event >> received (there is an awful if then else) and selects the type of event >> from the class ActivityEventType static fields. By this modification we >> can remove that if then else and use classname instead of the strings >> from ActivityEventType. > > Why do you need to do an instanceof? > >>>> And thirdly i thought about adding one field in AbstractFilterableEvent >>>> called /identifier/ and make all events extend directly this class. >>> Again I don't agree. A notion of event identifier is not linked to a Filterable event IMO but to the Event class itself. >>> >>>> And >>>> that /identifier/ would be used in a different way by the events >>>> themselves. But it would be used to retrieve infos about that events. >>>> And so, when logging an event, we could use classname + identifier. >>>> For example >>>> AnnotationAddedEvent + "This is my annotation" . >>>> CommentAddedEvent + "This is my comment" or the number of the comment. >>> I don't quite agree. If it's supposed to be an identifier it must be unique and leaving the unicity to the user will not guarantee it's unique. I'm not sure what you want to do: >>> 1) Be able to differentiate 2 events. For what reason? >>> 2) Be able to have some description of the events. Again for what reason? >>> >> The identifier field..I see it as a way so identify and specify more >> data about the event itself. For example, i think it is important do >> differentiate 2 CommentAddedEvent and not jusrt specify that they >> occurred in a list of events and i would do this by this field. >> The advantage of this field would be that it would be known in a generic >> way and access to it would be easily done. There would be no need of the >> sync you said between the listener and the emmiter. I aslways know i >> will receive a class extending AbstractFilterableEvent, I can get the >> identifier field. Maybe move it to Event itself ? > > How do you ensure unicity? And I don't think you're using this field in the correct way. You seem to be needing extra data about the event in order to perform some logging/display. The field to use for this is the data field. Having a data field in Event rather than in EventListener.onEvent() doesn't change anything for the sync... :) Right, except that it's not backwards compatible with the current implementation. We used to send the XWikiContext as the "data", and changing this would mean a very backwards incompatible change, although a change for the better. >>> Now if you need extra data, this is already taken into account in the onEvent method: >>> void onEvent(Event event, Object source, Object data); >>> >>> The data parameter is there to pass extra data. >>> The way to use it is to create compound objects if you need to pass several data. >>> >>> However for easiness of use, I'd agree to transform the signature into: >>> void onEvent(Event event, Object source, Object... data); >>> >>> All that said, we have a small issue in that the listener of events need to know what type of data get passed and thus emitter and listeners must be in sync. Thus a generic listener will always have a problem since it won't know the type of objects it receives (since they're different for each event). Thus for me, when you have a generic listener you also need an EventActionHandler that needs to be registered against the generic listener so that it know what to do with the extra data passed. >>> >>> How would you handle anonymous events in the Activity Stream (AS) listener? BTW what actions do you need to do with received events in the AS? >> About anonymous events. I do not know exactly what you mean by that. >> In AS i need to record them by doing a call to >> addDocumentActivityEvent(streamName, currentDoc, eventType, msgPrefix + >> eventType, params, context); > > Why only Document (Event can be about anything)? This is the name of the existing method. Nobody is proposing this name. If you don't like it, we can change it in the future. > In general I don't like this proposal as it is because it doesn't address the real problem. It's moving it from onEvent() to a field in Event, adding even more complexity since there would be two ways of passing information. I'm -1 on it right now, especially since it breaks API without solving the issue. We need to address the sync issue. > > Thanks > -Vincent > >> eventType would the the simple class name and the identifier would go in >> the params list. >> >> Thanks, >> Stefan >>> Thanks >>> -Vincent -- Sergiu Dumitriu http://purl.org/net/sergiu/
On Wed, Nov 10, 2010 at 09:08, Stefan abageru <[email protected]> wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
-1 for that, you can't force all events to extend AbstractFilterableEvent it does not always m
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Wed, Nov 10, 2010 at 10:20, Thomas Mortagne <[email protected]> wrote:
On Wed, Nov 10, 2010 at 09:08, Stefan abageru <[email protected]> wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
-1 for that, if you want data associated to the event you are supposed to find them in the onEvent parameters. Having an unique identifier will not always be enough whatever the event to know what happen (i don't event find any event for which it will be true). Note that remote observation manager have pretty much the same needs and already define event converter (or handler like Vincent call it) that extract from the event important parts in Serializable objects to send them to cluster members so that we send only absolutely necessary to know what happen. So it's exactly the same thing and we should use the same mechanism for both IMO. BTW we started to work on general event refactoring to have a common mechanism (and to improve ent API that really needs it) with Jean-Vincent but because of a lack of time I had to continue on my side with a remote observation only code but the needs still remain, you can see http://dev.xwiki.org/xwiki/bin/view/Design/ActivityComponent for the last state of this work.
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
-- Thomas Mortagne
On 11/10/2010 11:28 AM, Thomas Mortagne wrote:
On Wed, Nov 10, 2010 at 10:20, Thomas Mortagne <[email protected]> wrote:
On Wed, Nov 10, 2010 at 09:08, Stefan abageru<[email protected]> wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment. -1 for that, if you want data associated to the event you are supposed to find them in the onEvent parameters. Having an unique identifier will not always be enough whatever the event to know what happen (i don't event find any event for which it will be true).
IMO, the class name + the identifier would be enough to understand what happened. A simple example would be: CommentAddedEvent + "I think this is a nice document". The need to do an instanceof at this moment is given by the fact that there are different types of events and in each type there are different fields which contain informations about events. For example, for an Annotation event we have /identifier/ field, for a Comment event we have a /message/ field and we cannot retrieve that field unless we do the cast but after the check with instanceof. It is however clear that there are some fields in your proposal that we could add such as Date of the event. However, if my solution isn't as generic as it should be, maybe you guys can help with some ideas of design so that we can clean a little bit the events and Activity Stream part :). Thanks, Stefan
Note that remote observation manager have pretty much the same needs and already define event converter (or handler like Vincent call it) that extract from the event important parts in Serializable objects to send them to cluster members so that we send only absolutely necessary to know what happen. So it's exactly the same thing and we should use the same mechanism for both IMO. BTW we started to work on general event refactoring to have a common mechanism (and to improve ent API that really needs it) with Jean-Vincent but because of a lack of time I had to continue on my side with a remote observation only code but the needs still remain, you can see http://dev.xwiki.org/xwiki/bin/view/Design/ActivityComponent for the last state of this work.
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Wed, Nov 10, 2010 at 16:24, Stefan abageru <[email protected]> wrote:
On 11/10/2010 11:28 AM, Thomas Mortagne wrote:
On Wed, Nov 10, 2010 at 10:20, Thomas Mortagne <[email protected]> wrote:
On Wed, Nov 10, 2010 at 09:08, Stefan abageru<[email protected]> wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment. -1 for that, if you want data associated to the event you are supposed to find them in the onEvent parameters. Having an unique identifier will not always be enough whatever the event to know what happen (i don't event find any event for which it will be true).
IMO, the class name + the identifier would be enough to understand what happened. A simple example would be: CommentAddedEvent + "I think this is a nice document".
In this example you have no idea where this comments is located which is not very helpful IMO. When you see this in the stream it would be nice to have a link to the comment. If activity stream is only about printing some wild information without any context (sorry i did not followed activity stream conversations a lot) then your proposal is OK, otherwise it does not work IMO.
The need to do an instanceof at this moment is given by the fact that there are different types of events and in each type there are different fields which contain informations about events. For example, for an Annotation event we have /identifier/ field, for a Comment event we have a /message/ field and we cannot retrieve that field unless we do the cast but after the check with instanceof.
It is however clear that there are some fields in your proposal that we could add such as Date of the event.
However, if my solution isn't as generic as it should be, maybe you guys can help with some ideas of design so that we can clean a little bit the events and Activity Stream part :).
Thanks, Stefan
Note that remote observation manager have pretty much the same needs and already define event converter (or handler like Vincent call it) that extract from the event important parts in Serializable objects to send them to cluster members so that we send only absolutely necessary to know what happen. So it's exactly the same thing and we should use the same mechanism for both IMO. BTW we started to work on general event refactoring to have a common mechanism (and to improve ent API that really needs it) with Jean-Vincent but because of a lack of time I had to continue on my side with a remote observation only code but the needs still remain, you can see http://dev.xwiki.org/xwiki/bin/view/Design/ActivityComponent for the last state of this work.
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On 11/10/2010 06:20 PM, Thomas Mortagne wrote:
On Wed, Nov 10, 2010 at 16:24, Stefan abageru<[email protected]> wrote:
On 11/10/2010 11:28 AM, Thomas Mortagne wrote:
On Wed, Nov 10, 2010 at 10:20, Thomas Mortagne <[email protected]> wrote:
On Wed, Nov 10, 2010 at 09:08, Stefan abageru<[email protected]> wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment. -1 for that, if you want data associated to the event you are supposed to find them in the onEvent parameters. Having an unique identifier will not always be enough whatever the event to know what happen (i don't event find any event for which it will be true).
IMO, the class name + the identifier would be enough to understand what happened. A simple example would be: CommentAddedEvent + "I think this is a nice document".
In this example you have no idea where this comments is located which is not very helpful IMO. When you see this in the stream it would be nice to have a link to the comment.
There is a context, since the event knows for which document it occurs already. Still, my proposal was to use the object reference, not the comment content, so apart from knowing the document from the event, having the full reference will also identify the comment properly. Note that at the moment comment events use the object number as their identifier.
If activity stream is only about printing some wild information without any context (sorry i did not followed activity stream conversations a lot) then your proposal is OK, otherwise it does not work IMO.
The need to do an instanceof at this moment is given by the fact that there are different types of events and in each type there are different fields which contain informations about events. For example, for an Annotation event we have /identifier/ field, for a Comment event we have a /message/ field and we cannot retrieve that field unless we do the cast but after the check with instanceof.
It is however clear that there are some fields in your proposal that we could add such as Date of the event.
However, if my solution isn't as generic as it should be, maybe you guys can help with some ideas of design so that we can clean a little bit the events and Activity Stream part :).
Thanks, Stefan
Note that remote observation manager have pretty much the same needs and already define event converter (or handler like Vincent call it) that extract from the event important parts in Serializable objects to send them to cluster members so that we send only absolutely necessary to know what happen. So it's exactly the same thing and we should use the same mechanism for both IMO. BTW we started to work on general event refactoring to have a common mechanism (and to improve ent API that really needs it) with Jean-Vincent but because of a lack of time I had to continue on my side with a remote observation only code but the needs still remain, you can see http://dev.xwiki.org/xwiki/bin/view/Design/ActivityComponent for the last state of this work.
WDYT ?
Stefan
-- Sergiu Dumitriu http://purl.org/net/sergiu/
What about creating an EventLogger component with implementations for each of the events that need to be logged by the activity stream? When an event is triggered you lookup the corresponding EventLogger. If there is no EventLogger for the specific hint then ignore the event. Hope this helps, Marius On 11/10/2010 10:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On Wed, Nov 10, 2010 at 19:29, Marius Dumitru Florea <[email protected]> wrote:
What about creating an EventLogger component with implementations for each of the events that need to be logged by the activity stream? When an event is triggered you lookup the corresponding EventLogger. If there is no EventLogger for the specific hint then ignore the event.
(Which is exactly what I proposed and what remote observation manager system is doing except that it's sending the event on network instead of saving it in database but it's the same constraint: serializable data)
Hope this helps, Marius
On 11/10/2010 10:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
WDYT ?
Stefan _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On 11/10/2010 07:29 PM, Marius Dumitru Florea wrote:
What about creating an EventLogger component with implementations for each of the events that need to be logged by the activity stream? When an event is triggered you lookup the corresponding EventLogger. If there is no EventLogger for the specific hint then ignore the event.
The problem is that you have to write extra code to store the extra events. It's just moving the current if-else to a different level. It is more easily extensible, but I don't agree that any new component sending events has to have a dependency on the activity stream plugin to also be able to store those events in the stream.
Hope this helps, Marius
On 11/10/2010 10:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
-- Sergiu Dumitriu http://purl.org/net/sergiu/
On Wed, Nov 10, 2010 at 20:45, Sergiu Dumitriu <[email protected]> wrote:
On 11/10/2010 07:29 PM, Marius Dumitru Florea wrote:
What about creating an EventLogger component with implementations for each of the events that need to be logged by the activity stream? When an event is triggered you lookup the corresponding EventLogger. If there is no EventLogger for the specific hint then ignore the event.
The problem is that you have to write extra code to store the extra events. It's just moving the current if-else to a different level. It is more easily extensible, but I don't agree that any new component sending events has to have a dependency on the activity stream plugin to also be able to store those events in the stream.
I don't agree with the activity stream dependency. It's not about activity stream, it's a handler that extract important event informations in a serializable object. This system is then used by activity stream and remote observation manager to store or send events.
Hope this helps, Marius
On 11/10/2010 10:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
-- Sergiu Dumitriu http://purl.org/net/sergiu/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On 11/11/2010 01:16 AM, Thomas Mortagne wrote:
On Wed, Nov 10, 2010 at 20:45, Sergiu Dumitriu<[email protected]> wrote:
On 11/10/2010 07:29 PM, Marius Dumitru Florea wrote:
What about creating an EventLogger component with implementations for each of the events that need to be logged by the activity stream? When an event is triggered you lookup the corresponding EventLogger. If there is no EventLogger for the specific hint then ignore the event.
The problem is that you have to write extra code to store the extra events. It's just moving the current if-else to a different level. It is more easily extensible, but I don't agree that any new component sending events has to have a dependency on the activity stream plugin to also be able to store those events in the stream.
I don't agree with the activity stream dependency. It's not about activity stream, it's a handler that extract important event informations in a serializable object. This system is then used by activity stream and remote observation manager to store or send events.
Code speaks better than people. Here's a proposed first patch: http://jira.xwiki.org/jira/browse/XPAS-29
Hope this helps, Marius
On 11/10/2010 10:08 AM, Stefan abageru wrote:
Hi devs !
I thought about adding some changes in Events and Activity Stream.
First of all, I want to remove all the listeners from Activity Stream and make this class listen to AllEvent, making it capable of cathcing all events that occur in the wiki (and this way, when adding a new event, you wouldn't need any modifications to this class in order to catch it).
Secondly, I would like to modify onEvent function in the class so that it uses the simpleClassName of the event occurred in order to log the event (the eventType would be this simpleClassName...for example DocumentSavedEvent, AnnotationAddedEvent and so on).
And thirdly i thought about adding one field in AbstractFilterableEvent called /identifier/ and make all events extend directly this class. And that /identifier/ would be used in a different way by the events themselves. But it would be used to retrieve infos about that events. And so, when logging an event, we could use classname + identifier. For example AnnotationAddedEvent + "This is my annotation" . CommentAddedEvent + "This is my comment" or the number of the comment.
-- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (5)
-
Marius Dumitru Florea -
Sergiu Dumitriu -
Stefan abageru -
Thomas Mortagne -
Vincent Massol