07-04-05 10:45 PM
Hi Craig,
the changes look good! Two remarks:
- I noticed you kept the JDO1 version of method newObjectIdInstance
taking a String. This is to support backward compatibility with JDO1,
correct? If yes, I propose to add a note to the javadoc saying that this
method is sort of deprecated. The String method could delegate to the
Object method.
- Typo: the param javadoc on line 187 uses 'str' instead of 'obj' as the
parameter name of the parameter.
Regards Michael
> Hi,
>
> Final review. Thanks for your review of the first draft.
>
> Please take a look at these changes for API 20. They are needed to
> implement the latest changes to the JDO specification.
>
> Thanks,
>
> Craig
>
>------------------------------------------------------------------------
>
>Index: test/java/javax/jdo/identity/StringIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/StringIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/StringIdentityTest.java (working copy)
>@@ -70,4 +70,10 @@
> assertFalse ("Not equal StringIdentity instances compare equal.",
sc1.equals(sc3));
> assertFalse ("Not equal StringIdentity instances compare equal.",
sc3.equals(sc1));
> }
>+
>+ public void testGetKeyAsObject() {
>+ StringIdentity c1 = new StringIdentity(Object.class, "1");
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), "1
");
>+ }
>+
> }
>Index: test/java/javax/jdo/identity/IntIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/IntIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/IntIdentityTest.java (working copy)
>@@ -95,4 +95,14 @@
> assertFalse ("Not equal IntIdentity instances compare equal.", sc1
.equals(sc3));
> assertFalse ("Not equal IntIdentity instances compare equal.", sc3
.equals(sc1));
> }
>+ public void testGetKeyAsObjectPrimitive() {
>+ IntIdentity c1 = new IntIdentity(Object.class, 1);
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Integer(1));
>+ }
>+
>+ public void testGetKeyAsObject() {
>+ IntIdentity c1 = new IntIdentity(Object.class, new Integer(1));
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Integer(1));
>+ }
>+
> }
>Index: test/java/javax/jdo/identity/CharIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/CharIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/CharIdentityTest.java (working copy)
>@@ -105,4 +105,14 @@
> assertFalse ("Not equal CharIdentity instances compare equal.", sc
1.equals(sc3));
> assertFalse ("Not equal CharIdentity instances compare equal.", sc
3.equals(sc1));
> }
>+ public void testGetKeyAsObjectPrimitive() {
>+ CharIdentity c1 = new CharIdentity(Object.class, '1');
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Character('1'));
>+ }
>+
>+ public void testGetKeyAsObject() {
>+ CharIdentity c1 = new CharIdentity(Object.class, new Character('1'
));
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Character('1'));
>+ }
>+
> }
>Index: test/java/javax/jdo/identity/ShortIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/ShortIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/ShortIdentityTest.java (working copy)
>@@ -95,4 +95,14 @@
> assertFalse ("Not equal ShortIdentity instances compare equal.", s
c1.equals(sc3));
> assertFalse ("Not equal ShortIdentity instances compare equal.", s
c3.equals(sc1));
> }
>+ public void testGetKeyAsObjectPrimitive() {
>+ ShortIdentity c1 = new ShortIdentity(Object.class, (short)1);
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Short((short)1));
>+ }
>+
>+ public void testGetKeyAsObject() {
>+ ShortIdentity c1 = new ShortIdentity(Object.class, new Short((shor
t)1));
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Short((short)1));
>+ }
>+
> }
>Index: test/java/javax/jdo/identity/LongIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/LongIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/LongIdentityTest.java (working copy)
>@@ -95,4 +95,15 @@
> assertFalse ("Not equal LongIdentity instances compare equal.", sc
1.equals(sc3));
> assertFalse ("Not equal LongIdentity instances compare equal.", sc
3.equals(sc1));
> }
>+
>+ public void testGetKeyAsObjectPrimitive() {
>+ LongIdentity c1 = new LongIdentity(Object.class, 1L);
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Long(1L));
>+ }
>+
>+ public void testGetKeyAsObject() {
>+ LongIdentity c1 = new LongIdentity(Object.class, new Long(1L));
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Long(1L));
>+ }
>+
> }
>Index: test/java/javax/jdo/identity/ByteIdentityTest.java
> ========================================
===========================
>--- test/java/javax/jdo/identity/ByteIdentityTest.java (revision 202295)
>+++ test/java/javax/jdo/identity/ByteIdentityTest.java (working copy)
>@@ -95,4 +95,15 @@
> assertFalse ("Not equal ByteIdentity instances compare equal.", sc
1.equals(sc3));
> assertFalse ("Not equal ByteIdentity instances compare equal.", sc
3.equals(sc1));
> }
>+
>+ public void testGetKeyAsObjectPrimitive() {
>+ ByteIdentity c1 = new ByteIdentity(Object.class, (byte)1);
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Byte((byte)1));
>+ }
>+
>+ public void testGetKeyAsObject() {
>+ ByteIdentity c1 = new ByteIdentity(Object.class, new Byte((byte)1)
);
>+ assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), ne
w Byte((byte)1));
>+ }
>+
> }
>Index: src/java/javax/jdo/identity/SingleFieldIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/SingleFieldIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/SingleFieldIdentity.java (working copy)
>@@ -26,6 +26,10 @@
> import java.io.ObjectInput;
> import java.io.ObjectOutput;
>
>+import javax.jdo.JDOFatalInternalException;
>+
>+import javax.jdo.spi.I18NHelper;
>+
> /** This class is the abstract base class for all single field identity
> * classes. A common case of application identity uses exactly one
> * persistent field in the class to represent identity. In this case,
>@@ -36,6 +40,10 @@
> public abstract class SingleFieldIdentity
> implements Externalizable {
>
>+ /** The Internationalization message helper.
>+ */
>+ protected static I18NHelper msg = I18NHelper.getInstance ("javax.jdo.B
undle"); //NOI18N
>+
> /** The class of the target object.
> */
> transient private Class targetClass;
>@@ -47,6 +55,10 @@
> /** The hashCode.
> */
> protected int hashCode;
>+
>+ /** The key as an Object.
>+ */
>+ protected Object keyAsObject;
>
> /** Constructor with target class.
> * @param pcClass the class of the target
>@@ -80,6 +92,27 @@
> return targetClassName;
> }
>
>+ /** Return the key as an Object. The method is synchronized to avoid
>+ * race conditions in multi-threaded environments.
>+ * @return the key as an Object.
>+ * @since 2.0
>+ */
>+ public synchronized Object getKeyAsObject() {
>+ if (keyAsObject == null) {
>+ keyAsObject = createKeyAsObject();
>+ }
>+ return keyAsObject;
>+ }
>+
>+ /** Create the key as an Object.
>+ * @return the key as an Object;
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ throw new JDOFatalInternalException
>+ (msg.msg("EXC_CreateKeyAsObjectMustNotBeCalled"));
>+ }
>+
> /** Check the class and class name and object type. If restored
> * from serialization, class will be null so compare class name.
> * @param obj the other object
>Index: src/java/javax/jdo/identity/StringIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/StringIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/StringIdentity.java (working copy)
>@@ -30,11 +30,9 @@
> */
> public class StringIdentity extends SingleFieldIdentity {
>
>- /** The key.
>+ /** The key is stored in the superclass field keyAsObject.
> */
>- private String key;
>-
>-
>+
> /** Constructor with class and key.
> * @param pcClass the class
> * @param key the key
>@@ -43,8 +41,8 @@
> super (pcClass);
> if (key == null)
> throw new NullPointerException ();
>- this.key = key;
>- hashCode = hashClassName() ^ key.hashCode();
>+ this.keyAsObject = key;
>+ hashCode = hashClassName() ^ keyAsObject.hashCode();
> }
>
> /** Constructor only for Externalizable.
>@@ -56,14 +54,14 @@
> * @return the key
> */
> public String getKey () {
>- return key;
>+ return (String)keyAsObject;
> }
>
> /** Return the String form of the key.
> * @return the String form of the key
> */
> public String toString () {
>- return key;
>+ return (String)keyAsObject;
> }
>
> /** Determine if the other object represents the same object id.
>@@ -77,7 +75,7 @@
> return false;
> } else {
> StringIdentity other = (StringIdentity) obj;
>- return key.equals(other.key);
>+ return keyAsObject.equals(other.keyAsObject);
> }
> }
>
>@@ -86,7 +84,7 @@
> */
> public void writeExternal(ObjectOutput out) throws IOException {
> super.writeExternal (out);
>- out.writeObject(key);
>+ out.writeObject(keyAsObject);
> }
>
> /** Read this object. Read the superclass first.
>@@ -95,7 +93,6 @@
> public void readExternal(ObjectInput in)
> throws IOException, ClassNotFoundException {
> super.readExternal (in);
>- key = (String)in.readObject();
>- hashCode = hashClassName() ^ key.hashCode();
>+ keyAsObject = (String)in.readObject();
> }
> }
>Index: src/java/javax/jdo/identity/IntIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/IntIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/IntIdentity.java (working copy)
>@@ -47,6 +47,7 @@
> */
> public IntIdentity (Class pcClass, Integer key) {
> this (pcClass, key.intValue ());
>+ keyAsObject = key;
> }
>
>
>@@ -92,6 +93,14 @@
> }
> }
>
>+ /** Create the key as an Object.
>+ * @return the key as an Object
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ return new Integer(key);
>+ }
>+
> /** Write this object. Write the superclass first.
> * @param out the output
> */
>@@ -107,6 +116,5 @@
> throws IOException, ClassNotFoundException {
> super.readExternal (in);
> key = in.readInt();
>- hashCode = hashClassName() ^ key;
> }
> }
>Index: src/java/javax/jdo/identity/CharIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/CharIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/CharIdentity.java (working copy)
>@@ -56,6 +56,7 @@
> */
> public CharIdentity (Class pcClass, Character key) {
> this (pcClass, key.charValue ());
>+ keyAsObject = key;
> }
>
> /** Constructor with class and key. The String must have exactly one
>@@ -106,6 +107,14 @@
> }
> }
>
>+ /** Create the key as an Object.
>+ * @return the key as an Object
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ return new Character(key);
>+ }
>+
> /** Write this object. Write the superclass first.
> * @param out the output
> */
>Index: src/java/javax/jdo/identity/ShortIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/ShortIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/ShortIdentity.java (working copy)
>@@ -50,6 +50,7 @@
> */
> public ShortIdentity (Class pcClass, Short key) {
> this (pcClass, key.shortValue ());
>+ keyAsObject = key;
> }
>
> /** Constructor with class and key.
>@@ -94,6 +95,14 @@
> }
> }
>
>+ /** Create the key as an Object.
>+ * @return the key as an Object
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ return new Short(key);
>+ }
>+
> /** Write this object. Write the superclass first.
> * @param out the output
> */
>@@ -109,6 +118,5 @@
> throws IOException, ClassNotFoundException {
> super.readExternal (in);
> key = in.readShort();
>- hashCode = hashClassName() ^ key;
> }
> }
>Index: src/java/javax/jdo/identity/LongIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/LongIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/LongIdentity.java (working copy)
>@@ -50,6 +50,7 @@
> */
> public LongIdentity (Class pcClass, Long key) {
> this (pcClass, key.longValue ());
>+ keyAsObject = key;
> }
>
> /** Constructor with class and key.
>@@ -94,6 +95,14 @@
> }
> }
>
>+ /** Create the key as an Object.
>+ * @return the key as an Object
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ return new Long(key);
>+ }
>+
> /** Write this object. Write the superclass first.
> * @param out the output
> */
>@@ -109,6 +118,6 @@
> throws IOException, ClassNotFoundException {
> super.readExternal (in);
> key = in.readLong();
>- hashCode = hashClassName() ^ (int)key;
> }
>+
> }
>Index: src/java/javax/jdo/identity/ByteIdentity.java
> ========================================
===========================
>--- src/java/javax/jdo/identity/ByteIdentity.java (revision 202295)
>+++ src/java/javax/jdo/identity/ByteIdentity.java (working copy)
>@@ -50,6 +50,7 @@
> */
> public ByteIdentity(Class pcClass, Byte key) {
> this (pcClass, key.byteValue());
>+ keyAsObject = key;
> }
>
> /** Constructor with class and key.
>@@ -94,6 +95,14 @@
> }
> }
>
>+ /** Create the key as an Object.
>+ * @return the key as an Object
>+ * @since 2.0
>+ */
>+ protected Object createKeyAsObject() {
>+ return new Byte(key);
>+ }
>+
> /** Write this object. Write the superclass first.
> * @param out the output
> */
>@@ -109,6 +118,5 @@
> throws IOException, ClassNotFoundException {
> super.readExternal (in);
> key = in.readByte ();
>- hashCode = super.hashCode() ^ key;
> }
> }
>Index: src/java/javax/jdo/spi/JDOImplHelper.java
> ========================================
===========================
>--- src/java/javax/jdo/spi/JDOImplHelper.java (revision 202295)
>+++ src/java/javax/jdo/spi/JDOImplHelper.java (working copy)
>@@ -182,6 +182,18 @@
> }
>
> /** Create a new instance of the ObjectId class of this <code>Persiste
nceCapable</code>
>+ * class, using the <code>Object</code> form of the constructor.
>+ * @return the new ObjectId instance, or <code>null</code> if the clas
s is not registered.
>+ * @param str the <code>Object</code> form of the object id
>+ * @param pcClass the <code>PersistenceCapable</code> class.
>+ */
>+ public Object newObjectIdInstance (Class pcClass, Object obj) {
>+ Meta meta = getMeta (pcClass);
>+ PersistenceCapable pcInstance = meta.getPC();
>+ return pcInstance == null?null:pcInstance.jdoNewObjectIdInstance(o
bj);
>+ }
>+
>+ /** Create a new instance of the ObjectId class of this <code>Persiste
nceCapable</code>
> * class, using the <code>String</code> form of the constructor.
> * @return the new ObjectId instance, or <code>null</code> if the clas
s is not registered.
> * @param str the <code>String</code> form of the object id
>Index: src/java/javax/jdo/JDOHelper.java
> ========================================
===========================
>--- src/java/javax/jdo/JDOHelper.java (revision 202295)
>+++ src/java/javax/jdo/JDOHelper.java (working copy)
>@@ -30,6 +30,7 @@
> import java.lang.reflect.Method;
> import java.lang.reflect.InvocationTargetException;
>
>+import java.util.Map;
> import java.util.Properties;
>
> import javax.jdo.spi.I18NHelper;
>@@ -249,7 +250,7 @@
> * @see #getPersistenceManagerFactory(Properties
,ClassLoader)
> */
> public static PersistenceManagerFactory getPersistenceManagerFactory
>- (Properties props) {
>+ (Map props) {
> ClassLoader cl = Thread.currentThread().getContextClassLoader();
> return getPersistenceManagerFactory (props, cl);
> }
>@@ -287,15 +288,49 @@
> * @param cl a class loader to use to load the <code>PersistenceManage
rFactory</code> class.
> */
> public static PersistenceManagerFactory getPersistenceManagerFactory
>- (Properties props, ClassLoader cl) {
>+ (Map props, ClassLoader cl) {
> String pmfClassName = (String) props.get ("javax.jdo.PersistenceMa
nagerFactoryClass"); //NOI18N
> if (pmfClassName == null) {
> throw new JDOFatalUserException (msg.msg("EXC_NoClassNamePrope
rty")); // NOI18N
> }
>+ Method propsMethod = null;
>+ Exception propsGetMethodException = null;
>+ Method mapMethod = null;
>+ Exception mapGetMethodException = null;
>+ Method pmfMethod = null;
> try {
> Class pmfClass = cl.loadClass (pmfClassName);
>- Method pmfMethod = pmfClass.getMethod ("getPersistenceManagerF
actory", //NOI18N
>- new Class[] {Properties.class});
>+ try {
>+ propsMethod = pmfClass.getMethod("getPersistenceManagerFac
tory", //NOI18N
>+ new Class[] {Properties.class});
>+ } catch (NoSuchMethodException nsme) {
>+ propsGetMethodException = nsme;
>+ }
>+ try {
>+ mapMethod = pmfClass.getMethod("getPersistenceManagerFacto
ry", //NOI18N
>+ new Class[] {Map.class});
>+ } catch (NoSuchMethodException nsme) {
>+ mapGetMethodException = nsme;
>+ }
>+ /* If the parameter is not a Properties,
>+ * we need a mapMethod or else throw an exception.
>+ */
>+ if (!(props instanceof Properties)) {
>+ if (mapMethod != null) {
>+ pmfMethod = mapMethod;
>+ } else {
>+ throw mapGetMethodException;
>+ }
>+ } else { // the parameter is a Properties; use either met
hod.
>+ if (mapMethod != null) {
>+ pmfMethod = mapMethod;
>+ } else if (propsMethod != null) {
>+ pmfMethod = propsMethod;
>+ } else {
>+ throw mapGetMethodException;
>+ }
>+ }
>+
> return (PersistenceManagerFactory) pmfMethod.invoke (null, new
Object[] {props});
> } catch (ClassNotFoundException cnfe) {
> throw new JDOFatalUserException (msg.msg("EXC_ClassNotFound",
pmfClassName), cnfe); //NOI18N
>@@ -307,12 +342,12 @@
> Throwable nested = ite.getTargetException();
> if (nested instanceof JDOException) {
> throw (JDOException)nested;
>- } else throw new JDOFatalUserException (msg.msg("EXC_getPersis
tenceManagerFactory"), ite); //NOI18N
>+ } else throw new JDOFatalInternalException (msg.msg("EXC_getPe
rsistenceManagerFactory"), ite); //NOI18N
> } catch (Exception e) {
> throw new JDOFatalInternalException (msg.msg("ERR_UnexpectedEx
ception"), e); //NOI18N
> }
> }
>-
>+
> /**
> * Returns a {@link PersistenceManagerFactory} configured based
> * on the properties stored in the resource at
>Index: src/java/javax/jdo/Bundle.properties
> ========================================
===========================
>--- src/java/javax/jdo/Bundle.properties (revision 202295)
>+++ src/java/javax/jdo/Bundle.properties (working copy)
>@@ -46,3 +46,13 @@
> PersistenceManagerFactory at "{0}" from JNDI.
> EXC_StringWrongLength: There must be exactly one character in the id in th
e input String for CharIdentity.
> EXC_IllegalEventType:The event type is outside the range of valid event ty
pes.
>+EXC_ObjectIdentityStringConstruction: The instance could not be constructe
d from \
>+the parameter String "{0}". \nThe exception thrown was: "{1}". \
n\
>+Parsing the class name as "{2}" and key as "{3}".
> +EXC_ObjectIdentityStringConstructionNoD
elimiter: Missing delimiter ":".
> +EXC_ObjectIdentityStringConstructionToo
Short: Parameter is too short.
> +EXC_ObjectIdentityStringConstructionUsa
ge: The instance could not be const
ructed \
>+from the parameter String "{0}". \
>+\nThe parameter String is of the form "<className>:<keyString>".
>+EXC_CreateKeyAsObjectMustNotBeCalled: The method createKeyAsObject must no
t be called \
>+because the keyAsObject field must never be null for this class.
>
>
> ------------------------------------------------------------------------
>
>
> 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!
>
>
> 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!
>
>
> =
--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin
[ Post a follow-up to this message ]
|
