diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index 8c6879d821..96c97b0ba1 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.config.CanonicalWebUrlModule; import com.google.gerrit.server.config.CanonicalWebUrlProvider; import com.google.gerrit.server.config.GerritGlobalModule; import com.google.gerrit.server.config.MasterNodeStartup; +import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gerrit.sshd.SshModule; import com.google.gerrit.sshd.commands.MasterCommandModule; import com.google.gerrit.sshd.commands.SlaveCommandModule; @@ -185,6 +186,7 @@ public class Daemon extends SiteProgram { private Injector createSysInjector() { final List modules = new ArrayList(); + modules.add(SchemaVersionCheck.module()); modules.add(new LogFileCompressor.Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); if (httpd) { diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ExportReviewNotes.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ExportReviewNotes.java index 94cad95535..7b8c286ae3 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ExportReviewNotes.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ExportReviewNotes.java @@ -28,7 +28,6 @@ import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.GroupCacheImpl; import com.google.gerrit.server.cache.CachePool; import com.google.gerrit.server.config.ApprovalTypesProvider; -import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrlProvider; import com.google.gerrit.server.config.FactoryModule; @@ -36,6 +35,7 @@ import com.google.gerrit.server.git.CodeReviewNoteCreationException; import com.google.gerrit.server.git.CreateCodeReviewNotes; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LocalDiskRepositoryManager; +import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gwtorm.client.OrmException; import com.google.gwtorm.client.SchemaFactory; import com.google.inject.AbstractModule; @@ -91,6 +91,7 @@ public class ExportReviewNotes extends SiteProgram { gitInjector = dbInjector.createChildInjector(new AbstractModule() { @Override protected void configure() { + install(SchemaVersionCheck.module()); bind(ApprovalTypes.class).toProvider(ApprovalTypesProvider.class).in( Scopes.SINGLETON); bind(String.class).annotatedWith(CanonicalWebUrl.class) @@ -99,7 +100,6 @@ public class ExportReviewNotes extends SiteProgram { install(AccountCacheImpl.module()); install(GroupCacheImpl.module()); - install(new AuthConfigModule()); install(new FactoryModule() { @Override protected void configure() { diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ScanTrackingIds.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ScanTrackingIds.java index 45f560a006..8bf5f98f70 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ScanTrackingIds.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/ScanTrackingIds.java @@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gwtorm.client.OrmException; import com.google.gwtorm.client.SchemaFactory; import com.google.inject.Inject; @@ -71,7 +72,9 @@ public class ScanTrackingIds extends SiteProgram { } dbInjector = createDbInjector(MULTI_USER); - manager.add(dbInjector); + manager.add( + dbInjector, + dbInjector.createChildInjector(SchemaVersionCheck.module())); manager.start(); dbInjector.injectMembers(this); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java index a789546a5f..f61a7c6b97 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.config; import com.google.gerrit.common.auth.openid.OpenIdProviderPattern; import com.google.gerrit.reviewdb.AccountExternalId; import com.google.gerrit.reviewdb.AuthType; -import com.google.gerrit.reviewdb.SystemConfig; import com.google.gwtjsonrpc.server.SignedToken; import com.google.gwtjsonrpc.server.XsrfException; import com.google.inject.Inject; @@ -46,7 +45,7 @@ public class AuthConfig { private final boolean allowGoogleAccountUpgrade; @Inject - AuthConfig(@GerritServerConfig final Config cfg, final SystemConfig s) + AuthConfig(@GerritServerConfig final Config cfg) throws XsrfException { authType = toType(cfg); httpHeader = cfg.getString("auth", null, "httpheader"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfigModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfigModule.java index 33ea3fb663..e5d4ba8c31 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfigModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfigModule.java @@ -16,14 +16,12 @@ package com.google.gerrit.server.config; import static com.google.inject.Scopes.SINGLETON; -import com.google.gerrit.reviewdb.SystemConfig; import com.google.inject.AbstractModule; /** Creates {@link AuthConfig} from {@link GerritServerConfig}. */ public class AuthConfigModule extends AbstractModule { @Override protected void configure() { - bind(SystemConfig.class).toProvider(SystemConfigProvider.class).in(SINGLETON); bind(AuthConfig.class).in(SINGLETON); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersionCheck.java similarity index 62% rename from gerrit-server/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java rename to gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersionCheck.java index ba3e0fcb7f..9a59567f7e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SystemConfigProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersionCheck.java @@ -12,37 +12,43 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.gerrit.server.config; +package com.google.gerrit.server.schema; +import com.google.gerrit.lifecycle.LifecycleListener; +import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.reviewdb.CurrentSchemaVersion; import com.google.gerrit.reviewdb.ReviewDb; -import com.google.gerrit.reviewdb.SystemConfig; -import com.google.gerrit.server.schema.Current; -import com.google.gerrit.server.schema.SchemaVersion; import com.google.gwtorm.client.OrmException; import com.google.gwtorm.client.SchemaFactory; import com.google.inject.Inject; +import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.ProvisionException; -import java.util.List; +/** Validates the current schema version. */ +public class SchemaVersionCheck implements LifecycleListener { + public static Module module () { + return new LifecycleModule() { + @Override + protected void configure() { + listener().to(SchemaVersionCheck.class); + } + }; + } -/** Loads the {@link SystemConfig} from the database. */ -public class SystemConfigProvider implements Provider { private final SchemaFactory schema; @Current private final Provider version; @Inject - public SystemConfigProvider(final SchemaFactory schemaFactory, - @Current final Provider version) { + public SchemaVersionCheck(SchemaFactory schemaFactory, + @Current Provider version) { this.schema = schemaFactory; this.version = version; } - @Override - public SystemConfig get() { + public void start() { try { final ReviewDb db = schema.open(); try { @@ -50,33 +56,25 @@ public class SystemConfigProvider implements Provider { final int eVer = version.get().getVersionNbr(); if (sVer == null) { - throw new OrmException("Schema not yet initialized." + throw new ProvisionException("Schema not yet initialized." + " Run init to initialize the schema."); } if (sVer.versionNbr != eVer) { - throw new OrmException("Unsupported schema version " + throw new ProvisionException("Unsupported schema version " + sVer.versionNbr + "; expected schema version " + eVer + ". Run init to upgrade."); } - - final List all = db.systemConfig().all().toList(); - switch (all.size()) { - case 1: - return all.get(0); - case 0: - throw new OrmException("system_config table is empty"); - default: - throw new OrmException("system_config must have exactly 1 row;" - + " found " + all.size() + " rows instead"); - } } finally { db.close(); } } catch (OrmException e) { - throw new ProvisionException("Cannot read system_config", e); + throw new ProvisionException("Cannot read schema_version", e); } } + public void stop() { + } + private CurrentSchemaVersion getSchemaVersion(final ReviewDb db) { try { return db.schemaVersion().get(new CurrentSchemaVersion.Key()); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java index 90cb0d342c..20e0170a94 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java @@ -68,8 +68,6 @@ public class SchemaCreatorTest extends TestCase { // db.create(); db.assertSchemaVersion(); - final SystemConfig config = db.getSystemConfig(); - assertNotNull(config); // By default sitePath is set to the current working directory. // @@ -77,16 +75,7 @@ public class SchemaCreatorTest extends TestCase { if (sitePath.getName().equals(".")) { sitePath = sitePath.getParentFile(); } - assertEquals(sitePath.getAbsolutePath(), config.sitePath); - } - - public void testSubsequentGetReads() throws OrmException { - db.create(); - final SystemConfig exp = db.getSystemConfig(); - final SystemConfig act = db.getSystemConfig(); - - assertNotSame(exp, act); - assertEquals(exp.sitePath, act.sitePath); + assertEquals(sitePath.getAbsolutePath(), db.getSystemConfig().sitePath); } public void testCreateSchema_ApprovalCategory_CodeReview() diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryDatabase.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryDatabase.java index 8326051a97..82dcbea74c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryDatabase.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryDatabase.java @@ -22,7 +22,6 @@ import com.google.gerrit.server.GerritPersonIdentProvider; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePath; -import com.google.gerrit.server.config.SystemConfigProvider; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LocalDiskRepositoryManager; import com.google.gerrit.server.schema.Current; @@ -35,7 +34,6 @@ import com.google.gwtorm.jdbc.SimpleDataSource; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Key; -import com.google.inject.Provider; import junit.framework.TestCase; @@ -177,12 +175,13 @@ public class InMemoryDatabase implements SchemaFactory { } } - public SystemConfig getSystemConfig() { - return new SystemConfigProvider(this, new Provider() { - public SchemaVersion get() { - return schemaVersion; - } - }).get(); + public SystemConfig getSystemConfig() throws OrmException { + final ReviewDb c = open(); + try { + return c.systemConfig().get(new SystemConfig.Key()); + } finally { + c.close(); + } } public CurrentSchemaVersion getSchemaVersion() throws OrmException { diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index f1ac9454a1..0cbbac46a9 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -28,6 +28,7 @@ import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.schema.DataSourceProvider; import com.google.gerrit.server.schema.DatabaseModule; import com.google.gerrit.server.schema.SchemaModule; +import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gerrit.sshd.SshModule; import com.google.gerrit.sshd.commands.MasterCommandModule; import com.google.inject.AbstractModule; @@ -167,6 +168,7 @@ public class WebAppInitializer extends GuiceServletContextListener { modules.add(new GerritServerConfigModule()); } modules.add(new SchemaModule()); + modules.add(SchemaVersionCheck.module()); modules.add(new AuthConfigModule()); return dbInjector.createChildInjector(modules); }