From 9368da4ebfbec231ebf59c752a45189e11b47f90 Mon Sep 17 00:00:00 2001 From: Charlie Groves Date: Tue, 19 Oct 2010 00:29:31 +0000 Subject: [PATCH] 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. --- .../samskivert/depot/tools/GenRecordTask.java | 38 ++++++++++--------- .../depot/GeneratedValueRecord.java | 35 +++++++++++++++++ .../samskivert/depot/GeneratedValueTest.java | 34 +++++++++++++++++ .../java/com/samskivert/depot/TestBase.java | 2 +- .../java/com/samskivert/depot/TestRecord.java | 1 - 5 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 src/test/java/com/samskivert/depot/GeneratedValueRecord.java create mode 100644 src/test/java/com/samskivert/depot/GeneratedValueTest.java diff --git a/src/main/java/com/samskivert/depot/tools/GenRecordTask.java b/src/main/java/com/samskivert/depot/tools/GenRecordTask.java index 28a1483..104f570 100644 --- a/src/main/java/com/samskivert/depot/tools/GenRecordTask.java +++ b/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 com.google.common.collect.Maps; -import com.google.common.collect.Sets; +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 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++) { - processRecord(new File(fromDir, srcFiles[f])); + for (String srcFile : srcFiles) { + 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 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; diff --git a/src/test/java/com/samskivert/depot/GeneratedValueRecord.java b/src/test/java/com/samskivert/depot/GeneratedValueRecord.java new file mode 100644 index 0000000..3ec9c18 --- /dev/null +++ b/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 _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 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 +} diff --git a/src/test/java/com/samskivert/depot/GeneratedValueTest.java b/src/test/java/com/samskivert/depot/GeneratedValueTest.java new file mode 100644 index 0000000..364619a --- /dev/null +++ b/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> 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 recs = + dr.findAll(GeneratedValueRecord.class, new Where(GeneratedValueRecord.VALUE.eq(2))); + assertEquals(1, Iterables.getOnlyElement(recs).recordId); + } +} diff --git a/src/test/java/com/samskivert/depot/TestBase.java b/src/test/java/com/samskivert/depot/TestBase.java index 8e4861a..45f6db5 100644 --- a/src/test/java/com/samskivert/depot/TestBase.java +++ b/src/test/java/com/samskivert/depot/TestBase.java @@ -40,7 +40,7 @@ public abstract class TestBase * Note: 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"); diff --git a/src/test/java/com/samskivert/depot/TestRecord.java b/src/test/java/com/samskivert/depot/TestRecord.java index 3b3a6b4..fab5cbb 100644 --- a/src/test/java/com/samskivert/depot/TestRecord.java +++ b/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; /**