rzo1 commented on code in PR #227:
URL: https://github.com/apache/openjpa/pull/227#discussion_r4391454447
##########
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractPCData.java:
##########
@@ -243,6 +247,18 @@ protected Object toEmbeddedField(OpenJPAStateManager sm,
ValueMetaData vmd,
protected Object toData(FieldMetaData fmd, Object val, StoreContext ctx) {
if (val == null)
return null;
+ // a converted value is cached as its database value, so that each
+ // instance loaded from the cache gets its own value from the
+ // converter; it is not cached when the converter fails
+ if (fmd.isConvertedObject()) {
+ Object data;
+ try {
+ data = fmd.getExternalValue(val, ctx);
+ } catch (RuntimeException re) {
+ return NULL;
+ }
+ return (data == null) ? NULL : data;
Review Comment:
This block is gone: after the rebase onto master, #226 already caches every
converted value in its database form, so 3021 no longer changes
`AbstractPCData`.
##########
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java:
##########
@@ -4372,6 +4383,26 @@ void dirtyCheckWholeConverted() {
}
}
+ /**
+ * Put back the values of fields converted as a whole that were changed
+ * in place and not yet found by a flush, since they were not saved for
+ * rollback. Done before the instances are rolled back.
+ */
+ private void restoreWholeConverted() {
+ if (_wholeConverted == null || _wholeConverted.isEmpty()) {
+ return;
+ }
+
+ lock();
+ try {
+ for (StateManagerImpl sm : new ArrayList<>(_wholeConverted)) {
Review Comment:
Not needed, removed. Restoring a value does not add to or remove from the
set; the copy was only needed in `dirtyCheckWholeConverted()`, where dirtying
enlists the instance again.
##########
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DetachedStateManager.java:
##########
@@ -366,6 +388,45 @@ protected Object getDetachedObjectId(AttachManager manager,
return _oid;
}
+ /**
+ * Whether a field converted as a whole of this embedded instance
+ * converts differently than in the embedded instance its managed owner
+ * holds, since a value or element changed in place while detached does
+ * not mark the field dirty. Assumes a change when there is no such
+ * instance to compare with.
+ */
+ private boolean isEmbeddedWholeConvertedChanged(BrokerImpl broker,
+ ClassMetaData meta, OpenJPAStateManager owner,
+ ValueMetaData ownerMeta) {
+ FieldMetaData[] fmds = meta.getWholeConvertedFields();
+ if (fmds.length == 0) {
+ return false;
+ }
+ FieldMetaData ownerField = ownerMeta.getFieldMetaData();
+ if (ownerMeta != ownerField
+ || !owner.getLoaded().get(ownerField.getIndex())) {
+ return true;
+ }
+ OpenJPAStateManager cur = broker.getStateManager(
+ owner.fetchField(ownerField.getIndex(), false));
+ if (!(cur instanceof StateManagerImpl)) {
+ return true;
+ }
+ for (FieldMetaData fmd : fmds) {
+ int i = fmd.getIndex();
+ if (!_loaded.get(i)) {
+ continue;
+ }
+ provideField(i);
+ Object val = objval;
+ objval = null;
+ if (((StateManagerImpl) cur).isWholeConvertedChanged(i, val)) {
Review Comment:
Done, the cast is out of the loop now.
##########
openjpa-kernel/src/main/java/org/apache/openjpa/kernel/SavepointFieldManager.java:
##########
@@ -221,7 +221,10 @@ public void storeObjectField(int field, Object curVal) {
_field = proxy.copyDate((Date) curVal);
break;
case JavaTypes.OBJECT:
- _field = proxy.copyCustom(curVal);
+ // a converted value is copied through its converter
+ _field = (fmd.isConvertedObject())
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
