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.
When you use the setting convertors XML registration you have to register each class you want to convert. You cannot register just a base class and have all the subclasses being converted by the convertor. I will supply a patch that changes this behaviour. The patch is really simple but it is an API change so I am asking for a review. Also there goes my usual question: can such a change be considered for NB 5.0? I beleive it does not break anything but I will have to write the tests to verify. Background information: as you can see from the Settings API Convertor document (linked from the URL field) the registration of the convertor is real pain in the ... Doing it only once for number of classes really saves you not only time but also leads to much fewer errors in the (virtually) non debugable file (layer.xml).
This is for fast trak of course.
Created attachment 25548 [details] Proposed patch (without tests & docs) for review
5.0 is feature frozen so no changes to core APIs are likely to be accepted unless they are required for solving a serious defect. Regarding this change: seems undesirable to me. A superclass cannot in general know what information its subclasses might want to store during serialization. Similarly, a convertor designed for a particular class may or may not be able to correctly handle subclasses that might have additional fields, properties, etc. I would want to see some concrete examples of why this enhancement is necessary, in particular addressing the questions: 1. If one convertor is capable of correctly handling the superclass and all possible subclasses, then why are subclasses even needed? What additional behavior do they have that does not need to be reflected in their persistent state? 2. If a third party wrote a new subclass which does add logical entries to state (e.g. a new bean property), how will the convertor respond?
I'd like to add that change like this is not fully compatible. We may seriously break behaviour of existing users of the api. Imho that is reason for two things: 1. it is a bit too late to integrate to 5.0 2. we should find a solution that will require active participation to use the feature If I were asked to design the change to address #2, I would suggest to use a special marker the convertor that wish to support subclasses. That marker could be either a java method or xml attribute. That way all existing convertors would continue to work unchanged and people still would have a way to write convertor for subclasses.
Jesse: 1. we have such a convertor (cabable of handling subclasses). I agree that it is not common/required for the convertor to be able to do so. The subclasses are of course needed to handle additional properties and behaviour. We use XMLPropertiesConvertor (patched) - the subclasses of our base class (Tool) can add whatever they want to the properties. 2. The convertor must be ready to handle that. Jarda: 1. Agree 2. Agree ;-) As I wrote previously - when I have shown the doc to the people that they should create the layer configs they asked me whether I was crazy or what. They said: let's create a common superclass for all our tools (top components) that will handle the registration. I said: impossible in current NetBeans, they said patch NetBeans for us to make it possible. More background info: we don't want to use serialization for tools (top components) because the persistent state is not user user readable ... but window system uses standard NetBeans way to store them so I had to use the mechanism built into InstanceDataObject. But it did not allow me to swap the implementation to something that I wrote - or did I miss something?
Re. last para: as far as I know you can completely swap out the basic impl. You just need a DataLoader which can recognize your custom serialization format and provide an InstanceCookie. Presumably it should listen to changes in the persisted object and write them to disk. Or, if you want to use *.settings files, you can still bypass the core/settings module by using the settings.convertor attr and memory/your/base/Clazz{settings.providerPath} in your layer and make an Environment.Provider and something with a "public void write(Writer w, Object inst) throws IOException" method. The details don't seem to be documented anywhere but I'm sure you can figure it out by looking at sources.
Jesse, first paragraph: No. You cannot. By providing loader you would be able to converto from SFS --> memory. But not the other way around. Second paragraph: No. You cannot. Please check my patch. It takes the name of your class and performs search for the provider. If the name of the class is a subclass of my provider it is never found. I admit I did this patch over a year ago - but looking at the sources right now it is still relevant since there is no way to convert from memory to SFS for the subclasses without some kind of patch. Unless I am missing something obvious the classes Env and IDO are the only places providing the conversion from xml/memory --> objects. If I am wrong can you please point me to some working example that would convert an object from memory to SFS in a different way (allowing me to swap the impl)? I agree with Jarda that the patch can be enhanced to provide the subclass search only for the classes that will request it by providing a special attribute to the registration file under xml/memory.
Is this issue still active?
Yes, I plan to implement it for 6.0 if there are no objections. I intend to do it according to Yarda's comment, that you will have to supply a special attribute to activate the convertor for all subclasses.
Starting to work on the trunk patch ...
Created attachment 30203 [details] patch against trunk as of 20060503
I have attached a patch against trunk. Reassigning back to api reviews to give you a chance to comment/reject etc. I have implemented the new attribute per Jarda's suggestion + added test + updated doc and api-changes. If there will be no objections I plan to integrate this during the next week.
The last patch seems to be of acceptable quality, imho.
Looks good. Minor things: Please set an 'id' on the <change> in apichanges.xml. Also <summary> could probably be more specific, mentioning subclasses somehow. Unclosed <code> in EA_SUBCLASSES Javadoc. BTW -u is preferred for patch review. The IDE's CVS integration does not currently generate this (filed), so using cmdline CVS is best for creating patches.
Reassigning to myself. Will integrate with Jesse's suggestions applied.
IDE: [5/10/06 12:41 PM] Committing started cvs server: scheduling file `Bar3Setting.java' for addition cvs server: scheduling file `Bar4Setting.java' for addition cvs server: scheduling file `Bar2Setting.java' for addition cvs server: scheduling file `Bar1Setting.java' for addition cvs server: use 'cvs commit' to add these files permanently Checking in core/settings/api/doc/changes/apichanges.xml; /cvs/core/settings/api/doc/changes/apichanges.xml,v <-- apichanges.xml new revision: 1.7; previous revision: 1.6 done Checking in core/settings/src/org/netbeans/modules/settings/Env.java; /cvs/core/settings/src/org/netbeans/modules/settings/Env.java,v <-- Env.java new revision: 1.5; previous revision: 1.4 done RCS file: /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar2Setting.java,v done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar2Setting.java; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar2Setting.java,v <-- Bar2Setting.java initial revision: 1.1 done RCS file: /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar3Setting.java,v done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar3Setting.java; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar3Setting.java,v <-- Bar3Setting.java initial revision: 1.1 done RCS file: /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar1Setting.java,v done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar1Setting.java; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar1Setting.java,v <-- Bar1Setting.java initial revision: 1.1 done RCS file: /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar4Setting.java,v done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar4Setting.java; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/Bar4Setting.java,v <-- Bar4Setting.java initial revision: 1.1 done Checking in core/settings/manifest.mf; /cvs/core/settings/manifest.mf,v <-- manifest.mf new revision: 1.19; previous revision: 1.18 done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/convertors/data/mf-layer.xml; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/convertors/data/mf-layer.xml,v <-- mf-layer.xml new revision: 1.10; previous revision: 1.9 done Checking in core/settings/test/unit/src/org/netbeans/modules/settings/EnvTest.java; /cvs/core/settings/test/unit/src/org/netbeans/modules/settings/EnvTest.java,v <-- EnvTest.java new revision: 1.5; previous revision: 1.4 done Checking in openide/loaders/src/org/openide/loaders/InstanceDataObject.java; /cvs/openide/loaders/src/org/openide/loaders/InstanceDataObject.java,v <-- InstanceDataObject.java new revision: 1.30; previous revision: 1.29 done Checking in core/settings/api/doc/org/netbeans/spi/settings/doc-files/api.html; /cvs/core/settings/api/doc/org/netbeans/spi/settings/doc-files/api.html,v <-- api.html new revision: 1.12; previous revision: 1.11 done IDE: [5/10/06 12:41 PM] Committing finished