You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by mg...@apache.org on 2008/03/02 17:02:02 UTC
svn commit: r632775 - in
/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src:
main/java/org/apache/cayenne/access/
main/java/org/apache/cayenne/access/types/
test/java/org/apache/cayenne/access/types/
Author: mgentry
Date: Sun Mar 2 08:02:00 2008
New Revision: 632775
URL: http://svn.apache.org/viewvc?rev=632775&view=rev
Log:
First cut at CAY-994: Add extended enumeration support.
Added:
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/ExtendedEnumeration.java
Modified:
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/QueryLogger.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/EnumType.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/java/org/apache/cayenne/access/types/EnumTypeTest.java
Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/QueryLogger.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/QueryLogger.java?rev=632775&r1=632774&r2=632775&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/QueryLogger.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/QueryLogger.java Sun Mar 2 08:02:00 2008
@@ -25,6 +25,7 @@
import java.util.List;
import org.apache.cayenne.access.jdbc.ParameterBinding;
+import org.apache.cayenne.access.types.ExtendedEnumeration;
import org.apache.cayenne.conn.DataSourceInfo;
import org.apache.cayenne.map.DbAttribute;
import org.apache.cayenne.util.IDUtil;
@@ -121,7 +122,13 @@
else if (object instanceof Enum) {
buffer.append(object.getClass().getName()).append(".");
buffer.append(((Enum<?>) object).name()).append("=");
- buffer.append(((Enum<?>) object).ordinal());
+ if (object instanceof ExtendedEnumeration) {
+ Object value = ((ExtendedEnumeration) object).getDatabaseValue();
+ if (value instanceof String) buffer.append("'");
+ buffer.append(value);
+ if (value instanceof String) buffer.append("'");
+ }
+ else buffer.append(((Enum<?>) object).ordinal()); // FIXME -- this isn't quite right
}
else if (object instanceof ParameterBinding) {
sqlLiteralForObject(buffer, ((ParameterBinding) object).getValue());
Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/EnumType.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/EnumType.java?rev=632775&r1=632774&r2=632775&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/EnumType.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/EnumType.java Sun Mar 2 08:02:00 2008
@@ -23,15 +23,23 @@
import java.sql.CallableStatement;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.cayenne.CayenneRuntimeException;
import org.apache.cayenne.dba.TypesMapping;
import org.apache.cayenne.map.DbAttribute;
import org.apache.cayenne.validation.ValidationResult;
/**
- * An ExtendedType that handles an enum class. If Enum is mapped to a character column,
- * its name is used as persistent value; if it is mapped to a numeric column, its ordinal
- * (i.e. a position in enum class) is used.
+ * An ExtendedType that handles an enum class. If Enum is an instance of
+ * ExtendedEnumeration (preferred), use the mapped database value (provided
+ * by the getDatabaseValue() method) to map enumerations to the database. If
+ * the enum is not an instance of ExtendedEnumeration, fall back to a more
+ * simplistic mapping. If mapped to a character column, the name is used as
+ * the persistent value; if it is mapped to a numeric column, the ordinal
+ * value (i.e. a position in enum class) is used.
* <p>
* <i>Requires Java 1.5 or newer</i>
* </p>
@@ -42,7 +50,10 @@
public class EnumType<T extends Enum<T>> implements ExtendedType {
protected Class<T> enumClass;
- protected Object[] values;
+// protected Object[] values;
+
+ // Contains a mapping of database values (Integer or String) and the Enum for that value.
+ private Map<Object, Enum<T>> enumerationMappings = new HashMap<Object, Enum<T>>();
public EnumType(Class<T> enumClass) {
if (enumClass == null) {
@@ -53,7 +64,14 @@
try {
Method m = enumClass.getMethod("values");
- this.values = (Object[]) m.invoke(null);
+ Object[] values = (Object[]) m.invoke(null);
+
+ for (int i = 0; i < values.length; i++)
+ if (values[i] instanceof ExtendedEnumeration)
+ register((Enum<T>) values[i], ((ExtendedEnumeration) values[i]).getDatabaseValue());
+ else
+ register((Enum<T>) values[i], i);
+
}
catch (Exception e) {
throw new IllegalArgumentException("Class "
@@ -96,10 +114,16 @@
Enum<?> e = (Enum<?>) value;
if (TypesMapping.isNumeric(type)) {
- statement.setInt(pos, e.ordinal());
+ if (e instanceof ExtendedEnumeration)
+ statement.setInt(pos, (Integer) ((ExtendedEnumeration) e).getDatabaseValue());
+ else
+ statement.setInt(pos, e.ordinal());
}
else {
- statement.setString(pos, e.name());
+ if (e instanceof ExtendedEnumeration)
+ statement.setString(pos, (String) ((ExtendedEnumeration) e).getDatabaseValue());
+ else
+ statement.setString(pos, e.name());
}
}
else {
@@ -110,11 +134,13 @@
public Object materializeObject(ResultSet rs, int index, int type) throws Exception {
if (TypesMapping.isNumeric(type)) {
int i = rs.getInt(index);
- return (rs.wasNull() || index < 0) ? null : values[i];
+ return (rs.wasNull() || index < 0) ? null : lookup(i);
+// return (rs.wasNull() || index < 0) ? null : values[i];
}
else {
String string = rs.getString(index);
- return string != null ? Enum.valueOf(enumClass, string) : null;
+ return string != null ? lookup(string) : null;
+// return string != null ? Enum.valueOf(enumClass, string) : null;
}
}
@@ -123,11 +149,63 @@
if (TypesMapping.isNumeric(type)) {
int i = rs.getInt(index);
- return (rs.wasNull() || index < 0) ? null : values[i];
+ return (rs.wasNull() || index < 0) ? null : lookup(i);
+// return (rs.wasNull() || index < 0) ? null : values[i];
}
else {
String string = rs.getString(index);
- return string != null ? Enum.valueOf(enumClass, string) : null;
+ return string != null ? lookup(string) : null;
+// return string != null ? Enum.valueOf(enumClass, string) : null;
}
+ }
+
+ /**
+ * Register the given enum with the mapped database value.
+ */
+ private void register(Enum<T> enumeration, Object databaseValue)
+ {
+ // Check for duplicates.
+ if (enumerationMappings.containsKey(databaseValue) || enumerationMappings.containsValue(enumeration))
+ throw new CayenneRuntimeException("Enumerations/values may not be duplicated.");
+
+ // Store by database value/enum because we have to lookup by db value later.
+ enumerationMappings.put(databaseValue, enumeration);
+ }
+
+ /**
+ * Lookup the giving database value and return the matching enum.
+ */
+ private Enum<T> lookup(Object databaseValue)
+ {
+ if (enumerationMappings.containsKey(databaseValue) == false)
+ {
+ // All integers enums are mapped. Not necessarily all strings.
+ if (databaseValue instanceof Integer)
+ throw new CayenneRuntimeException("Missing enumeration mapping for " + getClassName() + " with value " + databaseValue + ".");
+
+ // Use the database value (a String) as the enum value.
+ return Enum.valueOf(enumClass, (String) databaseValue);
+ }
+
+ // Mapped value->enum exists, return it.
+ return enumerationMappings.get(databaseValue);
+ }
+
+
+ /**
+ * Returns the enumeration mapping for this enumerated data type. The
+ * key is the database value, the value is the actual enum.
+ */
+ public Map<Object, Enum<T>> getEnumerationMappings() {
+ return enumerationMappings;
+ }
+
+ /**
+ * Returns the values registered for this enumerated data type. Note that
+ * the order the values are returned in could differ from the ordinal order
+ * in which they are declared in the actual enum class.
+ */
+ public Collection<Enum<T>> values() {
+ return enumerationMappings.values();
}
}
Added: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/ExtendedEnumeration.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/ExtendedEnumeration.java?rev=632775&view=auto
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/ExtendedEnumeration.java (added)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/types/ExtendedEnumeration.java Sun Mar 2 08:02:00 2008
@@ -0,0 +1,8 @@
+package org.apache.cayenne.access.types;
+
+public interface ExtendedEnumeration
+{
+ // Return the value to be stored in the database for this enumeration. In
+ // actuallity, this should be an Integer or a String.
+ public Object getDatabaseValue();
+}
Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/java/org/apache/cayenne/access/types/EnumTypeTest.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/java/org/apache/cayenne/access/types/EnumTypeTest.java?rev=632775&r1=632774&r2=632775&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/java/org/apache/cayenne/access/types/EnumTypeTest.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/java/org/apache/cayenne/access/types/EnumTypeTest.java Sun Mar 2 08:02:00 2008
@@ -30,11 +30,12 @@
public void testConstructor() throws Exception {
EnumType type = new EnumType(MockEnum.class);
assertEquals(MockEnum.class.getName(), type.getClassName());
- assertEquals(MockEnum.values().length, type.values.length);
-
- for(int i = 0; i < MockEnum.values().length; i++) {
- assertSame(MockEnum.values()[i], type.values[i]);
- }
+ assertEquals(MockEnum.values().length, type.getEnumerationMappings().size());
+// assertEquals(MockEnum.values().length, type.values.length);
+
+// for(int i = 0; i < MockEnum.values().length; i++) {
+// assertSame(MockEnum.values()[i], type.values[i]);
+// }
}
public void testInvalidConstructor1() throws Exception {
Re: ExtendedEnumeration comments
Posted by Michael Gentry <bl...@gmail.com>.
Comments inline.
Thanks,
/dev/mrg
On Sun, Mar 2, 2008 at 12:18 PM, Andrus Adamchik <an...@objectstyle.org> wrote:
>
> Hi Michael,
>
> I have a few suggestions/comments/nitpicks.
>
> * Placing ExtendedEnumeration class in
> "org.apache.cayenne.access.types" will make it inaccessible on the ROP
> client (access.* packages are excluded from the client). So maybe put
> it under 'org.apache.cayenne'?
Like I told Ari, I never use ROP and was concerned of issues. Now I
know of at least one. I'll move it.
> * Since ExtendedEnumeration is a distinct case from just an Enum from
> the POV of ExtendedType, maybe instead of handling both via EnumType
> (and complicating the algorithm), create a separate ExtendedEnumType,
> and place the "instanceof" check in EnumTypeFactory to create an
> appropriate *Type.
I can look at that. Seems logical and I know I wanted to do more work
on it. That won't change the client-usage, though, which is all I was
trying to nail down. Obviously, moving the interface will, but no one
is using this yet.
> * ExtendedEnumeration is missing Apache license header.
Oops.
> * From the docs: "The preferred method is to implement the Cayenne
> ExtendedEnumeration interface and provide the database value for each
> enumeration." IMO it is not universally *preferred*, it depends on the
> situation at hand and user preferences. Both methods work equally fine
> from Cayenne POV. I think we should just describe how both methods
> work and what is the difference. It seems logical to start with
> vanilla Enums and go to ExtendedEnumeration next.
>
> [Hmm... the more I think of it, enums mapping doesn't even belong in
> the chapter on ExtendedTypes customization anymore, as this is all
> built in Cayenne and transparent. Maybe extract it to the "Modeling
> Object Layer" chapter in the modeler guide?]
>
> Andrus
>
The main reason I say preferred is because vanilla enums can have
unplanned gotchas with the numeric version. I think it is best to
steer them to the extended version, but point out the vanilla version,
too. That will allow them to make a better choice (hopefully)
I was thinking the same thing about the location of the enum docs
being under extended types when I was typing this stuff up, but I just
wanted to get something documented for now. It seems a good idea to
move it, but I didn't want to think too much about where right now
(and it seems there should be something in the Modeler Guide and the
Cayenne Guide, but I wasn't sure where in CG).
Re: ExtendedEnumeration comments
Posted by Andrus Adamchik <an...@objectstyle.org>.
I am not necessarily disagreeing with you. I am just viewing the
documentation flow differently. To me "basic" comes before "extended".
Andrus
On Mar 14, 2008, at 6:40 PM, Michael Gentry wrote:
> That's exactly what I meant by a more orderly world ... :-)
>
> Everything I work on seems to be maintenance/enhancement of legacy
> systems.
>
> I don't mind swapping the order they appear in the docs, but I still
> think the extended ones are more reliable and flexible. I tried to
> point out the shortcomings of the standard approach -- I just don't
> want anyone getting burned by the integer issue with them.
>
> /dev/mrg
>
>
> On Fri, Mar 14, 2008 at 12:34 PM, Andrus Adamchik
> <an...@objectstyle.org> wrote:
>> I am lucky to have fewer (if any) legacy systems to deal with in the
>> last 3 years.
>>
>> Andrus
>>
>>
>>
>>
>> On Mar 14, 2008, at 4:44 PM, Michael Gentry wrote:
>>> That just tells me you live in a more orderly world than I do. :-)
>>>
>>>
>>> On Fri, Mar 14, 2008 at 10:40 AM, Andrus Adamchik
>>> <an...@objectstyle.org> wrote:
>>>> I still think it is logical to start with "Standard" and then
>>>> move to
>>>> "Extended", but otherwise looks great.
>>>>
>>>> Thanks,
>>>> Andrus
>>>>
>>>>
>>>>
>>>>
>>>> On Mar 14, 2008, at 4:32 PM, Michael Gentry wrote:
>>>>
>>>>> I moved the enumeration documentation to here:
>>>>>
>>>>> http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object
>>>>> +Layer
>>>>>
>>>>> And deleted it off the Extended Types page. Let me know if that's
>>>>> what you were thinking.
>>>>>
>>>>> I also checked in the other changes to the code, so I think this
>>>>> can
>>>>> close out CAY-994 unless you think of anything else that should be
>>>>> done for it right away.
>>>>>
>>>>> Thanks!
>>>>>
>>>>
>>>>
>>>
>>
>>
>
Re: ExtendedEnumeration comments
Posted by Michael Gentry <bl...@gmail.com>.
That's exactly what I meant by a more orderly world ... :-)
Everything I work on seems to be maintenance/enhancement of legacy systems.
I don't mind swapping the order they appear in the docs, but I still
think the extended ones are more reliable and flexible. I tried to
point out the shortcomings of the standard approach -- I just don't
want anyone getting burned by the integer issue with them.
/dev/mrg
On Fri, Mar 14, 2008 at 12:34 PM, Andrus Adamchik
<an...@objectstyle.org> wrote:
> I am lucky to have fewer (if any) legacy systems to deal with in the
> last 3 years.
>
> Andrus
>
>
>
>
> On Mar 14, 2008, at 4:44 PM, Michael Gentry wrote:
> > That just tells me you live in a more orderly world than I do. :-)
> >
> >
> > On Fri, Mar 14, 2008 at 10:40 AM, Andrus Adamchik
> > <an...@objectstyle.org> wrote:
> >> I still think it is logical to start with "Standard" and then move to
> >> "Extended", but otherwise looks great.
> >>
> >> Thanks,
> >> Andrus
> >>
> >>
> >>
> >>
> >> On Mar 14, 2008, at 4:32 PM, Michael Gentry wrote:
> >>
> >>> I moved the enumeration documentation to here:
> >>>
> >>> http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object
> >>> +Layer
> >>>
> >>> And deleted it off the Extended Types page. Let me know if that's
> >>> what you were thinking.
> >>>
> >>> I also checked in the other changes to the code, so I think this can
> >>> close out CAY-994 unless you think of anything else that should be
> >>> done for it right away.
> >>>
> >>> Thanks!
> >>>
> >>
> >>
> >
>
>
Re: ExtendedEnumeration comments
Posted by Andrus Adamchik <an...@objectstyle.org>.
I am lucky to have fewer (if any) legacy systems to deal with in the
last 3 years.
Andrus
On Mar 14, 2008, at 4:44 PM, Michael Gentry wrote:
> That just tells me you live in a more orderly world than I do. :-)
>
>
> On Fri, Mar 14, 2008 at 10:40 AM, Andrus Adamchik
> <an...@objectstyle.org> wrote:
>> I still think it is logical to start with "Standard" and then move to
>> "Extended", but otherwise looks great.
>>
>> Thanks,
>> Andrus
>>
>>
>>
>>
>> On Mar 14, 2008, at 4:32 PM, Michael Gentry wrote:
>>
>>> I moved the enumeration documentation to here:
>>>
>>> http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object
>>> +Layer
>>>
>>> And deleted it off the Extended Types page. Let me know if that's
>>> what you were thinking.
>>>
>>> I also checked in the other changes to the code, so I think this can
>>> close out CAY-994 unless you think of anything else that should be
>>> done for it right away.
>>>
>>> Thanks!
>>>
>>
>>
>
Re: ExtendedEnumeration comments
Posted by Michael Gentry <bl...@gmail.com>.
That just tells me you live in a more orderly world than I do. :-)
On Fri, Mar 14, 2008 at 10:40 AM, Andrus Adamchik
<an...@objectstyle.org> wrote:
> I still think it is logical to start with "Standard" and then move to
> "Extended", but otherwise looks great.
>
> Thanks,
> Andrus
>
>
>
>
> On Mar 14, 2008, at 4:32 PM, Michael Gentry wrote:
>
> > I moved the enumeration documentation to here:
> >
> > http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object
> > +Layer
> >
> > And deleted it off the Extended Types page. Let me know if that's
> > what you were thinking.
> >
> > I also checked in the other changes to the code, so I think this can
> > close out CAY-994 unless you think of anything else that should be
> > done for it right away.
> >
> > Thanks!
> >
>
>
Re: ExtendedEnumeration comments
Posted by Andrus Adamchik <an...@objectstyle.org>.
I still think it is logical to start with "Standard" and then move to
"Extended", but otherwise looks great.
Thanks,
Andrus
On Mar 14, 2008, at 4:32 PM, Michael Gentry wrote:
> I moved the enumeration documentation to here:
>
> http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object
> +Layer
>
> And deleted it off the Extended Types page. Let me know if that's
> what you were thinking.
>
> I also checked in the other changes to the code, so I think this can
> close out CAY-994 unless you think of anything else that should be
> done for it right away.
>
> Thanks!
>
Re: ExtendedEnumeration comments
Posted by Michael Gentry <bl...@gmail.com>.
I moved the enumeration documentation to here:
http://cwiki.apache.org/confluence/display/CAYDOC/Modeling+Object+Layer
And deleted it off the Extended Types page. Let me know if that's
what you were thinking.
I also checked in the other changes to the code, so I think this can
close out CAY-994 unless you think of anything else that should be
done for it right away.
Thanks!
ExtendedEnumeration comments
Posted by Andrus Adamchik <an...@objectstyle.org>.
On Mar 2, 2008, at 6:02 PM, mgentry@apache.org wrote:
> Added:
> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/
> java/org/apache/cayenne/access/types/ExtendedEnumeration.java
> Modified:
> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/
> java/org/apache/cayenne/access/QueryLogger.java
> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/
> java/org/apache/cayenne/access/types/EnumType.java
> cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/test/
> java/org/apache/cayenne/access/types/EnumTypeTest.java
Hi Michael,
I have a few suggestions/comments/nitpicks.
* Placing ExtendedEnumeration class in
"org.apache.cayenne.access.types" will make it inaccessible on the ROP
client (access.* packages are excluded from the client). So maybe put
it under 'org.apache.cayenne'?
* Since ExtendedEnumeration is a distinct case from just an Enum from
the POV of ExtendedType, maybe instead of handling both via EnumType
(and complicating the algorithm), create a separate ExtendedEnumType,
and place the "instanceof" check in EnumTypeFactory to create an
appropriate *Type.
* ExtendedEnumeration is missing Apache license header.
* From the docs: "The preferred method is to implement the Cayenne
ExtendedEnumeration interface and provide the database value for each
enumeration." IMO it is not universally *preferred*, it depends on the
situation at hand and user preferences. Both methods work equally fine
from Cayenne POV. I think we should just describe how both methods
work and what is the difference. It seems logical to start with
vanilla Enums and go to ExtendedEnumeration next.
[Hmm... the more I think of it, enums mapping doesn't even belong in
the chapter on ExtendedTypes customization anymore, as this is all
built in Cayenne and transparent. Maybe extract it to the "Modeling
Object Layer" chapter in the modeler guide?]
Andrus