Re: [cp-patches] FYI: XMLEncoder - less dependencies & bugfix
Hi,
I fixed the ChangeLog entry. It now says:
2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
* java/beans/XMLEncoder.java:
(writeExpression): Added early return (fixes PR #25941).
(setExceptionListener, anonymous Class): Removed printStackTrace
call.
* java/beans/Encoder: Removed unused imports.
(setupDefaultPersistenceDelegates): Removed unneccessary
PersistenceDelegates for subclasses.
* java/beans/PersistenceDelegate:
(initialize): Use local variable as first argument as it was
intended once.
* java/beans/DefaultPersistenceDelegate:
(initialize): Added call to superclass' implementation, added
early return.
cya
Robert
Mark Wielaard wrote:
> Hi Robert,
>
> On Tue, 2006-01-24 at 18:07 +0100, Robert Schuster wrote:
>
>>The funny thing is that the whole behavior depends on three places. Two of
>>them
>>were implemented already because they are part of the spec. The final
>>component
>>consists only of one line (calling super.inititialize() in
>>DefaultPersistenceDelegate.initialize()).
>
>
> This cleans up the code very nicely. Good catch!
>
>
>>2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
>>
>>* java/beans/XMLEncoder.java:
>>(writeExpression): Added early return (fixes PR #25941).
>>* java/beans/Encoder: Removed unused imports.
>>(setupDefaultPersistenceDelegates): Removed unneccessary
>>PersistenceDelegates for subclasses.
>>* java/beans/PersistenceDelegate:
>>(initialize): Use local variable as first argument as it was
>>intended once.
>
>
> There was also this small change:
>
>
>>diff -u -r1.1 Encoder.java
>>--- java/beans/Encoder.java 8 Jan 2006 19:31:38 - 1.1
>>+++ java/beans/Encoder.java 24 Jan 2006 16:42:17 -
>>@@ -199,7 +173,6 @@
>> public void exceptionThrown(Exception e)
>> {
>> System.err.println("exception thrown: " + e);
>>-e.printStackTrace();
>> }
>> };
>> }
>
>
> Was that deliberate? And would it be a good idea to actually use a
> gnu.java.beans.decoder.DefaultExceptionListener here? And while I am
> suggesting changes, might it be a good idea to add a static
> getInstance() method to DefaultExceptionListener so we don't need to
> create a new Object each time and share the listener between all
> Decoders?
>
> Cheers,
>
> Mark
signature.asc
Description: OpenPGP digital signature
Re: [cp-patches] FYI: XMLEncoder - less dependencies & bugfix
Hi Mark,
> This cleans up the code very nicely. Good catch!
>
>
>>2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
>>
>>* java/beans/XMLEncoder.java:
>>(writeExpression): Added early return (fixes PR #25941).
>>* java/beans/Encoder: Removed unused imports.
>>(setupDefaultPersistenceDelegates): Removed unneccessary
>>PersistenceDelegates for subclasses.
>>* java/beans/PersistenceDelegate:
>>(initialize): Use local variable as first argument as it was
>>intended once.
>
>
> There was also this small change:
>
>
>>diff -u -r1.1 Encoder.java
>>--- java/beans/Encoder.java 8 Jan 2006 19:31:38 - 1.1
>>+++ java/beans/Encoder.java 24 Jan 2006 16:42:17 -
>>@@ -199,7 +173,6 @@
>> public void exceptionThrown(Exception e)
>> {
>> System.err.println("exception thrown: " + e);
>>-e.printStackTrace();
>> }
>> };
>> }
>
>
> Was that deliberate?
Uh good catch. This is another simple one line fix that I did while working on
the other stuff and it slipped through my ChangeLog writing cycle. I'll fix
that.
> And would it be a good idea to actually use a
> gnu.java.beans.decoder.DefaultExceptionListener here? And while I am
> suggesting changes, might it be a good idea to add a static
> getInstance() method to DefaultExceptionListener so we don't need to
> create a new Object each time and share the listener between all
> Decoders?
Ahh. Good one. I will do this ASAP.
Thanks for the review.
cya
Robert
signature.asc
Description: OpenPGP digital signature
Re: [cp-patches] FYI: XMLEncoder - less dependencies & bugfix
Hi Robert,
On Tue, 2006-01-24 at 18:07 +0100, Robert Schuster wrote:
> The funny thing is that the whole behavior depends on three places. Two of
> them
> were implemented already because they are part of the spec. The final
> component
> consists only of one line (calling super.inititialize() in
> DefaultPersistenceDelegate.initialize()).
This cleans up the code very nicely. Good catch!
> 2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
>
> * java/beans/XMLEncoder.java:
> (writeExpression): Added early return (fixes PR #25941).
> * java/beans/Encoder: Removed unused imports.
> (setupDefaultPersistenceDelegates): Removed unneccessary
> PersistenceDelegates for subclasses.
> * java/beans/PersistenceDelegate:
> (initialize): Use local variable as first argument as it was
> intended once.
There was also this small change:
> diff -u -r1.1 Encoder.java
> --- java/beans/Encoder.java 8 Jan 2006 19:31:38 - 1.1
> +++ java/beans/Encoder.java 24 Jan 2006 16:42:17 -
> @@ -199,7 +173,6 @@
>public void exceptionThrown(Exception e)
>{
> System.err.println("exception thrown: " + e);
> -e.printStackTrace();
>}
> };
>}
Was that deliberate? And would it be a good idea to actually use a
gnu.java.beans.decoder.DefaultExceptionListener here? And while I am
suggesting changes, might it be a good idea to add a static
getInstance() method to DefaultExceptionListener so we don't need to
create a new Object each time and share the listener between all
Decoders?
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
Re: [cp-patches] FYI: XMLEncoder - less dependencies & bugfix
Hi,
the changelog was missing the info about the changes in
DefaultPersistenceDelegate.
Fixed that.
2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
* java/beans/XMLEncoder.java:
(writeExpression): Added early return (fixes PR #25941).
* java/beans/Encoder: Removed unused imports.
(setupDefaultPersistenceDelegates): Removed unneccessary
PersistenceDelegates for subclasses.
* java/beans/PersistenceDelegate:
(initialize): Use local variable as first argument as it was
intended once.
* java/beans/DefaultPersistenceDelegate:
(initialize): Added call to superclass' implementation, added
early return.
Robert Schuster wrote:
> Hi,
> I spend some time finding out how the JDK makes it possible that a
> PersistenceDelegate registered for AbstractMap is used for all its subclasses.
>
> The funny thing is that the whole behavior depends on three places. Two of
> them
> were implemented already because they are part of the spec. The final
> component
> consists only of one line (calling super.inititialize() in
> DefaultPersistenceDelegate.initialize()).
>
> Along the lines I found out that the XMLEncoder gets into an endless recursion
> when it serializes a class which is not public. The problem is not specific to
> this case the recursion would happen in any when an exception occurs during
> the
> Expression.getValue() call. A simple early "return" fixes that. I have to
> admit
> that the wrong behavior slipped in when I copied most of
> Encoder.writeExpression's code into XMLEncoder.writeExpression. Anyway this
> fixes PR #25941 .
>
> The ChangeLog entry:
>
> 2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
>
> * java/beans/XMLEncoder.java:
> (writeExpression): Added early return (fixes PR #25941).
> * java/beans/Encoder: Removed unused imports.
> (setupDefaultPersistenceDelegates): Removed unneccessary
> PersistenceDelegates for subclasses.
> * java/beans/PersistenceDelegate:
> (initialize): Use local variable as first argument as it was
> intended once.
>
>
>
>
>
> Index: java/beans/DefaultPersistenceDelegate.java
> ===
> RCS file:
> /cvsroot/classpath/classpath/java/beans/DefaultPersistenceDelegate.java,v
> retrieving revision 1.1
> diff -u -r1.1 DefaultPersistenceDelegate.java
> --- java/beans/DefaultPersistenceDelegate.java8 Jan 2006 19:31:38
> - 1.1
> +++ java/beans/DefaultPersistenceDelegate.java24 Jan 2006 16:42:17
> -
> @@ -157,6 +157,23 @@
>protected void initialize(Class type, Object oldInstance, Object
> newInstance,
> Encoder out)
>{
> +// Calling the supertype's implementation of initialize makes it
> +// possible that descendants of classes like AbstractHashMap
> +// or Hashtable are serialized correctly. This mechanism grounds on
> +// two other facts:
> +// * Each class which has not registered a special purpose
> +// PersistenceDelegate is handled by a DefaultPersistenceDelegate
> +// instance.
> +// * PersistenceDelegate.initialize() is implemented in a way that it
> +// calls the initialize method of the superclass' persistence delegate.
> +super.initialize(type, oldInstance, newInstance, out);
> +
> +// Suppresses the writing of property setting statements when this
> delegate
> +// is not used for the exact instance type. By doing so the following
> code
> +// is called only once per object.
> +if (type != oldInstance.getClass())
> + return;
> +
> try
>{
> PropertyDescriptor[] propertyDescs = Introspector.getBeanInfo(
> Index: java/beans/Encoder.java
> ===
> RCS file: /cvsroot/classpath/classpath/java/beans/Encoder.java,v
> retrieving revision 1.1
> diff -u -r1.1 Encoder.java
> --- java/beans/Encoder.java 8 Jan 2006 19:31:38 - 1.1
> +++ java/beans/Encoder.java 24 Jan 2006 16:42:17 -
> @@ -44,15 +44,9 @@
> import gnu.java.beans.encoder.MapPersistenceDelegate;
> import gnu.java.beans.encoder.PrimitivePersistenceDelegate;
>
> -import java.util.ArrayList;
> +import java.util.AbstractCollection;
> import java.util.HashMap;
> -import java.util.HashSet;
> import java.util.IdentityHashMap;
> -import java.util.LinkedHashSet;
> -import java.util.LinkedList;
> -import java.util.TreeMap;
> -import java.util.TreeSet;
> -import java.util.Vector;
>
> /**
> * @author Robert Schuster ([EMAIL PROTECTED])
> @@ -123,31 +117,11 @@
> delegates.put(Object[].class, new ArrayPersistenceDelegate());
>
> pd = new CollectionPersistenceDelegate();
> -delegates.put(ArrayList.class, pd);
> -delegates.put(LinkedList.class, pd);
> -
[cp-patches] FYI: XMLEncoder - less dependencies & bugfix
Hi,
I spend some time finding out how the JDK makes it possible that a
PersistenceDelegate registered for AbstractMap is used for all its subclasses.
The funny thing is that the whole behavior depends on three places. Two of them
were implemented already because they are part of the spec. The final component
consists only of one line (calling super.inititialize() in
DefaultPersistenceDelegate.initialize()).
Along the lines I found out that the XMLEncoder gets into an endless recursion
when it serializes a class which is not public. The problem is not specific to
this case the recursion would happen in any when an exception occurs during the
Expression.getValue() call. A simple early "return" fixes that. I have to admit
that the wrong behavior slipped in when I copied most of
Encoder.writeExpression's code into XMLEncoder.writeExpression. Anyway this
fixes PR #25941 .
The ChangeLog entry:
2006-01-24 Robert Schuster <[EMAIL PROTECTED]>
* java/beans/XMLEncoder.java:
(writeExpression): Added early return (fixes PR #25941).
* java/beans/Encoder: Removed unused imports.
(setupDefaultPersistenceDelegates): Removed unneccessary
PersistenceDelegates for subclasses.
* java/beans/PersistenceDelegate:
(initialize): Use local variable as first argument as it was
intended once.
Index: java/beans/DefaultPersistenceDelegate.java
===
RCS file: /cvsroot/classpath/classpath/java/beans/DefaultPersistenceDelegate.java,v
retrieving revision 1.1
diff -u -r1.1 DefaultPersistenceDelegate.java
--- java/beans/DefaultPersistenceDelegate.java 8 Jan 2006 19:31:38 - 1.1
+++ java/beans/DefaultPersistenceDelegate.java 24 Jan 2006 16:42:17 -
@@ -157,6 +157,23 @@
protected void initialize(Class type, Object oldInstance, Object newInstance,
Encoder out)
{
+// Calling the supertype's implementation of initialize makes it
+// possible that descendants of classes like AbstractHashMap
+// or Hashtable are serialized correctly. This mechanism grounds on
+// two other facts:
+// * Each class which has not registered a special purpose
+// PersistenceDelegate is handled by a DefaultPersistenceDelegate
+// instance.
+// * PersistenceDelegate.initialize() is implemented in a way that it
+// calls the initialize method of the superclass' persistence delegate.
+super.initialize(type, oldInstance, newInstance, out);
+
+// Suppresses the writing of property setting statements when this delegate
+// is not used for the exact instance type. By doing so the following code
+// is called only once per object.
+if (type != oldInstance.getClass())
+ return;
+
try
{
PropertyDescriptor[] propertyDescs = Introspector.getBeanInfo(
Index: java/beans/Encoder.java
===
RCS file: /cvsroot/classpath/classpath/java/beans/Encoder.java,v
retrieving revision 1.1
diff -u -r1.1 Encoder.java
--- java/beans/Encoder.java 8 Jan 2006 19:31:38 - 1.1
+++ java/beans/Encoder.java 24 Jan 2006 16:42:17 -
@@ -44,15 +44,9 @@
import gnu.java.beans.encoder.MapPersistenceDelegate;
import gnu.java.beans.encoder.PrimitivePersistenceDelegate;
-import java.util.ArrayList;
+import java.util.AbstractCollection;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.IdentityHashMap;
-import java.util.LinkedHashSet;
-import java.util.LinkedList;
-import java.util.TreeMap;
-import java.util.TreeSet;
-import java.util.Vector;
/**
* @author Robert Schuster ([EMAIL PROTECTED])
@@ -123,31 +117,11 @@
delegates.put(Object[].class, new ArrayPersistenceDelegate());
pd = new CollectionPersistenceDelegate();
-delegates.put(ArrayList.class, pd);
-delegates.put(LinkedList.class, pd);
-delegates.put(Vector.class, pd);
-delegates.put(HashSet.class, pd);
-delegates.put(LinkedHashSet.class, pd);
-delegates.put(TreeSet.class, pd);
+delegates.put(AbstractCollection.class, pd);
pd = new MapPersistenceDelegate();
-delegates.put(HashMap.class, pd);
-delegates.put(TreeMap.class, pd);
+delegates.put(java.util.AbstractMap.class, pd);
delegates.put(java.util.Hashtable.class, pd);
-delegates.put(java.util.IdentityHashMap.class, pd);
-
-delegates.put(java.util.LinkedHashMap.class, pd);
-delegates.put(java.util.Properties.class, pd);
-
-delegates.put(java.awt.RenderingHints.class, pd);
-delegates.put(java.util.WeakHashMap.class, pd);
-delegates.put(javax.swing.UIDefaults.class, pd);
-
-// TODO: These classes need to be implemented first
-//delegates.put(java.security.AuthProvider.class, pd);
-//delegates.put(java.util.concurrent.ConcurrentHashMap.class
