SNOW-1895884: Token cache refactor (#2044)

Author: sfc-gh-dheyman

src/main/java/net/snowflake/client/config/SFClientConfigParser.java

@@ -58,11 +58,13 @@ public static SFClientConfig loadSFClientConfig(String configFilePath) throws IO
         derivedConfigFilePath = driverLocation;
       } else {
         // 4. Read SF_CLIENT_CONFIG_FILE_NAME if it is present in user home directory.
-        String userHomeFilePath =
-            Paths.get(systemGetProperty("user.home"), SF_CLIENT_CONFIG_FILE_NAME).toString();
-        if (Files.exists(Paths.get(userHomeFilePath))) {
-          logger.info("Using config file specified from home directory: {}", userHomeFilePath);
-          derivedConfigFilePath = userHomeFilePath;
+        String homeDirectory = systemGetProperty("user.home");
+        if (homeDirectory != null) {
+          String userHomeFilePath = Paths.get(homeDirectory, SF_CLIENT_CONFIG_FILE_NAME).toString();
+          if (Files.exists(Paths.get(userHomeFilePath))) {
+            logger.info("Using config file specified from home directory: {}", userHomeFilePath);
+            derivedConfigFilePath = userHomeFilePath;
+          }
         }
       }
     }

src/main/java/net/snowflake/client/core/FileCacheManager.java

@@ -4,6 +4,7 @@
 
 package net.snowflake.client.core;
 
+import static net.snowflake.client.core.FileUtil.isWritable;
 import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;
 import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetEnv;
 import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;
@@ -27,6 +28,7 @@
 import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Date;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import net.snowflake.client.log.SFLogger;
