16 Oct
2018
16 Oct
'18
6:07 p.m.
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
17 Oct
17 Oct
9:09 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
+1 to stopping event propagation when it's cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
--
Thomas Mortagne
9:44 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
Hi all,
I took the liberty to already create the PR, then you can see the impact
on the code: https://github.com/xwiki/xwiki-commons/pull/49
Simon.
On 10/17/18 9:09 AM, Thomas Mortagne wrote:
+1 to stopping event propagation when it's
cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
10:35 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
I'm OK.
<OFF TOPIC>
I'm just thinking about an other particular case:
Imagine you have 3 event listeners (A, B, C):
- A receives the event and perform some actions (saving something in the
database).
- B receives the event and cancels it
- C don't receive the event because it had been canceled
However, we may want to resend some infos to listener A so it can rollback
its actions (otherwise we end up with bad info in the database).
Do we have a strategy for this?
</OFF TOPIC>
Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne <thomas.mortagne(a)xwiki.com> a
écrit :
+1 to stopping event propagation when it's
cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
--
Guillaume Delhumeau (guillaume.delhumeau(a)xwiki.com)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
--
Thomas Mortagne
10:47 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
I'm OK.
<OFF TOPIC>
I'm just thinking about an other particular case:
Imagine you have 3 event listeners (A, B, C):
- A receives the event and perform some actions (saving something in the
database).
- B receives the event and cancels it
- C don't receive the event because it had been canceled
However, we may want to resend some infos to listener A so it can rollback
its actions (otherwise we end up with bad info in the database).
Do we have a strategy for this?
I don't think we have a strategy for that, but we might add a new method
in EventListener:
onRollback(CancelableEvent canceledEvent, Object source, Object data)
and store somewhere the list of called listener to be able to call their
rollback method if the event has been cancelled. Should do the trick, WDYT?
</OFF TOPIC>
Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne <thomas.mortagne(a)xwiki.com> a
écrit :
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
+1 to stopping event propagation when it's
cancelled
On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
--
Thomas Mortagne
11:29 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
Le mer. 17 oct. 2018 à 10:47, Simon Urli <simon.urli(a)xwiki.com> a écrit :
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
--
Guillaume Delhumeau (guillaume.delhumeau(a)xwiki.com)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project
On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
Sounds like a good idea, indeed. Or it could be called onCanceled() instead
of onRollback, since the rollback is what we expect, not the event that is
triggered.
I'm OK.
<OFF TOPIC>
I'm just thinking about an other particular case:
Imagine you have 3 event listeners (A, B, C):
- A receives the event and perform some actions (saving something in the
database).
- B receives the event and cancels it
- C don't receive the event because it had been canceled
However, we may want to resend some infos to listener A so it can
rollback
its actions (otherwise we end up with bad info in
the database).
Do we have a strategy for this?
I don't think we have a strategy for that, but we might add a new method
in EventListener:
onRollback(CancelableEvent canceledEvent, Object source, Object data)
and store somewhere the list of called listener to be able to call their
rollback method if the event has been cancelled. Should do the trick, WDYT?
</OFF TOPIC>
Le mer. 17 oct. 2018 à 09:09, Thomas Mortagne <thomas.mortagne(a)xwiki.com>
a
écrit :
> +1 to stopping event propagation when it's cancelled
> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com>
wrote:
>>
>> Hi everyone,
>>
>> the current behaviour of the ObservationManager is to always triggers
>> the listeners if it matches the events.
>> Now regarding the CancelableEvents, the match is only done on the type
>> of the event and some given filter rules, but never with its cancel
>> status: if an event is cancelled, the matching listeners are always
>> triggered.
>>
>> I propose to change that behaviour, to trigger listeners only if the
>> CancelableEvents are not canceled: basically, a cancelled event
wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
--
Thomas Mortagne
11:34 a.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
On 10/17/18 11:29 AM, Guillaume Delhumeau wrote:
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com <mailto:simon.urli@xwiki.com>
More about us at http://www.xwiki.com
--
Guillaume Delhumeau (guillaume.delhumeau(a)xwiki.com
<mailto:guillaume.delhumeau@xwiki.com>)
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
Le mer. 17 oct. 2018 à 10:47, Simon Urli
<simon.urli(a)xwiki.com
<mailto:simon.urli@xwiki.com>> a écrit :
On 10/17/18 10:35 AM, Guillaume Delhumeau wrote:
I'll create an issue in that way.
I'm OK.
<OFF TOPIC>
I'm just thinking about an other particular case:
Imagine you have 3 event listeners (A, B, C):
- A receives the event and perform some actions (saving something
in the
database).
- B receives the event and cancels it
- C don't receive the event because it had been canceled
However, we may want to resend some infos to listener A so it can
rollback
its actions (otherwise we end up with bad info in
the database).
Do we have a strategy for this?
I don't think we have a strategy for that, but we might add a new
method
in EventListener:
onRollback(CancelableEvent canceledEvent, Object source, Object data)
and store somewhere the list of called listener to be able to call
their
rollback method if the event has been cancelled. Should do the
trick, WDYT?
Sounds like a good idea, indeed. Or it could be called onCanceled()
instead of onRollback, since the rollback is what we expect, not the
event that is triggered.
> </OFF TOPIC>
>
Now Thomas just opens the debate about where to put the behaviour
change: I proposed in my PR to put it in AbstractCancelableEvents to
allow users redefine the behaviour if needed by overriding the method.
Thomas proposes
(https://github.com/xwiki/xwiki-commons/pull/49#issuecomment-430555665)
that we put it in DefaultObservationManager as we want the behaviour for
all CancelableEvents, not only those who inherits from
AbstractCancelableEvents. My concern here is that this behaviour
couldn't be changed easily if needed (unless using another kind of event).
WDYT?
Le mer. 17
oct. 2018 à 09:09, Thomas Mortagne
<thomas.mortagne(a)xwiki.com
<mailto:thomas.mortagne@xwiki.com>> a
écrit :
> +1 to stopping event propagation when it's cancelled
> On Tue, Oct 16, 2018 at 6:07 PM Simon Urli <simon.urli(a)xwiki.com
<mailto:simon.urli@xwiki.com>> wrote:
>>
>> Hi everyone,
>>
>> the current behaviour of the ObservationManager is to always
triggers
>> the listeners if it matches the events.
>> Now regarding the CancelableEvents, the match is only done on
the
type
>> of the event and some given filter rules,
but never with its cancel
>> status: if an event is cancelled, the matching listeners are always
>> triggered.
>>
>> I propose to change that behaviour, to trigger listeners only
if the
>> CancelableEvents are not canceled:
basically, a cancelled event
wouldn't
>> match any listener.
>>
>> My primary reason for wanting that change is that the current
behaviour
>> led to a bad UX: if an event triggers
multiple questions, no
matter if
one is cancelled, all questions will be asked to the
user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com <mailto:simon.urli@xwiki.com>
More about us at http://www.xwiki.com
--
Thomas Mortagne
2:21 p.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
-0. I would be OK to stop the event propagation if the list of event
listeners were *ordered* but AFAIK there's no order between event listeners
ATM. It should always be possible to catch an event, even if that event is
going to be canceled by some other listener:
* either by registering an event listener with higher priority than the one
canceling the event
* or by not stopping the event propagation and letting each listener decide
what to do based on the canceled state of the event
For instance, DOM events have this
https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-capture
If the capturing EventListener
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventListener>
wishes to prevent further processing of the event from occurring it may
call the stopProgagation method of the Event
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-Event>
interface. This will prevent further dispatch of the event, although
additional EventListeners
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventListener>
registered at the same hierarchy level will still receive the event.
So you can always catch the DOM event by registering the event listener on
the element that triggers the event (rather than on one of its parents).
Thanks,
Marius
On Tue, Oct 16, 2018 at 7:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
2:24 p.m.
New subject: [xwiki-devs] [Proposal] Change ObservationManager behaviour with CancelableEvents
An use case is to be able to log all the events that happen in XWiki. It
would be stupid to not be able to log an event because there's another
event listener that cancels the event before.
On Wed, Oct 17, 2018 at 3:21 PM Marius Dumitru Florea <
mariusdumitru.florea(a)xwiki.com> wrote:
-0. I would be OK to stop the event propagation if the
list of event
listeners were *ordered* but AFAIK there's no order between event
listeners ATM. It should always be possible to catch an event, even if that
event is going to be canceled by some other listener:
* either by registering an event listener with higher priority than the
one canceling the event
* or by not stopping the event propagation and letting each listener
decide what to do based on the canceled state of the event
For instance, DOM events have this
https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-capture
If the capturing EventListener
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventListener>
wishes to prevent further processing of the event from occurring it may
call the stopProgagation method of the Event
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-Event>
interface. This will prevent further dispatch of the event, although
additional EventListeners
<https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventListener>
registered at the same hierarchy level will still receive the event.
So you can always catch the DOM event by registering the event listener on
the element that triggers the event (rather than on one of its parents).
Thanks,
Marius
On Tue, Oct 16, 2018 at 7:07 PM Simon Urli <simon.urli(a)xwiki.com> wrote:
Hi everyone,
the current behaviour of the ObservationManager is to always triggers
the listeners if it matches the events.
Now regarding the CancelableEvents, the match is only done on the type
of the event and some given filter rules, but never with its cancel
status: if an event is cancelled, the matching listeners are always
triggered.
I propose to change that behaviour, to trigger listeners only if the
CancelableEvents are not canceled: basically, a cancelled event wouldn't
match any listener.
My primary reason for wanting that change is that the current behaviour
led to a bad UX: if an event triggers multiple questions, no matter if
one is cancelled, all questions will be asked to the user.
Do you know if the current behaviour is required at some places?
Else do you agree on changing it?
Simon
--
Simon Urli
Software Engineer at XWiki SAS
simon.urli(a)xwiki.com
More about us at http://www.xwiki.com
2214
days inactive
2215
days old
8 comments
4 participants
participants (4)
-
Guillaume Delhumeau -
Marius Dumitru Florea -
Simon Urli -
Thomas Mortagne