This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 124980 - Java's ClassIndex based features may be broken in Go To Type dialog
Summary: Java's ClassIndex based features may be broken in Go To Type dialog
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Navigation (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: apireviews
URL:
Keywords: API_REVIEW_FAST
Depends on: 125378
Blocks:
  Show dependency tree
 
Reported: 2008-01-09 16:49 UTC by Jan Lahoda
Modified: 2008-02-18 12:55 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Proposal for new API that allows waiting for opening/closing to finish (17.21 KB, patch)
2008-01-23 09:33 UTC, Jaroslav Tulach
Details | Diff
patch file against hg repo for projectui (12.97 KB, text/plain)
2008-01-30 14:19 UTC, Milan Kubec
Details
patch file against hg repo for projectuiapi (5.61 KB, text/plain)
2008-01-30 14:20 UTC, Milan Kubec
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2008-01-09 16:49:35 UTC
[recent sources]

After recent changes, the ProjectOpenedHooks are run asynchronously - nice. But I see quite a big problem. Consider the
following scenario:
-the user opens a lot of projects and stops the IDE
-the user installs a new version of NB which uses a different format of Java caches - the caches created during previous
runs of the IDE are therefore ignored (please note that the format of the caches may change at any time, we do not
guarantee any stability)
-the IDE starts, an editor is opened and the users invokes a feature that requires Java's ClassIndex (e.g. Go To Type,
smart completion, etc.). But there is no guarantee that the ProjectOpenedHooks have been run for all projects at this
point, so the GlobalPathRegistry (in java/api module) may not contain all the classpaths, and so the Java caches are not
guaranteed to be fully created (and the Java infrastructure might not even know that any project is opened, as the GPR
may be empty). So the user may get incomplete or inconsistent results.

Notes (to my knowledge):
-I do not see a way how to solve this (in the current state) in java/source or java/editor
-so far, it was recommended that the project types should register the classpath in the ProjectOpenedHook (the GPR
javadoc even states this)
-I think that, from java/source point of view, this is a semantically incompatible change in GPR
-the GPR itself is only a holder for classpath, so I do not see how it could solve this problem internally.

The most reasonable solution I see is to let Java infrastructure know that there are any POHs scheduled, which were not
executed yet (e.g. using a listener), so it can pretend the classpath scanning is running until all POHs are executed.
But any other solution is welcome. This should be well documented, for the advantage of other language supports.

I am filling this as a P1, as this might be a pretty bad experience for the user (and it seems like an incompatible API
change to me).
Comment 1 Jaroslav Tulach 2008-01-09 20:21:48 UTC
A note from:
http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-projectuiapi/org/netbeans/spi/project/ui/ProjectOpenedHook.html
says:

---
The meaning of these terms is intentionally left vague, but typically opening a project signals that the user may wish 
to work with it, so it would be a good idea to make sure caches are up to date, etc. It is perfectly possible to load 
and use (even run) projects which are not open, so any project type provider using this hook cannot rely on it for 
basic semantics.
---

I agree delaying the calls to OpenProjectHook is incompatible change. However change permitted by the API since its 
creation. I do not think this change can be attributed as P1 bug in projects/ui. Imho it is even not a bug, it is just 
a change.
Comment 2 Jan Lahoda 2008-01-09 21:36:04 UTC
Well, I know about the note in the POH's Javadoc, but:
-this is a note for the project type implementors, not for the language support - Java (and other language supports) do
not use this class. So it is not relevant to us, IMO.
-I think that this is a real problem for users
-this cannot be solved in java/source or java/editor or anywhere else in the Java infrastructure, to my knowledge
-I (as the initial evaluator for Java infra and Java editor bugs) will be the one who will loose time on evaluating bugs
resulting from your change, leading into my (our) inability to fix other functional or performance problems - but I/we
are not responsible for this, sorry.

You did the change, which broke the IDE for user - please propose a way to fix it. We (in Java infra) cannot. This is
more or less issue between Projects API and individual project types.
Comment 3 Tomas Zezula 2008-01-09 21:42:14 UTC
I fully agree with Honza. This is semantically non compatible API change that affects all the java/api users
(GlobalPathRegistry).
Comment 4 Tomas Zezula 2008-01-09 21:44:04 UTC
I fully agree with Honza. This is semantically non compatible API change that affects all the java/api users
(GlobalPathRegistry).
Comment 5 Jesse Glick 2008-01-09 22:09:58 UTC
Why is this a special issue? The user can at any time open a Java file with File > Open File (for example) and it is
expected to work, with the project unopened and GPR empty. GPR is just a hint as to which classpaths may be relevant for
existential queries; Go to Type may not be fully populated until the hooks have run and classpaths have been scanned,
but code completion etc. ought to work immediately (or show a Scanning message) regardless of project open hook timing.
Comment 6 Jan Lahoda 2008-01-09 22:22:02 UTC
Smart CC needs the ClassIndex ready in some cases to find all subclasses of a class. The "all" CC also needs it to find
all available classes for a given prefix.

