Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 154427 - Request for a standard API to provide a substitute during open
Request for a standard API to provide a substitute during open
Status: RESOLVED WONTFIX
Product: projects
Classification: Unclassified
Component: Generic Infrastructure
6.x
All All
: P3 (vote)
: 6.x
Assigned To: apireviews
issues@projects
: API_REVIEW_FAST
Depends on:
Blocks: 153937 154353 156396
  Show dependency treegraph
 
Reported: 2008-12-03 09:02 UTC by Jaroslav Tulach
Modified: 2009-02-19 22:56 UTC (History)
3 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
API changes, test, documentation (21.29 KB, patch)
2008-12-03 09:09 UTC, Jaroslav Tulach
Details | Diff
Simplified usage in ergonomics (3.73 KB, patch)
2008-12-03 09:09 UTC, Jaroslav Tulach
Details | Diff
Patch supporting delegation without use of reflection (8.81 KB, patch)
2008-12-22 10:25 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2008-12-03 09:02:39 UTC
The work on Ergonomics IDE[1] done so far reveals need more supported way of replacing a project with its proper 
substitute while the former is being opened. Here is a little enhancement that achieves that with the help of 
ProjectInitializationHook.
Comment 1 Jaroslav Tulach 2008-12-03 09:09:00 UTC
Created attachment 74455 [details]
API changes, test, documentation
Comment 2 Jaroslav Tulach 2008-12-03 09:09:37 UTC
Created attachment 74456 [details]
Simplified usage in ergonomics
Comment 3 Milos Kleint 2008-12-03 10:22:54 UTC
MK1: -1 on the whole concept. Open project list is just intended for representing the list of opened projects, not
mutate them. The ultimate source of project instances is ProjectManager. The instances of projects need to match or we
will get different instance of project via OpenProjects and via FileOwnerQuery, causing trouble all over the IDE.

Comment 4 Milan Kubec 2008-12-03 14:08:12 UTC
I don't like it either, we are adding one more possible source of problems during opening project phase. What is real
performance benefit of using ProjectInitializationHook?
Comment 5 Jaroslav Tulach 2008-12-04 09:01:07 UTC
Re. MK1. Right, the Project instances provided by ProjectManager and OpenProjects shall match.To ensure that in the 
proposed system, I suggest to add 

assert p == ProjectManager.getDefault().findProject(p.getProjectDirectory());

after the calls to doInitializeProject. That imho addresses the possible inconsistency issue appropriately.
Comment 6 Jesse Glick 2008-12-04 16:00:28 UTC
[JG01] This should not be a part of the public API. A friend API from the projectui module would be better.


[JG02] "if (np == null) return np;" is poor style. Return null explicitly if that is what you mean.


[JG03] What happens to projects with invalid metadata? Is the behavior of the Open Project dialog (i.e. to display the
error) unchanged?


[JG04] Why the change to ProjectOpenedHook.java, specifically why is this an @Override?


[JG05] I don't follow how the proposed assertion to satisfy MK1 would be helpful. Even if true, there may have been
other calls to PM.fP _prior_ to the project being opened. As I understand it (and I have not seen a very clear
explanation of what FeatureProjectFactory is doing), this would mean that there would be two distinct active Project's
in memory for the same project directory, which would break a very basic invariant in the project system and cause
unknown problems.

It seems to me that the simpler approach, which would require no API changes that I know of, would be for the FPF to
create a proxy Project which would remain active for the remainder of the IDE session or GC, delegating all functions to
the Lookup of the real Project once that is loaded.

Of course anyone casting a Project instance to a specific type would find their code broken, but that has always (since
4.0) been threatened as a possibility; correct code using Lookup would not be affected.


[JOKE01] Your patch assumes that p = np which has not yet been proven.
Comment 7 Milos Kleint 2008-12-08 20:43:54 UTC
I would prefer any solution not to mess project instances read by ProjectManager. Unlike jglick, I don't think we can
freely start using Project proxies, even though we warned in 4.0 we might. We've passed the point where we can actually
enforce it IMHO. Apart from straight hacky code, like casting and reflection calls, there's also objects in lookup being
passed the 'inner' project instance around, which could be eventually  compared to a given 'outer' project instance. I
believe i've seen it more than once in our codebase.

The only save viable solution to me seems to have connections points in 1. new project wizard, 2. open project dialog
and 3. possibly in ProjectFactory.isProject(). the last one is tricky and if FoD incorporates any UI to enable features,
it could end up in randomly popping up dialogs as projects are found. And the project can actually be found at rather
random occations. One example to demonstrate.

on project popup, the version control actions perform FileOwnerQuery query on all subfolders of the current project's
folder and if a project is found there, use the SharabilityQueryImplementation present in the child project's lookup for
triggering enabled/disabled state, similarly they do when "Show changes" dialogs are shown. Without a proper (real)
project backing up the location, we might end up in adding/committing user files that are not meant to be shared.


