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 134341 - ClassPathProviderMerger do not really merge the resulted ClassPath of CPProvider.
Summary: ClassPathProviderMerger do not really merge the resulted ClassPath of CPProvi...
Status: RESOLVED FIXED
Alias: None
Product: projects
Classification: Unclassified
Component: Generic Infrastructure (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Milos Kleint
URL:
Keywords: API, API_REVIEW_FAST
: 129439 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-05-01 18:33 UTC by lforet
Modified: 2008-05-20 15:57 UTC (History)
2 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
suggested api change along with implementation (27.51 KB, patch)
2008-05-12 13:08 UTC, Milos Kleint
Details | Diff
improved hg export of api change revisions (40.16 KB, patch)
2008-05-19 11:06 UTC, Milos Kleint
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description lforet 2008-05-01 18:33:00 UTC
ClassPathProviderMerger do not really merge the resulted ClassPath of CPProvider. 
Indeed, it seems that for a given FileObject the first convenient CPProvider found is returned and then the CPProvider
won't return the most exhaustive ClassPath.

A better behaviour is to merge the Classpath given by each CPProvider.
Comment 1 Milos Kleint 2008-05-12 13:08:14 UTC
Created attachment 61255 [details]
suggested api change along with implementation
Comment 2 Milos Kleint 2008-05-12 13:16:25 UTC
please review this change to java support apis. A single api method is added which creates a LookupMerger implementation
for ClassPathProvider instances. The resulting classpath is a merge of content from all providers.
Comment 3 Jesse Glick 2008-05-12 18:07:13 UTC
[JG01] Please do not use reflection to access ClassPath.impl. Either add an API to ClassPath; or do not try to get the
original ClassPathImplementation (I don't think you really need it, since you can already listen to a ClassPath and ask
for its entries).

[JG02] Is there a real need to have a defaultProvider? Can't you just put that default provider in your project lookup,
if you in fact have one?

[JG03] 'List semaphor' lacks a generic type (and is also misspelled). Could probably more clearly be replaced with an
AtomicInteger.
Comment 4 Milos Kleint 2008-05-13 14:22:25 UTC
jg01: I think I need the classpathimplementation. Entries won't help because I need to create a new instance of
PathResourceImplementation then and I have no way to reconstruct what should be the content (eg. if filtering is in
place or not)
in this context the reflection still seems as best solution.

jg02: the default provider is meant to make sure the project's own provider is always asked first and it's results are
first as well. the way lookup works currently I think I would get the same result as well, but it's not documented
anywhere (AFAIK) how the order in lookup results is  determined.. The layered ones should be ordered by content in
layer, that's for sure.

jg03: will do.
Comment 5 Jesse Glick 2008-05-13 16:18:19 UTC
I still don't see any compelling reason why you need the original PRI. Everything of interest is available through the
regular ClassPath API; mainly, the list of roots. In the case of includes, ClassPath.Entry.contains(String) directly
calls the FPRI.

(The distinction between CPI and PRI was just a mistake in the API; there is no difference that I know of between having
5 PRIs, each with one constant root, and firing PROP_RESOURCES, vs. having 1 constant PRI with 5 roots and firing
PROP_ROOTS.)
Comment 6 Milos Kleint 2008-05-14 09:41:29 UTC
jg01: well, without accessing the original CPIs, I'm forced to insert proxy FCPIs for every ClassPath entry that will
just delegate to the original.
the result will be longer stacks and increased memory footage, without any clear advantage. so the whole chain will look
like this (from outer most instance)
CP->FCPIs->CP-CPIs

and if I move the test that checks for the impl field to the ClassPathTest, then the reflection solution is also rather
safe to make.
Comment 7 Jesse Glick 2008-05-14 15:43:03 UTC
Regarding JG01: you only need one CPI and one FPRI per ClassPath.

If you still believe you need to access the original CPI, please do not use reflection. Introduce a proper API.

For that matter, is there any particular reason you are not using ClassPathSupport.createProxyClassPath(ClassPath...)?
Seems to me that this already does most of what you need. (You could still listen on Lookup changes in ClassPathProvider
if you want; it is not clear if this is a requirement.)
Comment 8 Milos Kleint 2008-05-19 11:06:20 UTC
Created attachment 61551 [details]
improved hg export of api change revisions
Comment 9 Milos Kleint 2008-05-19 11:09:35 UTC
the attached patch addressed JG01 and avoids using reflection for the price of adding more middlemen in the callstack.


Thanks for the review, I'd like to apply the change tomorrow.
Comment 10 Jesse Glick 2008-05-19 16:10:46 UTC
BTW you can just return null for PRI.getContent(), the method is useless.
Comment 12 Milos Kleint 2008-05-20 08:34:37 UTC
*** Issue 129439 has been marked as a duplicate of this issue. ***
Comment 13 Quality Engineering 2008-05-20 15:57:42 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #207 build
Changeset: http://hg.netbeans.org/main/rev/0578533e5301
User: Milos Kleint <mkleint@netbeans.org>
Log: #134341 additional api factory method to create a LookupMerger for ClassPathProvider implementations. Make use of it in j2se project.