Merge pull request 'Alternative method to provide dynamic arguments into sql query' (#28) from szumielxd/skript-db:master into master

Reviewed-on: #1

pom.xml

@@ -26,6 +26,10 @@
             <id>PaperMC</id>
             <url>https://repo.destroystokyo.com/repository/maven-public/</url>
         </repository>
+        <repository>
+            <id>sk89q</id>
+            <url>https://maven.sk89q.com/repo</url>
+        </repository>
         <repository>
             <id>skript</id>
             <url>https://repo.skriptlang.org/releases</url>
@@ -45,8 +49,8 @@
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.6.1</version>
                 <configuration>
-                    <source>17</source>
-                    <target>17</target>
+                    <source>11</source>
+                    <target>11</target>
                     <encoding>UTF-8</encoding>
                 </configuration>
             </plugin>

src/main/java/com/btk5h/skriptdb/SkriptValueWrapper.java

@@ -1,202 +0,0 @@
-package com.btk5h.skriptdb;
-
-import static java.sql.Types.*;
-
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Timestamp;
-import java.time.Instant;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.Function;
-import java.util.stream.IntStream;
-
-import ch.njol.skript.util.Date;
-import ch.njol.skript.util.Time;
-
-public class SkriptValueWrapper {
-	
-	private static final Map<Class<?>, SkriptValueWrapper> wrappersBySkript = new HashMap<>();
-	private static final Map<Integer, SkriptValueWrapper> wrappersBySql = new HashMap<>();
-	
-	
-	public static final SkriptValueWrapper FALLBACK = register(Object.class, Function.identity(), ResultSet::getObject);
-	public static final SkriptValueWrapper TEXT_VALUE = register(String.class, SkriptValueWrapper::asString, SkriptValueWrapper::getString, CHAR, VARCHAR, LONGVARCHAR, CLOB);
-	public static final SkriptValueWrapper BOOLEAN_VALUE = register(Boolean.class, SkriptValueWrapper::asBoolean, ResultSet::getBoolean, BOOLEAN);
-	public static final SkriptValueWrapper LONG_VALUE = register(Long.class, x -> x, SkriptValueWrapper::getLong, BIT, TINYINT, SMALLINT, INTEGER, BIGINT);
-	public static final SkriptValueWrapper DOUBLE_VALUE = register(Double.class, x -> x, SkriptValueWrapper::getDouble, FLOAT, REAL, DOUBLE, NUMERIC, DECIMAL);
-	public static final SkriptValueWrapper DATE_VALUE = register(String.class, SkriptValueWrapper::noParser, SkriptValueWrapper::getString, DATE);
-	public static final SkriptValueWrapper TIME_VALUE = register(Time.class, SkriptValueWrapper::asTime, SkriptValueWrapper::getTime, TIME);
-	public static final SkriptValueWrapper TIMESTAMP_VALUE = register(Date.class, SkriptValueWrapper::asTimestamp, SkriptValueWrapper::getTimestamp, TIMESTAMP);
-	public static final SkriptValueWrapper BINARY_VALUE = register(String.class, SkriptValueWrapper::noParser, SkriptValueWrapper::getBinary, TIMESTAMP);
-	
-	
-	
-	private final Class<?> skriptType;
-	private final Function<Object, Object> skToSqlMapper;
-	private final ResultFetcher<Object> sqlToSkMapper;
-	private final int[] validSqlTypes;
-	
-	
-	@SuppressWarnings("unchecked")
-	private <T> SkriptValueWrapper(Class<T> skriptType, Function<T, Object> skToSql, ResultFetcher<? super T> sqlToSk, int... validSqlTypes) {
-		this.skriptType = skriptType;
-		this.skToSqlMapper = (Function<Object, Object>) skToSql;
-		this.sqlToSkMapper = (ResultFetcher<Object>) sqlToSk;
-		this.validSqlTypes = IntStream.of(validSqlTypes)
-				.sorted()
-				.toArray();
-	}
-	
-	
-	public Object asSql(Object obj) {
-		return this.skToSqlMapper.apply(obj);
-	}
-	
-	
-	public boolean isSkApplicable(Class<?> type) {
-		return this.skriptType.isAssignableFrom(type);
-	}
-	
-	
-	public boolean isSqlApplicable(int sqlType) {
-		return Arrays.binarySearch(this.validSqlTypes, sqlType) >= 0;
-	}
-	
-	
-	public Object asSkript(ResultSet rs, int column) throws SQLException {
-		return this.sqlToSkMapper.fetch(rs, column);
-	}
-	
-	
-	public static SkriptValueWrapper getBySQLType(int type) {
-		return wrappersBySql.getOrDefault(type, FALLBACK);
-	}
-	
-	
-	public static SkriptValueWrapper getBySkriptType(Class<?> type) {
-		return wrappersBySkript.getOrDefault(type, FALLBACK);
-	}
-	
-	
-	public static SkriptValueWrapper getBySQLLType(int type) {
-		return switch (type) {
-			//case CHAR, VARCHAR, LONGVARCHAR, CLOB -> null; // string
-			//case BOOLEAN -> null; // boolean
-			//case BIT, TINYINT, SMALLINT, INTEGER, BIGINT -> null; // long
-			//case FLOAT, REAL, DOUBLE, NUMERIC, DECIMAL -> null; // double
-			//case DATE -> null; // string
-			//case TIME -> null; // time
-			//case TIMESTAMP -> null; // Date
-			//case BINARY, VARBINARY, LONGVARBINARY, BLOB -> null; // hex String
-			//case NULL, OTHER, JAVA_OBJECT, DISTINCT, STRUCT, ARRAY -> null; // just whatever it gives to me
-			default -> null;
-		};
-	}
-	
-	
-	private static <T> SkriptValueWrapper register(Class<T> skriptType, Function<T, Object> skToSql, ResultFetcher<? super T> sqlToSk, int... validSqlTypes) {
-		SkriptValueWrapper wrapper = new SkriptValueWrapper(skriptType, skToSql, sqlToSk, validSqlTypes);
-		wrappersBySkript.putIfAbsent(skriptType, wrapper);
-		for (int i : validSqlTypes) {
-			wrappersBySql.putIfAbsent(i, wrapper);
-		}
-		return wrapper;
-	}
-	
-	
-	private static String getString(ResultSet rs, int column) throws SQLException {
-		return rs.getString(column);
-	}
-	
-	private static String asString(Object obj) {
-		return obj != null ? String.valueOf(obj) : null;
-	}
-	
-	
-	private static Long getLong(ResultSet rs, int column) throws SQLException {
-		var val = rs.getLong(column);
-		if (!rs.wasNull()) {
-			return val;
-		}
-		return null;
-	}
-	
-	
-	private static Double getDouble(ResultSet rs, int column) throws SQLException {
-		var val = rs.getDouble(column);
-		if (!rs.wasNull()) {
-			return val;
-		}
-		return null;
-	}
-	
-	
-	private static final int TICKS_PER_HOUR = 1000;
-	private static final double TICKS_PER_MINUTE = 1000. / 60;
-	private static final int HOUR_ZERO = 6 * TICKS_PER_HOUR;
-	
-	private static Time getTime(ResultSet rs, int column) throws SQLException {
-		var time = rs.getTime(column);
-		if (time != null) {
-			var local = time.toLocalTime();
-			var minutes = local.getMinute();
-			var hours = local.getHour();
-			return new Time((int) Math.round(
-					hours * TICKS_PER_HOUR
-					- HOUR_ZERO
-					+ minutes * TICKS_PER_MINUTE));
-		}
-		return null;
-	}
-	
-	private static Object asTime(Time time) {
-		if (time != null) {
-			return new java.sql.Time(time.getTime());
-		}
-		return null;
-	}
-	
-	
-	private static Date getTimestamp(ResultSet rs, int column) throws SQLException {
-		return new Date(rs.getTimestamp(column).getTime());
-	}
-	
-	private static Object asTimestamp(Date date) {
-		return Timestamp.from(Instant.ofEpochMilli(date.getTimestamp()));
-	}
-	
-	private static Object asBoolean(Object obj) {
-		if (obj instanceof Number n) {
-			return n.doubleValue() > 0;
-		} else if (obj instanceof Boolean b) {
-			return b;
-		}
-		return null;
-	}
-	
-	
-	private static String getBinary(ResultSet rs, int column) throws SQLException {
-		byte[] bytes = rs.getBytes(column);
-		if (!rs.wasNull()) {
-			StringBuilder sb = new StringBuilder(bytes.length);
-			for (int i = 0; i < bytes.length; i++) {
-				sb.append(String.format("%02X", bytes[i]));
-			}
-			return sb.toString();
-		}
-		return null;
-	}
-	
-	
-	private static <T, U> U noParser(T t) {
-		throw new IllegalStateException("This value shouldn't have a parser");
-	}
-	
-	
-	private interface ResultFetcher<T> {
-		public T fetch(ResultSet rs, int index) throws SQLException;
-	}
-
-}

src/main/java/com/btk5h/skriptdb/skript/EffExecuteStatement.java

@@ -1,51 +1,33 @@
 package com.btk5h.skriptdb.skript;
 
+import ch.njol.skript.Skript;
+import ch.njol.skript.effects.Delay;
+import ch.njol.skript.lang.*;
+import ch.njol.skript.variables.Variables;
+import ch.njol.util.Kleenean;
+import ch.njol.util.Pair;
+import com.btk5h.skriptdb.SkriptDB;
+import com.btk5h.skriptdb.SkriptUtil;
+import com.zaxxer.hikari.HikariDataSource;
+import org.bukkit.Bukkit;
+import org.bukkit.event.Event;
+
+import javax.sql.DataSource;
+import javax.sql.rowset.CachedRowSet;
+import javax.sql.rowset.serial.SerialBlob;
+import javax.sql.rowset.serial.SerialException;
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
-import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.sql.Statement;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.regex.Pattern;
-import java.util.stream.Stream;
-
-import javax.sql.DataSource;
-import javax.sql.rowset.serial.SerialBlob;
-import javax.sql.rowset.serial.SerialException;
-
-import org.bukkit.Bukkit;
-import org.bukkit.event.Event;
-
-import com.btk5h.skriptdb.SkriptDB;
-import com.btk5h.skriptdb.SkriptUtil;
-import com.btk5h.skriptdb.SkriptValueWrapper;
-import com.zaxxer.hikari.HikariDataSource;
-
-import ch.njol.skript.Skript;
-import ch.njol.skript.effects.Delay;
-import ch.njol.skript.lang.Effect;
-import ch.njol.skript.lang.Expression;
-import ch.njol.skript.lang.SkriptParser;
-import ch.njol.skript.lang.Trigger;
-import ch.njol.skript.lang.TriggerItem;
-import ch.njol.skript.lang.Variable;
-import ch.njol.skript.lang.VariableString;
-import ch.njol.skript.variables.Variables;
-import ch.njol.util.Kleenean;
-import ch.njol.util.Pair;
 
 /**
  * Executes a statement on a database and optionally stores the result in a variable. Expressions
@@ -90,7 +72,7 @@ public class EffExecuteStatement extends Effect {
         DataSource ds = dataSource.getSingle(e);
         //if data source isn't set
         if (ds == null) {
-            return;
+        	return;
         }
         Pair<String, List<Object>> parsedQuery = parseQuery(e);
         String baseVariable = resultVariableName != null ? resultVariableName.toString(e).toLowerCase(Locale.ENGLISH) : null;
@@ -100,22 +82,22 @@ public class EffExecuteStatement extends Effect {
         //execute SQL statement
         if (Bukkit.isPrimaryThread()) {
             CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, parsedQuery), threadPool)
-                    .whenComplete((resources, err) -> {
-                        //handle last error syntax data
-                        resetLastSQLError();
-                        if (err instanceof CompletionException && err.getCause() instanceof SkriptDBQueryException) {
-                            setLastSQLError(err.getCause().getMessage());
-                        }
-                        //if local variables are present
-                        //bring back local variables
-                        //populate SQL data into variables
-                        if (!quickly) {
-                            Bukkit.getScheduler().runTask(SkriptDB.getInstance(),
-                                    () -> postExecution(e, locals, resources));
-                        } else {
-                            postExecution(e, locals, resources);
-                        }
-                    });
+		            .whenComplete((resources, err) -> {
+		            	//handle last error syntax data
+		            	resetLastSQLError();
+		            	if (err instanceof CompletionException && err.getCause() instanceof SkriptDBQueryException) {
+	            			setLastSQLError(err.getCause().getMessage());
+	            		}
+		                //if local variables are present
+		                //bring back local variables
+		                //populate SQL data into variables
+		                if (!quickly) {
+		                    Bukkit.getScheduler().runTask(SkriptDB.getInstance(),
+		                    		() -> postExecution(e, locals, resources));
+		                } else {
+		                	postExecution(e, locals, resources);
+		                }
+		            });
             // sync executed SQL query, same as above, just sync
         } else {
             isSync = true;
@@ -123,23 +105,23 @@ public class EffExecuteStatement extends Effect {
             resetLastSQLError();
             try {
                 resources = executeStatement(ds, baseVariable, parsedQuery);
-            } catch (SkriptDBQueryException err) {
-                //handle last error syntax data
-                setLastSQLError(err.getMessage());
-            }
+			} catch (SkriptDBQueryException err) {
+	            //handle last error syntax data
+				setLastSQLError(err.getMessage());
+			}
             //if local variables are present
             //bring back local variables
             //populate SQL data into variables
-            postExecution(e, locals, resources);
+			postExecution(e, locals, resources);
         }
     }
     
     private void postExecution(Event e, Object locals, Map<String, Object> resources) {
-        if (locals != null && getNext() != null) {
+    	if (locals != null && getNext() != null) {
             Variables.setLocalVariables(e, locals);
         }
         if (resources != null) {
-            resources.forEach((name, value) -> setVariable(e, name, value));
+        	resources.forEach((name, value) -> setVariable(e, name, value));
         }
         TriggerItem.walk(getNext(), e);
         //the line below is required to prevent memory leaks
@@ -163,19 +145,16 @@ public class EffExecuteStatement extends Effect {
             int queryArgCount = (int) ARGUMENT_PLACEHOLDER.matcher(queryString).results().count();
             if (queryArgCount != args.length) {
                 Skript.warning(String.format("Your query has %d question marks, but you provided %d arguments. (%s) [%s]",
-                        queryArgCount,
-                        args.length,
-                        queryArguments.toString(e, true),
-                        Optional.ofNullable(getTrigger())
-                                .map(Trigger::getDebugLabel)
-                                .orElse("unknown")));
+                		queryArgCount,
+                		args.length,
+                		queryArguments.toString(e, true),
+                		Optional.ofNullable(getTrigger())
+                				.map(Trigger::getDebugLabel)
+                				.orElse("unknown")));
                 args = Arrays.copyOf(args, queryArgCount);
             }
-            List<Object> argsList = Stream.of(args)
-            		.map(arg -> SkriptValueWrapper.getBySkriptType(arg.getClass()).asSql(arg))
-            		.toList();
-            return new Pair<>(query.getSingle(e), argsList);
-        } else if (query instanceof VariableString queryString && !queryString.isSimple()) {
+            return new Pair<>(query.getSingle(e), Arrays.asList(args));
+        } else if (query instanceof VariableString && !((VariableString) query).isSimple()) {
             return parseVariableQuery(e, (VariableString) query);
         }
         return new Pair<>(query.getSingle(e), null);
@@ -205,11 +184,11 @@ public class EffExecuteStatement extends Effect {
     }
     
     private Pair<String, Object> parseExpressionQuery(Expression<?> expr, Object expressionValue, boolean standaloneString) {
-        if (expr instanceof ExprUnsafe unsafe) {
+        if (expr instanceof ExprUnsafe) {
             if (standaloneString && expressionValue instanceof String) {
                 Skript.warning(
                         String.format("Unsafe may have been used unnecessarily. Try replacing 'unsafe %1$s' with %1$s",
-                        		unsafe.getRawExpression()));
+                                ((ExprUnsafe) expr).getRawExpression()));
             }
             return new Pair<>((String) expressionValue, null);
         } else {
@@ -222,7 +201,7 @@ public class EffExecuteStatement extends Effect {
 
     private Map<String, Object> executeStatement(DataSource ds, String baseVariable, Pair<String, List<Object>> query) throws SkriptDBQueryException {
         if (ds == null) {
-            throw new SkriptDBQueryException("Data source is not set");
+        	throw new SkriptDBQueryException("Data source is not set");
         }
         try (Connection conn = ds.getConnection()) {
             try (PreparedStatement stmt = createStatement(conn, query)) {
@@ -244,16 +223,14 @@ public class EffExecuteStatement extends Effect {
         }
 
         if (hasResultSet) {
-        	// TODO: Check if caching is even needed
-            //CachedRowSet crs = SkriptDB.getRowSetFactory().createCachedRowSet();
-            ResultSet rs = generatedKeys ? stmt.getGeneratedKeys() : stmt.getResultSet();
-            //crs.populate(rs);
+            CachedRowSet crs = SkriptDB.getRowSetFactory().createCachedRowSet();
+            crs.populate(generatedKeys ? stmt.getGeneratedKeys() : stmt.getResultSet());
 
             if (isList) {
-                return fetchQueryResultSet(rs, baseVariable);
+                return fetchQueryResultSet(crs, baseVariable);
             } else {
-            	rs.last();
-                return Map.of(baseVariable, rs.getRow());
+                crs.last();
+                return Map.of(baseVariable, crs.getRow());
             }
         } else if (!isList) {
             //if no results are returned and the specified variable isn't a list variable, put the affected rows count in the variable
@@ -262,25 +239,21 @@ public class EffExecuteStatement extends Effect {
         return Map.of();
     }
     
-    private Map<String, Object> fetchQueryResultSet(/*CachedRowSet*/ResultSet crs, String baseVariable) throws SQLException {
-    	Map<String, Object> variableList = new HashMap<>();
+    private Map<String, Object> fetchQueryResultSet(CachedRowSet crs, String baseVariable) throws SQLException {
+        Map<String, Object> variableList = new HashMap<>();
         ResultSetMetaData meta = crs.getMetaData();
         int columnCount = meta.getColumnCount();
-        SkriptValueWrapper[] wrappers = new SkriptValueWrapper[columnCount];
-        String[] columnNames = new String[columnCount];
+
         for (int i = 1; i <= columnCount; i++) {
             String label = meta.getColumnLabel(i);
             variableList.put(baseVariable + label, label);
-            columnNames[i - 1] = label;
-            wrappers[i - 1] = SkriptValueWrapper.getBySQLType(meta.getColumnType(i));
         }
-        
+
         int rowNumber = 1;
         while (crs.next()) {
             for (int i = 1; i <= columnCount; i++) {
-            	Object obj = wrappers[i - 1].asSkript(crs, i);
-                variableList.put(baseVariable + columnNames[i - 1].toLowerCase(Locale.ENGLISH)
-                        + Variable.SEPARATOR + rowNumber, obj);
+                variableList.put(baseVariable + meta.getColumnLabel(i).toLowerCase(Locale.ENGLISH)
+                        + Variable.SEPARATOR + rowNumber, crs.getObject(i));
             }
             rowNumber++;
         }
@@ -289,8 +262,8 @@ public class EffExecuteStatement extends Effect {
 
     private PreparedStatement createStatement(Connection conn, Pair<String, List<Object>> query) throws SQLException {
         PreparedStatement stmt = generatedKeys ? 
-                conn.prepareStatement(query.getFirst(), Statement.RETURN_GENERATED_KEYS)
-                : conn.prepareStatement(query.getFirst(), Statement.NO_GENERATED_KEYS);
+        		conn.prepareStatement(query.getFirst(), Statement.RETURN_GENERATED_KEYS)
+        		: conn.prepareStatement(query.getFirst(), Statement.NO_GENERATED_KEYS);
         if (query.getSecond() != null) {
             Iterator<Object> iter = query.getSecond().iterator();
             for (int i = 1; iter.hasNext(); i++) {
@@ -308,8 +281,8 @@ public class EffExecuteStatement extends Effect {
     }
 
     private String getString(Object[] objects, int index) {
-        if (index >= 0 && index < objects.length && objects[index] instanceof String str) {
-            return str;
+        if (index >= 0 && index < objects.length && objects[index] instanceof String) {
+            return (String) objects[index];
         }
         return null;
     }
@@ -318,13 +291,13 @@ public class EffExecuteStatement extends Effect {
 
         //fix mediumblob and similar column types, so they return a String correctly
         if (obj != null) {
-            if (obj instanceof byte[] bytes) {
-                obj = new String(bytes);
+            if (obj instanceof byte[]) {
+                obj = new String((byte[]) obj);
 
                 //in some servers instead of being byte array, it appears as SerialBlob (depends on mc version, 1.12.2 is bvte array, 1.16.5 SerialBlob)
-            } else if (obj instanceof SerialBlob blob) {
+            } else if (obj instanceof SerialBlob) {
                 try {
-                    obj = new String(blob.getBinaryStream().readAllBytes());
+                    obj = new String(((SerialBlob) obj).getBinaryStream().readAllBytes());
                 } catch (IOException | SerialException ex) {
                     ex.printStackTrace();
                 }
@@ -334,11 +307,11 @@ public class EffExecuteStatement extends Effect {
     }
     
     private static void resetLastSQLError() {
-        lastError = null;
+    	lastError = null;
     }
     
     private static void setLastSQLError(String error) {
-        lastError = error;
+    	lastError = error;
     }
 
     @Override
@@ -360,11 +333,12 @@ public class EffExecuteStatement extends Effect {
         }
         dataSource = (Expression<HikariDataSource>) exprs[1];
         if (exprs[2] != null) {
-            if (query instanceof VariableString queryString && !queryString.isSimple()) {
+            if (query instanceof VariableString && !((VariableString) query).isSimple()) {
                 Skript.warning("Your query string contains expresions, but you've also provided query arguments. Consider using `unsafe` keyword before your query.");
             }
             queryArguments = (Expression<Object>) exprs[2];
         }
+        ;
         Expression<?> resultHolder = exprs[3];
         quickly = parseResult.hasTag("quickly");
         if (resultHolder instanceof Variable) {
@@ -382,12 +356,12 @@ public class EffExecuteStatement extends Effect {
     
     public static class SkriptDBQueryException extends RuntimeException {
 
-        private static final long serialVersionUID = -1869895286406538884L;
-        
-        public SkriptDBQueryException(String message) {
-            super(message);
-        }
-        
+		private static final long serialVersionUID = -1869895286406538884L;
+		
+		public SkriptDBQueryException(String message) {
+			super(message);
+		}
+    	
     }
     
 }

src/main/java/com/btk5h/skriptdb/skript/ExprSQLQuery.java

@@ -30,8 +30,8 @@ public class ExprSQLQuery extends SimpleExpression<String> {
 
     @Override
     protected String[] get(Event e) {
-        if (e instanceof SQLQueryCompleteEvent sqlEvent) {
-            return new String[]{ sqlEvent.getQuery() };
+        if (e instanceof SQLQueryCompleteEvent) {
+            return new String[]{((SQLQueryCompleteEvent) e).getQuery()};
         }
         return null;
     }