Comment 8 Jaroslav Tulach 2008-12-11 20:56:41 UTC
Thanks for your comments. If I understand correctly, we all see two options:
1. delegate and proxy
2. create hooks to trigger the right modules.
Currently I am fully relying on option #1. I have proxy which delegates as much as possible to the real project, as 
soon as it is loaded. Things seem to work, just I am not really fond of the reflection used to call projectOpened() 
protected method and also I do not like that the instance returned from OpenProjects.getOpenProjects() is the fake one 
not the real one. I used to (and I still can if necessary) call OpenProjects.close and OpenProjects.open to replace 
the instances, however that seems to me too dynamic, stateful and errorprone. That is why I create the patch 
http://www.netbeans.org/nonav/issues/showattachment.cgi/74455/api.diff - it adds the only one missing (we have project 
wizards covered in completely different way) hook - it intercepts all calls to OpenProjects.open and makes sure that 
the fake project never appears there. It adds new interface, so previous usages remain unmodified. I agree with JG01 
that this does not need to be public API, if that is desirable, I can hide the API a bit.

I am willing to use both approaches in ergonomics: I will delegate as much as I can from the fake to the real project. 
However I'd also welcome an API to prevent my fake projects to be ever listed in OpenProjects.getOpenProjects - that 
can only contribute to the overall stability of the system.
Comment 9 Jesse Glick 2008-12-15 14:39:13 UTC
"I am not really fond of the reflection used to call projectOpened() protected method" - I would recommend just making
this method public. It is a general antipattern in our code that we try to prevent people from calling things from the
wrong direction, only to find later that in fact this is occasionally necessary for transparent proxying. In fact I know
it is common to want this to be public from unit tests.


"I do not like that the instance returned from OpenProjects.getOpenProjects() is the fake one not the real one" - yes
this is undesirable, and I agree with mkleint that it can cause issues with buggy modules. I just think these issues are
solvable as straightforward bug fixes, and it is _less dangerous_ than having both the fake and real Project instances
appear in the system at different times.

What I was trying to say earlier, and perhaps didn't express well: it is a basic invariant of the project system that
for a given project directory, there is at most one valid Project instance. The project system is layered so that non-UI
code (depending on projectapi) uses ProjectManager.findProject and expects that projects are loaded on request,
sometimes implicitly by e.g. FileOwnerQuery; whereas UI code (depending on projectuiapi) additionally uses OpenProjects
to see which projects are "open", meaning indicated by the user as somehow active and salient. Code which does not
specifically need to know about "openness" (including most queries) can and should ignore OpenProjects. If PM.fP reports
a Project instance for a given directory, even if that project is not open, this (usually lower-level) code will begin
using that Project (and the services in its Lookup); any substitute placed in OpenProjects later will probably not be
seen, and if both start to be used in parallel, it is hard to predict what chaos will result. That is why I feel more
comfortable with _consistent_ use of proxying, i.e. the "real" Project is never offered from ProjectManager.
Comment 10 Jaroslav Tulach 2008-12-22 10:25:12 UTC
Created attachment 75238 [details]
Patch supporting delegation without use of reflection
Comment 11 Jaroslav Tulach 2008-12-22 10:38:50 UTC
I guess we are still oscillating between attempt to 
1. delegate and proxy
2. create hooks to trigger the right modules.
Both approaches are possible and OK from my point of view. 

The patch for #2 is http://www.netbeans.org/nonav/issues/showattachment.cgi/74455/api.diff attached since beginning. 
If necessary its API can even be more hidden, so it is not available for non-friend API users.

Anyway Jesse seems to prefer solution #1. That is why I've just created patch to support that approach: 
http://www.netbeans.org/nonav/issues/showattachment.cgi/75238/ErgonomicsIncrementalOpenProject.diff
It is surprisingly simple - it just expects that the amount of ProjectOpenHooks can increase while one of them is 
called. If others are fine with #1, I am fine with this patch as well.
Comment 12 Milos Kleint 2008-12-22 14:41:44 UTC
Just for the record I don't wish there to be any commits to projects codebase without my in-depth review. Since I'm on
vacation now till end of Dec, the usual 1 week period doesn't apply here
Comment 13 Milos Kleint 2009-01-08 07:11:28 UTC
while your ProjectOpenHook based patch is probably technically correct, I don't understand what you need it for. A
Lookupmerger<ProjectOpenHook> in FOD mdule shall do the same job as well. Please state your usecase. What are you
planning to inject within the ProjectOpenHook?

At the same time I'm worried that you actually need something like that when doing #1 (proxying). It appears that you
are not really proxying at the point when the project is not opened yet..
Comment 14 Jaroslav Tulach 2009-01-08 09:07:50 UTC
I see. LookupMerger works: http://hg.netbeans.org/ergonomics/rev/ebc8ce972a81
It is slightly complicated to use it. I am not sure how to not specify path on SFS to read instances from, but things 
seem to work and there is no need for reflection anymore. Patch 
http://www.netbeans.org/nonav/issues/showattachment.cgi/75238/ErgonomicsIncrementalOpenProject.diff
is no longer needed.

I still leave this issue opened to discuss 
http://www.netbeans.org/nonav/issues/showattachment.cgi/74455/api.diff
and co.
Comment 15 Milos Kleint 2009-01-08 09:13:50 UTC
http://www.netbeans.org/nonav/issues/showattachment.cgi/74455/api.diff is conceptually wrong as has been discussed
before. What is there to discuss?
Comment 16 Jaroslav Tulach 2009-01-09 08:57:02 UTC
Per our yesterday's discussion I've just suppressed warning in ProjectsRootNode in case of ergonomics projects:
6749788f0206
As far as I am aware this was the only outstanding issue where == or instanceof or cast caused visible problems. But I 
will do audit of our sources later. Anyway this issue can be closed as wontfix.


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