Merge pull request 'cleaning' (#1) from cleaning into master

Reviewed-on: szumielxd/skript-db#1

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

@@ -1,37 +1,39 @@
 package com.btk5h.skriptdb;
 
-import ch.njol.skript.Skript;
-import ch.njol.skript.lang.Expression;
-import ch.njol.skript.lang.VariableString;
-
 import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Optional;
 
+import org.bukkit.event.Event;
+
+import ch.njol.skript.ScriptLoader;
+import ch.njol.skript.Skript;
+import ch.njol.skript.lang.Expression;
+import ch.njol.skript.lang.VariableString;
+import ch.njol.skript.lang.parser.ParserInstance;
+
 public class SkriptUtil {
 
     private static final Field STRING;
     private static final Field EXPR;
 
     static {
-    Field _FIELD = null;
+        STRING = tryGetOldStringField()
-    try {
+                .or(() -> tryGetNewStringField())
-      _FIELD = VariableString.class.getDeclaredField("string");
+                .orElseGet(() -> {
-      _FIELD.setAccessible(true);
-    } catch (NoSuchFieldException e) {
                     Skript.error("Skript's 'string' field could not be resolved.");
-      e.printStackTrace();
+                    return null;
-    }
+                });
-    STRING = _FIELD;
 
+        Field f = null;
         try {
             Optional<Class<?>> expressionInfo = Arrays.stream(VariableString.class.getDeclaredClasses())
                     .filter(cls -> cls.getSimpleName().equals("ExpressionInfo"))
                     .findFirst();
             if (expressionInfo.isPresent()) {
                 Class<?> expressionInfoClass = expressionInfo.get();
-        _FIELD = expressionInfoClass.getDeclaredField("expr");
+                f = expressionInfoClass.getDeclaredField("expr");
-        _FIELD.setAccessible(true);
+                f.setAccessible(true);
             } else {
                 Skript.error("Skript's 'ExpressionInfo' class could not be resolved.");
             }
@@ -39,7 +41,7 @@ public class SkriptUtil {
             e.printStackTrace();
             Skript.error("Skript's 'expr' field could not be resolved.");
         }
-    EXPR = _FIELD;
+        EXPR = f;
     }
 
     public static Object[] getTemplateString(VariableString vs) {
@@ -58,4 +60,35 @@ public class SkriptUtil {
         }
     }
     
+    @SuppressWarnings("deprecation")
+	public static boolean isCurrentEvent(Class<? extends Event> event) {
+        try {
+            Class.forName("ch.njol.skript.lang.parser.ParserInstance");
+            return ParserInstance.get().isCurrentEvent(event);
+        } catch (ClassNotFoundException e) {
+            return ScriptLoader.isCurrentEvent(event);
+        }
+    }
+    
+    
+    private static Optional<Field> tryGetOldStringField() {
+        try {
+            Field f = VariableString.class.getDeclaredField("string");
+            f.setAccessible(true);
+            return Optional.of(f);
+        } catch (NoSuchFieldException e) {
+            return Optional.empty();
+        }
+    }
+    
+    private static Optional<Field> tryGetNewStringField() {
+        try {
+            Field f = VariableString.class.getDeclaredField("strings");
+            f.setAccessible(true);
+            return Optional.of(f);
+        } catch (NoSuchFieldException e) {
+            return Optional.empty();
+        }
+    }
+
 }

src/main/java/com/btk5h/skriptdb/events/SQLQueryCompleteEvent.java

@@ -4,32 +4,26 @@ import org.bukkit.event.Event;
 import org.bukkit.event.HandlerList;
 
 public class SQLQueryCompleteEvent extends Event {
-    private final static HandlerList HANDLERS = new HandlerList();
+    
+	private static final HandlerList HANDLERS = new HandlerList();
+	
     private final String argument;
 
     public SQLQueryCompleteEvent(String argument) {
         super(true);
         this.argument = argument;
-        //  this.variables = variables;
-    }
-
-    public static HandlerList getHandlerList() {
-        return HANDLERS;
-    }
-
-    @Override
-    public String getEventName() {
-        return super.getEventName();
     }
 
     @Override
     public HandlerList getHandlers() {
-        return HANDLERS;
+        return getHandlerList();
     }
 
     public String getQuery() {
         return argument;
     }
 
-    //  public String getVariables() {return;}
+    public static HandlerList getHandlerList() {
+        return HANDLERS;
+    }
 }

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

@@ -21,8 +21,10 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.Statement;
 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;
@@ -51,104 +53,80 @@ public class EffExecuteStatement extends Effect {
 
     static {
         Skript.registerEffect(EffExecuteStatement.class,
-                "execute %string% (in|on) %datasource% " +
+                "[quickly:quickly] execute %string% (in|on) %datasource% " +
-                        "[with arg[ument][s] %-objects%] [and store [[the] (output|result)[s]] (to|in) [the] [var[iable]] %-objects%]",
+                        "[with arg[ument][s] %-objects%] [and store [[the] [keys:generated keys] (output|result)[s]] (to|in) [the] [var[iable]] %-objects%]");
-                "quickly execute %string% (in|on) %datasource% " +
-                        "[with arg[ument][s] %-objects%] [and store [[the] (output|result)[s]] (to|in) [the] [var[iable]] %-objects%]");
     }
 
     private Expression<String> query;
     private Expression<HikariDataSource> dataSource;
     private Expression<Object> queryArguments;
-    private VariableString var;
+    private VariableString resultVariableName;
     private boolean isLocal;
     private boolean isList;
     private boolean quickly;
+    private boolean generatedKeys;
     private boolean isSync = false;
 
-    private void continueScriptExecution(Event e, Object populatedVariables) {
-        lastError = null;
-        if (populatedVariables instanceof String) {
-            lastError = (String) populatedVariables;
-        } else {
-
-            if (getNext() != null) {
-                ((Map<String, Object>) populatedVariables).forEach((name, value) -> setVariable(e, name, value));
-            }
-        }
-        TriggerItem.walk(getNext(), e);
-    }
-
     @Override
     protected void execute(Event e) {
         DataSource ds = dataSource.getSingle(e);
-        Pair<String, List<Object>> query = parseQuery(e);
-        String baseVariable = var != null ? var.toString(e).toLowerCase(Locale.ENGLISH) : null;
         //if data source isn't set
-        if (ds == null) return;
+        if (ds == null) {
+        	return;
+        }
+        Pair<String, List<Object>> parsedQuery = parseQuery(e);
+        String baseVariable = resultVariableName != null ? resultVariableName.toString(e).toLowerCase(Locale.ENGLISH) : null;
+        
         Object locals = Variables.removeLocals(e);
 
         //execute SQL statement
         if (Bukkit.isPrimaryThread()) {
-            CompletableFuture<Object> sql = CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, query), threadPool);
+            CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, parsedQuery), threadPool)
-            sql.whenComplete((res, err) -> {
+		            .whenComplete((resources, err) -> {
-                if (err != null) {
-                    err.printStackTrace();
-                }
 		            	//handle last error syntax data
-                lastError = null;
+		            	resetLastSQLError();
-                if (res instanceof String) {
+		            	if (err instanceof CompletionException && err.getCause() instanceof SkriptDBQueryException) {
-                    lastError = (String) res;
+	            			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(), () -> {
+		                    Bukkit.getScheduler().runTask(SkriptDB.getInstance(),
-                        if (locals != null && getNext() != null) {
+		                    		() -> postExecution(e, locals, resources));
-                            Variables.setLocalVariables(e, locals);
-                        }
-                        if (!(res instanceof String)) {
-                            ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
-                        }
-                        TriggerItem.walk(getNext(), e);
-                        //the line below is required to prevent memory leaks
-                        Variables.removeLocals(e);
-                    });
 		                } else {
-                    if (locals != null && getNext() != null) {
+		                	postExecution(e, locals, resources);
-                        Variables.setLocalVariables(e, locals);
-                    }
-                    if (!(res instanceof String)) {
-                        ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
-                    }
-                    TriggerItem.walk(getNext(), e);
-                    //the line below is required to prevent memory leaks
-                    Variables.removeLocals(e);
 		                }
 		            });
             // sync executed SQL query, same as above, just sync
         } else {
             isSync = true;
-            Object resources = executeStatement(ds, baseVariable, query);
+            Map<String, Object> resources = null;
+            resetLastSQLError();
+            try {
+                resources = executeStatement(ds, baseVariable, parsedQuery);
+			} catch (SkriptDBQueryException err) {
 	            //handle last error syntax data
-            lastError = null;
+				setLastSQLError(err.getMessage());
-            if (resources instanceof String) {
-                lastError = (String) resources;
 			}
             //if local variables are present
             //bring back local variables
             //populate SQL data into variables
+			postExecution(e, locals, resources);
+        }
+    }
+    
+    private void postExecution(Event e, Object locals, Map<String, Object> resources) {
     	if (locals != null && getNext() != null) {
             Variables.setLocalVariables(e, locals);
         }
-            if (!(resources instanceof String)) {
+        if (resources != null) {
-                ((Map<String, Object>) 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
         Variables.removeLocals(e);
     }
-    }
 
     @Override
     protected TriggerItem walk(Event e) {
@@ -166,10 +144,16 @@ public class EffExecuteStatement extends Effect {
             String queryString = query.getSingle(e);
             int queryArgCount = (int) ARGUMENT_PLACEHOLDER.matcher(queryString).results().count();
             if (queryArgCount != args.length) {
-                Skript.warning("Your query has %d question marks, but you provided %d arguments.");
+                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")));
                 args = Arrays.copyOf(args, queryArgCount);
             }
-            return new Pair<>(query.getSingle(e), List.of(args));
+            return new Pair<>(query.getSingle(e), Arrays.asList(args));
         } else if (query instanceof VariableString && !((VariableString) query).isSimple()) {
             return parseVariableQuery(e, (VariableString) query);
         }
@@ -215,9 +199,9 @@ public class EffExecuteStatement extends Effect {
         }
     }
 
-    private Object executeStatement(DataSource ds, String baseVariable, Pair<String, List<Object>> query) {
+    private Map<String, Object> executeStatement(DataSource ds, String baseVariable, Pair<String, List<Object>> query) throws SkriptDBQueryException {
         if (ds == null) {
-            return "Data source is not set";
+        	throw new SkriptDBQueryException("Data source is not set");
         }
         try (Connection conn = ds.getConnection()) {
             try (PreparedStatement stmt = createStatement(conn, query)) {
@@ -229,25 +213,24 @@ public class EffExecuteStatement extends Effect {
                 return Map.of();
             }
         } catch (SQLException ex) {
-            return ex.getMessage();
+            throw new SkriptDBQueryException(ex.getMessage());
         }
     }
     
-    private Object processBaseVariable(String baseVariable, PreparedStatement stmt, boolean hasResultSet) throws SQLException {
+    private Map<String, Object> processBaseVariable(String baseVariable, PreparedStatement stmt, boolean hasResultSet) throws SQLException {
-        Map<String, Object> variableList = new HashMap<>();
         if (isList) {
             baseVariable = baseVariable.substring(0, baseVariable.length() - 1);
         }
 
         if (hasResultSet) {
             CachedRowSet crs = SkriptDB.getRowSetFactory().createCachedRowSet();
-            crs.populate(stmt.getResultSet());
+            crs.populate(generatedKeys ? stmt.getGeneratedKeys() : stmt.getResultSet());
 
             if (isList) {
                 return fetchQueryResultSet(crs, baseVariable);
             } else {
                 crs.last();
-                variableList.put(baseVariable, crs.getRow());
+                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
@@ -278,7 +261,9 @@ public class EffExecuteStatement extends Effect {
     }
 
     private PreparedStatement createStatement(Connection conn, Pair<String, List<Object>> query) throws SQLException {
-        PreparedStatement stmt = conn.prepareStatement(query.getFirst());
+        PreparedStatement stmt = generatedKeys ? 
+        		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++) {
@@ -306,7 +291,7 @@ public class EffExecuteStatement extends Effect {
 
         //fix mediumblob and similar column types, so they return a String correctly
         if (obj != null) {
-            if (obj.getClass().getName().equals("[B")) {
+            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)
@@ -321,6 +306,14 @@ public class EffExecuteStatement extends Effect {
         Variables.setVariable(name.toLowerCase(Locale.ENGLISH), obj, e, isLocal);
     }
     
+    private static void resetLastSQLError() {
+    	lastError = null;
+    }
+    
+    private static void setLastSQLError(String error) {
+    	lastError = error;
+    }
+
     @Override
     public String toString(Event e, boolean debug) {
         return "execute " + query.toString(e, debug) + " in " + dataSource.toString(e, debug);
@@ -328,8 +321,7 @@ public class EffExecuteStatement extends Effect {
 
     @SuppressWarnings("unchecked")
     @Override
-    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed,
+    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
-                        SkriptParser.ParseResult parseResult) {
         Expression<String> statementExpr = (Expression<String>) exprs[0];
         if (statementExpr instanceof VariableString || statementExpr instanceof ExprUnsafe) {
             query = statementExpr;
@@ -346,17 +338,30 @@ public class EffExecuteStatement extends Effect {
             }
             queryArguments = (Expression<Object>) exprs[2];
         }
+        ;
         Expression<?> resultHolder = exprs[3];
-        quickly = matchedPattern == 1;
+        quickly = parseResult.hasTag("quickly");
         if (resultHolder instanceof Variable) {
             Variable<?> varExpr = (Variable<?>) resultHolder;
-            var = varExpr.getName();
+            resultVariableName = varExpr.getName();
             isLocal = varExpr.isLocal();
             isList = varExpr.isList();
+            generatedKeys = parseResult.hasTag("keys");
         } else if (resultHolder != null) {
             Skript.error(resultHolder + " is not a variable");
             return false;
         }
         return true;
     }
+    
+    public static class SkriptDBQueryException extends RuntimeException {
+
+		private static final long serialVersionUID = -1869895286406538884L;
+		
+		public SkriptDBQueryException(String message) {
+			super(message);
+		}
+    	
+    }
+    
 }

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

@@ -32,8 +32,9 @@ public class ExprDataSource extends SimpleExpression<HikariDataSource> {
 
     static {
         Skript.registerExpression(ExprDataSource.class, HikariDataSource.class,
-                ExpressionType.COMBINED, "[the] data(base|[ ]source) [(of|at)] %string% " +
+                ExpressionType.COMBINED, "[the] data(base|[ ]source) [(of|at)] %string% "
-                        "[with [a] [max[imum]] [connection] life[ ]time of %-timespan%] " + "[[(using|with)] [a] driver %-string%]");
+                		+ "[with [a] [max[imum]] [connection] life[ ]time of %-timespan%] "
+                		+ "[[(using|with)] [a] driver %-string%]");
     }
 
     private Expression<String> url;
@@ -100,8 +101,7 @@ public class ExprDataSource extends SimpleExpression<HikariDataSource> {
 
     @SuppressWarnings("unchecked")
     @Override
-    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed,
+    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
-                        SkriptParser.ParseResult parseResult) {
         url = (Expression<String>) exprs[0];
         maxLifetime = (Expression<Timespan>) exprs[1];
         driver = (Expression<String>) exprs[2];

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

@@ -1,6 +1,10 @@
 package com.btk5h.skriptdb.skript;
 
-import ch.njol.skript.ScriptLoader;
+import org.bukkit.event.Event;
+
+import com.btk5h.skriptdb.SkriptUtil;
+import com.btk5h.skriptdb.events.SQLQueryCompleteEvent;
+
 import ch.njol.skript.Skript;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.ExpressionType;
@@ -8,8 +12,6 @@ import ch.njol.skript.lang.SkriptParser;
 import ch.njol.skript.lang.util.SimpleExpression;
 import ch.njol.skript.log.ErrorQuality;
 import ch.njol.util.Kleenean;
-import com.btk5h.skriptdb.events.SQLQueryCompleteEvent;
-import org.bukkit.event.Event;
 
 /**
  * Stores the error from the last executed statement, if there was one.
@@ -51,7 +53,7 @@ public class ExprSQLQuery extends SimpleExpression<String> {
 
     @Override
     public boolean init(final Expression<?>[] expressions, final int matchedPattern, final Kleenean isDelayed, final SkriptParser.ParseResult parseResult) {
-        if (!ScriptLoader.isCurrentEvent(SQLQueryCompleteEvent.class)) {
+        if (!SkriptUtil.isCurrentEvent(SQLQueryCompleteEvent.class)) {
             Skript.error("Cannot use 'sql query' outside of a complete of sql query event", ErrorQuality.SEMANTIC_ERROR);
             return false;
         }

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

@@ -53,8 +53,7 @@ public class ExprUnsafe extends SimpleExpression<String> {
 
     @SuppressWarnings("unchecked")
     @Override
-    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed,
+    public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
-                        SkriptParser.ParseResult parseResult) {
         stringExpression = (Expression<String>) exprs[0];
         rawExpression = parseResult.expr.substring("unsafe".length()).trim();
         return true;