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 183344 - API Review: SPI allowing plugging of providers handling source groups in GoTo File Dialog
Summary: API Review: SPI allowing plugging of providers handling source groups in GoTo...
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Search (show other bugs)
Version: 6.x
Hardware: PC Mac OS X
: P2 normal (vote)
Assignee: Tomas Zezula
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks: 171672 182884
  Show dependency tree
 
Reported: 2010-04-01 11:58 UTC by Tomas Zezula
Modified: 2010-04-09 04:50 UTC (History)
3 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
API diff (15.50 KB, patch)
2010-04-01 12:07 UTC, Tomas Zezula
Details | Diff
Complete diff (64.08 KB, patch)
2010-04-01 12:08 UTC, Tomas Zezula
Details | Diff
Patch with VV1 fixed (66.98 KB, patch)
2010-04-02 07:19 UTC, Tomas Zezula
Details | Diff
API diff with FileDescriptor (21.31 KB, patch)
2010-04-06 12:06 UTC, Tomas Zezula
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Zezula 2010-04-01 11:58:55 UTC
A friend spi in the jumpto module allowing custom FileProviders to participate in the GoTo File.
Usecase: The cnd does not use indexing api (parsing.api) and has a custom indexing mechanism. Currently the MakeProject registers roots in the GPR and let the parsing.api to do a file index. This causes two thread doing intensive IO running concurrently and degrades performance. There is also high memory consumption caused by concurrent running of these two indexers. The cnd team requires a SPI to be able to provide their own GoTo File data and not to start the parsing.api.
Comment 1 Tomas Zezula 2010-04-01 12:07:41 UTC
Created attachment 96522 [details]
API diff
Comment 2 Tomas Zezula 2010-04-01 12:08:18 UTC
Created attachment 96523 [details]
Complete diff
Comment 3 Vladimir Voskresensky 2010-04-01 12:44:05 UTC
Thomas, SPI looks nice. I only want to confirm support of one of our usecases.
Makeproject has a possibility to "Add External File" => we track list of files which are out of registered SourceGroups of MakeProject.
Such files can be correctly found by "Select In->Project" action, because we own them and return from FileOwnerQueryImplementation

VV1: If I understands correctly current proposed impl skip such cases, because 
our GoToFile provider would be still able to return external files, then you call FileUtil.getRelativePath and it would return null for such pair {source-root, external-file} and you skip such entry.
We'd like to display them as well.
Comment 4 Tomas Zezula 2010-04-01 16:32:31 UTC
VV1: Right, this use case is not supported. I have todo in the patch, that we can add an abstract FileDescriptor (like in GoTo type) which will provide name, path, project name, etc. In addition to Result.addFile() there will be more generic Result.addFileDescriptor() which should solve your use case.
I will add it. Thanks for pointing me on this problem, I didn't know about such a files.
Comment 5 Tomas Zezula 2010-04-02 07:19:12 UTC
Created attachment 96569 [details]
Patch with VV1 fixed
Comment 6 Tomas Zezula 2010-04-02 07:23:16 UTC
VV1: I didn't add the FileDescriptor as it's probably not needed. I rather changed the Result.addFile() to accept both files under source group (SG) root and external files. I've fixed the Javadoc of the addFile and added an unit test. When the file is under SG root relative path is displayed in the GoTo File, if not absolute path is displayed.
Vladimir, is this OK for you? Thanks
Comment 7 Vladimir Voskresensky 2010-04-02 12:41:39 UTC
Now path would be displayed as absolute and it could be very long string not visible in dialog.
In fact in our structures external files are stored as relative path if it makes path length smaller, like "../file.c" instead of "/net/machine/export/home/user/Projects/file.c" => we already control the best "display name"
Another reason for adding SPI could be performance:
The current impl works in terms of FileObjects, but our indexer works in terms of relative/absolute paths => we prevent expensive conversion to FOs. We can provide FO only infrastructure wants to open selected item in list and needs FO.
Comment 8 Tomas Zezula 2010-04-02 12:43:43 UTC
OK, I will add the SPI as well.
Comment 9 Jesse Glick 2010-04-02 17:41:08 UTC
[JG01] Seems that computeFiles should return a CancelableTask or similar rather than having a separate stateless cancel method. Otherwise the provider would need to hold a reference to the Thread or Task or whatever that is currently running, which seems awkward.

Or Thread.interrupt could simply be called on the thread running computeFiles, which could then decide to call pendingResult and return early.


[JG02] Typo: testExtenralFile
                     ^^

[JG03] I wonder if an implementation using e.g. /usr/bin/mlocate could be used. However its index is never completely up to date, whereas the indexer cache would be more up to date once created. Perhaps computeFiles should be able to return false (proceed to next provider) yet add some results? Then you could get some possibly stale results quickly, followed a bit later by more accurate results if you want to wait.
Comment 10 Tomas Zezula 2010-04-02 18:18:40 UTC
JG01: The provider does not create any Task or Thread. It's dispatched by the GoTo File Worker which synchromously. The Provider just calculates files and checks if the cancel was called (using AtomicBoolean or  volatile boolean) from EDT (key typed, dialog closed). If so it returns.

JG02: Thanks fixed

JG03: Perhaps computeFiles should be able to return false (proceed to next provider) yet add some results.
This is exactly how it works. But the Lucene file index has currently the highest priority, only when it does not cover the root the others are called. But the order  is implementation thing and can be changed if needed.
Comment 11 Tomas Zezula 2010-04-06 12:06:47 UTC
Created attachment 96760 [details]
API diff with FileDescriptor
Comment 12 Tomas Zezula 2010-04-06 12:11:20 UTC
I've added FileDescriptor allowing complete custom implementation as Vladimir requested.
If it's OK for cnd I will integrate it tomorrow.
Comment 13 Vladimir Voskresensky 2010-04-06 12:17:50 UTC
thanks
Comment 14 Vitezslav Stejskal 2010-04-06 13:50:17 UTC
The API looks ok. Perhaps FileProviderFactory's javadoc could mention that instances of the factory are supposed to be registered via @ServiceProvider (ie. in the default Lookup).
Comment 15 Tomas Zezula 2010-04-07 07:57:50 UTC
I am going to integrate the SPI.
Comment 16 Tomas Zezula 2010-04-07 08:24:51 UTC
Fixed in jet-main: 9a59d1812167
Comment 17 Quality Engineering 2010-04-09 04:50:35 UTC
Integrated into 'main-golden', will be available in build *201004090201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/9a59d1812167
User: Tomas Zezula <tzezula@netbeans.org>
Log: #183344:API Review: SPI allowing plugging of providers handling source groups in GoTo File Dialog