From 2f19f2ad464f53bd5bca1ba6fcc9b64ed497f4ab Mon Sep 17 00:00:00 2001 From: moparisthebest Date: Sun, 28 May 2017 23:38:10 -0400 Subject: [PATCH] Remove requirement for @JdbcMapper.Mapper to implement JdbcMapper, logic around requiring close() method or not --- .../jdbc/codegen/JdbcMapper.java | 20 +++-- .../jdbc/codegen/JdbcMapperFactory.java | 4 +- .../jdbc/codegen/JdbcMapperProcessor.java | 83 ++++++++++++++++--- .../jdbc/codegen/JdbcMapperTest.java | 2 +- .../jdbc/codegen/PersonDAO.java | 5 +- .../jdbc/codegen/PrestoPersonDAO.java | 4 +- .../jdbc/codegen/PrestoPersonDAOTest.java | 2 +- 7 files changed, 96 insertions(+), 24 deletions(-) diff --git a/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapper.java b/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapper.java index f8b7254..13296a0 100644 --- a/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapper.java +++ b/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapper.java @@ -25,8 +25,15 @@ public interface JdbcMapper extends Closeable { */ String jndiName() default ""; - boolean cachePreparedStatements() default true; + /** + * This defaults to true if a close() method exists to override/implement, false otherwise + */ + OptionalBool cachePreparedStatements() default OptionalBool.DEFAULT; + /** + * This defaults to SimpleSQLParser, PrestoSQLParser is another option for Java 8, or implement your own + * @return + */ Class sqlParser() default SQLParser.class; } @@ -38,7 +45,10 @@ public interface JdbcMapper extends Closeable { */ String value(); - OptionalBool cachePreparedStatement() default OptionalBool.INHERIT; + /** + * This defaults to the value of the class-level @JdbcMapper.Mapper.cachePreparedStatements annotation, but can be configured on a per-method level here + */ + OptionalBool cachePreparedStatement() default OptionalBool.DEFAULT; /** * Maximum array length. @@ -56,18 +66,18 @@ public interface JdbcMapper extends Closeable { } public enum OptionalBool { - INHERIT, + DEFAULT, TRUE, FALSE; - public boolean combine(final boolean inherited) { + public boolean combine(final boolean def) { switch (this) { case TRUE: return true; case FALSE: return false; } - return inherited; + return def; } } } diff --git a/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperFactory.java b/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperFactory.java index fe6c2ae..e0603e0 100644 --- a/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperFactory.java +++ b/common/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperFactory.java @@ -10,7 +10,7 @@ public abstract class JdbcMapperFactory { static final String SUFFIX = "Bean"; @SuppressWarnings("unchecked") - public static T create(final Class jdbcMapper, final Connection connection) { + public static T create(final Class jdbcMapper, final Connection connection) { try { return (T) Class.forName(jdbcMapper.getName() + SUFFIX).getConstructor(Connection.class).newInstance(connection); } catch (Throwable e) { @@ -18,7 +18,7 @@ public abstract class JdbcMapperFactory { } } - public static T create(final Class jdbcMapper) { + public static T create(final Class jdbcMapper) { return create(jdbcMapper, null); } } diff --git a/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java index d1a53eb..8e79a36 100644 --- a/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java +++ b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java @@ -146,17 +146,36 @@ public class JdbcMapperProcessor extends AbstractProcessor { } w.write("\n\t\tif (this.conn == null)\n" + "\t\t\tthrow new NullPointerException(\"Connection needs to be non-null for JdbcMapper...\");\n\t}\n" + - "\n\t@Override\n\tpublic Connection getConnection() {\n\t\treturn this.conn;\n\t}\n" + "\n\tpublic Connection getConnection() {\n\t\treturn this.conn;\n\t}\n" ); // loop through methods - final Types typeUtils = processingEnv.getTypeUtils(); int cachedPreparedStatements = 0; + ExecutableElement closeMethod = null; + boolean lookupCloseMethod = true; + final boolean defaultCachePreparedStatements; + switch (mapper.cachePreparedStatements()) { + case TRUE: + defaultCachePreparedStatements = true; + break; + case FALSE: + defaultCachePreparedStatements = false; + break; + default: + defaultCachePreparedStatements = (closeMethod = getCloseMethod(genClass)) != null; + lookupCloseMethod = false; + } + for (final Element methodElement : genClass.getEnclosedElements()) { // can only implement abstract methods if (methodElement.getKind() != ElementKind.METHOD || !methodElement.getModifiers().contains(Modifier.ABSTRACT)) continue; final ExecutableElement eeMethod = (ExecutableElement) methodElement; + if(lookupCloseMethod) + if((closeMethod = getCloseMethod(eeMethod)) != null) { + lookupCloseMethod = false; + continue; // skip close method + } final JdbcMapper.SQL sql = eeMethod.getAnnotation(JdbcMapper.SQL.class); if (sql == null || sql.value().isEmpty()) { processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "@JdbcMapper.SQL with non-empty query is required on abstract or interface methods", methodElement); @@ -199,7 +218,7 @@ public class JdbcMapperProcessor extends AbstractProcessor { w.write(" throws "); } for (final TypeMirror thrownType : thrownTypes) { - sqlExceptionThrown |= typeUtils.isSameType(thrownType, sqlExceptionType); + sqlExceptionThrown |= types.isSameType(thrownType, sqlExceptionType); w.write(thrownType.toString()); if (++count != numThrownTypes) w.write(", "); @@ -228,7 +247,7 @@ public class JdbcMapperProcessor extends AbstractProcessor { if (parsedSQl.isSelect()) w.write("\t\tResultSet rs = null;\n"); w.write("\t\ttry {\n\t\t\tps = "); - final boolean cachePreparedStatements = sql.cachePreparedStatement().combine(mapper.cachePreparedStatements()); + final boolean cachePreparedStatements = sql.cachePreparedStatement().combine(defaultCachePreparedStatements); if (cachePreparedStatements) { w.write("this.prepareStatement("); w.write(Integer.toString(cachedPreparedStatements)); @@ -286,6 +305,15 @@ public class JdbcMapperProcessor extends AbstractProcessor { w.write("\t}\n"); } + // look on super classes and interfaces recursively + if(lookupCloseMethod) + closeMethod = getCloseMethod(genClass); + + if(closeMethod == null && (cachedPreparedStatements > 0 || doJndi)) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "@Jdbc.Mapper extended classes with cachedPreparedStatements or jndiNames must have a public void close() method to override or implement, because they must be closed", genClass); + continue; + } + if (cachedPreparedStatements > 0) { w.write("\n\tprivate final PreparedStatement[] psCache = new PreparedStatement["); w.write(Integer.toString(cachedPreparedStatements)); @@ -296,13 +324,18 @@ public class JdbcMapperProcessor extends AbstractProcessor { } // close method - w.write("\n\t@Override\n\tpublic void close() {\n\t\t"); - if (cachedPreparedStatements > 0) - w.write("for(final PreparedStatement ps : psCache)\n\t\t\ttryClose(ps);\n\t\t"); - w.write("tryClose(conn);\n"); - if (doJndi) - w.write("\t\ttryClose(ctx);\n"); - w.write("\t}\n"); + if(closeMethod != null) { + // if cachedPreparedStatements > 0 or doJndi are true, class MUST have a close() method to override as it + // MUST be called to clean up + w.write("\n\t@Override\n\tpublic void close() {\n"); + if (cachedPreparedStatements > 0) + w.write("\t\tfor(final PreparedStatement ps : psCache)\n\t\t\ttryClose(ps);\n"); + if (doJndi) + w.write("\t\ttryClose(ctx);\n\t\tif(ctx != null)\n\t\t\ttryClose(conn);\n"); + if(closeMethod.getEnclosingElement().getKind() != ElementKind.INTERFACE && !closeMethod.getEnclosingElement().equals(genClass)) + w.write("\t\tsuper.close();\n"); + w.write("\t}\n"); + } // end close method w.write("}\n"); @@ -408,4 +441,32 @@ public class JdbcMapperProcessor extends AbstractProcessor { return Class.forName(tm.toString()); } } + + public ExecutableElement getCloseMethod(final TypeElement genClass) { + ExecutableElement ret = null; + for (final Element methodElement : genClass.getEnclosedElements()) { + if((ret = getCloseMethod(methodElement)) != null) + return ret; + } + // superclasses + final TypeMirror superclass = genClass.getSuperclass(); + if(superclass.getKind() == TypeKind.DECLARED && (ret = getCloseMethod((TypeElement)types.asElement(superclass))) != null) + return ret; + // interfaces + for(final TypeMirror iface : genClass.getInterfaces()) { + if(iface.getKind() == TypeKind.DECLARED && (ret = getCloseMethod((TypeElement)types.asElement(iface))) != null) + return ret; + } + return ret; + } + + public static ExecutableElement getCloseMethod(final Element element) { + return element.getKind() != ElementKind.METHOD ? null : getCloseMethod((ExecutableElement) element); + } + + public static ExecutableElement getCloseMethod(final ExecutableElement methodElement) { + return methodElement.getReturnType().getKind() == TypeKind.VOID && + methodElement.getSimpleName().toString().equals("close") && + methodElement.getParameters().isEmpty() ? methodElement : null; + } } diff --git a/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/JdbcMapperTest.java b/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/JdbcMapperTest.java index d256e07..af85884 100644 --- a/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/JdbcMapperTest.java +++ b/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/JdbcMapperTest.java @@ -24,7 +24,7 @@ public class JdbcMapperTest { @AfterClass public static void tearDown() throws Throwable { - tryClose(dao); + //tryClose(dao); } public PersonDAO getDao() { diff --git a/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/PersonDAO.java b/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/PersonDAO.java index d4ba2c2..a7647ae 100644 --- a/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/PersonDAO.java +++ b/jdbcmapper/src/test/java/com/moparisthebest/jdbc/codegen/PersonDAO.java @@ -2,6 +2,7 @@ package com.moparisthebest.jdbc.codegen; import com.moparisthebest.jdbc.dto.FieldPerson; +import java.io.Closeable; import java.sql.SQLException; import java.util.Iterator; import java.util.List; @@ -13,10 +14,10 @@ import java.util.Map; */ @JdbcMapper.Mapper( // jndiName = "bob", - cachePreparedStatements = false +// cachePreparedStatements = false // , sqlParser = SimpleSQLParser.class ) -public interface PersonDAO extends JdbcMapper { +public interface PersonDAO { @JdbcMapper.SQL("UPDATE person SET first_name = {firstName} WHERE last_name = {lastName}") int setFirstName(String firstName, String lastName); diff --git a/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAO.java b/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAO.java index d29429f..37deee1 100644 --- a/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAO.java +++ b/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAO.java @@ -13,8 +13,8 @@ import java.util.Map; */ @JdbcMapper.Mapper( // jndiName = "bob", - cachePreparedStatements = false - , sqlParser = PrestoSQLParser.class +// cachePreparedStatements = false, + sqlParser = PrestoSQLParser.class ) public interface PrestoPersonDAO extends PersonDAO { diff --git a/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAOTest.java b/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAOTest.java index c451596..fcf28b4 100644 --- a/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAOTest.java +++ b/presto-sqlparser/src/test/java/com/moparisthebest/jdbc/codegen/PrestoPersonDAOTest.java @@ -24,7 +24,7 @@ public class PrestoPersonDAOTest extends JdbcMapperTest { @AfterClass public static void tearDown() throws Throwable { - tryClose(dao); + //tryClose(dao); } public PersonDAO getDao() {