Bug 65950 - Setting convertors do not work for subclasses
Setting convertors do not work for subclasses
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: -- Other --
5.x
All All
: P2 (vote)
: 6.x
Assigned To: David Strupl
issues@platform
http://www.netbeans.org/download/dev/...
: API, API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-05 11:13 UTC by David Strupl
Modified: 2008-12-22 22:56 UTC (History)
0 users

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Proposed patch (without tests & docs) for review (3.52 KB, patch)
2005-10-05 11:36 UTC, David Strupl
Details | Diff
patch against trunk as of 20060503 (19.83 KB, patch)
2006-05-03 12:10 UTC, David Strupl
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Strupl 2005-10-05 11:13:11 UTC
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).
Comment 1 David Strupl 2005-10-05 11:36:04 UTC
This is for fast trak of course.
Comment 2 David Strupl 2005-10-05 11:36:56 UTC
Created attachment 25548 [details]
Proposed patch (without tests & docs) for review
Comment 3 Jesse Glick 2005-10-05 23:11:04 UTC
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?
Comment 4 Jaroslav Tulach 2005-10-06 07:00:25 UTC
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. 
Comment 5 David Strupl 2005-10-06 08:03:11 UTC
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?
Comment 6 Jesse Glick 2005-10-06 21:29:17 UTC
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.
Comment 7 David Strupl 2005-10-07 07:46:41 UTC
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.
Comment 8 David Strupl 2005-10-07 07:57:48 UTC
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.
Comment 9 Jesse Glick 2006-04-09 18:29:06 UTC
Is this issue still active?
Comment 10 David Strupl 2006-04-09 19:54:49 UTC
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.
Comment 11 David Strupl 2006-05-02 14:34:14 UTC
Starting to work on the trunk patch ...
Comment 12 David Strupl 2006-05-03 12:10:51 UTC
Created attachment 30203 [details]
patch against trunk as of 20060503
Comment 13 David Strupl 2006-05-03 12:12:53 UTC
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.
Comment 14 Jaroslav Tulach 2006-05-03 13:21:00 UTC
The last patch seems to be of acceptable quality, imho.
Comment 15 Jesse Glick 2006-05-03 16:56:01 UTC
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.
Comment 16 David Strupl 2006-05-10 11:11:19 UTC
Reassigning to myself. Will integrate with Jesse's suggestions applied.
Comment 17 David Strupl 2006-05-10 11:42:34 UTC
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


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo