|
Home > Archive > Apache JDO Project > July 2005 > Fix for JDO-77
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
|
|
| Craig Russell 2005-07-02, 5:45 pm |
| | |
| Michael Bouschen 2005-07-02, 5:45 pm |
| 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.
(2) The method checks for fields of specific types. Should we add date,
BigDecimal, BigInteger, float, double, Float and Double?
(3) We could use 'else if' when checking the field types.
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: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
| |
| Craig Russell 2005-07-03, 5:45 pm |
| | |
| Michael Bouschen 2005-07-03, 5:45 pm |
| Hi Craig,
this looks good, no remarks from my side.
Regards Michael
> Hi Michael,
>
> On Jul 2, 2005, at 11:58 AM, Michael Bouschen wrote:
>
>
>
> Done.
>
>
>
> Later. None of the identity classes uses these 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/ ObjectIdNotModifiedWhenObjectIdInstanceM
odified.java
> ========================================
===========================
>--- test/java/org/apache/jdo/tck/lifecycle/ ObjectIdNotModifiedWhenObjectIdInstanceM
odified.java (revision 208860)
>+++ test/java/org/apache/jdo/tck/lifecycle/ ObjectIdNotModifiedWhenObjectIdInstanceM
odified.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()) {
>
>
> ------------------------------------------------------------------------
>
>
>
> 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
|
|
|
|
|