Bug 71515 - Need ActionProvider to support blocking actions
Need ActionProvider to support blocking actions
Status: RESOLVED FIXED
Product: projects
Classification: Unclassified
Component: Generic Infrastructure
5.x
All All
: P3 with 2 votes (vote)
: 7.2
Assigned To: Jesse Glick
issues@projects
: API, API_REVIEW_FAST
Depends on: 209652
Blocks: 64991 70280 72385 185429 206208
  Show dependency treegraph
 
Reported: 2006-01-17 22:01 UTC by Jesse Glick
Modified: 2012-03-25 09:44 UTC (History)
7 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Start of patch with ActionProvider2 (151.53 KB, patch)
2012-03-13 00:04 UTC, Jesse Glick
Details | Diff
Rewritten patch based on ActionProgress (54.74 KB, patch)
2012-03-13 19:38 UTC, Jesse Glick
Details | Diff
Fix of bug #64991 using this API (19.94 KB, patch)
2012-03-13 21:58 UTC, Jesse Glick
Details | Diff
Minor updates to main patch (55.67 KB, patch)
2012-03-20 02:51 UTC, Jesse Glick
Details | Diff
Updated patch for #64991 (25.47 KB, patch)
2012-03-20 02:52 UTC, Jesse Glick
Details | Diff
Patch for #206208 (7.50 KB, patch)
2012-03-20 02:53 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2006-01-17 22:01:04 UTC
We need some way to call org.netbeans.spi.project.ActionProvider.invokeAction
and wait for it to finish. Consider e.g.

org.netbeans.modules.project.portfolios.PortfoliosTopComponent.ProjectProxyAction.actionPerformed:

                for (Iterator i = actionProviders.iterator(); i.hasNext();) {
                    Lookup lookup = (Lookup) lkpIterator.next();
                    ActionProvider ap = (ActionProvider) i.next();
                    ap.invokeAction(cmd, lookup);
                }

Really we want these actions to be invoked serially, not in parallel, since the
projects might have interdependencies and Ant builds should not be run in
parallel if one might be creating a JAR that another is using.

Your actions rewrite said something about a special token like "waitFinished"
for the ActionEvent - could we use that, perhaps? Fairly compatible. But ideal
would be to get a Task or an ExecutorTask or a ProgressHandle or something so
you could track progress properly.
Comment 1 Jesse Glick 2006-03-25 15:54:54 UTC
Since the actions like Build etc. work so poorly on groups due to this issue,
and since this kind of functionality is usually better invoked from a parent
project anyway, I am removing these actions from the groups module:

committed   * Up-To-Date  1.9         contrib/poorMansPortfolios/manifest.mf
committed   * Up-To-Date  1.13       
contrib/poorMansPortfolios/src/org/netbeans/modules/project/portfolios/PortfoliosTopComponent.java
Comment 2 Milos Kleint 2007-02-05 08:59:48 UTC
jesse, is this still relevant?
Comment 3 Jesse Glick 2007-02-05 18:17:37 UTC
Still a valid enhancement, though priority is I think lower.
Comment 4 Milos Kleint 2008-10-14 13:31:25 UTC
what would be the currently relevant api clients?
Comment 5 Jesse Glick 2008-11-07 03:28:25 UTC
I can't think of any current potential users. I thought there was something else that needed it, but I just grepped for
"71515" and did not find anything.
Comment 6 Antonin Nebuzelsky 2010-03-29 14:10:10 UTC
Change of default owner.
Comment 7 Jesse Glick 2010-05-06 19:20:07 UTC
Needed for a proper fix of #185429 (see a171a47f3b25).
Comment 8 Jaroslav Tulach 2010-05-07 00:21:22 UTC
Based on my fix of bug 185429 I'd suggest to create a static method:

pu. st. void invokeActionSynchronously(ActionProvider ap, String cmd, Lookup ctx)

such method would do the same think I did - e.g. create a ProxyLookup with ActionEvent("waitFinished"). The method should detect that the provider does not support synchronous execution (getSource()[0] not changed) and in such case log a warning with identification of the provider.
Comment 9 Martin Fousek 2011-12-22 13:24:15 UTC
+1 for Coherence module
Comment 10 Jesse Glick 2011-12-22 15:01:16 UTC
Will see if I can do this soon.
Comment 11 Jesse Glick 2012-03-13 00:04:31 UTC
Created attachment 116638 [details]
Start of patch with ActionProvider2

