Hi Michael,
On Jul 2, 2005, at 11:58 AM, Michael Bouschen wrote:
Hi Craig,
the changes look good!
I would like to propose a few improvements of the method mangleObject
you changed. Maybe this should be an extra check in, because they are
not related to the fix for JDO-77.
(1) We plan to add configuration running the TCK with a security
manager. Then the reflection access of method mangleObject should be
enclosed in a doPivileged block.
Done.
(2) The method checks for fields of specific types. Should we add
date, BigDecimal, BigInteger, float, double, Float and Double?
Later. None of the identity classes uses these types.
(3) We could use 'else if' when checking the field types.
Done.
Please check the attached.
Thanks,
Craig
=
------------------------------------------------------------------------
Index: test/java/org/apache/jdo/tck/JDO_Test.java
===================================================================
--- test/java/org/apache/jdo/tck/JDO_Test.java (revision 208860)
+++ test/java/org/apache/jdo/tck/JDO_Test.java (working copy)
@@ -20,10 +20,18 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
+
import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.List;
import java.util.Properties;
import java.util.Vector;
@@ -749,54 +757,75 @@
return NUM_STATES;
}
- /** This method mangles an object by changing all its public fields
+ /** This method mangles an object by changing all its non-static,
+ * non-final fields.
+ * It returns true if the object was mangled, and false if there
+ * are no fields to mangle.
*/
- protected void mangleObject (Object oid)
+ protected boolean mangleObject (Object oid)
throws Exception {
- Class oidClass = oid.getClass();
- Field[] fields = oidClass.getFields();
+ Field[] fields = getModifiableFields(oid);
+ if (fields.length == 0) return false;
for (int i = 0; i < fields.length; ++i) {
Field field = fields[i];
- field.setAccessible(true);
- if (debug)
- logger.debug("field" + i + " has name: " + field.getName() +
- " type: " + field.getType());
Class fieldType = field.getType();
if (fieldType == long.class) {
- field.setLong(oid, 10000L);
+ field.setLong(oid, 10000L + field.getLong(oid));
+ } else if (fieldType == int.class) {
+ field.setInt(oid, 10000 + field.getInt(oid));
+ } else if (fieldType == short.class) {
+ field.setShort(oid, (short)(10000 + field.getShort(oid)));
+ } else if (fieldType == byte.class) {
+ field.setByte(oid, (byte)(100 + field.getByte(oid)));
+ } else if (fieldType == char.class) {
+ field.setChar(oid, (char)(10 + field.getChar(oid)));
+ } else if (fieldType == String.class) {
+ field.set(oid, "This is certainly a challenge" +
(String)field.get(oid));
+ } else if (fieldType == Integer.class) {
+ field.set(oid, new Integer(10000 +
((Integer)field.get(oid)).intValue()));
+ } else if (fieldType == Long.class) {
+ field.set(oid, new Long(10000L +
((Long)field.get(oid)).longValue()));
+ } else if (fieldType == Short.class) {
+ field.set(oid, new Short((short)(10000 +
((Short)field.get(oid)).shortValue())));
+ } else if (fieldType == Byte.class) {
+ field.set(oid, new Byte((byte)(100 +
((Byte)field.get(oid)).byteValue())));
+ } else if (fieldType == Character.class) {
+ field.set(oid, new Character((char)(10 +
((Character)(field.get(oid))).charValue())));
}
- if (fieldType == int.class) {
- field.setInt(oid, 10000);
- }
- if (fieldType == short.class) {
- field.setShort(oid, (short)10000);
- }
- if (fieldType == byte.class) {
- field.setByte(oid, (byte)100);
- }
- if (fieldType == char.class) {
- field.setChar(oid, '0');
- }
- if (fieldType == String.class) {
- field.set(oid, "This is certainly a challenge");
- }
- if (fieldType == Integer.class) {
- field.set(oid, new Integer(10000));
- }
- if (fieldType == Long.class) {
- field.set(oid, new Long(10000L));
- }
- if (fieldType == Short.class) {
- field.set(oid, new Short((short)10000));
- }
- if (fieldType == Byte.class) {
- field.set(oid, new Byte((byte)100));
- }
- if (fieldType == Character.class) {
- field.set(oid, new Character('0'));
- }
}
+ return true;
}
+
+ /** Returns modifiable Fields of the class of the parameter.
+ * Fields are considered modifiable if they are not static or final.
+ * This method requires several permissions in order to run with
+ * a SecurityManager, hence the doPrivileged block:
+ * <ul>
+ * <li>ReflectPermission("suppressAccessChecks")</li>
+ * <li>RuntimePermission("accessDeclaredMembers")</li>
+ * </ul>
+ */
+ protected Field[] getModifiableFields(final Object obj) {
+ return (Field[])AccessController.doPrivileged(
+ new PrivilegedAction () {
+ public Object run () {
+ Class cls = obj.getClass();
+ List result = new ArrayList();
+ Field[] fields = cls.getFields();
+ for (int i = 0; i < fields.length; ++i) {
+ Field field = fields[i];
+ int modifiers = field.getModifiers();
+ if (Modifier.isFinal(modifiers) ||
+ Modifier.isStatic(modifiers))
+ continue;
+ field.setAccessible(true);
+ result.add(field);
+ }
+ return result.toArray(new Field[result.size()]);
+ }
+ }
+ );
+ }
/**
* Returns <code>true</code> if the current test runs with application
Index:
test/java/org/apache/jdo/tck/lifecycle/ObjectIdNotModifiedWhenObjectIdInstanceModified.java
===================================================================
---
test/java/org/apache/jdo/tck/lifecycle/ObjectIdNotModifiedWhenObjectIdInstanceModified.java
(revision 208860)
+++
test/java/org/apache/jdo/tck/lifecycle/ObjectIdNotModifiedWhenObjectIdInstanceModified.java
(working copy)
@@ -16,7 +16,9 @@
package org.apache.jdo.tck.lifecycle;
+import java.util.ArrayList;
import java.util.Iterator;
+import java.util.List;
import javax.jdo.Extent;
import javax.jdo.Transaction;
@@ -76,16 +78,45 @@
"Extent for StateTransitionObj should not be
empty");
}
extent.close(iter);
-
- for (int i=0; i<NUM_OBJECTS; i++)
- {
- Object objId=pm.getObjectId(obj[i]);
- mangleObject(objId);
- Object objId2 = pm.getObjectId(obj[i]); // get another
ObjectId copy
- if (objId.equals(objId2))
- fail(ASSERTION_FAILED,
- "object Id has been changed");
+ int failures = 0;
+ StringBuffer report = new StringBuffer("Failures comparing
oids.\n");
+ for (int i=0; i<NUM_OBJECTS; i++) {
+ Object objId1=pm.getObjectId(obj[i]);
+ String before=objId1.toString();
+ int objId1HashCode = objId1.hashCode();
+ Object objId2=pm.getObjectId(obj[i]);
+ if (!mangleObject(objId2)) {
+ /* The object id class is immutable, so the test succeeds.
*/
+ break;
+ }
+ int objId2HashCode = objId2.hashCode();
+ Object objId3 = pm.getObjectId(obj[i]); // get another
ObjectId copy
+ if (!(objId1.equals(objId3) && objId1HashCode !=
objId2HashCode)) {
+ /* The object id obtained after mangling the second object
id
+ * must equal the original object id, and the mangling must
+ * have changed the mangled id.
+ */
+ report.append("Index= ");
+ report.append(i);
+ report.append("\n");
+ report.append(" before= ");
+ report.append(before);
+ report.append("\n");
+ report.append("mangled= ");
+ report.append(objId2.toString());
+ report.append("\n");
+ report.append(" after= ");
+ report.append(objId3.toString());
+ report.append("\n");
+ ++failures;
+ }
}
+ if (failures != 0) {
+ if (debug) {
+ logger.debug(report.toString());
+ }
+ fail(ASSERTION_FAILED, "Failed to compare " + failures + " object
ids.");
+ }
pm.currentTransaction().commit();
} finally {
if (pm!=null && pm.currentTransaction().isActive()) {
------------------------------------------------------------------------
Regards Michael
Hi,
Please review this patch. It fixes an illegal access exception when
mangleObject tries to change a final field. The final field was
added to the PCPoint$Oid class.
The fix checks for final and static modifiers on the field before it
changes it.
I also changed the method to actually change each field, instead of
simply changing its value to a constant.
Thanks,
Craig
------------------------------------------------------------------------
Index: test/java/org/apache/jdo/tck/JDO_Test.java
===================================================================
--- test/java/org/apache/jdo/tck/JDO_Test.java (revision 208860)
+++ test/java/org/apache/jdo/tck/JDO_Test.java (working copy)
@@ -757,43 +757,47 @@
Field[] fields = oidClass.getFields();
for (int i = 0; i < fields.length; ++i) {
Field field = fields[i];
+ int modifiers = field.getModifiers();
+ if (java.lang.reflect.Modifier.isFinal(modifiers) ||
+ java.lang.reflect.Modifier.isStatic(modifiers))
+ break;
field.setAccessible(true);
if (debug) logger.debug("field" + i + "
has name: " + field.getName() +
" type: " + field.getType());
Class fieldType = field.getType();
if (fieldType == long.class) {
- field.setLong(oid, 10000L);
+ field.setLong(oid, 10000L + field.getLong(oid));
}
if (fieldType == int.class) {
- field.setInt(oid, 10000);
+ field.setInt(oid, 10000 + field.getInt(oid));
}
if (fieldType == short.class) {
- field.setShort(oid, (short)10000);
+ field.setShort(oid, (short)(10000 +
field.getShort(oid)));
}
if (fieldType == byte.class) {
- field.setByte(oid, (byte)100);
+ field.setByte(oid, (byte)(100 + field.getByte(oid)));
}
if (fieldType == char.class) {
- field.setChar(oid, '0');
+ field.setChar(oid, (char)(10 + field.getChar(oid)));
}
if (fieldType == String.class) {
- field.set(oid, "This is certainly a challenge");
+ field.set(oid, "This is certainly a challenge" +
(String)field.get(oid));
}
if (fieldType == Integer.class) {
- field.set(oid, new Integer(10000));
+ field.set(oid, new Integer(10000 +
((Integer)field.get(oid)).intValue()));
}
if (fieldType == Long.class) {
- field.set(oid, new Long(10000L));
+ field.set(oid, new Long(10000L +
((Long)field.get(oid)).longValue()));
}
if (fieldType == Short.class) {
- field.set(oid, new Short((short)10000));
+ field.set(oid, new Short((short)(10000 +
((Short)field.get(oid)).shortValue())));
}
if (fieldType == Byte.class) {
- field.set(oid, new Byte((byte)100));
+ field.set(oid, new Byte((byte)(100 +
((Byte)field.get(oid)).byteValue())));
}
if (fieldType == Character.class) {
- field.set(oid, new Character('0'));
+ field.set(oid, new Character((char)(10 +
((Character)(field.get(oid))).charValue())));
}
}
}
------------------------------------------------------------------------
Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:[EMAIL PROTECTED]
P.S. A good JDO? O, Gasp!
--
Michael Bouschen [EMAIL PROTECTED] Engineering GmbH
mailto:[EMAIL PROTECTED] http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin
Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:[EMAIL PROTECTED]
P.S. A good JDO? O, Gasp!
=
