empire-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eike Kettner <e...@eknet.org>
Subject Re: [jira] Updated: (EMPIREDB-97) Serialization of Empire-DB objects
Date Fri, 28 Jan 2011 12:01:30 GMT
Hi Rainer,

inline reply...

On [Fri, 28.01.2011 11:02], Rainer Döbele wrote:
> Hi Eike,
> 
> I share your concerns, but as the article says there is no way to unimplement an interface,
and hence the only option is to add the Serializeable interface only to the top level classes.


There is no need to unimplement, if it is not added to DBObject -
referring to the idea of tagging each single class instead of a very
top-level one. So each class below DBObject that is to be serializable
is tagged and DBReader is left out.

> In fact DBReader even is serializeable but only as long as there is no Resultset attached
to it.
> So basically we would throw an exception only if Resultset!=Null.

that's true!
 
> Of course we could just forget it as we expect an Exception anyway since Resultset is
not Serializeable.
> But IMO it would be cleaner if we explicitly define that somewhat like this:
> 
> private void writeObject(ObjectOutputStream out) throws IOException
> {
> 	if (rset!=null)
> 		throw new NotSerializableException("Unable to serialize an open reader");
> }
> 
> There is no need to implement readObject.
> For me this solution would be OK.

I agree. That would be ok for me, too. I'll try to apply this the next
couple of days and send you a patch.

Regards,
Eike


