On Thu, Nov 24, 2011 at 22:22, Vincent Massol <[email protected]> wrote:
> > On Nov 24, 2011, at 8:15 PM, Anca Luca wrote: > > > On 11/24/2011 05:31 PM, Vincent Massol wrote: > >> On Nov 24, 2011, at 4:55 PM, Denis Gervalle wrote: > >> > >>> On Thu, Nov 24, 2011 at 16:25, Vincent Massol<[email protected]> > wrote: > >>> > >>>> On Nov 24, 2011, at 4:06 PM, Denis Gervalle wrote: > >>>> > >>>>> On Thu, Nov 24, 2011 at 13:58, Vincent Massol<[email protected]> > >>>> wrote: > >>>>>> Hi devs, > >>>>>> > >>>>>> Summary: > >>>>>> ======== > >>>>>> > >>>>>> I'd like to add the notion of Priority to Event Listeners. The > reason is > >>>>>> that in some cases it's important that some listeners execute before > >>>> others. > >>>>>> The problem at hand: > >>>>>> ================= > >>>>>> > >>>>>> Here's a typical use case: When receiving the > ApplicationStartedEvent, > >>>> we > >>>>>> have lot of code that needs to initialize. Initialization order is > >>>>>> important (you can compare it to run levels in OS): for example some > >>>> init > >>>>>> must happen after the Database initialization has happened. > >>>>>> > >>>>>> Note that another solution exists for this use case: some > >>>> initializations > >>>>>> could introduce their own events (such as a DatabaseStartedEvent) > and > >>>> other > >>>>>> init could listen on those events instead of the generic > >>>>>> ApplicationStartedEvent. However I can see several drawbacks to > this: > >>>>>> * it's less generic than the priority solution > >>>>>> * it means creating more events > >>>>>> * but more importantly it means that modules will have strong > >>>> dependencies > >>>>>> (at maven level) on each other whereas it's not necessary and > shouldn't > >>>> be > >>>>>> the case. In our example use case: it means that inits that must > happen > >>>>>> after database is started will need to depend on oldcore (which is > >>>> where DB > >>>>>> is started ATM) > >>>>>> > >>>>>> Proposal: > >>>>>> ======== > >>>>>> > >>>>>> * Don't break backward compat in Observation module > >>>>>> * Introduce a PrioritizedEventListener interface that adds a > >>>> getPriority() > >>>>>> method > >>>>>> * Modify ObservationManager implementation to take into account > >>>> priorities > >>>>>> * In order to make it simple I propose to have only a single > priority > >>>> per > >>>>>> Listener and not a priority per event supported by a given listener > >>>>>> > >>>>>> General Context > >>>>>> ============= > >>>>>> > >>>>>> To give some context here's what I'd like to do on the medium term: > >>>>>> > >>>>>> * Step 1: Introduce notion of priority in EventListeners > >>>>>> * Step 2: Refactor XWiki init to use an EventListener on AppStarted > with > >>>>>> low priority > >>>>>> * Step 3: Refactor wiki macros to use an EventListener on AppStarted > >>>> with > >>>>>> priority value lower than at step2 > >>>>>> * Step 4: Write an EventListener for the new UI Extensions with a > >>>> priority > >>>>>> value higher than the one of step2<-- this is the initial goal that > >>>> led > >>>>>> me to make this proposal ;) > >>>>>> > >>>>>> WDYT? > >>>>>> > >>>>> Sounds good if not overkill for the goal. > >>>>> An simpler alternative would be to have more than a single AppStarted > >>>>> event, like there is more than one starting level in Linux. > >>>>> Let say one level before XWiki, the one during, the one after ? is > there > >>>> so > >>>>> many other use case ? > >>>> > >>> Well, I have said +1 anyway, but... > >>> > >>> > >>>> Yes this is very close to the other solution I explained above. > >>>> > >>> Surely. > >>> > >>> > >>>> But it's far less generic and introduces knowing stuff you don't > really > >>>> need to know. For me it's a "poorman" implementation of priorities. > >>>> > >>> Well, it depends on the pursued goal. What I mostly dislike in priority > >>> systems, it the management of priorities. When you say that you are > >>> listening on a let says DatabaseStarted event, you clearly explain > what you > >>> really need. On the other hand, when you say that you want to be run at > >>> priority 100 of the AppStartedEvent, without another document saying > that > >>> starting at 100, database is ready, and ensuring this in the database > >>> module, you do not really know what it means really. > >> Yes it's static dependency vs loose dependency. One is statically typed > the other is documentation. > >> > >> But that's the only way I know to not have modules depend on each other. > >> > >> I like the static way of course but what you proposed while it's good > for this specific use case doesn't solve all other use cases, which is why > I'm ambivalent about it and why I think the generic solution is a bit > better (I could be convinced otherwise with enough arguing and if enough > devs prefer the non generic way ;)). > > > > I sort of agree with Dennis on this one. Numbered priorities always have > conflicts, and in order to make sure you place yourself in the proper place > (before this stuff, after this other stuff) you need to check (in the code? > in a documentation about priority numbers?) the numbers that stuff are > using. > > Or did I understand the proposal wrong and it's not about priority > numbers? > > > > It's really a good idea BUT, as we've discussed before, numbered > priorities are always very tricky, since you can never be sure what it > means when you put 100 or 0 or some other number x (what do other listeners > declare? will my listener be before or after a certain other listener?). > > If you can propose a better solution I'm all for it. I just don't know a > better solution. > > FYI I've reviewed the following I could think about: > * Specific events added by other listeners (as I mentioned in my first > mail) > * Ability to say 'before' and 'after' other events > * Priority numbers > > And by very far the one that I've found with the most pros is the priority > number solution (which we use in several other places in xwiki code base > whenever we need order: macros, etc). > > > I would think all the number-based priority levels endup being used as a > static list-based priority levels (there are a few well-known values that > are used), > > Yes that's the idea. Special numbers are documented and are "API"s. Except > they are "decoupled APIs" (vs the static coupling you get with the other > solutions). > > > since you can't really use a continuous interval (you need "before this > standard listener" or "after this other standard listener", if we're > talking about non-standard listeners, numbers are not much more helpful > since you can never know what will be all the priorities declared by all > those listeners and where would yours be placed). > > That's not true. In practice this still works and this is what makes > numbers powerful vs other solutions. They're flexible and you can always > (ok, almost always) tune the number to get the behavior you wish to have > which is definitely not even imaginable with the other 2 solutions I listed > above since they're not "dynamic" solutions and cannot adapt to something > unknown from the onset. > All what both of you said are true. But I think that what is worrying me and Anka most finally, is that priority defeat the goal of a traditional observer or listener pattern, since it will induce some coupling between sibling listeners. You can almost (since priority are on listeners and not listener/event pair) get the behavior you want for a defined distribution, but changes in any sibling listeners of the same event may defeat your initial behavior due to the "loose" but "still" coupling priorities will introduce. > Now I'm open to hear about a better solution. I just don't see one. > I do not know much myself, I am not sure these exists really. In a similar case, the usual solution is to introduce a intermediate controller that will divide the single event into smaller ones, allowing listeners a more granular notification. This is static priorities, but these does not introduce coupling between listeners. So basically, what I was saying is that introducing flexible priority in listeners may cause more problems than solutions. Since I have already given my +1, I will keep it, but I would not have done so, and I would have preferred the more usual way of adding more events, especially for the current UC you have to solve, which is mainly concentrated on the single AppStartEvent. Denis > > Thanks > -Vincent > > > Thanks, > > Anca > > > >> > >> Thanks > >> -Vincent > >> > >>>> I don't know about other use cases right now but I can imagine a lot > of > >>>> them (for example, some extensions that needs open office to be > >>>> initialized, etc). We need to remember that we're developing a > platform and > >>>> not just for XE's needs. The extra amount of work is negligible IMO > not to > >>>> do it (btw I have a working version locally already, I just need to > fine > >>>> tune it a bit for improved performance maybe). > >>>> > >>> > >>> > >>>> Thanks > >>>> -Vincent > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
