db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Bouschen <mbo.t...@spree.de>
Subject Re: Changes for review
Date Mon, 04 Jul 2005 16:53:08 GMT
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(), new Integer(1));
>+    }
>+
>+    public void testGetKeyAsObject() {
>+        IntIdentity c1 = new IntIdentity(Object.class, new Integer(1));
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new 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.", sc1.equals(sc3));
>         assertFalse ("Not equal CharIdentity instances compare equal.", sc3.equals(sc1));
>     }
>+    public void testGetKeyAsObjectPrimitive() {
>+        CharIdentity c1 = new CharIdentity(Object.class, '1');
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new Character('1'));
>+    }
>+
>+    public void testGetKeyAsObject() {
>+        CharIdentity c1 = new CharIdentity(Object.class, new Character('1'));
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new 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.", sc1.equals(sc3));
>         assertFalse ("Not equal ShortIdentity instances compare equal.", sc3.equals(sc1));
>     }
>+    public void testGetKeyAsObjectPrimitive() {
>+        ShortIdentity c1 = new ShortIdentity(Object.class, (short)1);
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new Short((short)1));
>+    }
>+
>+    public void testGetKeyAsObject() {
>+        ShortIdentity c1 = new ShortIdentity(Object.class, new Short((short)1));
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new 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.", sc1.equals(sc3));
>         assertFalse ("Not equal LongIdentity instances compare equal.", sc3.equals(sc1));
>     }
>+    
>+    public void testGetKeyAsObjectPrimitive() {
>+        LongIdentity c1 = new LongIdentity(Object.class, 1L);
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new Long(1L));
>+    }
>+
>+    public void testGetKeyAsObject() {
>+        LongIdentity c1 = new LongIdentity(Object.class, new Long(1L));
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new 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.", sc1.equals(sc3));
>         assertFalse ("Not equal ByteIdentity instances compare equal.", sc3.equals(sc1));
>     }
>+    
>+    public void testGetKeyAsObjectPrimitive() {
>+        ByteIdentity c1 = new ByteIdentity(Object.class, (byte)1);
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new Byte((byte)1));
>+    }
>+
>+    public void testGetKeyAsObject() {
>+        ByteIdentity c1 = new ByteIdentity(Object.class, new Byte((byte)1));
>+        assertEquals("keyAsObject doesn't match.", c1.getKeyAsObject(), new 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.Bundle"); //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>PersistenceCapable</code>
>+     * class, using the <code>Object</code> form of the constructor.
>+     * @return the new ObjectId instance, or <code>null</code> if the class
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(obj);
>+    }
>+    
>+    /** Create a new instance of the ObjectId class of this <code>PersistenceCapable</code>
>      * class, using the <code>String</code> form of the constructor.
>      * @return the new ObjectId instance, or <code>null</code> if the class
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>PersistenceManagerFactory</code>
class.
>      */
>     public static PersistenceManagerFactory getPersistenceManagerFactory
>-            (Properties props, ClassLoader cl) {
>+            (Map props, ClassLoader cl) {
>         String pmfClassName = (String) props.get ("javax.jdo.PersistenceManagerFactoryClass");
//NOI18N
>         if (pmfClassName == null) {
>             throw new JDOFatalUserException (msg.msg("EXC_NoClassNameProperty")); //
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 ("getPersistenceManagerFactory",  //NOI18N
>-                new Class[] {Properties.class});
>+            try {
>+                propsMethod = pmfClass.getMethod("getPersistenceManagerFactory",  //NOI18N
>+                    new Class[] {Properties.class});
>+            } catch (NoSuchMethodException nsme) {
>+                propsGetMethodException = nsme;
>+            }
>+            try {
>+                mapMethod = pmfClass.getMethod("getPersistenceManagerFactory",  //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 method.
>+                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_getPersistenceManagerFactory"),
ite); //NOI18N
>+            } else throw new JDOFatalInternalException (msg.msg("EXC_getPersistenceManagerFactory"),
ite); //NOI18N
>         } catch (Exception e) {
>             throw new JDOFatalInternalException (msg.msg("ERR_UnexpectedException"),
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 the input String
for CharIdentity.
> EXC_IllegalEventType:The event type is outside the range of valid event types.
>+EXC_ObjectIdentityStringConstruction: The instance could not be constructed from \
>+the parameter String "{0}". \nThe exception thrown was: "{1}". \n\
>+Parsing the class name as "{2}" and key as "{3}".
>+EXC_ObjectIdentityStringConstructionNoDelimiter: Missing delimiter ":".
>+EXC_ObjectIdentityStringConstructionTooShort: Parameter is too short.
>+EXC_ObjectIdentityStringConstructionUsage: The instance could not be constructed \
>+from the parameter String "{0}". \
>+\nThe parameter String is of the form "<className>:<keyString>".
>+EXC_CreateKeyAsObjectMustNotBeCalled: The method createKeyAsObject must not 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			


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message