Bug 333930 - [api] provide ability to get notified of changes to task data and submits
Summary: [api] provide ability to get notified of changes to task data and submits
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P2 enhancement with 1 vote (vote)
Target Milestone: 3.7   Edit
Assignee: Sam Davis CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed, noteworthy, plan
Depends on: 370620
Blocks:
  Show dependency tree
 
Reported: 2011-01-10 20:05 EST by Thomas Ehrnhoefer CLA
Modified: 2012-02-15 12:35 EST (History)
4 users (show)

See Also:


Attachments
patch (9.53 KB, patch)
2012-02-01 19:51 EST, Sam Davis CLA
no flags Details | Diff
mylyn/context/zip (15.06 KB, application/octet-stream)
2012-02-01 19:51 EST, Sam Davis CLA
no flags Details
changes to ExtensionPointReader (2.59 KB, patch)
2012-02-02 18:30 EST, Sam Davis CLA
no flags Details | Diff
patch (16.79 KB, patch)
2012-02-14 21:01 EST, Sam Davis CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Ehrnhoefer CLA 2011-01-10 20:05:38 EST
...so one can listen to submits without creation/initiating them.
Comment 1 Steffen Pingel CLA 2011-01-10 21:12:57 EST
Sounds like a good idea. I would suggest doing the same for ITaskDataManagerListener. This would probably need to happen through an extension point to ensure that clients are notified regardless of bundle activation.
Comment 2 Jan Mauersberger CLA 2011-07-12 10:36:17 EDT
Exactly what I was looking for. The interface at least for submits could look similar to the IJobChangeListener interface, meaning hooks like aboutToSubmit(), submitted(). I think no veto support is required.
Comment 3 Sam Davis CLA 2012-02-01 19:51:24 EST
Created attachment 210414 [details]
patch

Steffen, what do you think of this? I considered reusing SubmitTaskJob instead of defining a new interface but the problem is that it if threw an exception it would break all submits.
Comment 4 Sam Davis CLA 2012-02-01 19:51:26 EST
Created attachment 210415 [details]
mylyn/context/zip
Comment 5 Steffen Pingel CLA 2012-02-01 20:17:36 EST
That's a good start. SubmitJobListener is indeed a poor name, it should have been called SubmitJobParticipant. How about we create a TaskJobListener class? Here a few suggestions:

* To make the listener interface extensible in the future I would prefer an abstract class and event class (like SubmitJobEvent) that encapsulates the event parameters so we can add additional properties as needed, e.g. the job result.
* The extension handler should use ExtensionPointReader and create extensions as lazy as possible. 
* We should consider adding a connectorKind field to the extension point to support listening for specific connectors only. This would require an enhancement to ExtensionPointReader to support filtering of extensions.

Can you push a code review for the next iteration?
Comment 6 Sam Davis CLA 2012-02-02 02:24:54 EST
Sure. I like those suggestions, but I'm unsure how it could be made more lazy, other than only loading listeners for a specific connector when that connector does a submit, which may or may not be more efficient.
Comment 7 Steffen Pingel CLA 2012-02-02 05:38:05 EST
(In reply to comment #6)
> Sure. I like those suggestions, but I'm unsure how it could be made more lazy,
> other than only loading listeners for a specific connector when that connector
> does a submit, which may or may not be more efficient.

I guess I was thinking that the extension point should be parsed when the event is fired but that wouldn't make much of a difference. It doesn't seem quite right though to store a static reference to listeners in SubmitTaskJob. That should be handled in TaskJobFactory and injected through the constructor.
Comment 8 Sam Davis CLA 2012-02-02 18:08:53 EST
I've created the following review: I26f2ccd9: NEW - bug 333930: [api] provide ability to get notified of changes to task data and submits  https://bugs.eclipse.org/bugs/show_bug.cgi?id=333930
http://review.mylyn.org/#change,254
Comment 9 Sam Davis CLA 2012-02-02 18:30:59 EST
Created attachment 210480 [details]
changes to ExtensionPointReader

changes to ExtensionPointReader to support filtering elements by attribute value
Comment 10 Steffen Pingel CLA 2012-02-03 17:11:22 EST
Thanks Sam. Can you open a new bug for the ExtensionPointReader enhancement? I would also like to consider adding support for a priority attribute. 

Can you move the review to the master branch? It looks like it's currently building against the e_3_7_m_3_6_x branch: http://review.mylyn.org/#change,254
Comment 11 Sam Davis CLA 2012-02-03 19:11:30 EST
New review: http://review.mylyn.org/#change,262 (the original had no change id).
Comment 12 Sam Davis CLA 2012-02-07 16:46:22 EST
As discussed on bug 370772, we should also include the old task in the event.
Comment 13 Jan Mauersberger CLA 2012-02-08 03:30:57 EST
Sam, could you please also consider a hook before the task is actually submitted - something like "aboutToSubmit()" as mentioned in comment:2? I think that kind of pre-submit hook is required at least in some cases we have.
Comment 14 Sam Davis CLA 2012-02-08 13:25:59 EST
(In reply to comment #13)
> Sam, could you please also consider a hook before the task is actually submitted
> - something like "aboutToSubmit()" as mentioned in comment:2? I think that kind
> of pre-submit hook is required at least in some cases we have.

That makes sense to me and should be trivial to add.
Comment 15 Sam Davis CLA 2012-02-13 18:00:58 EST
I have updated the review.
Comment 16 Steffen Pingel CLA 2012-02-14 14:41:53 EST
Thanks! That looks good. Please add a few lines of documentation to TaskJobListener and attach a patch. I'll then go ahead and merge the contribution.
Comment 17 Sam Davis CLA 2012-02-14 21:01:51 EST
Created attachment 211018 [details]
patch

Note that I changed the id of the extension point to match the schema file name
Comment 18 Steffen Pingel CLA 2012-02-15 12:33:33 EST
Thanks! I have committed the patch. I'll push it as soon as git access is restored.
Comment 19 Steffen Pingel CLA 2012-02-15 12:35:52 EST
To use the new API register an extension for the org.eclipse.mylyn.tasks.core.taskJobListener extension point that extends TaskJobListener. The listener instance is notified before task submissions takes place and after it has completed.