From 7056f6c05c3e7277f819e1c227b5fed624382ea2 Mon Sep 17 00:00:00 2001
From: Par Winzell <zell@threerings.net>
Date: Mon, 10 Mar 2008 22:28:11 +0000
Subject: [PATCH] Don't try to build an UPDATE statement at all if no key were
 set on the record sent into store(). Also throw a sensible (if scaredy-cat)
 exception rather than an NPE if no WHERE clause were given to an UPDATE. We
 probably do want to allow rampant nutty 20,000-row UPDATEs at some point, but
 not because of an accidental null key.

---
 src/java/com/samskivert/jdbc/depot/BuildVisitor.java |  3 +++
 .../com/samskivert/jdbc/depot/DepotRepository.java   | 12 ++++++++----
 src/java/com/samskivert/jdbc/depot/clause/Join.java  |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java
index 0a59e1a..447e1c8 100644
--- a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java
+++ b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java
@@ -418,6 +418,9 @@ public abstract class BuildVisitor implements ExpressionVisitor
     public void visit (UpdateClause<? extends PersistentRecord> updateClause)
         throws Exception
     {
+        if (updateClause.getWhereClause() == null) {
+            throw new SQLException("I dare not currently perform UPDATE without a WHERE clause.");
+        }
         Class<? extends PersistentRecord> pClass = updateClause.getPersistentClass();
         _innerClause = true;
 
diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java
index a78f56c..0fa1e6d 100644
--- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java
+++ b/src/java/com/samskivert/jdbc/depot/DepotRepository.java
@@ -572,26 +572,30 @@ public abstract class DepotRepository
         final UpdateClause<T> update =
             new UpdateClause<T>(pClass, key, marsh._columnFields, record);
         final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
-        builder.newQuery(update);
+
+        // if our primary key isn't null, we start by trying to update rather than insert
+        if (key != null) {
+            builder.newQuery(update);
+        }
 
         final boolean[] created = new boolean[1];
         _ctx.invoke(new CachingModifier<T>(record, key, key) {
             public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = null;
                 try {
-                    // if our primary key isn't null, update rather than insert the record before
-                    // been persisted and insert
                     if (_key != null) {
+                        // run the update
                         stmt = builder.prepare(conn);
                         int mods = stmt.executeUpdate();
                         if (mods > 0) {
+                            // if it succeeded, we're done
                             return mods;
                         }
                         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
+                    // an insertion: first, set any auto-generated column values
                     Set<String> identityFields =
                         marsh.generateFieldValues(conn, liaison, _result, false);
 
diff --git a/src/java/com/samskivert/jdbc/depot/clause/Join.java b/src/java/com/samskivert/jdbc/depot/clause/Join.java
index 4ee9ee7..dbbcbf3 100644
--- a/src/java/com/samskivert/jdbc/depot/clause/Join.java
+++ b/src/java/com/samskivert/jdbc/depot/clause/Join.java
@@ -89,6 +89,7 @@ public class Join extends QueryClause
     public void addClasses (Collection<Class<? extends PersistentRecord>> classSet)
     {
         classSet.add(_joinClass);
+        _joinCondition.addClasses(classSet);
     }
 
     /** Indicates the type of join to be performed. */