Hi all,

I just added Java 5.0 annotation support to ObjectDB, following the work that was contr
ibuted by Andy (http://issues.apache.org/jira/browse/JDO-403).
Andy did a great job, but still there might be a little room for improvement
s. Following are suggestions that might be considered.

1. Using simple annotations instead of complex attributes
For instance:
@PrimaryKey
instead of:
@Field(primaryKey=true)
(or simply @Id as in JPA).

And another example:
@Embedded / @Embedded(false)
Instead of:
@Field(embedded="true") / @Field(embedded="false")
which also solves the need to use boolean as string in order to get a third 
state, which doesn't look good.

This could be done at least for the more common attributes.

Also, maybe @Index should be defined without unique option, by:
public @interface Indexed { boolean value() default true; }
or maybe even by:
public @interface Unique {}
and simply used with:
@Indexed
Do we really need so many different methods to define a unique index?

2. Discarding @Array, @Collection and @Map
It seems logical to move the attributes of @Array, @Collection and @Map to @
Element, @Key and @Value. The separation (whose origin is in the XML) is unc
lear to me.

3. Merging all annotations to one package
Good separation seems to be very difficult. For instance - @Index, which is 
currently in orm has both columns and fields as attributes. Fields are not r
elated to orm - just columns. If the separation remains, at least moving @In
dex, @Indices, @Unique, @Uniques, @Element, @Key and @Value that contain als
o non orm information should be pulled out to the main package.

4. Type attributes should be Class rather than String
For instance, in Field:
Class fieldType();
instead of:
String fieldType() default "";

5. RecursionDepth
Should be added somehow to the fields in @FetchGroup (and removed from @Fiel
d if unused).

Regards,

Ilan Kirsh
ObjectDB Software
http://www.objectdb.com

[ Post a follow-up to this message ]

[ Post a follow-up to this message ]

Hi Craig,

Thanks for your prompt response.

I understand the limitation of boolean attributes, therefore, I suggested to
 move from attribute to annotation.

This way we will have 3 options:
(1) @Indexed or @Indexed(true)
(2) @Indexed(false)
(3) Not using @Indexed at all - which means implied

Regards,

Ilan

----- Original Message ----- 
From: Craig L Russell 
To: Ilan Kirsh 
Cc: jdo-dev@db.apache.org ; JDO Expert Group 
Sent: Tuesday, January 16, 2007 2:53 AM
Subject: Re: Thoughts about JDO 2.1 Annotation


Hi Ilan, 


Thanks for your constructive comments. I've not yet had a detailed look at A
ndy's proposed annotations, and I'll consider your comments when I review th
em.


I have thought about public @interface Indexed { boolean value() defaul
t true; } and there is an issue in general with using boolean annotations. I
f you don't define a default value for a property, then you must declare it 
each time. But if you define a default value, you cannot tell whether the co
mpiler provided it as a default or the user declared it. So for me it makes 
sense to use String indexed() default ""; as Andy suggested.


More later,


Craig


On Jan 15, 2007, at 4:16 PM, Ilan Kirsh wrote:


Hi all,

I just added Java 5.0 annotation support to ObjectDB, following the work that was contr
ibuted by Andy (http://issues.apache.org/jira/browse/JDO-403).
Andy did a great job, but still there might be a little room for improvement
s. Following are suggestions that might be considered.

1. Using simple annotations instead of complex attributes
For instance:
@PrimaryKey
instead of:
@Field(primaryKey=true)
(or simply @Id as in JPA).

And another example:
@Embedded / @Embedded(false)
Instead of:
@Field(embedded="true") / @Field(embedded="false")
which also solves the need to use boolean as string in order to get a third 
state, which doesn't look good.

This could be done at least for the more common attributes.

Also, maybe @Index should be defined without unique option, by:
public @interface Indexed { boolean value() default true; }
or maybe even by:
public @interface Unique {}
and simply used with:
@Indexed
Do we really need so many different methods to define a unique index?

2. Discarding @Array, @Collection and @Map
It seems logical to move the attributes of @Array, @Collection and @Map to @
Element, @Key and @Value. The separation (whose origin is in the XML) is unc
lear to me.

3. Merging all annotations to one package
Good separation seems to be very difficult. For instance - @Index, which is 
currently in orm has both columns and fields as attributes. Fields are not r
elated to orm - just columns. If the separation remains, at least moving @In
dex, @Indices, @Unique, @Uniques, @Element, @Key and @Value that contain als
o non orm information should be pulled out to the main package.

4. Type attributes should be Class rather than String
For instance, in Field:
Class fieldType();
instead of:
String fieldType() default "";

5. RecursionDepth
Should be added somehow to the fields in @FetchGroup (and removed from @Fiel
d if unused).

Regards,

Ilan Kirsh
ObjectDB Software
http://www.objectdb.com




Craig Russell

Architect, Sun Java Enterprise System http://java.sun.com/products/jdo

408 276-5638 mailto:Craig.Russell@sun.com

P.S. A good JDO? O, Gasp!

[ Post a follow-up to this message ]

Hi Ilan,

always good to see some ideas on improving the annotations

> 1. Using simple annotations instead of complex attributes
> For instance:
>     @PrimaryKey
> instead of:
>     @Field(primaryKey=true)
> (or simply @Id as in JPA).

OK, but the other point of view is where we have a field that needs several
attributes setting (e.g NullValue, Dependent, PrimaryKey, etc) we end up wit
h
a long list of annotations against it. In that case a single annotation is
cleaner IMHO. But yes I agree in the case of say one or two attributes to be
added that it would add more flexibility and be simpler.

It doesn't have to be a question of whether we have the attributes in Field 
OR
have things like PrimaryKey. We could allow both.

> Also, maybe @Index should be defined without unique option, by:
>     public @interface Indexed { boolean value() default true; }
> or maybe even by:
>     public @interface Unique {}
> and simply used with:
>     @Indexed
> Do we really need so many different methods to define a unique index?

Some ideas :-
1. I'd remove "indexed" attribute from @Field since we can have @Index
specified on the field.
2. I'd remove "unique" attribute from @Field since we can have @Unique
specified on the field.
3. I'd change @Index to make "name" and "table" default to "" (so we can the
n
just specify @Index against a field (this then means the same as
"indexed=true")
4. I'd change @Unique to make "name" and "table" default to "" (so we can th
en
just specify @Unique against a field (this then means the same as
"unique=true")

@Index is defined with a "unique" option for RDBMS convenience IIRC.


> 2. Discarding @Array, @Collection and @Map
> It seems logical to move the attributes of @Array, @Collection and @Map to
> @Element, @Key and @Value. The separation (whose origin is in the XML) is
> unclear to me.

Agreed.
The separation in MetaData is JDO1 backwards compatibility (i.e the "element
",
"key", "value" didnt exist in JDO1 and the attributes of their types etc wer
e
already on the array, collection, map so were left there ... I think).
Obviously with annotations we dont have to consider backwards compatibility.

> 3. Merging all annotations to one package
> Good separation seems to be very difficult. For instance - @Index, which i
s
> currently in orm has both columns and fields as attributes. Fields are not
> related to orm - just columns. If the separation remains, at least moving
> @Index, @Indices, @Unique, @Uniques, @Element, @Key and @Value that contai
n
> also non orm information should be pulled out to the main package.

No complaints from me. I had them together at the start and received a reque
st
to split them. As you say, what to include as "orm" and what not is not
clear. ODBMS usually allow Index, Unique etc whereas these are in the ORM
DTD.

If the issue here is discouraging users from putting "ORM" info as annotatio
ns
then documentation (and annotation tools) is the way to do that.

> 4. Type attributes should be Class rather than String
> For instance, in Field:
>     Class fieldType();
> instead of:
>     String fieldType() default "";

I think that should be
Class fieldType() void.class;
because otherwise it becomes "required".

Similarly PersistenceCapable.objectIdClass

> 5. RecursionDepth
> Should be added somehow to the fields in @FetchGroup (and removed from
> @Field if unused).

I was originally intending on having Field[] as an attribute of FetchGro
up.
Since we only really need the field name and the recursion-depth there it
would be nice to provide notation just for those. What this notation would b
e
is not clear to me however.


--
Andy

[ Post a follow-up to this message ]

Hi Andy,
 
>
> OK, but the other point of view is where we have a field that needs
> several
> attributes setting (e.g NullValue, Dependent, PrimaryKey, etc) we end up
> with
> a long list of annotations against it. In that case a single annotation is
> cleaner IMHO. But yes I agree in the case of say one or two attributes to
> be
> added that it would add more flexibility and be simpler.

This is true.

> It doesn't have to be a question of whether we have the attributes in
> Field OR
> have things like PrimaryKey. We could allow both.

Seems as a good solution. At least @PrimaryKey should be a spearate
annotation.

Other annotations that can be considered:
@Embedded, @Serialized, @Dependent, @Fetch
and maybe less important but still an option:
@Transient, @Required

Also, @Embedded can indicate the combination of:
@Field(embedded="true")
@Element(embedded="true") (after moving from Array and Collection)
@Key(embedded="true") (after moving from Map)
@Value(embedded="true") (after moving from Map)
(anything that is relevant from the above).

i.e. not exactly duplication of @Field(embedded="true"), but rather a
shortcut
for a new more powerful operation that can be equivalent to JPA's @Embedded
and will cover most real uses.

Same for @Serialized and @Dependent, @Index and @Unique
(mainly for collections, but maybe also for maps when keys and values should
share the same setting, or maybe @Serialized and @Dependent should only
cover map values and @Index and @Unique only map keys).
 
>
> Some ideas :-
> 1. I'd remove "indexed" attribute from @Field since we can have @Index
> specified on the field.
> 2. I'd remove "unique" attribute from @Field since we can have @Unique
> specified on the field.
> 3. I'd change @Index to make "name" and "table" default to "" (so we can
> then
> just specify @Index against a field (this then means the same as
> "indexed=true")
> 4. I'd change @Unique to make "name" and "table" default to "" (so we can
> then
> just specify @Unique against a field (this then means the same as
> "unique=true")

I noticed now that you already defined @Index and @Unique with:
@Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD})
so the steps above (at least steps 3 and 4) are essential.

