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/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
