db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Armin Waibel" <armin.wai...@code-au-lait.de>
Subject Re: [OJB] Issue #OJB136 modified [VOTE]
Date Thu, 06 Mar 2003 13:08:34 GMT
Hi,

> i'm really not sure about conversion of autoIncrement values.
>
> cons:
> - the autoincrement value is calculated based on the type of the field
> and thus conversion shouldn't be necessary
>
> pros:
> - i'm sure there's at least one case where conversion makes sense...
>
Say we use a class PrimaryKey

class PrimaryKey
{
    private Long key;
    public PrimaryKey()
    {
    ......
}
as the primary key field of a persistent object and we want
to use autoincrement for that field too.

> if we do convert autoIncrement values we'll need an additional method
in
> FieledConversion because sqlToJava might fail.
> sqlJava is the mehod called when reading from a ResultSet,  so i
propose
> to name the new method:
>
> autoIncrementToJava
>
> what do you think ?

The problem only occurs when someone use 'autoincrement'
and need fieldConversion too, right?
And only in the sql-->java direction?
Seems in that case the SequenceManager/BrokerHelper
be an 'anti-pattern' ;-)

A compromise that should work in 90% of all cases without
adopt new methods could be to do the field conversion only
in the 'object case'.
...
                // Object
                else if (!f.getType().isPrimitive())
                {
                    // only assign a value if attribute == null
                    if (f.get(obj) == null)
                    {
                        result =
broker.serviceSequenceManager().getUniqueObject(fmd);
                        // ### here we do field conversion to support
user defined fields
                        result =
fmd.getFieldConversion().sqlToJava(result);
                        if (result != null)
The pitfall is the
broker.serviceSequenceManager().getUniqueObject(fmd)
method, because we don't know what type of object the
sequence manager returns.
This depends on the SM implementation
and not on the jdbc-type of the persistent-class field.

Thus it could be difficult to write a adequate FieldConversion
AND the #sqlToJava(...) method in fact does a 'javaToJava'
conversion, exactly a 'sequenceManagerToJava' conversion.
The same problem arise when we add a new method.

Conclusion, I think the only transparent way is to document that
autoincrement fields do not support field conversion. Or do I
overlook somthing?

regards,
Armin

>
> jakob
>
> Jakob Braeuchi wrote:
>
> > hi david,
> >
> > sorry, you're right there's _no_ double conversion. but we have a
test
> > case failing after applying this patch.
> >
> > FieldConversionTest fails because of  conversion :
> >
> > in BrokerHelper  the autoIncr value in result is _Long_ because of
the
> > typ of the field
> >
> >       result = new
> > Long(broker.serviceSequenceManager().getUniqueLong(fmd));
> >       // reflect autoincrement value back into object
> >       f.set(obj, fmd.getFieldConversion().sqlToJava(result));
> >
> > the fieldconversion (inner class FieldConversionLongToInteger)
defined
> > for this field checks for _Integer_ which fails:
> >
> >        public Object sqlToJava(Object source) throws
ConversionException
> >        {
> >            if (!(source instanceof Integer))
> >            {
> >                throw new ConversionException(
> >                        "Wrong java field type when java-->sql,
> > expected java.lang.Integer, found "
> >                        + source.getClass());
> >            }
> >            return new Long(((Integer) source).longValue());
> >        }
> >
> > maybe we should make field conversion more tolerant ???
> >
> > jakob
> >
> > David Marquard wrote:
> >
> >> That's not right! The patch doesn't result in double conversion.
The
> >> current
> >> (unpatched) code does something like this:
> >>
> >> result = broker.serviceSequenceManager().getUniqueObject(fmd);
> >> // reflect autoincrement value back into object
> >> f.set(obj, result);
> >>           ^^^^^^
> >> As you can see, an *unconverted* object is being retrieved from the
> >> sequence
> >> manager and set directly into a persistent field without a field
> >> conversion
> >> being run on it. The patch changes that second line to:
> >>
> >> f.set(obj, fmd.getFieldConversion().sqlToJava(result));
> >>
> >> which properly converts the object before setting it in the
persistent
> >> field. The converted object *is not* returned from
> >> getAutoIncrement(), and
> >> thus does not cause double conversions. In fact, callers of
> >> getAutoIncrement() won't see any difference at all.
> >>
> >> Dave
> >>
> >> -----Original Message-----
> >> From: Jakob Braeuchi [mailto:jbraeuchi@hotmail.com]
> >> Sent: Wednesday, March 05, 2003 8:33 AM
> >> To: Dave Marquard
> >> Cc: ojb-dev@db.apache.org
> >> Subject: [OJB] Issue #OJB136 modified
> >>
> >>
> >>
> >> Issue OJB136 has just been modified by user brj
> >>
> >> You can view the issue detail at the following URL:
> >>     <http://scarab.werken.com/scarab/issues/id/OJB136>
> >>
> >> The following modifications were made to this issue:
> >>
> >> getAutoIncrement is called be other mehods of BrokerHelper which DO
> >> field
> >> conversion.
> >> applying this patch results in double conversion.
> >>
> >>
>
>> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> >> For additional commands, e-mail: ojb-dev-help@db.apache.org
> >>
> >>
> >>
> >>
> >
> >
>
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> > For additional commands, e-mail: ojb-dev-help@db.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
>
>
>


Mime
View raw message