Create SpannerOptions in Module instead of Configuration Remove unnecessary Configuration class, move functionality to Module. Update tests to function with aforementioned change, no need for faux Configuration any more. Change-Id: I39b52ed74e42eeff0be973279c0a1510848b4354
diff --git a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Configuration.java deleted file mode 100644 index 7f34313..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Configuration.java +++ /dev/null
@@ -1,83 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.spannerrefdb; - -import com.google.auth.oauth2.GoogleCredentials; -import com.google.cloud.spanner.DatabaseId; -import com.google.cloud.spanner.SpannerOptions; -import com.google.common.base.Strings; -import com.google.common.flogger.FluentLogger; -import com.google.gerrit.extensions.annotations.PluginName; -import com.google.gerrit.server.config.PluginConfigFactory; -import com.google.inject.Inject; -import com.google.inject.Singleton; -import java.io.FileInputStream; -import java.io.IOException; -import org.eclipse.jgit.lib.Config; - -@Singleton -class Configuration { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - public static final String DATABASE_KEY = "database"; - public static final String INSTANCE_KEY = "instance"; - public static final String EMULATOR_KEY = "useEmulator"; - public static final String CREDENTIALS_KEY = "credentialsPath"; - public static final String SECTION = "ref-database"; - public static final String SUBSECTION = "spanner"; - private final String spannerInstance; - private final String spannerDatabase; - private SpannerOptions options; - - @Inject - Configuration(PluginConfigFactory configFactory, @PluginName String pluginName) - throws IOException { - Config cfg = configFactory.getGlobalPluginConfig(pluginName); - this.spannerInstance = getString(cfg, SECTION, SUBSECTION, INSTANCE_KEY, "spanner-instance"); - this.spannerDatabase = getString(cfg, SECTION, SUBSECTION, DATABASE_KEY, "global-refdb"); - boolean useEmulator = cfg.getBoolean(SECTION, SUBSECTION, EMULATOR_KEY, false); - if (useEmulator) { - this.options = SpannerOptions.newBuilder().build(); - logger.atInfo().log( - "Using local Spanner emulator for global-refdb; Spanner credentials will not be read."); - } else { - String credentialsPath = getString(cfg, SECTION, SUBSECTION, CREDENTIALS_KEY, null); - GoogleCredentials credentials = - GoogleCredentials.fromStream(new FileInputStream(credentialsPath)); - this.options = SpannerOptions.newBuilder().setCredentials(credentials).build(); - } - } - - final DatabaseId getDatabaseId() { - return DatabaseId.of(options.getProjectId(), spannerInstance, spannerDatabase); - } - - final SpannerOptions getOptions() { - return options; - } - - void setOptions(SpannerOptions options) { - this.options = options; - } - - private String getString( - Config cfg, String section, String subsection, String name, String defaultValue) { - String value = cfg.getString(section, subsection, name); - if (!Strings.isNullOrEmpty(value)) { - return value; - } - return defaultValue; - } -}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Module.java b/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Module.java index df30ab7..c146a9c 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/Module.java
@@ -15,19 +15,35 @@ package com.googlesource.gerrit.plugins.spannerrefdb; import com.gerritforge.gerrit.globalrefdb.GlobalRefDatabase; -import com.google.cloud.spanner.DatabaseAdminClient; +import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.DatabaseId; +import com.google.cloud.spanner.SpannerOptions; +import com.google.common.base.Strings; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.server.config.PluginConfigFactory; import com.google.inject.Provides; import com.google.inject.Scopes; import com.google.inject.Singleton; import com.google.inject.assistedinject.FactoryModuleBuilder; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import org.eclipse.jgit.lib.Config; class Module extends LifecycleModule { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + public static final String DATABASE_KEY = "database"; + public static final String INSTANCE_KEY = "instance"; + public static final String EMULATOR_KEY = "useEmulator"; + public static final String CREDENTIALS_KEY = "credentialsPath"; + public static final String SECTION = "ref-database"; + public static final String SUBSECTION = "spanner"; + @Override protected void configure() { logger.atInfo().log("Configuring Cloud Spanner for global refdb."); @@ -43,13 +59,49 @@ @Provides @Singleton - public DatabaseAdminClient createDatabaseAdminClient(Configuration configuration) { - return configuration.getOptions().getService().getDatabaseAdminClient(); + private Config Configuration(PluginConfigFactory configFactory, @PluginName String pluginName) { + return configFactory.getGlobalPluginConfig(pluginName); } @Provides @Singleton - public DatabaseClient createDatabaseClient(Configuration configuration) { - return configuration.getOptions().getService().getDatabaseClient(configuration.getDatabaseId()); + public SpannerOptions createSpannerOptions(Config cfg) throws FileNotFoundException, IOException { + SpannerOptions options; + boolean useEmulator = cfg.getBoolean(SECTION, SUBSECTION, EMULATOR_KEY, false); + if (useEmulator) { + options = SpannerOptions.newBuilder().build(); + logger.atInfo().log( + "Using local Spanner emulator for global-refdb; Spanner credentials will not be read."); + } else { + String credentialsPath = getString(cfg, SECTION, SUBSECTION, CREDENTIALS_KEY, null); + GoogleCredentials credentials = + GoogleCredentials.fromStream(new FileInputStream(credentialsPath)); + options = SpannerOptions.newBuilder().setCredentials(credentials).build(); + } + return options; + } + + @Provides + @Singleton + public DatabaseClient createDatabaseClient(Config cfg, SpannerOptions options) { + return options.getService().getDatabaseClient(createDatabaseId(cfg, options)); + } + + @Provides + @Singleton + public DatabaseId createDatabaseId(Config cfg, SpannerOptions options) { + String spannerInstance = getString(cfg, SECTION, SUBSECTION, INSTANCE_KEY, "spanner-instance"); + String spannerDatabase = getString(cfg, SECTION, SUBSECTION, DATABASE_KEY, "global-refdb"); + + return DatabaseId.of(options.getProjectId(), spannerInstance, spannerDatabase); + } + + private String getString( + Config cfg, String section, String subsection, String name, String defaultValue) { + String value = cfg.getString(section, subsection, name); + if (!Strings.isNullOrEmpty(value)) { + return value; + } + return defaultValue; } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/SpannerLifeCycleManager.java b/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/SpannerLifeCycleManager.java index 9c80b63..5783857 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/SpannerLifeCycleManager.java +++ b/src/main/java/com/googlesource/gerrit/plugins/spannerrefdb/SpannerLifeCycleManager.java
@@ -17,8 +17,10 @@ import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError; import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.spanner.DatabaseAdminClient; +import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerException; +import com.google.cloud.spanner.SpannerOptions; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.inject.Inject; @@ -30,13 +32,13 @@ class SpannerLifeCycleManager implements LifecycleListener { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final Configuration configuration; private DatabaseAdminClient dbAdminClient; + private DatabaseId dbId; @Inject - SpannerLifeCycleManager(Configuration configuration, DatabaseAdminClient dbAdminClient) { - this.configuration = configuration; - this.dbAdminClient = dbAdminClient; + SpannerLifeCycleManager(SpannerOptions options, DatabaseId dbId) { + this.dbAdminClient = options.getService().getDatabaseAdminClient(); + this.dbId = dbId; } @Override @@ -68,14 +70,12 @@ try { OperationFuture<Void, UpdateDatabaseDdlMetadata> op = dbAdminClient.updateDatabaseDdl( - configuration.getDatabaseId().getInstanceId().getInstance(), - configuration.getDatabaseId().getDatabase(), + dbId.getInstanceId().getInstance(), + dbId.getDatabase(), Arrays.asList(statement), null); op.get(); - logger.atInfo().log( - "Created table in database [%s] with statement %s", - configuration.getDatabaseId(), statement); + logger.atInfo().log("Created table in database [%s] with statement %s", dbId, statement); } catch (Exception e) { Throwable cause = e.getCause(); if (cause instanceof SpannerException) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/spannerrefdb/EmulatedSpannerRefDb.java b/src/test/java/com/googlesource/gerrit/plugins/spannerrefdb/EmulatedSpannerRefDb.java index 7ce34ed..c555044 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/spannerrefdb/EmulatedSpannerRefDb.java +++ b/src/test/java/com/googlesource/gerrit/plugins/spannerrefdb/EmulatedSpannerRefDb.java
@@ -14,18 +14,10 @@ package com.googlesource.gerrit.plugins.spannerrefdb; -import static com.googlesource.gerrit.plugins.spannerrefdb.Configuration.DATABASE_KEY; -import static com.googlesource.gerrit.plugins.spannerrefdb.Configuration.EMULATOR_KEY; -import static com.googlesource.gerrit.plugins.spannerrefdb.Configuration.INSTANCE_KEY; -import static com.googlesource.gerrit.plugins.spannerrefdb.Configuration.SECTION; -import static com.googlesource.gerrit.plugins.spannerrefdb.Configuration.SUBSECTION; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - import com.google.cloud.NoCredentials; import com.google.cloud.spanner.Database; -import com.google.cloud.spanner.DatabaseAdminClient; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.Instance; import com.google.cloud.spanner.InstanceAdminClient; import com.google.cloud.spanner.InstanceConfigId; @@ -33,11 +25,8 @@ import com.google.cloud.spanner.InstanceInfo; import com.google.cloud.spanner.Spanner; import com.google.cloud.spanner.SpannerOptions; -import com.google.gerrit.server.config.PluginConfigFactory; -import java.io.IOException; import java.util.Collections; import java.util.concurrent.ExecutionException; -import org.eclipse.jgit.lib.Config; import org.junit.Ignore; import org.testcontainers.containers.SpannerEmulatorContainer; import org.testcontainers.utility.DockerImageName; @@ -47,17 +36,17 @@ public static final String PROJECT_ID = "test"; public static final String SPANNER_INSTANCE_ID = "spanner-instance"; public static final String SPANNER_DATABASE_ID = "global-refdb"; - private static final String pluginName = "spanner-refdb"; private final SpannerEmulatorContainer container; private final Spanner spanner; + private final SpannerOptions spannerOptions; private final InstanceAdminClient instanceAdminClient; private final Instance spannerInstance; private final Database spannerDatabase; + private final DatabaseId dbId; private final DatabaseClient databaseClient; private final SpannerRefDatabase spannerRefDb; - private final Configuration pluginConfig; public EmulatedSpannerRefDb() throws Exception { container = @@ -69,12 +58,13 @@ "Spanner emulator container started and is listening on %s", container.getEmulatorGrpcEndpoint())); - spanner = getEmulatorOptions().getService(); + spannerOptions = getEmulatorOptions(); + spanner = spannerOptions.getService(); instanceAdminClient = spanner.getInstanceAdminClient(); spannerInstance = createSpannerInstance(); + dbId = DatabaseId.of(spannerOptions.getProjectId(), SPANNER_INSTANCE_ID, SPANNER_DATABASE_ID); spannerDatabase = spannerInstance.createDatabase(SPANNER_DATABASE_ID, Collections.emptyList()).get(); - pluginConfig = createEmulatorConfiguration(); createSchema(); databaseClient = createDatabaseClient(); Lock.LockFactory lockFactory = @@ -88,8 +78,7 @@ } private void createSchema() { - DatabaseAdminClient dbAdminClient = spanner.getDatabaseAdminClient(); - SpannerLifeCycleManager lcm = new SpannerLifeCycleManager(pluginConfig, dbAdminClient); + SpannerLifeCycleManager lcm = new SpannerLifeCycleManager(spannerOptions, dbId); lcm.start(); } @@ -135,23 +124,6 @@ } private DatabaseClient createDatabaseClient() { - return createDatabaseClient(pluginConfig); - } - - private Configuration createEmulatorConfiguration() throws IOException { - Config refDbConfig = new Config(); - refDbConfig.setString(SECTION, SUBSECTION, INSTANCE_KEY, SPANNER_INSTANCE_ID); - refDbConfig.setString(SECTION, SUBSECTION, DATABASE_KEY, SPANNER_DATABASE_ID); - refDbConfig.setBoolean(SECTION, SUBSECTION, EMULATOR_KEY, true); - - PluginConfigFactory cfgFactory = mock(PluginConfigFactory.class); - when(cfgFactory.getGlobalPluginConfig(pluginName)).thenReturn(refDbConfig); - Configuration spannerConfig = new Configuration(cfgFactory, pluginName); - spannerConfig.setOptions(getEmulatorOptions()); - return spannerConfig; - } - - private DatabaseClient createDatabaseClient(Configuration configuration) { - return configuration.getOptions().getService().getDatabaseClient(configuration.getDatabaseId()); + return spannerOptions.getService().getDatabaseClient(dbId); } }