> @Index is defined with a "unique" option for RDBMS convenience IIRC.
>
> 
>
> Agreed.
> The separation in MetaData is JDO1 backwards compatibility (i.e the
> "element",
> "key", "value" didnt exist in JDO1 and the attributes of their types etc
> were
> already on the array, collection, map so were left there ... I think).
> Obviously with annotations we dont have to consider backwards
> compatibility.
> 
>
> No complaints from me. I had them together at the start and received a
> request
> to split them. As you say, what to include as "orm" and what not is not
> clear. ODBMS usually allow Index, Unique etc whereas these are in the ORM
> DTD.
>
> If the issue here is discouraging users from putting "ORM" info as
> annotations
> then documentation (and annotation tools) is the way to do that.
> 
>
> I think that should be
> Class fieldType() void.class;
> because otherwise it becomes "required".
>
> Similarly PersistenceCapable.objectIdClass

I guess you are right, just 'default' seems to be missing:
Class fieldType() default void.class;
 
>
> I was originally intending on having Field[] as an attribute of
> FetchGroup.
> Since we only really need the field name and the recursion-depth there it
> would be nice to provide notation just for those. What this notation would
> be
> is not clear to me however.
> --
> Andy

Maybe we need another annotation @FieldFetch.
IMO reusing fields in fetch groups is also confusing in the XML metadata.

