Refactor Backend.setupSchema to return schema info (#8753)

projectnessie · Jun 6, 2024 · f60c800 · f60c800
1 parent e4660d1
commit f60c800
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 99 deletions.
diff --git a/...kus-common/src/main/java/org/projectnessie/quarkus/providers/storage/PersistProvider.java b/...kus-common/src/main/java/org/projectnessie/quarkus/providers/storage/PersistProvider.java
@@ -28,6 +28,7 @@
 import jakarta.enterprise.inject.Produces;
 import jakarta.inject.Inject;
 import jakarta.inject.Singleton;
+import java.util.Optional;
 import org.projectnessie.quarkus.config.QuarkusStoreConfig;
 import org.projectnessie.quarkus.config.VersionStoreConfig;
 import org.projectnessie.quarkus.config.VersionStoreConfig.VersionStoreType;
@@ -83,11 +84,7 @@ public Backend produceBackend() {
 
   public void closeBackend(@Disposes Backend backend) throws Exception {
     if (backend != null) {
-      String info = backend.configInfo();
-      if (!info.isEmpty()) {
-        info = " (" + info + ")";
-      }
-      LOGGER.info("Stopping storage for {}{}", versionStoreConfig.getVersionStoreType(), info);
+      LOGGER.info("Stopping storage for {}", versionStoreConfig.getVersionStoreType());
       backend.close();
     }
   }
@@ -123,18 +120,13 @@ public Persist producePersist(MeterRegistry meterRegistry) {
     }
 
     Backend b = backend.get();
-    b.setupSchema();
+    Optional<String> info = b.setupSchema();
 
     LOGGER.info("Creating/opening version store {} ...", versionStoreType);
 
     PersistFactory persistFactory = b.createFactory();
     Persist persist = persistFactory.newPersist(storeConfig);
 
-    String info = b.configInfo();
-    if (!info.isEmpty()) {
-      info = " (" + info + ")";
-    }
-
     CacheSizing cacheSizing =
         CacheSizing.builder()
             .fixedSizeInMB(storeConfig.cacheCapacityMB())
@@ -167,7 +159,11 @@ public Persist producePersist(MeterRegistry meterRegistry) {
       cacheInfo = "without objects cache";
     }
 
-    LOGGER.info("Using {} version store{}, {}", versionStoreType, info, cacheInfo);
+    LOGGER.info(
+        "Using {} version store{}, {}",
+        versionStoreType,
+        info.map(s -> " (" + s + ")").orElse(""),
+        cacheInfo);
 
     return persist;
   }

diff --git a/.../bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTableBackend.java b/.../bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTableBackend.java
@@ -44,6 +44,7 @@
 import java.util.ArrayDeque;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import java.util.Queue;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -123,7 +124,7 @@ public void close() {
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     if (tableAdminClient == null) {
       // If BigTable admin client is not available, check at least that the required tables exist.
       boolean refs = checkTableNoAdmin(tableRefsId);
@@ -134,11 +135,11 @@ public void setupSchema() {
           tableRefs,
           tableObjs);
       LOGGER.info("No Bigtable admin client available, skipping schema setup");
-      return;
+    } else {
+      checkTable(tableRefs, FAMILY_REFS);
+      checkTable(tableObjs, FAMILY_OBJS);
     }
-
-    checkTable(tableRefs, FAMILY_REFS);
-    checkTable(tableObjs, FAMILY_OBJS);
+    return tableAdminClient != null ? Optional.empty() : Optional.of("no admin client");
   }
 
   private boolean checkTableNoAdmin(TableId table) {
@@ -226,9 +227,4 @@ private static Filter repoFilter(List<ByteString> prefixes) {
       return filter;
     }
   }
-
-  @Override
-  public String configInfo() {
-    return this.tableAdminClient != null ? "" : " (no admin client)";
-  }
 }