But we do not need to talk about CC: the GPR specifically speaks about "Fast Open" (i.e. Go to Type) as one of the
intended usage patters, and there is a regression in this feature due to this commit.
Comment 7 Jesse Glick 2008-01-09 22:38:09 UTC
Well, you may have to wait a bit for Go to Type (~ Fast Open) to work. In exchange, you get to do other things
immediately. If you want to wait for as long as it used to take to start up, then GtT should be ready.

For code completion, you can use the classpath of the file being edited; there is no need for GPR at all.
Comment 8 Jan Lahoda 2008-01-09 22:55:09 UTC
Well, you may have to wait a bit for Go to Type (~ Fast Open) to work: *the user* needs to wait an arbitrarily long time
before he/she gets correct results, and there is currently no way he/she could be informed that the results are
incomplete. Sorry, but this is simply wrong.

I am probably missing something really obvious in the CC case: the CC (including "all" CC) answers (often) in a second
or so. If the classpath is completely unknown (and so it was never scanned, which is more or less what happens), how can
we make it answer that fast? I mean, how to implement query like "give me all classes whose simple name starts with
'Stri'" for a source root that is seen for the first time, and make this query finish in a second?

I was thinking about this more and more things may be broken by this - but it all boils down to the same thing - the
ClassIndex (and GPR) is no up-to-date, and the Java infra has no way to find this out.

Moreover: I do not see anything in the GPR javadoc that would specifically say it is a hint.

Please note that I do not ask for rollback of this commit - I am asking for constructive solution of this problem.
Comment 9 Tomas Zezula 2008-01-09 23:01:12 UTC
Neither all completion nor the smart completion can work in a fast way when there are no caches (ClassPaths weren't
registered in the GPR). They need metadata (list of all classes, inheritance tree) which are created during the
background scan and are not very cheap to create.
Comment 10 Jaroslav Tulach 2008-01-09 23:12:36 UTC
So far I have heard just speculations about what can get broken. Anyone knows a case that really is broken?
Comment 11 Jan Lahoda 2008-01-09 23:22:25 UTC
OK:
-(a few hours ago) I have created a module that slowed down POH for J2SE Projects - and the Go to Type really broke (I
had two projects opened, cleared the caches, started the IDE and saw references only from the first project in the GtT)
-it is quite likely that something similar will happen for apisupport projects, because the POH of it is likely to be
very slow. Feel free to check.
-it is quite possible that no real issues will appear for J2SE Projects and many other projects types because they POHs
are likely to be fast - but I do not think that "likely" is enough in this case. From what I know, it may break.
Comment 12 Jesse Glick 2008-01-09 23:24:29 UTC
Go to Type is already wrong if you opened a file whose project is not open. Anyway this is just a shortcut for locating
the desired class in the Projects tree.

If CC is invoked on a file whose ClassPath.getClassPath includes as-yet-unscanned entries, then these should be scanned,
a progress bar displayed, and "Scanning" shown in the popup until it is done. Obviously this will not happen in a
second. Nor does CC in 6.0 if you open a file from a project which was just opened; you have to wait for the IDE to
finish scanning.

GPR has always been intended as a supplemental heuristic API only, for code which needs to get a list of possibly
relevant classpaths without any clear starting point, where the primary API for getting exact information is
ClassPath.getClassPath.
Comment 13 Tomas Zezula 2008-01-09 23:40:31 UTC
>GPR has always been intended as a supplemental heuristic API only, for code which needs to get a list of possibly
>relevant classpaths without any clear starting point
As far as I remember the GPR was firstly used to let the editor to know for which roots it should build the old code
completion databases. When the javacore was integrated it was used for the same, to let the javacore to know for which
roots it should build the MDR storages. Retouche reused this pattern as well.
The code completion can invoke the background compilation when it firstly meets the ClassPath, but it will make the IDE
to perform in very fuzzy way.

Comment 14 Jaroslav Tulach 2008-01-10 12:32:44 UTC
Guys, it is really not wise to have conversations in issuezilla, imho. Better to chat, email or edit a wiki page.

As a response to my question, Jan raised a concern about GoTo Type dialog: True, it can contain wrong results. Let 
search for solution of this issue in: http://wiki.netbeans.org/wiki/view/NavigationImprovementsDesc