Ilan

[ Post a follow-up to this message ]

Hi Ilan,

> At least @PrimaryKey should be a separate annotation.

Yes. Of course it already exists ... only it wasn't defined for Field/Method
usage.
 
>
> I guess you are right, just 'default' seems to be missing:
>     Class fieldType() default void.class;

Yes
 
> Maybe we need another annotation @FieldFetch.
> IMO reusing fields in fetch groups is also confusing in the XML metadata.

I agree, but I prefer @FetchField ;-)


I've now included some of these changes in JPOX CVS :-
1. Removed @Collection, @Map, @Array and embodied attributes in @Element,
@Key, @Value
2. Changed all attributes that represent a class to be of type Class and not
String
3. Added defaults for "name", "table" on @Unique, @Index, @ForeignKey so use
rs
can specify just the annotation with no attributes
4. Added @FetchField to represent a field within a @FetchGroup (with
attributes name and recursionDepth). Removed "recursionDepth" from @Field
5. Updated @PrimaryKey to be usable at Field/Method level as well as Class
level.


Latest javadocs at the following links have them :-
http://www.jpox.org/docs/1_2/javado.../>
ummary.html
http://www.jpox.org/docs/1_2/javado...ge-summary.html


Still to do:-
1. @Field "indexed" and "unique" attributes likely to be removed.
2. Put in single package (javax.jdo.annotations)
3. Consider making more of the field-level annotations specifiable without
attributes.