diff --git a/...ssandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java b/...ssandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java
@@ -85,13 +85,9 @@
 import org.projectnessie.versioned.storage.common.exceptions.UnknownOperationResultException;
 import org.projectnessie.versioned.storage.common.persist.Backend;
 import org.projectnessie.versioned.storage.common.persist.PersistFactory;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public final class CassandraBackend implements Backend {
 
-  private static final Logger LOGGER = LoggerFactory.getLogger(CassandraBackend.class);
-
   private final CassandraBackendConfig config;
   private final boolean closeClient;
 
@@ -377,7 +373,7 @@ public void close() {
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     Metadata metadata = session.getMetadata();
     Optional<KeyspaceMetadata> keyspace = metadata.getKeyspace(config.keyspace());
 
@@ -406,6 +402,13 @@ public void setupSchema() {
         CREATE_TABLE_OBJS,
         Stream.concat(Stream.of(COL_REPO_ID), COLS_OBJS_ALL.stream()).collect(toImmutableSet()),
         List.of(COL_REPO_ID, COL_OBJ_ID));
+    return Optional.of(
+        "keyspace: "
+            + config.keyspace()
+            + " DDL timeout: "
+            + config.ddlTimeout()
+            + " DML timeout: "
+            + config.dmlTimeout());
   }
 
   private void createTableIfNotExists(
@@ -484,16 +487,6 @@ private List<String> checkColumns(TableMetadata table, Set<CqlColumn> expectedCo
     return missing;
   }
 
-  @Override
-  public String configInfo() {
-    return "keyspace: "
-        + config.keyspace()
-        + " DDL timeout: "
-        + config.ddlTimeout()
-        + " DML timeout: "
-        + config.dmlTimeout();
-  }
-
   @Override
   public void eraseRepositories(Set<String> repositoryIds) {
     if (repositoryIds == null || repositoryIds.isEmpty()) {

diff --git a/...rage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Backend.java b/...rage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Backend.java
@@ -16,16 +16,22 @@
 package org.projectnessie.versioned.storage.common.persist;
 
 import jakarta.annotation.Nonnull;
+import java.util.Optional;
 import java.util.Set;
 
 public interface Backend extends AutoCloseable {
 
-  void setupSchema();
+  /**
+   * Set up the schema for the backend and create the necessary tables / collections / column
+   * families.
+   *
+   * @return any optional information about the schema setup, such as the database name; to be
+   *     displayed when the application starts.
+   */
+  Optional<String> setupSchema();
 
   @Nonnull
   PersistFactory createFactory();
 
-  String configInfo();
-
   void eraseRepositories(Set<String> repositoryIds);
 }
diff --git a/.../dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBBackend.java b/.../dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBBackend.java
@@ -21,6 +21,7 @@
 
 import jakarta.annotation.Nonnull;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -79,9 +80,10 @@ public void close() {
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     createIfMissing(tableRefs);
     createIfMissing(tableObjs);
+    return Optional.empty();
   }
 
   private void createIfMissing(String name) {
@@ -137,11 +139,6 @@ private static void verifyKeySchema(TableDescription description) {
             description.tableName(), KEY_NAME));
   }
 
-  @Override
-  public String configInfo() {
-    return "";
-  }
-
   @Override
   public void eraseRepositories(Set<String> repositoryIds) {
     if (repositoryIds == null || repositoryIds.isEmpty()) {

diff --git a/.../inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryBackend.java b/.../inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryBackend.java
@@ -18,6 +18,7 @@
 import jakarta.annotation.Nonnull;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
@@ -48,11 +49,8 @@ public void close() {
   }
 
   @Override
-  public void setupSchema() {}
-
-  @Override
-  public String configInfo() {
-    return "";
+  public Optional<String> setupSchema() {
+    return Optional.empty();
   }
 
   @Override

diff --git a/...oned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcBackend.java b/...oned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcBackend.java
@@ -50,8 +50,8 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.sql.DataSource;
@@ -71,10 +71,6 @@ public final class JdbcBackend implements Backend {
   private final String createTableRefsSql;
   private final String createTableObjsSql;
 
-  private final AtomicBoolean first = new AtomicBoolean(true);
-  private String catalog;
-  private String schema;
-
   @SuppressWarnings("removal")
   public JdbcBackend(
       @Nonnull JdbcBackendConfig config,
@@ -187,23 +183,11 @@ public PersistFactory createFactory() {
   Connection borrowConnection() throws SQLException {
     Connection c = dataSource.getConnection();
     c.setAutoCommit(false);
-    if (first.compareAndSet(true, false)) {
-      catalog = c.getCatalog();
-      schema = c.getSchema();
-      if (catalog == null) {
-        LOGGER.warn(
-            "Could not determine catalog name from JDBC properties: schema checks might fail");
-      }
-      if (schema == null) {
-        LOGGER.warn(
-            "Could not determine schema name from JDBC properties: schema checks might fail");
-      }
-    }
     return c;
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     try (Connection conn = borrowConnection()) {
       Integer nameTypeId = databaseSpecific.columnTypeIds().get(NAME);
       Integer objIdTypeId = databaseSpecific.columnTypeIds().get(OBJ_ID);
@@ -228,6 +212,19 @@ public void setupSchema() {
           Stream.concat(Stream.of(COL_REPO_ID), COLS_OBJS_ALL.keySet().stream())
               .collect(Collectors.toSet()),
           ImmutableMap.of(COL_REPO_ID, nameTypeId, COL_OBJ_ID, objIdTypeId));
+      StringBuilder info = new StringBuilder();
+      String s = conn.getCatalog();
+      if (s != null && !s.isEmpty()) {
+        info.append("catalog: ").append(s);
+      }
+      s = conn.getSchema();
+      if (s != null && !s.isEmpty()) {
+        if (info.length() > 0) {
+          info.append(", ");
+        }
+        info.append("schema: ").append(s);
+      }
+      return Optional.of(info.toString());
     } catch (SQLException e) {
       throw new RuntimeException(e);
     }
@@ -313,23 +310,6 @@ private static String sortedColumnNames(Collection<?> input) {
     return input.stream().map(Object::toString).sorted().collect(Collectors.joining(","));
   }
 
-  @Override
-  public String configInfo() {
-    StringBuilder info = new StringBuilder();
-    String s = catalog;
-    if (s != null && !s.isEmpty()) {
-      info.append("catalog: ").append(s);
-    }
-    s = schema;
-    if (s != null && !s.isEmpty()) {
-      if (info.length() > 0) {
-        info.append(", ");
-      }
-      info.append("schema: ").append(s);
-    }
-    return info.toString();
-  }
-
   @Override
   public void eraseRepositories(Set<String> repositoryIds) {
     if (repositoryIds == null || repositoryIds.isEmpty()) {

diff --git a/...age/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBBackend.java b/...age/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBBackend.java
@@ -25,11 +25,13 @@
 import com.mongodb.client.MongoDatabase;
 import jakarta.annotation.Nonnull;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 import org.bson.Document;
 import org.bson.conversions.Bson;
 import org.projectnessie.versioned.storage.common.persist.Backend;
+import org.projectnessie.versioned.storage.common.persist.PersistFactory;
 
 public class MongoDBBackend implements Backend {
 
@@ -68,7 +70,7 @@ private synchronized void initialize() {
 
   @Override
   @Nonnull
-  public MongoDBPersistFactory createFactory() {
+  public PersistFactory createFactory() {
     initialize();
     return new MongoDBPersistFactory(this);
   }
@@ -81,13 +83,9 @@ public synchronized void close() {
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     initialize();
-  }
-
-  @Override
-  public String configInfo() {
-    return "database name: " + config.databaseName();
+    return Optional.of("database name: " + config.databaseName());
   }
 
   @Override

diff --git a/...age/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBBackend.java b/...age/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBBackend.java
@@ -28,6 +28,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
@@ -144,8 +145,9 @@ c, new ColumnFamilyOptions().optimizeUniversalStyleCompaction()))
   }
 
   @Override
-  public void setupSchema() {
+  public Optional<String> setupSchema() {
     initialize();
+    return Optional.of("database path: " + config.databasePath());
   }
 
   @Nonnull
@@ -155,11 +157,6 @@ public PersistFactory createFactory() {
     return new RocksDBPersistFactory(this);
   }
 
-  @Override
-  public String configInfo() {
-    return "database path: " + config.databasePath();
-  }
-
   RocksDBRepo repo(StoreConfig config) {
     return repositories.computeIfAbsent(config.repositoryId(), r -> new RocksDBRepo());
   }