From 129120794a77c5c3e97180eee7eb8791e8df02c0 Mon Sep 17 00:00:00 2001 From: samskivert Date: Tue, 16 Sep 2008 21:33:22 +0000 Subject: [PATCH] Amazingly, we had apparently never tried to insert a preconfigured row into a table that had field generators on its primary key. The code just blindly overwrite the supplied primary key and ran the generators anyway. We now only run the generators if the record has no primary key. This means we can't use field generators on non-primary-key fields, but I'm not sure we ever meant to support that in the first place. git-svn-id: https://samskivert.googlecode.com/svn/trunk@2431 6335cc39-0255-0410-8fd6-9bcaacd3b74c --- .../jdbc/depot/DepotMarshaller.java | 18 ++++++----- .../jdbc/depot/DepotRepository.java | 32 ++++++++----------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java index 4b70f2f8..0697d217 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java +++ b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java @@ -358,7 +358,9 @@ public class DepotMarshaller int nulls = 0; for (int ii = 0; ii < _pkColumns.size(); ii++) { FieldMarshaller field = _pkColumns.get(ii); - if ((values[ii] = (Comparable)field.getField().get(object)) == null) { + values[ii] = (Comparable)field.getField().get(object); + if (values[ii] == null || + (values[ii] instanceof Number && ((Number)values[ii]).intValue() == 0)) { nulls++; } } @@ -474,14 +476,14 @@ public class DepotMarshaller } /** - * Go through the registered {@link ValueGenerator}s for our persistent object and run the - * ones that match the current postFactum phase, filling in the fields on the supplied object - * while we go. This method used to generate a key; that is now a separate step. + * Go through the registered {@link ValueGenerator}s for our persistent object and run the ones + * that match the current postFactum phase, filling in the fields on the supplied object while + * we go. * - * The return value is only non-empty for the !postFactum phase, in which case it is a set - * of field names that are associated with {@link IdentityValueGenerator}, because these need - * special handling in the INSERT (specifically, 'DEFAULT' must be supplied as a value in - * the eventual SQL). + * The return value is only non-empty for the !postFactum phase, in which case it is a set of + * field names that are associated with {@link IdentityValueGenerator}, because these need + * special handling in the INSERT (specifically, 'DEFAULT' must be supplied as a value in the + * eventual SQL). */ public Set generateFieldValues ( Connection conn, DatabaseLiaison liaison, Object po, boolean postFactum) diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java index d6fa9f36..95b9eb9b 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java +++ b/src/java/com/samskivert/jdbc/depot/DepotRepository.java @@ -315,12 +315,11 @@ public abstract class DepotRepository return _ctx.invoke(new CachingModifier(record, key, key) { @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { - // set any auto-generated column values - Set identityFields = - marsh.generateFieldValues(conn, liaison, _result, false); - // if needed, update our modifier's key so that it can cache our results + Set identityFields = Collections.emptySet(); if (_key == null) { + // set any auto-generated column values + identityFields = marsh.generateFieldValues(conn, liaison, _result, false); updateKey(marsh.getPrimaryKey(_result, false)); } @@ -329,15 +328,13 @@ public abstract class DepotRepository PreparedStatement stmt = builder.prepare(conn); try { int mods = stmt.executeUpdate(); - - // run any post-factum value generators - marsh.generateFieldValues(conn, liaison, _result, true); - - // and check once more if a key now exists + // run any post-factum value generators and potentially generate our key if (_key == null) { + marsh.generateFieldValues(conn, liaison, _result, true); updateKey(marsh.getPrimaryKey(_result, false)); } return mods; + } finally { JDBCUtil.close(stmt); } @@ -739,13 +736,12 @@ public abstract class DepotRepository JDBCUtil.close(stmt); } - // if the update modified zero rows or the primary key was obviously unset, do - // an insertion: first, set any auto-generated column values - Set identityFields = - marsh.generateFieldValues(conn, liaison, _result, false); - - // update our modifier's key so that it can cache our results + // if the update modified zero rows or the primary key was unset, insert + Set identityFields = Collections.emptySet(); if (_key == null) { + // first, set any auto-generated column values + identityFields = marsh.generateFieldValues(conn, liaison, _result, false); + // update our modifier's key so that it can cache our results updateKey(marsh.getPrimaryKey(_result, false)); } @@ -754,11 +750,9 @@ public abstract class DepotRepository stmt = builder.prepare(conn); int mods = stmt.executeUpdate(); - // run any post-factum value generators - marsh.generateFieldValues(conn, liaison, _result, true); - - // and check once more if a key now exists + // run any post-factum value generators and potentially generate our key if (_key == null) { + marsh.generateFieldValues(conn, liaison, _result, true); updateKey(marsh.getPrimaryKey(_result, false)); } created[0] = true;