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 155962 - Human friendly way to register XML property based convertor
Summary: Human friendly way to register XML property based convertor
Alias: None
Product: platform
Classification: Unclassified
Component: Options&Settings (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Jaroslav Tulach
Depends on: 156158
  Show dependency tree
Reported: 2008-12-22 21:00 UTC by Jaroslav Tulach
Modified: 2009-02-19 22:53 UTC (History)
1 user (show)

See Also:
Exception Reporter:

New annotation to add to objects willing to convert themselves to and from Properties (21.02 KB, patch)
2008-12-23 12:06 UTC, Jaroslav Tulach
Details | Diff
Improved patch with ability to do "readResolve" and its usage in TC wizard (38.48 KB, patch)
2008-12-30 12:30 UTC, Jaroslav Tulach
Details | Diff
Patch to integrate on Saturday (40.14 KB, patch)
2009-01-07 09:39 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-22 21:00:22 UTC
The semantics of registration of Convertors are beyond intellectual capabilities of majority of NetBeans API users. 
They require triple layer entries, all synchronized with each other and yet each slightly different. Even the example 
in the javadoc used to be wrong (fixed in 6.5). This is all very errorprone.

Yet there is simple fix. Let's use the new annotation processor infrastructure to make simple things really simple.
Comment 1 Jaroslav Tulach 2008-12-23 12:06:32 UTC
Created attachment 75267 [details]
New annotation to add to objects willing to convert themselves to and from Properties
Comment 2 Jaroslav Tulach 2008-12-23 12:10:24 UTC
1. if anyone comes better name for the annotation, perfect
2. the change is useful of its own
3. it would be nice if TopComponent's template used the annotation instead of "Resolver"
Comment 3 Jaroslav Tulach 2008-12-30 12:24:26 UTC
During testing I found bug 156158. It needs to be fixed before the convertors get widely used in TopComponent wizard.
Comment 4 Jaroslav Tulach 2008-12-30 12:30:42 UTC
Created attachment 75360 [details]
Improved patch with ability to do "readResolve" and its usage in TC wizard
Comment 5 Jiri Skrivanek 2009-01-05 14:07:17 UTC
I looked at it and haven't found anything wrong with it. Just make it buildable with JDK1.5.
Comment 6 Jesse Glick 2009-01-05 17:10:04 UTC
I would prefer that this effort be spent on creating a better way to persist TC state in general (using NbPreferences),
so we can stop using the settings module generally, but this at least seems better than what we have now.

[JG01] Import the annotation, rather than using FQN, in the template.

[JG02] Specifying a public ID for a nonexistent DTD in an annotation is bizarre. Can we fix settings to not require a
"DTD" at all? Failing that, could the annotation at least generate a "public ID" based on the FQN of the class?

[JG03] The patch in NbInstaller line 598 is unnecessary.

[JG04] I'm not really clear on why NbInstaller is being patched at all. Who is creating duplicate entries and why? How
is this related to the rest of the patch?

[JG05] The annotation belongs in the spi package, not api. Any API would be for requesting that objects be read or
stored, whereas the SPI is for defining how storage should work for your objects.

[JG06] apichanges fails to mention the change to readProperties semantics.

[JG07] For ignoreChanges, "all" (or the array {"all"}) should be a constant defined in the annotation.

[JG08] The AP should probably be checking roundEnv.processingOver().

[JG09] The AP should verify that the class contains valid read/writeProperties methods, or whatever it is the runtime
will need, in addition to the constructor.

[JG10] instantiableClassOrMethod seems to have a type arg which is always null. Delete.

[JG11] convElem[1] seems to be ignored. Is it even possible to use a methodvalue convertor? AFAIK it is not, and you do
not declare ElementType.METHOD anyway. So simplify this code.

[JG12] Patch to is unnecessary, do it separately.
Comment 7 Jaroslav Tulach 2009-01-07 09:29:00 UTC
JG01: OK.
JG02: I leave it as it is. The Public ID encode FQN: "-//org.myorg.pkg//ClassName//EN"
JG03,04: Duplicate entries are created for modules on classpath. Which happens especially in unit tests. It makes 
debugging hard and I think we want to eliminate the duplicates as soon as possible. Anyway, it is unrelated change to 
this issue, but I guess we want to do it.
JG05: The spi is for those who write own convertors. api is for those who use them. This annotation is use of existing 
convertor. That is how I convinced myself to put it into api package. Also it belongs next to already existing 
api.Factory (layer API).
JG06: OK.
JG07: OK. 

Code cleaned as suggested. Thanks for the review.
Comment 8 Jaroslav Tulach 2009-01-07 09:39:43 UTC
Created attachment 75527 [details]
Patch to integrate on Saturday
Comment 9 Jaroslav Tulach 2009-01-10 06:16:27 UTC
compiles OK in
Comment 10 Jesse Glick 2009-01-13 02:06:37 UTC
I still think JG02 is weird. The format of the file being written is in fact predetermined; you can put whatever value
you like as the attribute here, because there is no such DTD anywhere in existence. It is an unfortunate relic of the
Convertors architecture that a "DTD public ID" is required as a unique identifier for the convertor, but we can and IMHO
should hide this fact from annotation users, who already have a unique ID: the class being converted. (The settings
module should in this case call Utilities.translateNames to allow for the usual refactorings.)