> Regards
> Rainer
> 
> 
> 
> Eike Kettner wrote:
> > from: Eike Kettner [mailto:news@eknet.org]
> > to: empire-db-dev@incubator.apache.org
> > re: Re: [jira] Updated: (EMPIREDB-97) Serialization of Empire-DB
> > objects
> > 
> > Hi Rainer,
> > 
> > I will add the serialVersionUID and send one patch of everything.
> > 
> > Referring to "Stop Serialization": This is of course a solution, but it
> > feels a bit strange to me. The class implements Serializable but throws
> > Exceptions in its read/writeObject methods. So it will work for any
> > method that accepts arguments of type Serializable, but will then throw
> > runtime exceptions. (There are probably runtime exceptions thrown
> > anyways, because ResultSet is not serializable?) What I mean is, the
> > type says "yes", but the implementation says "no". I could imaging that
> > this approach is used if you have no "source-access" to the class you
> > are extending. Then you don't have much choice. What do you think?
> > 
> > Regards,
> > Eike
> > 
> > On [Fri, 28.01.2011 09:36], Rainer Döbele wrote:
> > > Hi Eike,
> > >
> > > yes we already have that on our list.
> > > For the upcoming release 2.1. we will leave things as they are, but
> > this will be the next big thing.
> > >
> > > If you want to add the sercialVersionID's that would be fine.
> > > You can send a complete patch including your first changes if you
> > want.
> > >
> > > About DBReader: In the following article
> > >
> > http://java.sun.com/developer/technicalArticles/Programming/serializatio
> > n/
> > > you will find a paragraph titled "Stop That Serialization".
> > > I guess this is what we need.
> > >
> > > Regards
> > > Rainer
> > >
> > >
> > > Eike Kettner wrote:
> > > > from: Eike Kettner [mailto:news@eknet.org]
> > > > to: empire-db-dev@incubator.apache.org
> > > > re: Re: [jira] Updated: (EMPIREDB-97) Serialization of Empire-DB
> > > > objects
> > > >
> > > > Hello Rainer,
> > > > Hello Francis,
> > > >
> > > > I must admit that I was a bit confused when I first had contact to
> > > > ErrorObject. It remembered me to coding C. I also think that
> > inheritance
> > > > shouldn't be "overused" and so I would like to see ErrorObject
> > disappear
> > > > :) But, of course, this is really some work, which must be done
> > > > carefully. Just wanted you to know, that I'm happy that there is an
> > > > effort like EMPIREDB-95.
> > > >
> > > > Thanks and regards,
> > > > Eike
> > > >
> > > > On [Thu, 27.01.2011 09:53], Rainer Döbele wrote:
> > > > > Hi Francis,
> > > > >
> > > > > yes I myself have asked this question a while ago, whether we
> > should
> > > > keep this old style error handling as an option.
> > > > > Currently we can switch between Exception-mode and an exception-
> > less
> > > > mode with Boolean return values for most functions to indicate an
> > error.
> > > > >
> > > > > I agree that this exception-less mode is not really needed and I
> > have
> > > > no problem getting rid of it and only do Exceptions in the future.
> > > > However it only makes sense if we also change the return values of
> > most
> > > > functions and thus there is a lot to rethink an test - which means
> > quite
> > > > a bit of work.
> > > > >
> > > > > I would suggest to leave it for now until we have finished our
> > 2.1.
> > > > release.
> > > > > This should then be a major task for a 2.2. release.
> > > > >
> > > > > I thought we have an issue for this already but could not find
> > any.
> > > > > I have now changed EMPIREDB-95 which was "Code cleanup and review"
> > > > (did quite mean anything) to "Remove optional support for old style
> > > > error handing".
> > > > >
> > > > > Regards
> > > > > Rainer
> > > > >
> > > > >
> > > > > Francis De Brabandere wrote:
> > > > > > from: Francis De Brabandere [mailto:francisdb@gmail.com]
> > > > > > to: empire-db-dev@incubator.apache.org
> > > > > > re: Re: [jira] Updated: (EMPIREDB-97) Serialization of Empire-DB
> > > > > > objects
> > > > > >
> > > > > > Rainer,
> > > > > >
> > > > > > We probably already had a discussion about this ErrorObject
> > idea. I
> > > > > > still see this as some kind of anti-pattern from old times where
> > > > > > inheritance was the key to everything.
> > > > > >
> > > > > > Could you (again) explain me what the added value of this
> > > > ErrorObject
> > > > > > is (ignoring backwards compatibility)? I'll add that info to
the
> > > > wiki
> > > > > > in case I forget once again or somebody else asks the same
> > question.
> > > > > >
> > > > > > Cheers,
> > > > > > Francis
> > > > > >
> > > > > > On Wed, Jan 26, 2011 at 11:36 PM, Rainer Döbele
> > <doebele@esteam.de>
> > > > > > wrote:
> > > > > > > Hi Eike,
> > > > > > >
> > > > > > > I can follow your conclusion and agree that making ErrorObject
> > > > > > serializeable makes no sense (why would anyone want to serialize
> > an
> > > > > > error anyway?), whereas serializing EmpireException is fine.
> > > > > > >
> > > > > > > I have not applied your patch yet (but I will do), but
there
> > is
> > > > one
> > > > > > more thing that came to my mind:
> > > > > > > DBReader is not serializeable as it requires a reference
to an
> > > > open
> > > > > > java.sql.ResultSet.
> > > > > > > We have to make sure, that that a NotSerializeable exception
> > is
> > > > > > thrown, when attempting to serialize this class.
> > > > > > >
> > > > > > > Thanks and regards,
> > > > > > > Rainer
> > > > > > >
> > > > > > >
> > > > > > > Eike Kettner (JIRA) wrote:
> > > > > > >> from: Eike Kettner (JIRA) [mailto:empire-db-
> > > > dev@incubator.apache.org]
> > > > > > >> to: empire-db-commits@incubator.apache.org
> > > > > > >> re: [jira] Updated: (EMPIREDB-97) Serialization of
Empire-DB
> > > > > > >> objects
> > > > > > >>
> > > > > > >>
> > > > > > >>      [ https://issues.apache.org/jira/browse/EMPIREDB-
> > > > > > >> 97?page=com.atlassian.jira.plugin.system.issuetabpanels:all-
> > > > tabpanel
> > > > > > ]
> > > > > > >>
> > > > > > >> Eike Kettner updated EMPIREDB-97:
> > > > > > >> ---------------------------------
> > > > > > >>
> > > > > > >>     Attachment: 1_dbobject.patch
> > > > > > >>                 0_exception.patch
> > > > > > >>
> > > > > > >> Hi there,
> > > > > > >>
> > > > > > >> As I started working on the serialization thing, I
figured
> > that
> > > > it's
> > > > > > >> not a good idea to let ErrorObject implement Serializable.
> > Nearly
> > > > > > every
> > > > > > >> object extends ErrorObject and for some (and especially
for
> > > > objects
> > > > > > to
> > > > > > >> come) it is not desireable to be serializable. I think
its
> > not
> > > > that
> > > > > > >> good
> > > > > > >> to open so many object for serialization. Then there
are
> > already
> > > > > > >> classes
> > > > > > >> that hold non-serializable references (I found
> > XMLConfiguration
> > > > to be
> > > > > > >> such a class). So, I think it could be a source of
bugs to
> > open
> > > > every
> > > > > > >> object this way...
> > > > > > >>
> > > > > > >> The other side is, that ErrorObject uses a static ThreadLocal
> > to
> > > > hold
> > > > > > >> error info. This wouldn't be serialized anyways. This
means
> > > > > > >> EmpireException wouldn't be serializable even if ErrorObject
> > > > > > implements
> > > > > > >> Serializable.
> > > > > > >>
> > > > > > >> Instead I chose to serialize EmpireException by using
a
> > > > serializable
> > > > > > >> implementation of ErrorInfo. The major difference here:
> > > > > > EmpireException
> > > > > > >> does not hold a reference to the concrete object anymore,
but
> > > > only a
> > > > > > >> copy of the error infos. IMHO, this is good for an
exception,
> > but
> > > > I
> > > > > > >> have
> > > > > > >> no glue to what extend users rely on
> > > > EmpireException#getErrorObject
> > > > > > to
> > > > > > >> return a DBTable, XMLConfiguration etc. This is applied
with
> > the
> > > > > > first
> > > > > > >> patch.
> > > > > > >>
> > > > > > >> Then I chose to let DBObject implement Serializable.
I think
> > that
> > > > > > it's
> > > > > > >> nice if data model objects are serializable. Plain
SQL
> > strings
> > > > are
> > > > > > >> serializable and so I think objects like DBCommand
or
> > > > DBOrderByExpr
> > > > > > >> (that represent parts of SQL) should be serializable,
too :).
> > > > This is
> > > > > > >> applied in the second patch.
> > > > > > >>
> > > > > > >> Regards,
> > > > > > >> Eike
> > > > > > >>
> > > > > > >>
> > > > > > >> > Serialization of Empire-DB objects
> > > > > > >> > ----------------------------------
> > > > > > >> >
> > > > > > >> >                 Key: EMPIREDB-97
> > > > > > >> >                 URL:
> > > > > > https://issues.apache.org/jira/browse/EMPIREDB-
> > > > > > >> 97
> > > > > > >> >             Project: Empire-DB
> > > > > > >> >          Issue Type: Wish
> > > > > > >> >          Components: Core
> > > > > > >> >            Reporter: Eike Kettner
> > > > > > >> >         Attachments: 0_exception.patch, 1_dbobject.patch
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > Looking at class EmpireException, it holds references
to
> > two
> > > > non-
> > > > > > >> serializable objects: ErrorObject and ErrorType which
breaks
> > the
> > > > > > >> contract with the Exception API.
> > > > > > >> > Now, it would be great for several use-cases to
have
> > Empire-DB
> > > > > > >> objects serializable. If ErrorObject would be serializable,
> > it
> > > > would
> > > > > > >> first make EmpireException serializable (assuming ErrorType
> > to be
> > > > > > >> serializable) and next it would make every other DBXyz
object
> > in
> > > > this
> > > > > > >> hierarchy serializable.
> > > > > > >> > Here is for reference the mail thread from users@
mailing
> > list:
> > > > > > >> > ------------------------------
> > > > > > >> > On Sat, Jan 22, 2011 at 7:26 PM, Eike Kettner
> > <news@eknet.org>
> > > > > > wrote:
> > > > > > >> > > Hi Rainer and Francis,
> > > > > > >> > >
> > > > > > >> > > thanks for your quick replies and for giving
this a
> > chance.
> > > > > > >> Serializing
> > > > > > >> > > an exception is sure not something massive
used, however
> > > > > > sometimes
> > > > > > >> it is
> > > > > > >> > > quite a nice feature. For example, a JMSLogger
sends log
> > > > events
> > > > > > to
> > > > > > >> a
> > > > > > >> > > broker, and there exceptions are serialized.
Well, I see
> > that
> > > > > > this
> > > > > > >> is
> > > > > > >> > > not used often, and more or less a "special
case" :).
> > Still,
> > > > I
> > > > > > >> would
> > > > > > >> > > consider a non-serializable exception a small
"bug" -
> > just
> > > > > > because
> > > > > > >> it's
> > > > > > >> > > dictated by the java api.
> > > > > > >> > >
> > > > > > >> > > I had a quick look at the sources and as
far as I can
> > see, it
> > > > > > >> should be
> > > > > > >> > > ok to make "everything" serializable. There
is always the
> > > > > > >> > > "serializable-drawback" to consider: users
can save
> > objects
> > > > on
> > > > > > disk
> > > > > > >> and
> > > > > > >> > > later try to load them with a new version
of empire-db,
> > where
> > > > > > class
> > > > > > >> > > definitions have changed. Well, I think one
can live with
> > > > this,
> > > > > > and
> > > > > > >> it
> > > > > > >> > > does not apply to many other use-cases of
serialization
> > (rmi,
> > > > > > >> > > serialization used in wicket or messaging),
because
> > objects
> > > > are
> > > > > > >> > > serialized only for a short amount of time.
> > > > > > >> > >
> > > > > > >> > > Regards,
> > > > > > >> > > Eike
> > > > > > >> > >
> > > > > > >> > > On [Sat, 22.01.2011 13:49], Rainer D=F6bele
wrote:
> > > > > > >> > >> Hi Eike,
> > > > > > >> > >>
> > > > > > >> > >> I agree with Francis that I don't quite
see the point
> > for
> > > > > > >> serializing an Exception, although I must admit that
> > > > > > >> java.lang.Throwable is Serializable.
> > > > > > >> > >>
> > > > > > >> > >> But then I agree that we should consider
making DBObject
> > or
> > > > > > >> ErrorObject serializeable which then would apply to
the
> > entire
> > > > object
> > > > > > >> hierarchy.
> > > > > > >> > >> Regards
> > > > > > >> > >>
> > > > > > >> > >> Rainer
> > > > > > >> > >>
> > > > > > >> > >>
> > > > > > >> > >> Francis De Brabandere wrote:
> > > > > > >> > >> > from: Francis De Brabandere
> > [mailto:francisdb@gmail.com]
> > > > > > >> > >> > to: empire-db-user@incubator.apache.org
> > > > > > >> > >> > re: Re: Serialization of EmpireException
> > > > > > >> > >> >
> > > > > > >> > >> > Hi Eike,
> > > > > > >> > >> >
> > > > > > >> > >> > I see no reason for not making them
Serializable.
> > > > > > >> > >> >
> > > > > > >> > >> > Rainer?
> > > > > > >> > >> >
> > > > > > >> > >> > Cheers,
> > > > > > >> > >> > Francis
> > > > > > >> > >> >
> > > > > > >> > >> > On Fri, Jan 21, 2011 at 4:31 PM,
Eike Kettner
> > > > <news@eknet.org>
> > > > > > >> wrote:
> > > > > > >> > >> > >
> > > > > > >> > >> > > Hello,
> > > > > > >> > >> > >
> > > > > > >> > >> > > I was trying to serialize EmpireException
but ran
> > into
> > > > an
> > > > > > >> error.
> > > > > > >> > >> > > EmpireException is marked as
Serializable
> > > > (RuntimeException)
> > > > > > >> > >> > > but it holds references to
ErrorObject and ErrorType
> > > > which
> > > > > > are
> > > > > > >> not
> > > > > > >> > >> > > serializable. Hence a NotSerializableException
is
> > > > thrown.
> > > > > > >> > >> > >
> > > > > > >> > >> > > When asking this, I like to
ask whether there is a
> > > > thought
> > > > > > >> about making
> > > > > > >> > >> > > some model objects like DBRowset
DBTable etc
> > > > serializable.
> > > > > > >> Since most or all
> > > > > > >> > >> > > DBXyz objects hold model information
only it should
> > be
> > > > okay
> > > > > > >> for them to
> > > > > > >> > >> > > be serializable, imho? I use
messaging and often
> > Apache
> > > > > > Wicket
> > > > > > >> which
> > > > > > >> > >> > > both use serialization, that's
why I'm asking this.
> > (For
> > > > > > >> example, I'd
> > > > > > >> > >> > > like to pass around where and
order-by expressions).
> > > > > > >> > >> > >
> > > > > > >> > >> > > Kind Regards,
> > > > > > >> > >> > > Eike
> > > > > > >> > >> > >
> > > > > > >> > >> > >
> > > > > > >> > >> > >
> > > > > > >> > >> >
> > > > > > >> > >> >
> > > > > > >> > >> >
> > > > > > >> > >> > --
> > > > > > >> > >> > http://www.somatik.be
> > > > > > >> > >> > Microsoft gives you windows, Linux
gives you the whole
> > > > house.
> > > > > > >> > >>
> > > > > > >> > >
> > > > > > >>
> > > > > > >> --
> > > > > > >> This message is automatically generated by JIRA.
> > > > > > >> -
> > > > > > >> You can reply to this email to add a comment to the
issue
> > online.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > http://www.somatik.be
> > > > > > Microsoft gives you windows, Linux gives you the whole house.
> > > > >
> > > >
> > > > --
> > > > email: eike@eknet.org   https://www.eknet.org  pgp: 481161A0
> > >
> > 
> > --
> > email: eike@eknet.org   https://www.eknet.org  pgp: 481161A0
> 

-- 
email: eike@eknet.org   https://www.eknet.org  pgp: 481161A0

Mime
View raw message