Before my question, I've heard also concerns about code completion, etc. However I think that they cannot be really be 
turned into a problematic usecase, as before showing code completion, the classpath scan for given source root has to 
finish and this is not affected by Projects Open Hook at all. That is why I do not consider that a problem. If you 
disagree, or you think you found another problematic issue, please create wiki page and add link into this issue.
Comment 15 Jaroslav Tulach 2008-01-11 10:00:51 UTC
I'll take your silence as an agreement with my latest post. We'll fix the Go To Type dialog for 6.1M1. Pavel will 
working on that, if he needs help, I'll be around. Also unless other problematic cases are found I adjust the IZ 
category to match Go To Type dialog source location.
Comment 16 Pavel Flaska 2008-01-17 11:36:18 UTC
Work still in progress. I do not think this is a blocker for M1. Changing priority.
Comment 17 Jaroslav Tulach 2008-01-23 09:33:30 UTC
Created attachment 55417 [details]
Proposal for new API that allows waiting for opening/closing to finish
Comment 18 Jaroslav Tulach 2008-01-23 09:35:31 UTC
As we discussed yesterday, here is an API to allow notification when list of open projects is changing. It works on 
start up, as well during regular open/close operations. Milan, Jan, Tomáš please review if this OK and satisfactory.
Comment 19 Pavel Flaska 2008-01-23 15:19:36 UTC
Implemented warning in "Go To Type" dialog. Other parts still in progress...

Checking in java/sourceui/src/org/netbeans/modules/java/source/ui/Bundle.properties;
/cvs/java/sourceui/src/org/netbeans/modules/java/source/ui/Bundle.properties,v  <--  Bundle.properties
new revision: 1.4; previous revision: 1.3
done
Checking in java/sourceui/src/org/netbeans/modules/java/source/ui/JavaTypeProvider.java;
/cvs/java/sourceui/src/org/netbeans/modules/java/source/ui/JavaTypeProvider.java,v  <--  JavaTypeProvider.java
new revision: 1.12; previous revision: 1.11
done
RCS file: /cvs/utilities/jumpto/src/org/netbeans/modules/jumpto/resources/warning.png,v
done
Checking in utilities/jumpto/src/org/netbeans/modules/jumpto/resources/warning.png;
/cvs/utilities/jumpto/src/org/netbeans/modules/jumpto/resources/warning.png,v  <--  warning.png
initial revision: 1.1
done
Checking in utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToPanel.form;
/cvs/utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToPanel.form,v  <--  GoToPanel.form
new revision: 1.6; previous revision: 1.5
done
Checking in utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToPanel.java;
/cvs/utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToPanel.java,v  <--  GoToPanel.java
new revision: 1.10; previous revision: 1.9
done
Checking in utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToTypeAction.java;
/cvs/utilities/jumpto/src/org/netbeans/modules/jumpto/type/GoToTypeAction.java,v  <--  GoToTypeAction.java
new revision: 1.15; previous revision: 1.14
done
Comment 20 Pavel Flaska 2008-01-25 10:59:48 UTC
That is the change I also need in modified "Go To Type..." java-type provider. I've used it in my prototyping work and
it quite fits in with my needs.
Comment 21 Milan Kubec 2008-01-30 14:19:44 UTC
Created attachment 55765 [details]
patch file against hg repo for projectui
Comment 22 Milan Kubec 2008-01-30 14:20:15 UTC
Created attachment 55766 [details]
patch file against hg repo for projectuiapi
Comment 23 Milan Kubec 2008-01-30 14:21:14 UTC
I've attached patch files of Jarda's change against hg repo updated to current sources.
Comment 24 Milos Kleint 2008-02-01 08:53:33 UTC
MK1: the new api assumes (and the attached test shows it) that anything that is called from the OpenProjectHook
implementations cannot use this new method safely. The documentation should somehow reflect it (both in OpenProjectList
and OpenedProjectHook).  

      protected void projectOpened() {
             opened++;
+            assertFalse("Working on", OpenProjects.getDefault().openProjects().isDone());
+            RequestProcessor.getDefault().post(this).waitFinished();
+            assertNotNull("some result computed", result);
+            assertEquals("It is time out exception", TimeoutException.class, result.getClass());
         }
       public void run() {
+            try {
+                result = OpenProjects.getDefault().openProjects().get(100, TimeUnit.MILLISECONDS);
+            } catch (Exception ex) {
+                result = ex;
+    }
Comment 25 Jaroslav Tulach 2008-02-06 09:52:06 UTC
Re. MK1: actually the test shows that while a project is opened, any other thread querying for the list of opened 
projects is blocked. This is imho already properly documented.
Comment 26 Milan Kubec 2008-02-06 14:44:54 UTC
API suggested by Jarda was commited:

http://hg.netbeans.org/main/rev/bc857e7522a0
Comment 27 Pavel Flaska 2008-02-18 12:55:52 UTC
Wait until opening finished.

changeset c0823af2f4e4 in main
details: http://hg.netbeans.org/main?cmd=changeset;node=c0823af2f4e4
description:
        Wait for lazy projects initialization.