From a6c5fc7b52cced2861067e9261fa829449edffee Mon Sep 17 00:00:00 2001 From: moparisthebest Date: Tue, 6 Mar 2018 01:04:44 -0500 Subject: [PATCH] Implement optional compile-time SQL checking with explain plan --- .../jdbc/codegen/JdbcMapperProcessor.java | 74 ++++++-- .../jdbc/codegen/SQLChecker.java | 14 ++ .../jdbc/codegen/SimpleSQLChecker.java | 179 ++++++++++++++++++ .../com/moparisthebest/jdbc/ArrayInList.java | 23 ++- .../jdbc/OracleArrayInList.java | 21 +- .../com/moparisthebest/jdbc/QueryMapper.java | 2 + 6 files changed, 282 insertions(+), 31 deletions(-) create mode 100644 jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SQLChecker.java create mode 100644 jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SimpleSQLChecker.java 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 2543699..ab183fb 100644 --- a/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java +++ b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/JdbcMapperProcessor.java @@ -1,7 +1,9 @@ package com.moparisthebest.jdbc.codegen; +import com.moparisthebest.jdbc.ArrayInList; import com.moparisthebest.jdbc.Cleaner; import com.moparisthebest.jdbc.MapperException; +import com.moparisthebest.jdbc.OracleArrayInList; import javax.annotation.processing.*; import javax.lang.model.SourceVersion; @@ -28,7 +30,7 @@ import static com.moparisthebest.jdbc.codegen.JdbcMapperFactory.SUFFIX; * Created by mopar on 5/24/17. */ @SupportedAnnotationTypes("com.moparisthebest.jdbc.codegen.JdbcMapper.Mapper") -@SupportedOptions({"jdbcMapper.databaseType", "jdbcMapper.arrayNumberTypeName", "jdbcMapper.arrayStringTypeName", "JdbcMapper.allowedMaxRowParamNames"}) +@SupportedOptions({"jdbcMapper.databaseType", "jdbcMapper.arrayNumberTypeName", "jdbcMapper.arrayStringTypeName", "jdbcMapper.allowedMaxRowParamNames", "jdbcMapper.sqlCheckerClass"}) @SupportedSourceVersion(SourceVersion.RELEASE_5) public class JdbcMapperProcessor extends AbstractProcessor { @@ -47,8 +49,8 @@ public class JdbcMapperProcessor extends AbstractProcessor { RELEASE_8 = rl8; } - private static Types types; - private static Messager messager; + static Types types; + static Messager messager; public static Types getTypes() { return types; @@ -58,14 +60,15 @@ public class JdbcMapperProcessor extends AbstractProcessor { return messager; } - private TypeMirror sqlExceptionType, stringType, numberType, utilDateType, readerType, clobType, jdbcMapperType, - byteArrayType, inputStreamType, fileType, blobType, sqlArrayType, collectionType, calendarType, cleanerType; + static TypeMirror sqlExceptionType, stringType, numberType, utilDateType, readerType, clobType, jdbcMapperType, + byteArrayType, inputStreamType, fileType, blobType, sqlArrayType, collectionType, calendarType, cleanerType, enumType; //IFJAVA8_START - private TypeMirror instantType, localDateTimeType, localDateType, localTimeType, zonedDateTimeType, offsetDateTimeType, offsetTimeType; + static TypeMirror instantType, localDateTimeType, localDateType, localTimeType, zonedDateTimeType, offsetDateTimeType, offsetTimeType; //IFJAVA8_END private TypeElement cleanerElement; private JdbcMapper.DatabaseType defaultDatabaseType; private String defaultArrayNumberTypeName, defaultArrayStringTypeName; + private SQLChecker sqlChecker; private Set allowedMaxRowParamNames; private CompileTimeResultSetMapper rsm; @@ -114,14 +117,25 @@ public class JdbcMapperProcessor extends AbstractProcessor { cleanerElement = elements.getTypeElement(Cleaner.class.getCanonicalName()); cleanerType = types.getDeclaredType(cleanerElement, types.getWildcardType(null, null)); - final String databaseType = processingEnv.getOptions().get("JdbcMapper.databaseType"); + enumType = types.getDeclaredType(elements.getTypeElement(Enum.class.getCanonicalName()), types.getWildcardType(null, null)); + + final String databaseType = processingEnv.getOptions().get("jdbcMapper.databaseType"); defaultDatabaseType = databaseType == null ? JdbcMapper.DatabaseType.STANDARD : JdbcMapper.DatabaseType.valueOf(databaseType); - defaultArrayNumberTypeName = processingEnv.getOptions().get("JdbcMapper.arrayNumberTypeName"); + defaultArrayNumberTypeName = processingEnv.getOptions().get("jdbcMapper.arrayNumberTypeName"); if (defaultArrayNumberTypeName == null || defaultArrayNumberTypeName.isEmpty()) defaultArrayNumberTypeName = defaultDatabaseType.arrayNumberTypeName; - defaultArrayStringTypeName = processingEnv.getOptions().get("JdbcMapper.arrayStringTypeName"); + defaultArrayStringTypeName = processingEnv.getOptions().get("jdbcMapper.arrayStringTypeName"); if (defaultArrayStringTypeName == null || defaultArrayStringTypeName.isEmpty()) defaultArrayStringTypeName = defaultDatabaseType.arrayStringTypeName; + final String sqlCheckerClass = processingEnv.getOptions().get("jdbcMapper.sqlCheckerClass"); + if(sqlCheckerClass != null) { + try { + sqlChecker = (SQLChecker) Class.forName(sqlCheckerClass).newInstance(); + } catch (Throwable e) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Error instantiating class specified by jdbcMapper.sqlCheckerClass, needs to implement SQLChecker and have a public no-arg constructor:" + toString(e)); + } + } String allowedMaxRowParamNames = processingEnv.getOptions().get("JdbcMapper.allowedMaxRowParamNames"); if (allowedMaxRowParamNames == null || allowedMaxRowParamNames.isEmpty()) @@ -162,6 +176,22 @@ public class JdbcMapperProcessor extends AbstractProcessor { arrayStringTypeName = !mapper.arrayStringTypeName().isEmpty() ? mapper.arrayStringTypeName() : (mapper.databaseType() == defaultDatabaseType ? defaultArrayStringTypeName : mapper.databaseType().arrayStringTypeName); } + final ArrayInList arrayInList; + if(sqlChecker != null) { + switch(databaseType) { + case ORACLE: + arrayInList = new OracleArrayInList(arrayNumberTypeName, arrayStringTypeName); + break; + case STANDARD: + arrayInList = new ArrayInList(arrayNumberTypeName, arrayStringTypeName); + break; + default: + // no support + arrayInList = null; + } + } else { + arrayInList = null; + } final String sqlParserMirror = getSqlParser(mapper).toString(); //final SQLParser parser = new SimpleSQLParser();//(SQLParser)Class.forName(mapper.sqlParser().getCanonicalName()).newInstance(); //final SQLParser parser = mapper.sqlParser().equals(SQLParser.class) ? new SimpleSQLParser() : mapper.sqlParser().newInstance(); @@ -380,7 +410,7 @@ public class JdbcMapperProcessor extends AbstractProcessor { w.write("conn.prepareStatement("); } w.write('"'); - w.write(sqlStatement.replace("\n", "\\n").replace("\"", "\\\"")); + w.write(sqlStatement.replace("\"", "\\\"").replace("\n", "\\n\" +\n\t\t\t \"")); w.write("\");\n"); // now bind parameters @@ -439,6 +469,9 @@ public class JdbcMapperProcessor extends AbstractProcessor { w.write("\t\t}\n"); w.write("\t}\n"); + + if(sqlChecker != null) + sqlChecker.checkSql(processingEnv, genClass, mapper, databaseType, eeMethod, sqlStatement, bindParams, arrayInList); } // look on super classes and interfaces recursively @@ -501,10 +534,7 @@ public class JdbcMapperProcessor extends AbstractProcessor { tryClose(w); } } catch (Exception e) { - final StringWriter sw = new StringWriter(); - sw.write('\n'); - e.printStackTrace(new PrintWriter(sw)); - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, e.getMessage() + sw.toString(), element); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, e.getMessage() + toString(e), element); return false; } return true; @@ -876,4 +906,20 @@ public class JdbcMapperProcessor extends AbstractProcessor { return false; } } + + public static String toString(final Throwable e) { + StringWriter sw = null; + PrintWriter pw = null; + try { + sw = new StringWriter(); + sw.write(String.format("%n")); + pw = new PrintWriter(sw); + e.printStackTrace(new PrintWriter(pw)); + pw.flush(); + return sw.toString(); + } finally { + tryClose(sw); + tryClose(pw); + } + } } diff --git a/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SQLChecker.java b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SQLChecker.java new file mode 100644 index 0000000..979d8e1 --- /dev/null +++ b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SQLChecker.java @@ -0,0 +1,14 @@ +package com.moparisthebest.jdbc.codegen; + +import com.moparisthebest.jdbc.ArrayInList; + +import javax.annotation.processing.ProcessingEnvironment; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import java.sql.SQLException; +import java.util.List; + +public interface SQLChecker { + void checkSql(final ProcessingEnvironment processingEnv, final TypeElement classElement, final JdbcMapper.Mapper mapper, final JdbcMapper.DatabaseType databaseType, final ExecutableElement method, final String sqlStatement, final List bindParams, final ArrayInList arrayInList) throws SQLException; +} diff --git a/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SimpleSQLChecker.java b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SimpleSQLChecker.java new file mode 100644 index 0000000..dfe4f25 --- /dev/null +++ b/jdbcmapper/src/main/java/com/moparisthebest/jdbc/codegen/SimpleSQLChecker.java @@ -0,0 +1,179 @@ +package com.moparisthebest.jdbc.codegen; + +import com.moparisthebest.jdbc.ArrayInList; +import com.moparisthebest.jdbc.QueryMapper; + +import javax.annotation.processing.Messager; +import javax.annotation.processing.ProcessingEnvironment; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.ArrayType; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.tools.Diagnostic; +import java.io.ByteArrayInputStream; +import java.io.StringReader; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import static com.moparisthebest.jdbc.TryClose.tryClose; +import static com.moparisthebest.jdbc.codegen.JdbcMapperProcessor.*; + +public class SimpleSQLChecker implements SQLChecker { + @Override + public void checkSql(final ProcessingEnvironment processingEnv, final TypeElement classElement, final JdbcMapper.Mapper mapper, final JdbcMapper.DatabaseType databaseType, final ExecutableElement method, final String sqlStatement, final List bindParams, final ArrayInList arrayInList) throws SQLException { + Connection conn = null; + QueryMapper qm = null; + try { + conn = getConnection(classElement, mapper, databaseType); + if (conn == null) { + getMessager().printMessage(Diagnostic.Kind.ERROR, "connection must be non-null for testing", classElement); + return; + } + conn.setAutoCommit(false); + qm = new QueryMapper(conn); + qm.executeUpdate(getSqlToExecute(classElement, conn, databaseType, sqlStatement), getFakeBindParams(bindParams, conn, arrayInList)); + } catch (Exception e) { + handleException(getMessager(), e, classElement, method); + } finally { + if (conn != null) { + try { + conn.rollback(); + conn.setAutoCommit(true); + } catch (Throwable e) { + // ignore + } + tryClose(conn); + } + tryClose(qm); + } + } + + public Connection getConnection(final TypeElement classElement, final JdbcMapper.Mapper mapper, final JdbcMapper.DatabaseType databaseType) throws SQLException { + if (mapper.jndiName().isEmpty()) { + throw new RuntimeException("@JdbcMapper.jndiName must be non-null for testing"); + } + return JdbcMapperFactory.connectionFactory(mapper.jndiName()).create(); + } + + public String getSqlToExecute(final TypeElement classElement, final Connection conn, final JdbcMapper.DatabaseType databaseType, final String sqlStatement) { + switch (databaseType) { + case ORACLE: + // oracle, being terrible, gives this error message for explain plans on MERGES + // so we will just execute those directly instead... + // ORA-00600: internal error code, arguments: [qctfrc : bfc], [22], [0], [5], [1], [1], [2], [371], [], [], [], [] + // we are assuming length is at least 5 after being trimmed, if anyone can come up with a valid SQL statement that is shorter let me know... + if (sqlStatement.trim().substring(0, 5).toUpperCase().equals("MERGE")) + return sqlStatement; + return "EXPLAIN PLAN FOR " + sqlStatement; + case STANDARD: + return "EXPLAIN " + sqlStatement; + } + return sqlStatement; + } + + public void handleException(final Messager messager, final Exception e, final TypeElement classElement, final ExecutableElement method) { + if (e.getMessage().startsWith("ORA-02291: integrity constraint")) + return; // since we make up values and rollback we can ignore this, it means SQL is correct at least + getMessager().printMessage(Diagnostic.Kind.ERROR, e.getMessage(), method); + } + + public Collection getFakeBindParams(final List bindParams, final Connection conn, final ArrayInList arrayInList) throws SQLException { + final Collection ret = new ArrayList(bindParams.size()); + for (final VariableElement bindParam : bindParams) { + ret.add(getFakeBindParam(bindParam, conn, arrayInList)); + } + return ret; + } + + private static final String defaultString = "n"; // oracle considers empty strings to be null... + + public Object getFakeBindParam(final VariableElement param, final Connection conn, final ArrayInList arrayInList) throws SQLException { + final TypeMirror o = param.asType(); + // special behavior + if (param instanceof InListVariableElement) { + final TypeMirror componentType; + if (o.getKind() == TypeKind.ARRAY) { + componentType = ((ArrayType) o).getComponentType(); + } else if (o.getKind() == TypeKind.DECLARED && types.isAssignable(o, collectionType)) { + final DeclaredType dt = (DeclaredType) o; + componentType = dt.getTypeArguments().get(0); + } else { + getMessager().printMessage(Diagnostic.Kind.MANDATORY_WARNING, "invalid in-list-variable type, returning null", ((InListVariableElement) param).delegate); + return null; + } + if (types.isAssignable(componentType, numberType)) { + return arrayInList.toArray(conn, true, new Long[]{0L}); // todo: not quite right, oh well for now + } else if (types.isAssignable(componentType, stringType) || types.isAssignable(componentType, enumType)) { + return arrayInList.toArray(conn, false, new String[]{defaultString}); + } else { + getMessager().printMessage(Diagnostic.Kind.MANDATORY_WARNING, "invalid in-list-variable type, returning null", ((InListVariableElement) param).delegate); + return null; + } + } + final JdbcMapper.Blob blob = param.getAnnotation(JdbcMapper.Blob.class); + if (blob != null) { + return new ByteArrayInputStream(new byte[1]); + } else { + final JdbcMapper.Clob clob = param.getAnnotation(JdbcMapper.Clob.class); + if (clob != null) { + return new StringReader(defaultString); + } + } + // end special behavior + // we are going to put most common ones up top so it should execute faster normally + if (types.isAssignable(o, stringType)) { + return defaultString; + } else if (o.getKind().isPrimitive() || types.isAssignable(o, numberType)) { + switch (o.getKind()) { + case BOOLEAN: + return true; + case BYTE: + case SHORT: + case INT: + return 0; + case LONG: + return 0L; + case CHAR: + return ' '; + case FLOAT: + return 0F; + case DOUBLE: + return 0D; + } + return null;//new BigDecimal(0D); // todo: not quite right, maybe good enough + // java.util.Date support, put it in a Timestamp + } else if (types.isAssignable(o, utilDateType)) { + return new java.sql.Timestamp(0); + } + //IFJAVA8_START + else if (types.isAssignable(o, instantType) || types.isAssignable(o, localDateTimeType) || types.isAssignable(o, zonedDateTimeType) || types.isAssignable(o, offsetDateTimeType)) { + return new java.sql.Timestamp(0); + } else if (types.isAssignable(o, localDateType)) { + return new java.sql.Date(0); + } else if (types.isAssignable(o, localTimeType) || types.isAssignable(o, offsetTimeType)) { + return new java.sql.Time(0); + } + //IFJAVA8_END + // CLOB support + else if (types.isAssignable(o, readerType) || types.isAssignable(o, clobType)) { + return new StringReader(defaultString); + } else if (types.isAssignable(o, inputStreamType) || types.isAssignable(o, blobType) || types.isAssignable(o, fileType)) { + return new ByteArrayInputStream(new byte[1]); + } else if (types.isAssignable(o, byteArrayType)) { + return new byte[1]; + } else if (types.isAssignable(o, sqlArrayType)) { + getMessager().printMessage(Diagnostic.Kind.MANDATORY_WARNING, "java.sql.Array not yet supported for getFakeBindParam, returning null", param); + return null; + } else { + // shouldn't get here ever, if we do the types should be more specific + getMessager().printMessage(Diagnostic.Kind.MANDATORY_WARNING, "unsupported type for getFakeBindParam, returning null", param); + return null; + } + } +} diff --git a/querymapper/src/main/java/com/moparisthebest/jdbc/ArrayInList.java b/querymapper/src/main/java/com/moparisthebest/jdbc/ArrayInList.java index 17c5a1e..e92db62 100644 --- a/querymapper/src/main/java/com/moparisthebest/jdbc/ArrayInList.java +++ b/querymapper/src/main/java/com/moparisthebest/jdbc/ArrayInList.java @@ -16,18 +16,31 @@ public class ArrayInList implements InList { return instance; } - protected ArrayInList() { + protected final String numericType, otherType; + + public ArrayInList(final String numericType, final String otherType) { + this.numericType = numericType; + this.otherType = otherType; + } + + public ArrayInList() { + this("number", "text"); } protected String columnAppend(final String columnName) { return "(" + columnName + " = ANY(?))"; } + public Array toArray(final Connection conn, final String typeName, final Object[] elements) throws SQLException { + return conn.createArrayOf(typeName, elements); + } + + public Array toArray(final Connection conn, final boolean numeric, final Object[] elements) throws SQLException { + return toArray(conn, numeric ? numericType : otherType, elements); + } + protected Array toArray(final Connection conn, final Collection values) throws SQLException { - return conn.createArrayOf( - values.iterator().next() instanceof Number ? "number" : "text", - values.toArray() - ); + return toArray(conn, values.iterator().next() instanceof Number, values.toArray()); } public InListObject inList(final Connection conn, final String columnName, final Collection values) throws SQLException { diff --git a/querymapper/src/main/java/com/moparisthebest/jdbc/OracleArrayInList.java b/querymapper/src/main/java/com/moparisthebest/jdbc/OracleArrayInList.java index 542d61e..40d02ba 100644 --- a/querymapper/src/main/java/com/moparisthebest/jdbc/OracleArrayInList.java +++ b/querymapper/src/main/java/com/moparisthebest/jdbc/OracleArrayInList.java @@ -34,25 +34,22 @@ public class OracleArrayInList extends ArrayInList { createArray = ca; } - protected OracleArrayInList() { + public OracleArrayInList(final String numberType, final String otherType) { + super(numberType, otherType); + } + + public OracleArrayInList() { + this("ARRAY_NUM_TYPE", "ARRAY_STR_TYPE"); } protected String columnAppend(final String columnName) { return "(" + columnName + " IN(select column_value from table(?)))"; } - protected Array toArray(final Connection conn, final Collection values) throws SQLException { - /* - return conn.unwrap(oracle.jdbc.OracleConnection.class).createOracleArray( - values.iterator().next() instanceof Number ? "ARRAY_NUM_TYPE" : "ARRAY_STR_TYPE", - values.toArray() - ); - */ + public Array toArray(final Connection conn, final String typeName, final Object[] elements) throws SQLException { + //return conn.unwrap(oracle.jdbc.OracleConnection.class).createOracleArray(typeName, elements); try { - return (Array) createArray.invoke(conn.unwrap(oracleConnection), - values.iterator().next() instanceof Number ? "ARRAY_NUM_TYPE" : "ARRAY_STR_TYPE", - values.toArray() - ); + return (Array) createArray.invoke(conn.unwrap(oracleConnection), typeName, elements); } catch (SQLException e) { throw e; } catch (Exception e) { diff --git a/querymapper/src/main/java/com/moparisthebest/jdbc/QueryMapper.java b/querymapper/src/main/java/com/moparisthebest/jdbc/QueryMapper.java index 5a396aa..34a6a02 100644 --- a/querymapper/src/main/java/com/moparisthebest/jdbc/QueryMapper.java +++ b/querymapper/src/main/java/com/moparisthebest/jdbc/QueryMapper.java @@ -179,6 +179,8 @@ public class QueryMapper implements JdbcMapper { ps.setBlob(index, (java.sql.Blob) o); else if (o instanceof ArrayInList.ArrayListObject) ps.setArray(index, ((ArrayInList.ArrayListObject) o).getArray()); + else if (o instanceof java.sql.Array) + ps.setArray(index, (java.sql.Array) o); else ps.setObject(index, o); // probably won't get here ever, but just in case... /*