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.
Summary: | ClassPathProviderMerger do not really merge the resulted ClassPath of CPProvider. | ||
---|---|---|---|
Product: | projects | Reporter: | lforet <lforet> |
Component: | Generic Infrastructure | Assignee: | Milos Kleint <mkleint> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | mkleint, pjiricka |
Priority: | P3 | Keywords: | API, API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Attachments: |
suggested api change along with implementation
improved hg export of api change revisions |
Description
lforet
2008-05-01 18:33:00 UTC
Created attachment 61255 [details]
suggested api change along with implementation
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. [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. 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. 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.) 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. 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.) Created attachment 61551 [details]
improved hg export of api change revisions
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. BTW you can just return null for PRI.getContent(), the method is useless. http://hg.netbeans.org/main/rev/0578533e5301 http://hg.netbeans.org/main/rev/3453765319d9 http://hg.netbeans.org/main/rev/267cb054ef11 and update of j2ee project types http://hg.netbeans.org/main/rev/bf86633cf312 *** Issue 129439 has been marked as a duplicate of this issue. *** 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. |