Bugs item #837092, was opened at 2003-11-06 12:06
Message generated for change (Comment added) made by loubyansky
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=376685&aid=837092&group_id=22866
Category: JBossCMP
Group: v3.2
>Status: Closed
>Resolution: Fixed
Priority: 5
Submitted By: Adrian Price (adrianprice)
Assigned to: Alexey Loubyansky (loubyansky)
Summary: CMP Field Update Optimization is Broken
Initial Comment:
The mechanism for optimizing updates to CMP fields is
broken, because it makes the erroneous assumption that
the value being stored is immutable.
o.j.e.p.c.j.b.JDBCFieldBridge.setInstanceValue() or
thereabouts ignores the update if
oldValue().equals(newValue). Whilst this works for
primitives (or rather their immutable java.lang.*
wrappers), it breaks completely for mutable
serializable objects in a BLOB, because if one
retrieves such an object from a CMP field, modifies it
then updates the CMP field with the same, modified
object, that change will never get flushed to the
database and the update will have been ignored.
class MyBean implements Serializable {
int x, y;
// getters & setters...
// The correct implementation of equals() for this
class.
public boolean equals(Object o) {
if (!(o instanceof MyBean))
return false;
MyBean that = (MyBean)o;
return this == that || this.x = that.x &&
this.y = that.y;
}
/*
// Horrid kludge necessary to force JBossCMP to
store me :(
public boolean equals(Object o) {
return false;
}
*/
}
MyEntityLocal entity =
MyEntityLocalHome.findByPrimaryKey(pk);
// Retrieve bean from CMP field and modify it.
MyBean bean entity.getBean(); // CMP field getter
bean.x = 666;
bean.y = 777;
// Store modified bean. Setting field to null first
forces JBossCMP
// to recognize the update in 3.0.4 and 3.2.1, but not
3.2.2.
entity.setBean(null); // (shouldn't be necessary anyway)
entity.setBean(bean);
// TXN commit...
// asserts fail, bean not persisted. Uncommenting
Horrid Kludge forces JBossCMP v. 3.2.2 to recognize &
persist the update.
bean entity.getBean();
assert(bean.x == 666 && bean.y == 777);
----------------------------------------------------------------------
>Comment By: Alexey Loubyansky (loubyansky)
Date: 2003-11-18 01:50
Message:
Logged In: YES
user_id=543482
Fixed in Branch_3_2 and HEAD.
By default, CMP fields of primitive types, their wrappers,
java.lang.String and audit and version fields of type
java.util.Date rely on equals() implementation when checked
for dirty state and have check-dirty-after-get=false. Other
fields, by default, rely on invalid-unless-null dirty
checking strategy, i.e. if the field was null and a new
value is null it is considered valid, otherwise - invalid;
and have check-dirty-after-get=true.
----------------------------------------------------------------------
Comment By: Adrian Price (adrianprice)
Date: 2003-11-07 21:45
Message:
Logged In: YES
user_id=580837
I still don't believe equals() is a reliable way to determine
whether the field value is dirty, because by definition (this ==
this) is always true. The case I'm describing is where you
pass the exact same object instance to the cmp setter as
was initially retrieved from the cmp field getter. In that case
equals() will always return true, even if the object is actually
dirty. The only way round that would be to use some dirty
trick to clone the original value in order to remember what it
was. If the object isn't clonable you'd have to deserialize it
twice in order to keep a pristine copy, but that's an
expensive operation. I don't believe JBoss actually does
clone the initial value because if it did, clonedValue.equals
(newValue) would return false if the object in question did
not override Object.equals(), which simply checks identity
(this == that).
I think the correct solution is to treat the field as dirty if
oldFieldValue == newFieldValue. You could add some smarts
to check the object's class and ignore the immutable
java.lang.* objects.
I've looked at the source for JDBCTypeFactory, and I'm
convinced the flaw is in the implementation of the SIMPLE
anonymous inner class that implements
CMPFieldStateFactory. I think the isStateValid() method
should read:
public boolean isStateValid(Object state, Object fieldValue)
{
// Must treat identical references as dirty,
// because equals() won't tell you the object
// has changed.
return state == null ? fieldValue == null : state !=
fieldValue && state.equals(fieldValue);
}
----------------------------------------------------------------------
Comment By: Alexey Loubyansky (loubyansky)
Date: 2003-11-07 19:53
Message:
Logged In: YES
user_id=543482
That is not completely correct. Check this link, for example
https://sourceforge.net/tracker/?func=detail&aid=816762&group_id=22866&atid=381174.
Current implementation relies on equals() implementation.
So, that is not the CMP engine who assumes your object is
not dirty. But you implementing equals() that way.
I can expose the mechanism that is used for Map, List, Set,
array to application developer and offer you to provide an
implementation of the following interface for MyBean to
solve the problem.
public interface CMPFieldStateFactory
{
/**
* Calculates and returns an object that represents the
state of the field value.
* The states produced by this method will be used to
check whether the field
* is dirty at commit time.
*
* @param fieldValue field's value.
* @return an object representing the field's state.
*/
Object getFieldState(Object fieldValue);
/**
* Checks whether the field's state <code>state</code>
* is equal to the field value's state (possibly,
calculated with
* the <code>getFieldState()</code> method).
*
* @param state the state to compare with field value's
state.
* @param fieldValue field's value, the state of which
will be compared
* with <code>state</code>.
* @return true if <code>state</code> equals to
<code>fieldValue</code>'s state.
*/
boolean isStateValid(Object state, Object fieldValue);
}
----------------------------------------------------------------------
Comment By: Adrian Price (adrianprice)
Date: 2003-11-06 12:10
Message:
Logged In: YES
user_id=580837
penultimate line should read:
bean = entity.getBean();
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=376685&aid=837092&group_id=22866
-------------------------------------------------------
This SF. Net email is sponsored by: GoToMyPC
GoToMyPC is the fast, easy and secure way to access your computer from
any Web browser or wireless device. Click here to Try it Free!
https://www.gotomypc.com/tr/OSDN/AW/Q4_2003/t/g22lp?Target=mm/g22lp.tmpl
_______________________________________________
JBoss-Development mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/jboss-development
