Revert minimizeJar due to bug & fix sync queries in non-main thread

Reviewed-on: #20

README.md

@@ -5,11 +5,18 @@
 
 ### Difference from original skript-db
 - Fixed local variables disappearance in newer Skript versions (very hacky fix, but it works, so that's good!)
-- Thread-pool size is now automatically increasing on demand to use of CachedThreadPool, instead of a fixed hard-coded number
-- Uses a newer version of HikariCP
-- Only meant to be used by newer Minecraft versions (1.8 is not supported)
+- Thread-pool size is now automatically increasing on demand with use of CachedThreadPool, instead of a fixed hard-coded number
+- Uses newer versions of dependencies (Increased performance and security)
 - Replaced `synchronously execute` with `quickly execute`, which allows to speed up queries by 50ms with some risk
+- SQL Driver is configurable
+- A few variable type related bugs fixed
+- Uses Java 11 instead of Java 8
 
+### Installation
+1. Use 1.8+ Minecraft server version.
+2. Use Skript 2.5+ (1.8 Skript fork is needed if you're using 1.8)
+3. Use Java 11+ (If you use 1.8, a spigot fork is needed to support Java 11+)
+4. Put skript-db in plugins folder and restart the server
 ### Expression `Data Source` => `datasource`
 Stores the connection information for a data source. This should be saved to a variable in a
  `script load` event or manually through an effect command.
@@ -78,3 +85,10 @@ execute unsafe {fully dynamic query} in {sql}
 ```
 
 ---
+### Configuration
+plugins/skript-db/config.yml
+```
+# Only change this if you wish to use a different driver than Java's default, like MariaDB driver.
+# If you use MariaDB, its driver is shaded together with skript-db, so you can just specify: "org.mariadb.jdbc.Driver"
+sql-driver-class-name: "default"
+```

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>com.btk5h</groupId>
     <artifactId>skript-db</artifactId>
-    <version>1.3.5</version>
+    <version>1.3.7</version>
     <packaging>jar</packaging>
 
     <repositories>
@@ -19,8 +19,8 @@
             <url>https://jitpack.io</url>
         </repository>
         <repository>
-            <id>commons-pool2</id>
-            <url>https://mvnrepository.com/artifact/org.apache.commons/commons-pool2</url>
+            <id>mvnrepository</id>
+            <url>https://mvnrepository.com</url>
         </repository>
         <repository>
             <id>PaperMC</id>

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

@@ -10,7 +10,6 @@ import com.btk5h.skriptdb.SkriptDB;
 import com.btk5h.skriptdb.SkriptUtil;
 import com.zaxxer.hikari.HikariDataSource;
 import org.bukkit.Bukkit;
-import org.bukkit.entity.Player;
 import org.bukkit.event.Event;
 
 import javax.sql.DataSource;
@@ -91,49 +90,32 @@ public class EffExecuteStatement extends Effect {
         Object locals = Variables.removeLocals(e);
 
         //execute SQL statement
-        CompletableFuture<Object> sql =
-                CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, query), threadPool);
+
+        CompletableFuture<Object> sql = null;
+        Object resources = null;
+        if (!sync) {
+            sql = CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, query), threadPool);
+        } else {
+            resources = executeStatement(ds, baseVariable, query);
+        }
 
         //when SQL statement is completed
         boolean finalSync = sync;
-        sql.whenComplete((res, err) -> {
-            if (err != null) {
-                err.printStackTrace();
-            }
-
-            //handle last error syntax data
-            lastError = null;
-            if (res instanceof String) {
-                lastError = (String) res;
-            }
-
-            if (getNext() != null) {
-                //if local variables are present
-                //bring back local variables
-
-                //populate SQL data into variables
-                if (!(res instanceof String)) {
-
-                    //also set variables in the sql query complete event
-
-                    //TEMPORARILY DISABLED, AS THIS WOULD WORSEN PERFORMANCE OF THE QUERIES AND NOT BE USED BY MOST PEOPLE.
-                    //I may add config option to enable this later?
-
-                    //SQLQueryCompleteEvent event = new SQLQueryCompleteEvent(this.query.getSingle(e));
-                    //((Map<String, Object>) res).forEach((name, value) -> setVariable(event, name, value));
-                    //SkriptDB.getPlugin(SkriptDB.class).getServer().getPluginManager().callEvent(event);
+        if (sql != null) {
+            sql.whenComplete((res, err) -> {
+                if (err != null) {
+                    err.printStackTrace();
                 }
-                if (isSync || finalSync) {
-                    if (locals != null) {
-                        Variables.setLocalVariables(e, locals);
-                    }
-                    if (!(res instanceof String)) {
-                        ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
-                    }
-                    TriggerItem.walk(getNext(), e);
-                    Variables.removeLocals(e);
-                } else {
-                    Bukkit.getScheduler().runTask(SkriptDB.getInstance(), () -> {
+                //handle last error syntax data
+                lastError = null;
+                if (res instanceof String) {
+                    lastError = (String) res;
+                }
+                if (getNext() != null) {
+                    //if local variables are present
+                    //bring back local variables
+                    //populate SQL data into variables
+                    if (isSync || finalSync) {
                         if (locals != null) {
                             Variables.setLocalVariables(e, locals);
                         }
@@ -141,14 +123,45 @@ public class EffExecuteStatement extends Effect {
                             ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
                         }
                         TriggerItem.walk(getNext(), e);
-                        //the line below is required to prevent memory leaks
-                        //no functionality difference notice with it being removed from my test, but the memory gets filled with leaks
-                        //so it must be kept
                         Variables.removeLocals(e);
-                    });
+                    } else {
+                        Bukkit.getScheduler().runTask(SkriptDB.getInstance(), () -> {
+                            if (locals != null) {
+                                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
+                            //no functionality difference notice with it being removed from my test, but the memory gets filled with leaks
+                            //so it must be kept
+                            Variables.removeLocals(e);
+                        });
+                    }
                 }
+            });
+        // sync executed SQL query, same as above, just sync
+        } else {
+            //handle last error syntax data
+            lastError = null;
+            if (resources instanceof String) {
+                lastError = (String) resources;
             }
-        });
+            if (getNext() != null) {
+                //if local variables are present
+                //bring back local variables
+                //populate SQL data into variables
+                if (locals != null) {
+                    Variables.setLocalVariables(e, locals);
+                }
+                if (!(resources instanceof String)) {
+                    ((Map<String, Object>) resources).forEach((name, value) -> setVariable(e, name, value));
+                }
+                TriggerItem.walk(getNext(), e);
+                Variables.removeLocals(e);
+            }
+        }
     }
 
     @Override
@@ -305,15 +318,17 @@ public class EffExecuteStatement extends Effect {
     private void setVariable(Event e, String name, Object obj) {
 
         //fix mediumblob and similar column types, so they return a String correctly
-        if (obj.getClass().getName().equals("[B")) {
-            obj = new String((byte[]) obj);
+        if (obj != null) {
+            if (obj.getClass().getName().equals("[B")) {
+                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) {
-            try {
-                obj = new String(((SerialBlob) obj).getBinaryStream().readAllBytes());
-            } catch (IOException | SerialException ex) {
-                ex.printStackTrace();
+                //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) {
+                try {
+                    obj = new String(((SerialBlob) obj).getBinaryStream().readAllBytes());
+                } catch (IOException | SerialException ex) {
+                    ex.printStackTrace();
+                }
             }
         }
         Variables.setVariable(name.toLowerCase(Locale.ENGLISH), obj, e, isLocal);