From 6d745b424799ea5f04e77be4bf98cfdeec7706a8 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Thu, 29 Oct 2009 17:38:47 +0000 Subject: [PATCH] Roll back Charlie's constructor patch. It's an admirable idea but it just adds too much unwieldy boilerplate to large records, and neither of us like the idea of on/off switches. We'll just have to construct by hand. --- .../samskivert/depot/tools/GenRecordTask.java | 107 +++++------------- .../depot/tools/record_constructor.tmpl | 9 -- 2 files changed, 27 insertions(+), 89 deletions(-) delete mode 100644 src/java/com/samskivert/depot/tools/record_constructor.tmpl diff --git a/src/java/com/samskivert/depot/tools/GenRecordTask.java b/src/java/com/samskivert/depot/tools/GenRecordTask.java index 7066f89..9afeef7 100644 --- a/src/java/com/samskivert/depot/tools/GenRecordTask.java +++ b/src/java/com/samskivert/depot/tools/GenRecordTask.java @@ -20,16 +20,6 @@ package com.samskivert.depot.tools; -import java.lang.annotation.Annotation; -import java.lang.reflect.Field; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; - -import java.util.List; -import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.File; @@ -37,6 +27,20 @@ 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; + +import java.util.List; +import java.util.Map; +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 org.apache.tools.ant.BuildException; import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.Task; @@ -44,18 +48,12 @@ 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.ImmutableMap; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; - 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.Column; -import com.samskivert.depot.annotation.GeneratedValue; import com.samskivert.depot.annotation.Id; import com.samskivert.depot.annotation.Transient; import com.samskivert.depot.impl.DepotUtil; @@ -101,8 +99,8 @@ public class GenRecordTask extends Task DirectoryScanner ds = fs.getDirectoryScanner(getProject()); File fromDir = fs.getDir(getProject()); String[] srcFiles = ds.getIncludedFiles(); - for (String srcFile : srcFiles) { - processRecord(new File(fromDir, srcFile)); + for (int f = 0; f < srcFiles.length; f++) { + processRecord(new File(fromDir, srcFiles[f])); } } } @@ -145,40 +143,17 @@ public class GenRecordTask extends Task } boolean isAbstract = Modifier.isAbstract(rclass.getModifiers()); - // determine our primary key fields for getKey() generation (if we're not an abstract) and - // fields that need to be filled in for insertion to make a constructor + // determine our primary key fields for getKey() generation (if we're not an abstract) List kflist = Lists.newArrayList(); - List requiredFields= Lists.newArrayList(); if (!isAbstract) { // 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 (!isPersistentField(field)) { - continue; - } if (hasAnnotation(field, Id.class)) { kflist.add(field); - if (hasAnnotation(field, GeneratedValue.class)) { - continue; - } + continue; } - // We can't actually get the annotation as Column.class due to classloader hijinks, - // so prepare some reflection hoops and jump through them to get the values - Object ann = getAnnotation(field, Column.class); - if (ann != null) { - try { - Method defVal = ann.getClass().getMethod("defaultValue"); - Method nullable = ann.getClass().getMethod("nullable"); - if(!StringUtil.isBlank((String)defVal.invoke(ann)) || - (Boolean)nullable.invoke(ann)) { - continue; - } - } catch (Exception e) { - throw new RuntimeException(e); - } - } - requiredFields.add(field); } } @@ -189,6 +164,12 @@ public class GenRecordTask extends Task flist.add(field); } } + Set declared = Sets.newHashSet(); + for (Field field : rclass.getDeclaredFields()) { + if (isPersistentField(field)) { + declared.add(field); + } + } // slurp our source file into newline separated strings String[] lines = null; @@ -210,7 +191,6 @@ public class GenRecordTask extends Task int bstart = -1, bend = -1; int nstart = -1, nend = -1; int mstart = -1, mend = -1; - int finalc = -1; for (int ii = 0; ii < lines.length; ii++) { String line = lines[ii].trim(); @@ -228,10 +208,6 @@ public class GenRecordTask extends Task } } - // look for the final line that starts a constructor - } else if(CONSTRUCTOR_PATTERN.matcher(line).find()) { - finalc = ii; - // track the last } on a line by itself and we'll call that the end of the class body } else if (line.equals("}")) { bend = ii; @@ -292,23 +268,6 @@ public class GenRecordTask extends Task fsection.append(mergeTemplate(COL_TMPL, fsubs)); } - // Add a constructor that fills in all required fields if there isn't a constructor out of - // the generated section and we have some required fields - if (!requiredFields.isEmpty() && (finalc == -1 || (finalc > nstart && finalc < nend))) { - StringBuilder args = new StringBuilder(); - StringBuilder assignments = new StringBuilder(); - for (Field arg : requiredFields) { - String name = arg.getName(); - if (args.length() > 0) { - args.append(", "); - } - args.append(GenUtil.simpleName(arg)).append(" ").append(name); - assignments.append("\n this.").append(name).append(" = ").append(name).append(';'); - } - fsection.append(mergeTemplate(CONS_TMPL, ImmutableMap.of("argList", args.toString(), - "assignments", assignments.toString(), "record", rname))); - } - // generate our methods section StringBuilder msection = new StringBuilder(); @@ -436,20 +395,15 @@ public class GenRecordTask extends Task } } - protected static boolean hasAnnotation (Field field, Class annotation) - { - return getAnnotation(field, annotation) != null; - } - - protected static Annotation getAnnotation (Field field, Class annotation) + protected static boolean hasAnnotation (Field field, Class annotation) { // iterate becase getAnnotation() fails if we're dealing with multiple classloaders for (Annotation a : field.getAnnotations()) { if (annotation.getName().equals(a.annotationType().getName())) { - return a; + return true; } } - return null; + return false; } /** @@ -508,9 +462,6 @@ public class GenRecordTask extends Task /** Specifies the path to the key code template. */ protected static final String KEY_TMPL = "com/samskivert/depot/tools/record_key.tmpl"; - /** Specifies the path to the constructor code template. */ - protected static final String CONS_TMPL = "com/samskivert/depot/tools/record_constructor.tmpl"; - // markers protected static final String MARKER = "// AUTO-GENERATED: "; protected static final String FIELDS_START = MARKER + "FIELDS START"; @@ -524,8 +475,4 @@ public class GenRecordTask extends Task /** A regular expression for matching the class or interface declaration. */ protected static final Pattern NAME_PATTERN = Pattern.compile( "^\\s*public\\s+(?:abstract\\s+)?(interface|class)\\s+(\\w+)(\\W|$)"); - - /** A regular expression for matching constructor declarations. */ - protected static final Pattern CONSTRUCTOR_PATTERN = Pattern.compile( - "^\\s*(public|protected|private)\\s+\\w+\\s*\\("); } diff --git a/src/java/com/samskivert/depot/tools/record_constructor.tmpl b/src/java/com/samskivert/depot/tools/record_constructor.tmpl deleted file mode 100644 index 81afe41..0000000 --- a/src/java/com/samskivert/depot/tools/record_constructor.tmpl +++ /dev/null @@ -1,9 +0,0 @@ - public @record@ () {} - - /** - * Creates this record with all values required for insertion. Any nullable or default values - * can be filled in before insertion. - */ - public @record@ (@argList@) - {@assignments@ - }