--
Andy

[ Post a follow-up to this message ]

> Same for @Serialized and @Dependent, @Index and @Unique
> (mainly for collections, but maybe also for maps when keys and values
> should share the same setting, or maybe @Serialized and @Dependent should
> only cover map values and @Index and @Unique only map keys).

@Serialized is a valid annotation to add but not for what you're suggesting.
It should serialise the whole field. If the field is not a container this
makes no difference but it does for containers (maps/collections/arrays). If
the field is a collection then the whole collection would be serialised with
this annotations. This is different to serialising the element only. Best
visualised with the case (with an RDBMS) and you want a join table and you
have a List and want to serialise the element into one column of the join
table. This is where @Element(serialized="true") is used - so the join table
has 3 columns ... an FK back to the owner, a List order, and a serialised
element. Similarly for Maps ... @Serialized would serialise the map as a
whole, whereas @Key(serialized="true")/@Value(serialized="true") would
serialise the key/value into any join table. The distinction is important.

I'm against the @Index/@Unique on an array/collection/map field implying
things regarding indexing the element/key/value. Just doesn't work for maps
and I don't like the assumptions that the JPA spec makes.

I'm also against having an @Dependent annotation since maps have 2 component
s,
and JPA falls apart on this one too [Its @Cascade applies to Value only,
 so
what happens when a key needs cascading ? undefined!]


--
Andy

[ Post a follow-up to this message ]

> @Serialized is a valid annotation to add but not for what you're
> suggesting.
> It should serialise the whole field. If the field is not a container this
> makes no difference but it does for containers (maps/collections/arrays).
> If
> the field is a collection then the whole collection would be serialised
> with
> this annotations. This is different to serialising the element only. Best
> visualised with the case (with an RDBMS) and you want a join table and you
> have a List and want to serialise the element into one column of the join
> table. This is where @Element(serialized="true") is used - so the join
> table
> has 3 columns ... an FK back to the owner, a List order, and a serialised
> element. Similarly for Maps ... @Serialized would serialise the map as a
> whole, whereas @Key(serialized="true")/@Value(serialized="true") would
> serialise the key/value into any join table. The distinction is important.

Actually, this is more or less what I expected.
i.e. @Serialized is equivalent to @Field(serialized="true").

> I'm against the @Index/@Unique on an array/collection/map field implying
> things regarding indexing the element/key/value. Just doesn't work for
> maps
> and I don't like the assumptions that the JPA spec makes.
>
> I'm also against having an @Dependent annotation since maps have 2
> components,
> and JPA falls apart on this one too [Its @Cascade applies to Value onl
y,
> so
> what happens when a key needs cascading ? undefined!]

OK. That makes sense.

@Embedded (and maybe also @Transient , @EagerFetch) for fields / methods
as well as @EmbeddedOnly for types should still be considered.

Ilan

[ Post a follow-up to this message ]