A cleaner API going forward - builds blocking nature directly into the SPI, and elininates the awkwardness of three separate methods when just one would suffice - but cumbersome to convert existing ActionProvider implementations due to the different control flow.
Comment 12 _ tboudreau 2012-03-13 00:32:30 UTC
I'm reaching the conclusion that Future is almost always evil;  it says to the caller "please!  block on me!".  Almost any place that you have the signature

public Future<T> doSomething() {
}

you could replace it with something like

public void doSomething (Callback<T> callback);
static abstract class Callback<T> {
   void callback(T val);
   <E extends Exception> void onFail(E thrown) throws E { throw thrown; }
}

(parameterizing on an exception is a little weird, but it allows the same code to handle rethrow of runtime and checked exceptions without special-cases for each).


While you can still write a Callback which could, say, call this.notifyAll(), and the caller could block on that, an API like that doesn't give the impression that the natural way to use it is to block on it.

Or at the least, wrap all Futures in a wrapper instance which will throw an exception if waited on in the event thread.
Comment 13 Jesse Glick 2012-03-13 14:02:47 UTC
I am already working on an alternate API involving a callback/listener of a kind. Though at first blush this seems like it would be less work to use in an existing ActionProvider.invokeAction implementation, the situation is complicated by the fact that many such implementations reschedule their work into a request processor, yet it is necessary to call a start method on the listener within the dynamic scope of the method... so while easier than returning Callable it is not trivial. Will post a new patch when I have it working.
Comment 14 Jesse Glick 2012-03-13 17:52:59 UTC
(In reply to comment #7)
> Needed for a proper fix of #185429 (see a171a47f3b25).

Actually 43e85fce76d0 (bug #198275) removed the call site of this, so the ActionEvent trick was dead code; will just replace it with the standard API.
Comment 15 Jesse Glick 2012-03-13 19:38:35 UTC
Created attachment 116685 [details]
Rewritten patch based on ActionProgress
Comment 16 Jesse Glick 2012-03-13 19:41:00 UTC
Please review the revised patch. The SPI seems fairly straightforward to support from existing ActionProvider implementations.

Since the only API caller so far is the Coherence module, which I probably cannot even test, I will work on implementations of bug #206208 and/or bug #64991 to verify that everything works together naturally.
Comment 17 J Bachorik 2012-03-13 21:39:54 UTC
Would it be possible to smuggle in an indication that the action has not been executed at all? Eg. when you call Run/Debug/Profile on a class without a valid main method the BaseActionProvider actually skips the execution and displays a warning about missing main method. It would be great if an invoker of AP.invokeAction() could get back the information that the action was not, in fact, executed.
Comment 18 Jesse Glick 2012-03-13 21:58:20 UTC
Created attachment 116689 [details]
Fix of bug #64991 using this API
Comment 19 Jesse Glick 2012-03-13 22:04:30 UTC
(In reply to comment #17)
> Would it be possible to smuggle in an indication that the action has not been
> executed at all?

That is what the caller may infer when started() is never called. (Or else the provider just does not support #71515 yet; there is no way to differentiate those cases.)

> when you call Run/Debug/Profile on a class without a valid
> main method the BaseActionProvider actually skips the execution and displays a
> warning about missing main method.

Right. In the current patch in this circumstance I think BAP will call started() followed by finished(false), but the Javadoc permits it to call neither, which would probably be simpler to implement... so if that is useful to you I will do it that way.

BTW the patch for #64991 does not work to enable Project [Main] Project on a multiselection. Not sure if that is due to the use of ProjectActionPerformer, which cannot deal with multiselections. Anyway this is not a serious use case; the point is to be able to use Test 2 Projects, Generate Javadoc for 2 Projects, Clean & Build 2 Projects, etc., which all work.
Comment 20 Martin Fousek 2012-03-14 06:45:47 UTC
(In reply to comment #16)
> Since the only API caller so far is the Coherence module, which I probably
> cannot even test, 

The patch looks well from Coherence point of view. Thanks Jesse.
Comment 21 J Bachorik 2012-03-14 09:49:34 UTC
(In reply to comment #19)
> (In reply to comment #17)
> > Would it be possible to smuggle in an indication that the action has not been
> > executed at all?
> 
> That is what the caller may infer when started() is never called. (Or else the
> provider just does not support #71515 yet; there is no way to differentiate
> those cases.)

What I need is to get a notification that the action is actually to be executed so I can initialize the profiler infrastructure. However, I think I might be able to infer such situation from the already available information.

> 
> > when you call Run/Debug/Profile on a class without a valid
> > main method the BaseActionProvider actually skips the execution and displays a
> > warning about missing main method.
> 
> Right. In the current patch in this circumstance I think BAP will call
> started() followed by finished(false), but the Javadoc permits it to call
> neither, which would probably be simpler to implement... so if that is useful
> to you I will do it that way.
> 
> BTW the patch for #64991 does not work to enable Project [Main] Project on a
> multiselection. Not sure if that is due to the use of ProjectActionPerformer,
> which cannot deal with multiselections. Anyway this is not a serious use case;
> the point is to be able to use Test 2 Projects, Generate Javadoc for 2
> Projects, Clean & Build 2 Projects, etc., which all work.

Profiling multiple projects doesn't make sense either so I am fine with it. But what I find missing is the integration of ActionProgress into Rerun action in ant based projects. Currently, if you re-run an ant based action you will not receive any notifications, right?
Comment 22 Jesse Glick 2012-03-14 15:12:55 UTC
(In reply to comment #21)
> what I find missing is the integration of ActionProgress into Rerun action in
> ant based projects. Currently, if you re-run an ant based action you will not
> receive any notifications, right?

Right, because this action was not launched by the caller, so there is no reason to send notifications about it. ActionProgress is simply a workaround for the fact that ActionProvider.invokeAction as originally designed does not block or return Future. Once the original action has completed, it is irrelevant whether the user chooses to rerun some Ant target later.

I am not really sure what your use case is, but I do not think this API is what you are looking for. Anyway I thought you were using #206196 to run profiler infrastructure?
Comment 23 J Bachorik 2012-03-15 12:50:26 UTC
(In reply to comment #22)

Probably this will be a bit off-topic WRT the API review but anyway
> (In reply to comment #21)
> > what I find missing is the integration of ActionProgress into Rerun action in
> > ant based projects. Currently, if you re-run an ant based action you will not
> > receive any notifications, right?
> 
> Right, because this action was not launched by the caller, so there is no
> reason to send notifications about it. ActionProgress is simply a workaround
> for the fact that ActionProvider.invokeAction as originally designed does not
> block or return Future. Once the original action has completed, it is
> irrelevant whether the user chooses to rerun some Ant target later.
> 
> I am not really sure what your use case is, but I do not think this API is what
> you are looking for. Anyway I thought you were using #206196 to run profiler
> infrastructure?

The #206196 is used to simplify starting the profiled application. But we need to start the so-called profiler client as well. The client then attaches to the started application and displays the live results etc. I can not rely on the #206196 infra for starting the profiler client because it fails when re-runing profiling on ant based projects - the Rerun action implementation doesn't use startup extenders and directly executes the last used ant target with cached properties. 

Of course, I can solve this with a custom ant task but I would rather avoid it if possible.
Comment 24 Jesse Glick 2012-03-20 00:08:42 UTC
(In reply to comment #23)
> I can not rely on the
> #206196 infra for starting the profiler client because it fails when re-runing
> profiling on ant based projects - the Rerun action implementation doesn't use
> startup extenders and directly executes the last used ant target with cached
> properties.

Earlier I suggested you file this as an issue blocking #206196. Either way, ActionProgress is not intended for this kind of use case - it is off topic here.

> I can solve this with a custom ant task

Which is how Debug Project is currently implemented: <nbjpdastart> generates a server port number to pass to the target JVM, and in parallel launches the IDE client. StartupExtender would probably be a cleaner solution, though.
Comment 25 Jesse Glick 2012-03-20 02:51:15 UTC
Created attachment 116895 [details]
Minor updates to main patch

Planned for tomorrow.
Comment 26 Jesse Glick 2012-03-20 02:52:46 UTC
Created attachment 116896 [details]
Updated patch for #64991
Comment 27 Jesse Glick 2012-03-20 02:53:41 UTC
Created attachment 116897 [details]
Patch for #206208
Comment 28 Jesse Glick 2012-03-20 14:47:55 UTC
core-main #f6c1c3eba99c
Comment 29 Quality Engineering 2012-03-24 10:59:36 UTC
Integrated into 'main-golden', will be available in build *201203240400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f6c1c3eba99c
User: Jesse Glick <jglick@netbeans.org>
Log: #71515: API for blocking project actions.
Comment 30 Quality Engineering 2012-03-25 09:44:11 UTC
Integrated into 'main-golden', will be available in build *201203250401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f6c1c3eba99c
User: Jesse Glick <jglick@netbeans.org>
Log: #71515: API for blocking project actions.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo