From 935bd346212a9f9562c1cc5f62ee97ef2dafe8d9 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Sun, 14 Jan 2007 20:40:54 +0000 Subject: [PATCH] Some fixes and documentation from Zell. --- .../jdbc/depot/DepotMarshaller.java | 42 ++++++++++++++++--- .../jdbc/depot/DepotRepository.java | 25 +++++++---- .../com/samskivert/jdbc/depot/Modifier.java | 39 ++++++++++++----- 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java index 431fba2..c948b1a 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java +++ b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java @@ -251,22 +251,54 @@ public class DepotMarshaller } /** - * Returns a key configured with the primary key of the supplied object. Throws an exception - * if the persistent object did not declare a primary key. + * Returns a key configured with the primary key of the supplied object. If all the fields are + * null, this method returns null. An exception is thrown if some of the fields are null and + * some are not, or if the object does not declare a primary key. */ public Key getPrimaryKey (Object object) + { + return getPrimaryKey(object, true); + } + + /** + * Returns a key configured with the primary key of the supplied object. If all the fields are + * null, this method returns null. If some of the fields are null and some are not, an + * exception is thrown. If the object does not declare a primary key and the second argument is + * true, this method throws an exception; if it's false, the method returns null. + */ + public Key getPrimaryKey (Object object, boolean requireKey) { if (!hasPrimaryKey()) { - throw new UnsupportedOperationException( - _pclass.getName() + " does not define a primary key"); + if (requireKey) { + throw new UnsupportedOperationException( + _pclass.getName() + " does not define a primary key"); + } + return null; } + try { Comparable[] values = new Comparable[_pkColumns.size()]; + boolean hasNulls = false; for (int ii = 0; ii < _pkColumns.size(); ii++) { FieldMarshaller field = _pkColumns.get(ii); values[ii] = (Comparable) field.getField().get(object); + if (values[ii] == null || Integer.valueOf(0).equals(values[ii])) { + // if this is the first null we see but not the first field, freak out + if (!hasNulls && ii > 0) { + throw new IllegalArgumentException( + "Persistent object's primary key fields are mixed null and non-null."); + } + hasNulls = true; + } else if (hasNulls) { + // if this is a non-null field and we've previously seen nulls, also freak + throw new IllegalArgumentException( + "Persistent object's primary key fields are mixed null and non-null."); + } } - return makePrimaryKey(values); + + // if all the fields were null, return null, else build a key + return hasNulls ? null : makePrimaryKey(values); + } catch (IllegalAccessException iae) { throw new RuntimeException(iae); } diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java index a56e95d..5b62109 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java +++ b/src/java/com/samskivert/jdbc/depot/DepotRepository.java @@ -189,7 +189,9 @@ public class DepotRepository throws PersistenceException { final DepotMarshaller marsh = _ctx.getMarshaller(record.getClass()); - return _ctx.invoke(new CachingModifier(null, null) { + final Key key = marsh.getPrimaryKey(record, false); + // key will be null if record was supplied without a primary key + return _ctx.invoke(new CachingModifier(record, key, key) { public int invoke (Connection conn) throws SQLException { // update our modifier's key so that it can cache our results updateKey(marsh.assignPrimaryKey(conn, record, false)); @@ -198,7 +200,6 @@ public class DepotRepository int mods = stmt.executeUpdate(); // check again in case we have a post-factum key generator updateKey(marsh.assignPrimaryKey(conn, record, true)); - setInstance(record); return mods; } finally { JDBCUtil.close(stmt); @@ -218,7 +219,10 @@ public class DepotRepository { final DepotMarshaller marsh = _ctx.getMarshaller(record.getClass()); final Key key = marsh.getPrimaryKey(record); - return _ctx.invoke(new CachingModifier(key, key) { + if (key == null) { + throw new IllegalArgumentException("Can't update record with null primary key."); + } + return _ctx.invoke(new CachingModifier(record, key, key) { public int invoke (Connection conn) throws SQLException { PreparedStatement stmt = marsh.createUpdate(conn, record, key); try { @@ -241,7 +245,10 @@ public class DepotRepository { final DepotMarshaller marsh = _ctx.getMarshaller(record.getClass()); final Key key = marsh.getPrimaryKey(record); - return _ctx.invoke(new CachingModifier(key, key) { + if (key == null) { + throw new IllegalArgumentException("Can't update record with null primary key."); + } + return _ctx.invoke(new CachingModifier(record, key, key) { public int invoke (Connection conn) throws SQLException { PreparedStatement stmt = marsh.createUpdate(conn, record, key, modifiedFields); try { @@ -408,13 +415,13 @@ public class DepotRepository { final DepotMarshaller marsh = _ctx.getMarshaller(record.getClass()); final Key key = marsh.hasPrimaryKey() ? marsh.getPrimaryKey(record) : null; - return _ctx.invoke(new CachingModifier(key, key) { + return _ctx.invoke(new CachingModifier(record, key, key) { public int invoke (Connection conn) throws SQLException { PreparedStatement stmt = null; try { - // if our primary key is null or is the integer 0, assume the record has never + // if our primary key isn't null, update rather than insert the record // before been persisted and insert - if (key != null && !Integer.valueOf(0).equals(key)) { + if (key != null) { stmt = marsh.createUpdate(conn, record, key); int mods = stmt.executeUpdate(); if (mods > 0) { @@ -429,7 +436,6 @@ public class DepotRepository stmt = marsh.createInsert(conn, record); int mods = stmt.executeUpdate(); updateKey(marsh.assignPrimaryKey(conn, record, true)); - setInstance(record); return mods; } finally { @@ -450,6 +456,9 @@ public class DepotRepository { @SuppressWarnings("unchecked") Class type = (Class)record.getClass(); Key primaryKey = _ctx.getMarshaller(type).getPrimaryKey(record); + if (primaryKey == null) { + throw new IllegalArgumentException("Can't delete record with null primary key."); + } return deleteAll(type, primaryKey, primaryKey); } diff --git a/src/java/com/samskivert/jdbc/depot/Modifier.java b/src/java/com/samskivert/jdbc/depot/Modifier.java index 632274f..317ff26 100644 --- a/src/java/com/samskivert/jdbc/depot/Modifier.java +++ b/src/java/com/samskivert/jdbc/depot/Modifier.java @@ -53,23 +53,29 @@ public abstract class Modifier } /** - * A simple modifier that updates the cache with its modified object on completion. The derived - * class must call {@link #setInstance} to inform the modifier of its object at some point - * during the modification operation. + * A convenience modifier that can perform cache updates in addition to invalidation: + * - Before {@link #invoke(Connection)}, the {@link CacheInvalidator} is run, if given. + * - After {@link #invoke(Connection)}, the cache is updated with the modified object, + * presuming both _key and _result are non-null. These variables may be set or modified + * during execution in addition to being supplied to the constructor. */ public static abstract class CachingModifier extends Modifier { - protected CachingModifier (CacheKey key, CacheInvalidator invalidator) + /** + * Construct a new CachingModifier with the given result, cache key, and invalidator, + * all of which are optional, and may also be set during execution. + */ + protected CachingModifier (T result, CacheKey key, CacheInvalidator invalidator) { super(invalidator); + _result = result; _key = key; } - protected void setInstance (T result) - { - _result = result; - } - + /** + * Update this {@link CachingModifier}'s cache key, e.g. during insertion when a + * persistent object first receives a generated key. + */ protected void updateKey (CacheKey key) { if (key != null) { @@ -77,11 +83,12 @@ public abstract class Modifier } } - @Override + @Override // from Modifier public void cacheUpdate (PersistenceContext ctx) { super.cacheUpdate(ctx); - if (_key != null) { + // if we have both a key and a record, cache + if (_key != null && _result != null) { ctx.cacheStore(_key, _result); } } @@ -90,13 +97,23 @@ public abstract class Modifier protected T _result; } + /** + * Overriden to perform the actual database modifications represented by this object; + * should return the number of modified rows. + */ public abstract int invoke (Connection conn) throws SQLException; + /** + * Constructs a {@link Modifier} without a cache invalidator. + */ public Modifier () { this(null); } + /** + * Constructs a {@link Modifier} with the given cache invalidator. + */ public Modifier (CacheInvalidator invalidator) { _invalidator = invalidator;