Skip classes in GenRecordTask if they have @GeneratedValue on a field without @Id. I'd rather stop

the build, but other errors in class processing seem to lead to skipping, so I'm not rocking the
boat.

If @GeneratedValue is on a field without an @Id, depot creates the necessary indices to generate the
value, but it doesn't actually do the generation on insertion.  Rather than making it work without a
good use case for it, I'm adding this check to keep people from stumbling into a broken design.

src/main/java/com/samskivert/depot/tools/GenRecordTask.java

@@ -20,13 +20,6 @@
 
 package com.samskivert.depot.tools;
 
-import java.io.BufferedReader;
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileReader;
-import java.io.FileWriter;
-import java.io.IOException;
-
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
@@ -37,9 +30,12 @@ import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import com.google.common.collect.Lists;
+import java.io.BufferedReader;
-import com.google.common.collect.Maps;
+import java.io.BufferedWriter;
-import com.google.common.collect.Sets;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
 
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.DirectoryScanner;
@@ -48,12 +44,17 @@ import org.apache.tools.ant.types.FileSet;
 import org.apache.tools.ant.types.Reference;
 import org.apache.tools.ant.util.ClasspathUtils;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+
 import com.samskivert.io.StreamUtil;
 import com.samskivert.util.ClassUtil;
 import com.samskivert.util.GenUtil;
 import com.samskivert.util.StringUtil;
 
 import com.samskivert.depot.PersistentRecord;
+import com.samskivert.depot.annotation.GeneratedValue;
 import com.samskivert.depot.annotation.Id;
 import com.samskivert.depot.annotation.Transient;
 import com.samskivert.depot.impl.DepotUtil;
@@ -92,15 +93,15 @@ public class GenRecordTask extends Task
         try {
             _prclass = _cloader.loadClass(PersistentRecord.class.getName());
         } catch (Exception e) {
-            throw new BuildException("Can't resolve InvocationListener", e);
+            throw new BuildException("Can't resolve PersistentRecord", e);
         }
 
         for (FileSet fs : _filesets) {
             DirectoryScanner ds = fs.getDirectoryScanner(getProject());
             File fromDir = fs.getDir(getProject());
             String[] srcFiles = ds.getIncludedFiles();
-            for (int f = 0; f < srcFiles.length; f++) {
+            for (String srcFile : srcFiles) {
-                processRecord(new File(fromDir, srcFiles[f]));
+                processRecord(new File(fromDir, srcFile));
             }
         }
     }
@@ -141,18 +142,21 @@ public class GenRecordTask extends Task
             // System.err.println("Skipping " + rclass.getName() + "...");
             return;
         }
-        boolean isAbstract = Modifier.isAbstract(rclass.getModifiers());
 
         // determine our primary key fields for getKey() generation (if we're not an abstract)
         List<Field> kflist = Lists.newArrayList();
-        if (!isAbstract) {
+        if (!Modifier.isAbstract(rclass.getModifiers())) {
             // determine which fields make up our primary key; we'd just use Class.getFields() but
             // that returns things in a random order whereas ClassUtil returns fields in
             // declaration order starting from the top-most class and going down the line
             for (Field field : ClassUtil.getFields(rclass)) {
                 if (hasAnnotation(field, Id.class)) {
                     kflist.add(field);
-                    continue;
+                } else if (hasAnnotation(field, GeneratedValue.class)) {
+                    System.err.println("Skipping " + rclass.getName() + ".  Field '" +
+                        field.getName() + "' has @GeneratedValue, which may only used on primary " +
+                        "keys with @Id.");
+                    return;
                 }
             }
         }
@@ -408,7 +412,7 @@ public class GenRecordTask extends Task
 
     protected static boolean hasAnnotation (Field field, Class<?> annotation)
     {
-        // iterate becase getAnnotation() fails if we're dealing with multiple classloaders
+        // iterate because getAnnotation() fails if we're dealing with multiple classloaders
         for (Annotation a : field.getAnnotations()) {
             if (annotation.getName().equals(a.annotationType().getName())) {
                 return true;

src/test/java/com/samskivert/depot/GeneratedValueRecord.java

@@ -0,0 +1,35 @@
+package com.samskivert.depot;
+
+import com.samskivert.depot.annotation.GeneratedValue;
+import com.samskivert.depot.annotation.Id;
+import com.samskivert.depot.expression.ColumnExp;
+
+public class GeneratedValueRecord extends PersistentRecord
+{
+    // AUTO-GENERATED: FIELDS START
+    public static final Class<GeneratedValueRecord> _R = GeneratedValueRecord.class;
+    public static final ColumnExp RECORD_ID = colexp(_R, "recordId");
+    public static final ColumnExp VALUE = colexp(_R, "value");
+    // AUTO-GENERATED: FIELDS END
+
+    public static final int SCHEMA_VERSION = 1;
+
+    @Id @GeneratedValue
+    public int recordId;
+
+    public int value;
+
+    // AUTO-GENERATED: METHODS START
+    /**
+     * Create and return a primary {@link Key} to identify a {@link GeneratedValueRecord}
+     * with the supplied key values.
+     */
+    public static Key<GeneratedValueRecord> getKey (int recordId)
+    {
+        return newKey(_R, recordId);
+    }
+
+    /** Register the key fields in an order matching the getKey() factory. */
+    static { registerKeyFields(RECORD_ID); }
+    // AUTO-GENERATED: METHODS END
+}

src/test/java/com/samskivert/depot/GeneratedValueTest.java

@@ -0,0 +1,34 @@
+package com.samskivert.depot;
+
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Test;
+
+import com.google.common.collect.Iterables;
+
+import com.samskivert.depot.clause.Where;
+
+import static org.junit.Assert.assertEquals;
+
+public class GeneratedValueTest
+{
+    @Test public void insertAutoGeneratedValue ()
+    {
+        PersistenceContext pc = TestBase.createPersistenceContext();
+        DepotRepository dr = new DepotRepository(pc) {
+            @Override
+            protected void getManagedRecords (Set<Class<? extends PersistentRecord>> classes) {
+                classes.add(GeneratedValueRecord.class);
+            }
+        };
+        GeneratedValueRecord rec = new GeneratedValueRecord();
+        rec.value = 2;
+        assertEquals(0, rec.recordId);
+        assertEquals(1, dr.insert(rec));
+        assertEquals(1, rec.recordId);
+        List<GeneratedValueRecord> recs =
+            dr.findAll(GeneratedValueRecord.class, new Where(GeneratedValueRecord.VALUE.eq(2)));
+        assertEquals(1, Iterables.getOnlyElement(recs).recordId);
+    }
+}

src/test/java/com/samskivert/depot/TestBase.java

@@ -40,7 +40,7 @@ public abstract class TestBase
      * <b>Note:</b> the in-memory HSQL database is shared for the duration of the VM, so tests have
      * to clean up after themselves to avoid conflicting with one another.
      */
-    protected PersistenceContext createPersistenceContext ()
+    protected static PersistenceContext createPersistenceContext ()
     {
         Properties props = new Properties();
         props.put("default.driver", "org.hsqldb.jdbcDriver");

src/test/java/com/samskivert/depot/TestRecord.java

@@ -27,7 +27,6 @@ import com.samskivert.util.StringUtil;
 
 import com.samskivert.depot.annotation.Entity;
 import com.samskivert.depot.annotation.Id;
-import com.samskivert.depot.annotation.Index;
 import com.samskivert.depot.expression.ColumnExp;
 
 /**