Hi, I think it is OK to break it. The public API is ISerializer while your proposed changes are internals of JavaSerializer, imo.
On Fri, Nov 25, 2011 at 6:24 PM, James Carman <[email protected]> wrote: > While doing a bit of debugging today, I noticed that I didn't see that > nice serialization issue message that tells me which field is the > problem. So, I started looking into it (with some direction from > martin-g on IRC): > > The Problem > > In the new JavaSerializer class, it has a CheckerOutputStream which > extends ObjectOutputStream. The intent is to use the > ObjectOutputStream.writeObjectOverride() support. However, the > writeObjectOverride() method is never called unless you use the no-arg > constructor from the ObjectOutputStream class (which sets the > "enableOverride" flag to true). The CheckerOutputStream uses the > ObjectOutputStream(OutputStream) constructor in its constructor. > Worse yet, even if the writeObjectOverride() method were to be called, > it would create a StackOverflowError because it's calling the > super.writeObject() method which is what called it in the first place > (infinite recursion). > > My Proposed Solution > > I propose we do the following: > > 1. Change the following JavaSerializer method: > > ObjectOutputStream newObjectOutputStream(OutputStream out) > > to > > ObjectOutput newObjectOutput(OutputStream out) > > 2. Change CheckerOutputStream to implement ObjectOutput instead of > extending ObjectOutputStream. > > 3. Have CheckerOutputStream delegate its ObjectOutput methods to an > internal ObjectOutputStream object. > > Now, this would break the existing API because you're changing the > signature of the newObjectOutputSream() method, but I think this is > how it should have been written in the first place (to the interface, > not the class). > > What do you guys think? > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