@@ -43,7 +45,6 @@ class FileCacheManager {
   private String cacheDirectorySystemProperty;
   private String cacheDirectoryEnvironmentVariable;
   private String baseCacheFileName;
-  private long cacheExpirationInMilliseconds;
   private long cacheFileLockExpirationInMilliseconds;
 
   private File cacheFile;
@@ -74,12 +75,6 @@ FileCacheManager setBaseCacheFileName(String baseCacheFileName) {
     return this;
   }
 
-  FileCacheManager setCacheExpirationInSeconds(long cacheExpirationInSeconds) {
-    // converting from seconds to milliseconds
-    this.cacheExpirationInMilliseconds = cacheExpirationInSeconds * 1000;
-    return this;
-  }
-
   FileCacheManager setCacheFileLockExpirationInSeconds(long cacheFileLockExpirationInSeconds) {
     this.cacheFileLockExpirationInMilliseconds = cacheFileLockExpirationInSeconds * 1000;
     return this;
@@ -90,17 +85,22 @@ FileCacheManager setOnlyOwnerPermissions(boolean onlyOwnerPermissions) {
     return this;
   }
 
+  synchronized String getCacheFilePath() {
+    return cacheFile.getAbsolutePath();
+  }
+
   /**
    * Override the cache file.
    *
    * @param newCacheFile a file object to override the default one.
    */
-  void overrideCacheFile(File newCacheFile) {
+  synchronized void overrideCacheFile(File newCacheFile) {
     if (!newCacheFile.exists()) {
       logger.debug("Cache file doesn't exists. File: {}", newCacheFile);
     }
     if (onlyOwnerPermissions) {
-      FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(
+      FileUtil.throwWhenFilePermissionsWiderThanUserOnly(newCacheFile, "Override cache file");
+      FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(
           newCacheFile, "Override cache file");
     } else {
       FileUtil.logFileUsage(cacheFile, "Override cache file", false);
@@ -110,7 +110,7 @@ void overrideCacheFile(File newCacheFile) {
     this.baseCacheFileName = newCacheFile.getName();
   }
 
-  FileCacheManager build() {
+  synchronized FileCacheManager build() {
     // try to get cacheDir from system property or environment variable
     String cacheDirPath =
         this.cacheDirectorySystemProperty != null
@@ -134,32 +134,30 @@ FileCacheManager build() {
     if (cacheDirPath != null) {
       this.cacheDir = new File(cacheDirPath);
     } else {
-      // use user home directory to store the cache file
-      String homeDir = systemGetProperty("user.home");
-      if (homeDir != null) {
-        // Checking if home directory is writable.
-        File homeFile = new File(homeDir);
-        if (!homeFile.canWrite()) {
-          logger.debug("Home directory not writeable, skip using cache", false);
-          homeDir = null;
-        }
-      }
-      if (homeDir == null) {
-        // if still home directory is null, no cache dir is set.
+      this.cacheDir = getDefaultCacheDir();
+    }
+    if (cacheDir == null) {
+      return this;
+    }
+    if (!cacheDir.exists()) {
+      try {
+        Files.createDirectories(
+            cacheDir.toPath(),
+            PosixFilePermissions.asFileAttribute(
+                Stream.of(
+                        PosixFilePermission.OWNER_READ,
+                        PosixFilePermission.OWNER_WRITE,
+                        PosixFilePermission.OWNER_EXECUTE)
+                    .collect(Collectors.toSet())));
+      } catch (IOException e) {
+        logger.info(
+            "Failed to create the cache directory: {}. Ignored. {}",
+            e.getMessage(),
+            cacheDir.getAbsoluteFile());
         return this;
       }
-      if (Constants.getOS() == Constants.OS.WINDOWS) {
-        this.cacheDir =
-            new File(
-                new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches");
-      } else if (Constants.getOS() == Constants.OS.MAC) {
-        this.cacheDir = new File(new File(new File(homeDir, "Library"), "Caches"), "Snowflake");
-      } else {
-        this.cacheDir = new File(new File(homeDir, ".cache"), "snowflake");
-      }
     }
-
-    if (!this.cacheDir.mkdirs() && !this.cacheDir.exists()) {
+    if (!this.cacheDir.exists()) {
       logger.debug(
           "Cannot create the cache directory {}. Giving up.", this.cacheDir.getAbsolutePath());
       return this;
@@ -199,12 +197,72 @@ FileCacheManager build() {
     return this;
   }
 
-  /** Reads the cache file. */
-  JsonNode readCacheFile() {
-    if (cacheFile == null || !this.checkCacheLockFile()) {
-      // no cache or the cache is not valid.
+  static File getDefaultCacheDir() {
+    if (Constants.getOS() == Constants.OS.LINUX) {
+      String xdgCacheHome = getXdgCacheHome();
+      if (xdgCacheHome != null) {
+        return new File(xdgCacheHome, "snowflake");
+      }
+    }
+
+    String homeDir = getHomeDirProperty();
+    if (homeDir == null) {
+      // if still home directory is null, no cache dir is set.
+      return null;
+    }
+    if (Constants.getOS() == Constants.OS.WINDOWS) {
+      return new File(
+          new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches");
+    } else if (Constants.getOS() == Constants.OS.MAC) {
+      return new File(new File(new File(homeDir, "Library"), "Caches"), "Snowflake");
+    } else {
+      return new File(new File(homeDir, ".cache"), "snowflake");
+    }
+  }
+
+  private static String getXdgCacheHome() {
+    String xdgCacheHome = systemGetEnv("XDG_CACHE_HOME");
+    if (xdgCacheHome != null && isWritable(xdgCacheHome)) {
+      return xdgCacheHome;
+    }
+    return null;
+  }
+
+  private static String getHomeDirProperty() {
+    String homeDir = systemGetProperty("user.home");
+    if (homeDir != null && isWritable(homeDir)) {
+      return homeDir;
+    }
+    return null;
+  }
+
+  synchronized <T> T withLock(Supplier<T> supplier) {
+    if (cacheFile == null) {
+      logger.error("No cache file assigned", false);
+      return null;
+    }
+    if (cacheLockFile == null) {
+      logger.error("No cache lock file assigned", false);
       return null;
+    } else if (cacheLockFile.exists()) {
+      deleteCacheLockIfExpired();
     }
+
+    if (!tryToLockCacheFile()) {
+      logger.debug("Failed to lock the file. Skipping cache operation", false);
+      return null;
+    }
+    try {
+      return supplier.get();
+    } finally {
+      if (!unlockCacheFile()) {
+        logger.debug("Failed to unlock cache file", false);
+      }
+    }
+  }
+
+  /** Reads the cache file. */
+  synchronized JsonNode readCacheFile() {
     try {
       if (!cacheFile.exists()) {
         logger.debug("Cache file doesn't exists. File: {}", cacheFile);
@@ -215,7 +273,8 @@ JsonNode readCacheFile() {
           new InputStreamReader(new FileInputStream(cacheFile), DEFAULT_FILE_ENCODING)) {
 
         if (onlyOwnerPermissions) {
-          FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Read cache");
+          FileUtil.throwWhenFilePermissionsWiderThanUserOnly(cacheFile, "Read cache");
+          FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Read cache");
           FileUtil.throwWhenOwnerDifferentThanCurrentUser(cacheFile, "Read cache");
         } else {
           FileUtil.logFileUsage(cacheFile, "Read cache", false);
@@ -228,38 +287,29 @@ JsonNode readCacheFile() {
     return null;
   }
 
-  void writeCacheFile(JsonNode input) {
+  synchronized void writeCacheFile(JsonNode input) {
     logger.debug("Writing cache file. File: {}", cacheFile);
-    if (cacheFile == null || !tryLockCacheFile()) {
-      // no cache file or it failed to lock file
-      logger.debug(
-          "No cache file exists or failed to lock the file. Skipping writing the cache", false);
-      return;
-    }
-    // NOTE: must unlock cache file
     try {
       if (input == null) {
         return;
       }
       try (Writer writer =
           new OutputStreamWriter(new FileOutputStream(cacheFile), DEFAULT_FILE_ENCODING)) {
         if (onlyOwnerPermissions) {
-          FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Write to cache");
+          FileUtil.throwWhenFilePermissionsWiderThanUserOnly(cacheFile, "Write to cache");
+          FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(
+              cacheFile, "Write to cache");
         } else {
           FileUtil.logFileUsage(cacheFile, "Write to cache", false);
         }
         writer.write(input.toString());
       }
     } catch (IOException ex) {
       logger.debug("Failed to write the cache file. File: {}", cacheFile);
-    } finally {
-      if (!unlockCacheFile()) {
-        logger.debug("Failed to unlock cache file", false);
-      }
     }
   }
 
-  void deleteCacheFile() {
+  synchronized void deleteCacheFile() {
     logger.debug("Deleting cache file. File: {}, lock file: {}", cacheFile, cacheLockFile);
 
     if (cacheFile == null) {
@@ -277,76 +327,52 @@ void deleteCacheFile() {
    *
    * @return true if success or false
    */
-  private boolean tryLockCacheFile() {
+  private synchronized boolean tryToLockCacheFile() {
     int cnt = 0;
     boolean locked = false;
-    while (cnt < 100 && !(locked = lockCacheFile())) {
+    while (cnt < 5 && !(locked = lockCacheFile())) {
       try {
-        Thread.sleep(100);
+        Thread.sleep(10);
       } catch (InterruptedException ex) {
         // doesn't matter
       }
       ++cnt;
     }
     if (!locked) {
-      logger.debug("Failed to lock the cache file.", false);
+      deleteCacheLockIfExpired();
+      if (!lockCacheFile()) {
+        logger.debug("Failed to lock the cache file.", false);
+      }
     }
     return locked;
   }
 
-  /**
-   * Lock cache file by creating a lock directory
-   *
-   * @return true if success or false
-   */
-  private boolean lockCacheFile() {
-    return cacheLockFile.mkdirs();
-  }
-
-  /**
-   * Unlock cache file by deleting a lock directory
-   *
-   * @return true if success or false
-   */
-  private boolean unlockCacheFile() {
-    return cacheLockFile.delete();
-  }
-
-  private boolean checkCacheLockFile() {
+  private synchronized void deleteCacheLockIfExpired() {
     long currentTime = new Date().getTime();
-    long cacheFileTs = fileCreationTime(cacheFile);
-
-    if (!cacheLockFile.exists()
-        && cacheFileTs > 0
-        && currentTime - this.cacheExpirationInMilliseconds <= cacheFileTs) {
-      logger.debug("No cache file lock directory exists and cache file is up to date.", false);
-      return true;
-    }
-
     long lockFileTs = fileCreationTime(cacheLockFile);
     if (lockFileTs < 0) {
-      // failed to get the timestamp of lock directory
-      return false;
-    }
-    if (lockFileTs < currentTime - this.cacheFileLockExpirationInMilliseconds) {
+      logger.debug("Failed to get the timestamp of lock directory");
+    } else if (lockFileTs < currentTime - this.cacheFileLockExpirationInMilliseconds) {
       // old lock file
-      if (!cacheLockFile.delete()) {
-        logger.debug("Failed to delete the directory. Dir: {}", cacheLockFile);
-        return false;
+      try {
+        if (!cacheLockFile.delete()) {
+          logger.debug("Failed to delete the directory. Dir: {}", cacheLockFile);
+        } else {
+          logger.debug("Deleted expired cache lock directory.", false);
+        }
+      } catch (Exception e) {
+        logger.debug(
+            "Failed to delete the directory. Dir: {}, Error: {}", cacheLockFile, e.getMessage());
       }
-      logger.debug("Deleted the cache lock directory, because it was old.", false);
-      return currentTime - this.cacheExpirationInMilliseconds <= cacheFileTs;
     }
-    logger.debug("Failed to lock the file. Ignored.", false);
-    return false;
   }
 
   /**
    * Gets file/dir creation time in epoch (ms)
    *
    * @return epoch time in ms
    */
-  private static long fileCreationTime(File targetFile) {
+  private static synchronized long fileCreationTime(File targetFile) {
     if (!targetFile.exists()) {
       logger.debug("File not exists. File: {}", targetFile);
       return -1;
@@ -361,7 +387,21 @@ private static long fileCreationTime(File targetFile) {
     return -1;
   }
 
-  String getCacheFilePath() {
-    return cacheFile.getAbsolutePath();
+  /**
+   * Lock cache file by creating a lock directory
+   *
+   * @return true if success or false
+   */
+  private synchronized boolean lockCacheFile() {
+    return cacheLockFile.mkdirs();
+  }
+
+  /**
+   * Unlock cache file by deleting a lock directory
+   *
+   * @return true if success or false
+   */
+  private synchronized boolean unlockCacheFile() {
+    return cacheLockFile.delete();
   }
 }