feat: add slf44-api to replace use of standard streams

Closes: #802
xerial · Oct 3, 2023 · 21c77a4 · 21c77a4
1 parent ebdda17
commit 21c77a4
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 29 deletions.
diff --git a/pom.xml b/pom.xml
@@ -407,6 +407,11 @@
     </profiles>
 
     <dependencies>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+            <version>2.0.9</version>
+        </dependency>
         <!--
         This dependency makes compilation on non-GraalVM versions possible.
         The dependency should however never actually be required by end-users because if they require

diff --git a/src/main/java/org/sqlite/JDBC.java b/src/main/java/org/sqlite/JDBC.java
@@ -19,16 +19,18 @@
 import java.sql.*;
 import java.util.Properties;
 import java.util.logging.Logger;
+import org.slf4j.LoggerFactory;
 import org.sqlite.jdbc4.JDBC4Connection;
 
 public class JDBC implements Driver {
+    private static final org.slf4j.Logger logger = LoggerFactory.getLogger(JDBC.class);
     public static final String PREFIX = "jdbc:sqlite:";
 
     static {
         try {
             DriverManager.registerDriver(new JDBC());
         } catch (SQLException e) {
-            e.printStackTrace();
+            logger.atError().setCause(e).log();
         }
     }
 

diff --git a/src/main/java/org/sqlite/SQLiteJDBCLoader.java b/src/main/java/org/sqlite/SQLiteJDBCLoader.java
@@ -39,6 +39,8 @@
 import java.util.Properties;
 import java.util.UUID;
 import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sqlite.util.LibraryLoaderUtil;
 import org.sqlite.util.OSInfo;
 import org.sqlite.util.StringUtils;
@@ -55,6 +57,7 @@
  * @author leo
  */
 public class SQLiteJDBCLoader {
+    private static final Logger logger = LoggerFactory.getLogger(SQLiteJDBCLoader.class);
 
     private static final String LOCK_EXT = ".lck";
     private static boolean extracted = false;
@@ -98,14 +101,14 @@ static void cleanup() {
                                     try {
                                         Files.delete(nativeLib);
                                     } catch (Exception e) {
-                                        System.err.println(
-                                                "Failed to delete old native lib: "
-                                                        + e.getMessage());
+                                        logger.atError()
+                                                .setCause(e)
+                                                .log("Failed to delete old native lib");
                                     }
                                 }
                             });
         } catch (IOException e) {
-            System.err.println("Failed to open directory: " + e.getMessage());
+            logger.atError().setCause(e).log("Failed to open directory");
         }
     }
 
@@ -220,7 +223,7 @@ private static boolean extractAndLoadLibraryFile(
             }
             return loadNativeLibrary(targetFolder, extractedLibFileName);
         } catch (IOException e) {
-            e.printStackTrace();
+            logger.atError().setCause(e).log();
             return false;
         }
     }
@@ -244,7 +247,7 @@ private static InputStream getResourceAsStream(String name) {
             connection.setUseCaches(false);
             return connection.getInputStream();
         } catch (IOException e) {
-            e.printStackTrace();
+            logger.atError().setCause(e).log();
             return null;
         }
     }
@@ -264,12 +267,12 @@ private static boolean loadNativeLibrary(String path, String name) {
                 System.load(new File(path, name).getAbsolutePath());
                 return true;
             } catch (UnsatisfiedLinkError e) {
-                System.err.println(
-                        "Failed to load native library:"
-                                + name
-                                + ". osinfo: "
-                                + OSInfo.getNativeLibFolderPathForCurrentOS());
-                e.printStackTrace();
+                logger.atError()
+                        .setCause(e)
+                        .setMessage("Failed to load native library: {}. osinfo: {}")
+                        .addArgument(name)
+                        .addArgument(OSInfo::getNativeLibFolderPathForCurrentOS)
+                        .log();
                 return false;
             }
 
@@ -283,8 +286,9 @@ private static boolean loadNativeLibraryJdk() {
             System.loadLibrary(LibraryLoaderUtil.NATIVE_LIB_BASE_NAME);
             return true;
         } catch (UnsatisfiedLinkError e) {
-            System.err.println("Failed to load native library through System.loadLibrary");
-            e.printStackTrace();
+            logger.atError()
+                    .setCause(e)
+                    .log("Failed to load native library through System.loadLibrary");
             return false;
         }
     }
@@ -393,6 +397,7 @@ public static String getVersion() {
      * executable, and we're eliminating the IO operations as well.
      */
     public static final class VersionHolder {
+        private static final Logger logger = LoggerFactory.getLogger(VersionHolder.class);
         private static final String VERSION;
 
         static {
@@ -414,7 +419,9 @@ public static final class VersionHolder {
                     version = version.trim().replaceAll("[^0-9\\.]", "");
                 }
             } catch (IOException e) {
-                e.printStackTrace();
+                logger.atError()
+                        .setCause(e)
+                        .log("Could not read version from file: {}", versionFile);
             }
             VERSION = version;
         }

diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java b/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java
@@ -21,13 +21,17 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sqlite.SQLiteConnection;
+import org.sqlite.core.CoreDatabaseMetaData;
 import org.sqlite.core.CoreStatement;
 import org.sqlite.jdbc3.JDBC3DatabaseMetaData.ImportedKeyFinder.ForeignKey;
 import org.sqlite.util.QueryUtils;
 import org.sqlite.util.StringUtils;
 
-public abstract class JDBC3DatabaseMetaData extends org.sqlite.core.CoreDatabaseMetaData {
+public abstract class JDBC3DatabaseMetaData extends CoreDatabaseMetaData {
+    private static final Logger logger = LoggerFactory.getLogger(JDBC3DatabaseMetaData.class);
 
     private static String driverName;
     private static String driverVersion;
@@ -961,14 +965,14 @@ public ResultSet getColumns(String c, String s, String tblNamePattern, String co
                         try {
                             rsColAutoinc.close();
                         } catch (Exception e) {
-                            e.printStackTrace();
+                            logger.atError().setCause(e).log();
                         }
                     }
                     if (statColAutoinc != null) {
                         try {
                             statColAutoinc.close();
                         } catch (Exception e) {
-                            e.printStackTrace();
+                            logger.atError().setCause(e).log();
                         }
                     }
                 }
@@ -1119,7 +1123,7 @@ public ResultSet getColumns(String c, String s, String tblNamePattern, String co
                 try {
                     rs.close();
                 } catch (Exception e) {
-                    e.printStackTrace();
+                    logger.atError().setCause(e).log();
                 }
             }
         }

diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
@@ -8,6 +8,8 @@
 import java.sql.SQLWarning;
 import java.sql.Statement;
 import java.util.Arrays;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sqlite.ExtendedCommand;
 import org.sqlite.ExtendedCommand.SQLExtension;
 import org.sqlite.SQLiteConnection;
@@ -92,8 +94,10 @@ public ResultSet executeQuery(String sql) throws SQLException {
     }
 
     static class BackupObserver implements ProgressObserver {
+        private static final Logger logger = LoggerFactory.getLogger(BackupObserver.class);
+
         public void progress(int remaining, int pageCount) {
-            System.out.printf("remaining:%d, page count:%d%n", remaining, pageCount);
+            logger.atInfo().log("remaining:{}, page count:{}", remaining, pageCount);
         }
     }
 

diff --git a/src/main/java/org/sqlite/util/OSInfo.java b/src/main/java/org/sqlite/util/OSInfo.java
@@ -31,13 +31,16 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Provides OS name and architecture name.
  *
  * @author leo
  */
 public class OSInfo {
+    private static final Logger logger = LoggerFactory.getLogger(OSInfo.class);
     protected static ProcessRunner processRunner = new ProcessRunner();
     private static final HashMap<String, String> archMapping = new HashMap<>();
 
@@ -158,7 +161,7 @@ static String getHardwareName() {
         try {
             return processRunner.runAndWaitFor("uname -m");
         } catch (Throwable e) {
-            System.err.println("Error while running uname -m: " + e.getMessage());
+            logger.atError().setCause(e).log("Error while running uname -m");
             return "unknown";
         }
     }
@@ -219,8 +222,9 @@ static String resolveArmArchType() {
                         return "armv7";
                     }
                 } else {
-                    System.err.println(
-                            "WARNING! readelf not found. Cannot check if running on an armhf system, armel architecture will be presumed.");
+                    logger.atWarn()
+                            .log(
+                                    "readelf not found. Cannot check if running on an armhf system, armel architecture will be presumed");
                 }
             } catch (IOException | InterruptedException e) {
                 // ignored: fall back to "arm" arch (soft-float ABI)

diff --git a/src/test/java/org/sqlite/MultipleClassLoaderTest.java b/src/test/java/org/sqlite/MultipleClassLoaderTest.java
@@ -37,6 +37,8 @@
 import java.nio.file.Paths;
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -86,10 +88,21 @@ public void multipleClassLoader() throws Throwable {
         if (classesDir == null) {
             fail("Couldn't find classes under test.");
         }
+
+        // find the slf4j-api jar
+        Optional<String> slf4jApi =
+                Arrays.stream(stringUrls)
+                        .filter(s -> s.contains("org/slf4j/slf4j-api/"))
+                        .findFirst();
+        if (!slf4jApi.isPresent()) fail("Couldn't find slf4j-api");
+
         // Create a JAR file out the classes and resources
         File jarFile = File.createTempFile("jar-for-test-", ".jar");
         createJar(classesDir, classesDirPrefix, jarFile);
-        URL[] jarUrl = new URL[] {jarFile.toPath().toUri().toURL()};
+        URL[] jarUrl =
+                new URL[] {
+                    jarFile.toPath().toUri().toURL(), Paths.get(slf4jApi.get()).toUri().toURL()
+                };
 
         final AtomicInteger completedThreads = new AtomicInteger(0);
         ExecutorService pool = Executors.newFixedThreadPool(4);

diff --git a/src/test/java/org/sqlite/architecture/CodingRulesTest.java b/src/test/java/org/sqlite/architecture/CodingRulesTest.java
@@ -1,21 +1,27 @@
 package org.sqlite.architecture;
 
+import static com.tngtech.archunit.base.DescribedPredicate.not;
+import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo;
+import static com.tngtech.archunit.lang.conditions.ArchPredicates.are;
 import static com.tngtech.archunit.library.GeneralCodingRules.*;
 
+import com.tngtech.archunit.core.domain.JavaClasses;
 import com.tngtech.archunit.core.importer.ImportOption;
 import com.tngtech.archunit.junit.AnalyzeClasses;
 import com.tngtech.archunit.junit.ArchTest;
 import com.tngtech.archunit.lang.ArchRule;
+import org.sqlite.util.OSInfo;
 
 @AnalyzeClasses(
         packages = "org.sqlite",
         importOptions = {ImportOption.DoNotIncludeTests.class})
 class CodingRulesTest {
 
-    //    Disabled for now, see https://github.com/xerial/sqlite-jdbc/issues/802
-    //    @ArchTest
-    private final ArchRule no_access_to_standard_streams =
-            NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS;
+    @ArchTest
+    void no_access_to_standard_streams(JavaClasses importedClasses) {
+        NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS.check(
+                importedClasses.that(are(not(equivalentTo(OSInfo.class)))));
+    }
 
     //    Disabled for now
     //    @ArchTest