Merge "Fix font for gr-button 'links'"
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index 728d630..815cda8 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -137,14 +137,23 @@
To continue with the full migration after running the trial migration, use
either the online or offline migration steps as normal. To revert to
ReviewDb-only, remove `noteDb.changes.read` and `noteDb.changes.write` from
-`gerrit.config` and restart Gerrit.
+`notedb.config` and restart Gerrit.
== Configuration
-The migration process works by setting a configuration option in `gerrit.config`
+The migration process works by setting a configuration option in `notedb.config`
for each step in the process, then performing the corresponding data migration.
-In general, users should not set these options manually; this section serves
-primarily as a reference.
+
+Config options are read from `notedb.config` first, falling back to
+`gerrit.config`. If editing config manually, you may edit either file, but the
+migration process itself only touches `notedb.config`. This means if your
+`gerrit.config` is managed with Puppet or a similar tool, it can overwrite
+`gerrit.config` without affecting the migration process. You should not manage
+`notedb.config` with Puppet, but you may copy values back into `gerrit.config`
+and delete `notedb.config` at some later point after completing the migration.
+
+In general, users should not set the options described below manually; this
+section serves primarily as a reference.
- `noteDb.changes.write=true`: During a ReviewDb write, the state of the change
in NoteDb is written to the `note_db_state` field in the `Change` entity.
diff --git a/gerrit-acceptance-framework/BUILD b/gerrit-acceptance-framework/BUILD
index 5a7f3b7..8a95346 100644
--- a/gerrit-acceptance-framework/BUILD
+++ b/gerrit-acceptance-framework/BUILD
@@ -1,6 +1,12 @@
load("//tools/bzl:java.bzl", "java_library2")
+load("//tools/bzl:junit.bzl", "junit_tests")
-SRCS = glob(["src/test/java/com/google/gerrit/acceptance/*.java"])
+TEST_SRCS = ["src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java"]
+
+SRCS = glob(
+ ["src/test/java/com/google/gerrit/acceptance/*.java"],
+ exclude = TEST_SRCS,
+)
PROVIDED = [
"//gerrit-common:annotations",
@@ -77,3 +83,14 @@
title = "Gerrit Acceptance Test Framework Documentation",
visibility = ["//visibility:public"],
)
+
+junit_tests(
+ name = "acceptance_framework_tests",
+ srcs = TEST_SRCS,
+ deps = [
+ ":lib",
+ "//lib:guava",
+ "//lib:truth",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ ],
+)
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfig.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfig.java
index c651d48..bd4f653 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfig.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfig.java
@@ -40,7 +40,7 @@
}
for (String section : s.getSections()) {
for (String subsection : s.getSubsections(section)) {
- for (String name : s.getNames(section, subsection)) {
+ for (String name : s.getNames(section, subsection, true)) {
setStringList(
section,
subsection,
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java
new file mode 100644
index 0000000..49c23e3
--- /dev/null
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java
@@ -0,0 +1,118 @@
+// Copyright (C) 2017 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.google.gerrit.acceptance;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import java.io.File;
+import java.nio.file.Files;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.util.FS;
+import org.junit.Test;
+
+public class MergeableFileBasedConfigTest {
+ @Test
+ public void mergeNull() throws Exception {
+ MergeableFileBasedConfig cfg = newConfig();
+ cfg.setString("foo", null, "bar", "value");
+ String expected = "[foo]\n\tbar = value\n";
+ assertConfig(cfg, expected);
+ cfg.merge(null);
+ assertConfig(cfg, expected);
+ }
+
+ @Test
+ public void mergeFlatConfig() throws Exception {
+ MergeableFileBasedConfig cfg = newConfig();
+ cfg.setString("foo", null, "bar1", "value1");
+ cfg.setString("foo", null, "bar2", "value2");
+ cfg.setString("foo", "sub", "bar1", "value1");
+ cfg.setString("foo", "sub", "bar2", "value2");
+
+ assertConfig(
+ cfg,
+ "[foo]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = value2\n"
+ + "[foo \"sub\"]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = value2\n");
+
+ Config toMerge = new Config();
+ toMerge.setStringList("foo", null, "bar2", ImmutableList.of("merge1", "merge2"));
+ toMerge.setStringList("foo", "sub", "bar2", ImmutableList.of("merge1", "merge2"));
+ cfg.merge(toMerge);
+
+ assertConfig(
+ cfg,
+ "[foo]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = merge1\n"
+ + "\tbar2 = merge2\n"
+ + "[foo \"sub\"]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = merge1\n"
+ + "\tbar2 = merge2\n");
+ }
+
+ @Test
+ public void mergeStackedConfig() throws Exception {
+ MergeableFileBasedConfig cfg = newConfig();
+ cfg.setString("foo", null, "bar1", "value1");
+ cfg.setString("foo", null, "bar2", "value2");
+ cfg.setString("foo", "sub", "bar1", "value1");
+ cfg.setString("foo", "sub", "bar2", "value2");
+
+ assertConfig(
+ cfg,
+ "[foo]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = value2\n"
+ + "[foo \"sub\"]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = value2\n");
+
+ Config base = new Config();
+ Config toMerge = new Config(base);
+ base.setStringList("foo", null, "bar2", ImmutableList.of("merge1", "merge2"));
+ base.setStringList("foo", "sub", "bar2", ImmutableList.of("merge1", "merge2"));
+ cfg.merge(toMerge);
+
+ assertConfig(
+ cfg,
+ "[foo]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = merge1\n"
+ + "\tbar2 = merge2\n"
+ + "[foo \"sub\"]\n"
+ + "\tbar1 = value1\n"
+ + "\tbar2 = merge1\n"
+ + "\tbar2 = merge2\n");
+ }
+
+ private MergeableFileBasedConfig newConfig() throws Exception {
+ File f = File.createTempFile(getClass().getSimpleName(), ".config");
+ f.deleteOnExit();
+ return new MergeableFileBasedConfig(f, FS.detect());
+ }
+
+ private void assertConfig(MergeableFileBasedConfig cfg, String expected) throws Exception {
+ assertThat(cfg.toText()).isEqualTo(expected);
+ cfg.save();
+ assertThat(new String(Files.readAllBytes(cfg.getFile().toPath()), UTF_8)).isEqualTo(expected);
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java
index 0517675..8c50b90 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java
@@ -61,6 +61,7 @@
@NoHttpd
public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest {
private StoredConfig gerritConfig;
+ private StoredConfig noteDbConfig;
private Project.NameKey project;
private Change.Id changeId;
@@ -69,10 +70,14 @@
public void setUp() throws Exception {
assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF);
gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
+ // Unlike in the running server, for tests, we don't stack notedb.config on gerrit.config.
+ noteDbConfig = new FileBasedConfig(sitePaths.notedb_config.toFile(), FS.detect());
}
@Test
public void rebuildOneChangeTrialMode() throws Exception {
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertNoAutoMigrateConfig(noteDbConfig);
assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
setUpOneChange();
@@ -101,6 +106,8 @@
@Test
public void migrateOneChange() throws Exception {
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertNoAutoMigrateConfig(noteDbConfig);
assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
setUpOneChange();
@@ -128,6 +135,8 @@
assertThat(db.changes().get(id2)).isNull();
}
}
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertAutoMigrateConfig(noteDbConfig, false);
}
@Test
@@ -151,6 +160,40 @@
@Test
public void onlineMigrationViaDaemon() throws Exception {
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertNoAutoMigrateConfig(noteDbConfig);
+
+ testOnlineMigration(u -> startServer(u.module(), "--migrate-to-note-db", "true"));
+
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertAutoMigrateConfig(noteDbConfig, false);
+ }
+
+ @Test
+ public void onlineMigrationViaConfig() throws Exception {
+ assertNoAutoMigrateConfig(gerritConfig);
+ assertNoAutoMigrateConfig(noteDbConfig);
+
+ testOnlineMigration(
+ u -> {
+ gerritConfig.setBoolean("noteDb", "changes", "autoMigrate", true);
+ gerritConfig.save();
+ return startServer(u.module());
+ });
+
+ // Auto-migration is turned off in notedb.config, which takes precedence, but is still on in
+ // gerrit.config. This means Puppet can continue overwriting gerrit.config without turning
+ // auto-migration back on.
+ assertAutoMigrateConfig(gerritConfig, true);
+ assertAutoMigrateConfig(noteDbConfig, false);
+ }
+
+ @FunctionalInterface
+ private interface StartServerWithMigration {
+ ServerContext start(IndexUpgradeController u) throws Exception;
+ }
+
+ private void testOnlineMigration(StartServerWithMigration start) throws Exception {
assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
int prevVersion = ChangeSchemaDefinitions.INSTANCE.getPrevious().getVersion();
int currVersion = ChangeSchemaDefinitions.INSTANCE.getLatest().getVersion();
@@ -166,7 +209,7 @@
setOnlineUpgradeConfig(true);
IndexUpgradeController u = new IndexUpgradeController(1);
- try (ServerContext ctx = startServer(u.module(), "--migrate-to-note-db", "true")) {
+ try (ServerContext ctx = start.start(u)) {
ChangeIndexCollection indexes = ctx.getInjector().getInstance(ChangeIndexCollection.class);
assertThat(indexes.getSearchIndex().getSchema().getVersion()).isEqualTo(prevVersion);
@@ -199,8 +242,8 @@
}
private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
- gerritConfig.load();
- assertThat(NotesMigrationState.forConfig(gerritConfig)).hasValue(expected);
+ noteDbConfig.load();
+ assertThat(NotesMigrationState.forConfig(noteDbConfig)).hasValue(expected);
}
private ReviewDb openUnderlyingReviewDb(ServerContext ctx) throws Exception {
@@ -209,6 +252,17 @@
.open();
}
+ private static void assertNoAutoMigrateConfig(StoredConfig cfg) throws Exception {
+ cfg.load();
+ assertThat(cfg.getString("noteDb", "changes", "autoMigrate")).isNull();
+ }
+
+ private static void assertAutoMigrateConfig(StoredConfig cfg, boolean expected) throws Exception {
+ cfg.load();
+ assertThat(cfg.getString("noteDb", "changes", "autoMigrate")).isNotNull();
+ assertThat(cfg.getBoolean("noteDb", "changes", "autoMigrate", false)).isEqualTo(expected);
+ }
+
private void setOnlineUpgradeConfig(boolean enable) throws Exception {
gerritConfig.load();
gerritConfig.setBoolean("index", null, "onlineUpgrade", enable);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index f08a16e..8aa2497 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -32,6 +32,7 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -65,6 +66,7 @@
import org.junit.Test;
@Sandboxed
+@UseLocalDisk
@NoHttpd
public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
private static final String INVALID_STATE = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
@@ -85,12 +87,13 @@
@Inject private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
@Inject private Sequences sequences;
- private FileBasedConfig gerritConfig;
+ private FileBasedConfig noteDbConfig;
@Before
public void setUp() throws Exception {
assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF);
- gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
+ // Unlike in the running server, for tests, we don't stack notedb.config on gerrit.config.
+ noteDbConfig = new FileBasedConfig(sitePaths.notedb_config.toFile(), FS.detect());
assertNotesMigrationState(REVIEW_DB);
}
@@ -365,27 +368,27 @@
migrate(b -> b.setStopAtStateForTesting(WRITE));
assertNotesMigrationState(WRITE);
- assertThat(NoteDbMigrator.getAutoMigrate(gerritConfig)).isFalse();
+ assertThat(NoteDbMigrator.getAutoMigrate(noteDbConfig)).isFalse();
migrate(b -> b.setAutoMigrate(true).setStopAtStateForTesting(READ_WRITE_NO_SEQUENCE));
assertNotesMigrationState(READ_WRITE_NO_SEQUENCE);
- assertThat(NoteDbMigrator.getAutoMigrate(gerritConfig)).isTrue();
+ assertThat(NoteDbMigrator.getAutoMigrate(noteDbConfig)).isTrue();
migrate(b -> b);
assertNotesMigrationState(NOTE_DB);
- assertThat(NoteDbMigrator.getAutoMigrate(gerritConfig)).isFalse();
+ assertThat(NoteDbMigrator.getAutoMigrate(noteDbConfig)).isFalse();
}
private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected);
- gerritConfig.load();
- assertThat(NotesMigrationState.forConfig(gerritConfig)).hasValue(expected);
+ noteDbConfig.load();
+ assertThat(NotesMigrationState.forConfig(noteDbConfig)).hasValue(expected);
}
private void setNotesMigrationState(NotesMigrationState state) throws Exception {
- gerritConfig.load();
- state.setConfigValues(gerritConfig);
- gerritConfig.save();
+ noteDbConfig.load();
+ state.setConfigValues(noteDbConfig);
+ noteDbConfig.save();
notesMigration.setFrom(state);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritConfig.java
index fdc3b7f..0715f8e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritConfig.java
@@ -18,10 +18,12 @@
import org.eclipse.jgit.lib.Config;
class GerritConfig extends Config {
+ private final Config gerritConfig;
private final SecureStore secureStore;
- GerritConfig(Config baseConfig, SecureStore secureStore) {
- super(baseConfig);
+ GerritConfig(Config noteDbConfigOverGerritConfig, Config gerritConfig, SecureStore secureStore) {
+ super(noteDbConfigOverGerritConfig);
+ this.gerritConfig = gerritConfig;
this.secureStore = secureStore;
}
@@ -42,4 +44,11 @@
}
return super.getStringList(section, subsection, name);
}
+
+ @Override
+ public String toText() {
+ // Only show the contents of gerrit.config, hiding the implementation detail that some values
+ // may come from secure.config (or another secure store) and notedb.config.
+ return gerritConfig.toText();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigProvider.java
index 494b63a..1a59c52 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigProvider.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigProvider.java
@@ -14,11 +14,18 @@
package com.google.gerrit.server.config;
+import static java.util.stream.Collectors.joining;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.securestore.SecureStore;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -41,19 +48,46 @@
@Override
public Config get() {
- FileBasedConfig cfg = new FileBasedConfig(site.gerrit_config.toFile(), FS.DETECTED);
-
- if (!cfg.getFile().exists()) {
+ FileBasedConfig baseConfig = loadConfig(null, site.gerrit_config);
+ if (!baseConfig.getFile().exists()) {
log.info("No " + site.gerrit_config.toAbsolutePath() + "; assuming defaults");
- return new GerritConfig(cfg, secureStore);
}
+ FileBasedConfig noteDbConfigOverBaseConfig = loadConfig(baseConfig, site.notedb_config);
+ checkNoteDbConfig(noteDbConfigOverBaseConfig);
+
+ return new GerritConfig(noteDbConfigOverBaseConfig, baseConfig, secureStore);
+ }
+
+ private static FileBasedConfig loadConfig(@Nullable Config base, Path path) {
+ FileBasedConfig cfg = new FileBasedConfig(base, path.toFile(), FS.DETECTED);
try {
cfg.load();
} catch (IOException | ConfigInvalidException e) {
throw new ProvisionException(e.getMessage(), e);
}
+ return cfg;
+ }
- return new GerritConfig(cfg, secureStore);
+ private static void checkNoteDbConfig(FileBasedConfig noteDbConfig) {
+ List<String> bad = new ArrayList<>();
+ for (String section : noteDbConfig.getSections()) {
+ if (section.equals(NotesMigration.SECTION_NOTE_DB)) {
+ continue;
+ }
+ for (String subsection : noteDbConfig.getSubsections(section)) {
+ noteDbConfig
+ .getNames(section, subsection, false)
+ .forEach(n -> bad.add(section + "." + subsection + "." + n));
+ }
+ noteDbConfig.getNames(section, false).forEach(n -> bad.add(section + "." + n));
+ }
+ if (!bad.isEmpty()) {
+ throw new ProvisionException(
+ "Non-NoteDb config options not allowed in "
+ + noteDbConfig.getFile()
+ + ":\n"
+ + bad.stream().collect(joining("\n")));
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java
index c27f5b9..3748bfd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java
@@ -53,6 +53,7 @@
public final Path gerrit_config;
public final Path secure_config;
+ public final Path notedb_config;
public final Path ssl_keystore;
public final Path ssh_key;
@@ -100,6 +101,7 @@
gerrit_config = etc_dir.resolve("gerrit.config");
secure_config = etc_dir.resolve("secure.config");
+ notedb_config = etc_dir.resolve("notedb.config");
ssl_keystore = etc_dir.resolve("keystore");
ssh_key = etc_dir.resolve("ssh_host_key");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java
new file mode 100644
index 0000000..6b1fd85
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeReportFormatter.java
@@ -0,0 +1,87 @@
+// Copyright (C) 2017 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.google.gerrit.server.git;
+
+import com.google.gerrit.reviewdb.client.Change;
+
+public interface ChangeReportFormatter {
+ public static class Input {
+ private final Change change;
+ private String subject;
+ private Boolean draft;
+ private Boolean edit;
+ private Boolean isPrivate;
+ private Boolean wip;
+
+ public Input(Change change) {
+ this.change = change;
+ }
+
+ public Input setPrivate(boolean isPrivate) {
+ this.isPrivate = isPrivate;
+ return this;
+ }
+
+ public Input setDraft(boolean draft) {
+ this.draft = draft;
+ return this;
+ }
+
+ public Input setEdit(boolean edit) {
+ this.edit = edit;
+ return this;
+ }
+
+ public Input setWorkInProgress(boolean wip) {
+ this.wip = wip;
+ return this;
+ }
+
+ public Input setSubject(String subject) {
+ this.subject = subject;
+ return this;
+ }
+
+ public Change getChange() {
+ return change;
+ }
+
+ public String getSubject() {
+ return subject == null ? change.getSubject() : subject;
+ }
+
+ public boolean isDraft() {
+ return draft == null ? Change.Status.DRAFT == change.getStatus() : draft;
+ }
+
+ public boolean isEdit() {
+ return edit == null ? false : edit;
+ }
+
+ public boolean isPrivate() {
+ return isPrivate == null ? change.isPrivate() : isPrivate;
+ }
+
+ public boolean isWorkInProgress() {
+ return wip == null ? change.isWorkInProgress() : wip;
+ }
+ }
+
+ String newChange(Input input);
+
+ String changeUpdated(Input input);
+
+ String changeClosed(Input input);
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
new file mode 100644
index 0000000..297770c
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
@@ -0,0 +1,66 @@
+// Copyright (C) 2017 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.google.gerrit.server.git;
+
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.inject.Inject;
+
+public class DefaultChangeReportFormatter implements ChangeReportFormatter {
+ private final String canonicalWebUrl;
+
+ @Inject
+ DefaultChangeReportFormatter(@CanonicalWebUrl String canonicalWebUrl) {
+ this.canonicalWebUrl = canonicalWebUrl;
+ }
+
+ @Override
+ public String newChange(ChangeReportFormatter.Input input) {
+ return formatChangeUrl(canonicalWebUrl, input);
+ }
+
+ @Override
+ public String changeUpdated(ChangeReportFormatter.Input input) {
+ return formatChangeUrl(canonicalWebUrl, input);
+ }
+
+ @Override
+ public String changeClosed(ChangeReportFormatter.Input input) {
+ return String.format(
+ "change %s closed", ChangeUtil.formatChangeUrl(canonicalWebUrl, input.getChange()));
+ }
+
+ private String formatChangeUrl(String url, Input input) {
+ StringBuilder m =
+ new StringBuilder()
+ .append(" ")
+ .append(ChangeUtil.formatChangeUrl(url, input.getChange()))
+ .append(" ")
+ .append(ChangeUtil.cropSubject(input.getSubject()));
+ if (input.isDraft()) {
+ m.append(" [DRAFT]");
+ }
+ if (input.isEdit()) {
+ m.append(" [EDIT]");
+ }
+ if (input.isPrivate()) {
+ m.append(" [PRIVATE]");
+ }
+ if (input.isWorkInProgress()) {
+ m.append(" [WIP]");
+ }
+ return m.toString();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModule.java
index 15a9d74..92514da 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModule.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import org.eclipse.jgit.transport.PostUploadHook;
@@ -26,5 +27,7 @@
factory(MetaDataUpdate.InternalFactory.class);
bind(MetaDataUpdate.Server.class);
DynamicSet.bind(binder(), PostUploadHook.class).to(UploadPackMetricsHook.class);
+ DynamicItem.itemOf(binder(), ChangeReportFormatter.class);
+ DynamicItem.bind(binder(), ChangeReportFormatter.class).to(DefaultChangeReportFormatter.class);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 4f613af..b1edb92 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -66,6 +66,7 @@
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicMap.Entry;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -93,13 +94,13 @@
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.SetHashtagsOp;
import com.google.gerrit.server.config.AllProjectsName;
-import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
+import com.google.gerrit.server.git.ChangeReportFormatter;
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeOpRepoManager;
@@ -318,7 +319,6 @@
private final Sequences seq;
private final SetHashtagsOp.Factory hashtagsFactory;
private final SshInfo sshInfo;
- private final String canonicalWebUrl;
private final SubmoduleOp.Factory subOpFactory;
private final TagCache tagCache;
private final CreateRefControl createRefControl;
@@ -370,10 +370,10 @@
private Task closeProgress;
private Task commandProgress;
private MessageSender messageSender;
+ private final ChangeReportFormatter changeFormatter;
@Inject
ReceiveCommits(
- @CanonicalWebUrl String canonicalWebUrl,
AccountResolver accountResolver,
AccountsUpdate.Server accountsUpdate,
AllProjectsName allProjectsName,
@@ -406,6 +406,7 @@
SubmoduleOp.Factory subOpFactory,
TagCache tagCache,
CreateRefControl createRefControl,
+ DynamicItem<ChangeReportFormatter> changeFormatterProvider,
@Assisted ProjectControl projectControl,
@Assisted ReceivePack rp,
@Assisted AllRefsWatcher allRefsWatcher,
@@ -416,9 +417,10 @@
this.accountsUpdate = accountsUpdate;
this.allProjectsName = allProjectsName;
this.batchUpdateFactory = batchUpdateFactory;
- this.canonicalWebUrl = canonicalWebUrl;
this.changeInserterFactory = changeInserterFactory;
this.commitValidatorsFactory = commitValidatorsFactory;
+ this.changeFormatter = changeFormatterProvider.get();
+ this.user = projectControl.getUser().asIdentifiedUser();
this.db = db;
this.editUtil = editUtil;
this.hashtagsFactory = hashtagsFactory;
@@ -454,7 +456,6 @@
// Immutable fields derived from constructor arguments.
repo = rp.getRepository();
- user = projectControl.getUser().asIdentifiedUser();
project = projectControl.getProject();
labelTypes = projectControl.getProjectState().getLabelTypes();
permissions = permissionBackend.user(user).project(project.getNameKey());
@@ -604,15 +605,7 @@
addMessage("");
addMessage("New Changes:");
for (CreateRequest c : created) {
- addMessage(
- formatChangeMessage(
- canonicalWebUrl,
- c.change,
- c.change.getSubject(),
- c.change.getStatus() == Change.Status.DRAFT,
- false, // edit
- c.change.isPrivate(),
- c.change.isWorkInProgress()));
+ addMessage(changeFormatter.newChange(new ChangeReportFormatter.Input(c.change)));
}
addMessage("");
}
@@ -655,55 +648,27 @@
} else {
subject = u.info.getSubject();
}
+
if (isPrivate == null) {
isPrivate = u.notes.getChange().isPrivate();
}
if (wip == null) {
wip = u.notes.getChange().isWorkInProgress();
}
- addMessage(
- formatChangeMessage(
- canonicalWebUrl,
- u.notes.getChange(),
- subject,
- u.replaceOp != null && u.replaceOp.getPatchSet().isDraft(),
- edit,
- isPrivate,
- wip));
+
+ ChangeReportFormatter.Input input =
+ new ChangeReportFormatter.Input(u.notes.getChange())
+ .setSubject(subject)
+ .setDraft(u.replaceOp != null && u.replaceOp.getPatchSet().isDraft())
+ .setEdit(edit)
+ .setPrivate(isPrivate)
+ .setWorkInProgress(wip);
+ addMessage(changeFormatter.changeUpdated(input));
}
addMessage("");
}
}
- private static String formatChangeMessage(
- String url,
- Change change,
- String subject,
- boolean draft,
- boolean edit,
- boolean isPrivate,
- boolean wip) {
- StringBuilder m =
- new StringBuilder()
- .append(" ")
- .append(ChangeUtil.formatChangeUrl(url, change))
- .append(" ")
- .append(ChangeUtil.cropSubject(subject));
- if (draft) {
- m.append(" [DRAFT]");
- }
- if (edit) {
- m.append(" [EDIT]");
- }
- if (isPrivate) {
- m.append(" [PRIVATE]");
- }
- if (wip) {
- m.append(" [WIP]");
- }
- return m.toString();
- }
-
private void insertChangesAndPatchSets() {
ReceiveCommand magicBranchCmd = magicBranch != null ? magicBranch.cmd : null;
if (magicBranchCmd != null && magicBranchCmd.getResult() != NOT_ATTEMPTED) {
@@ -1713,7 +1678,7 @@
private boolean requestReplace(
ReceiveCommand cmd, boolean checkMergedInto, Change change, RevCommit newCommit) {
if (change.getStatus().isClosed()) {
- reject(cmd, "change " + ChangeUtil.formatChangeUrl(canonicalWebUrl, change) + " closed");
+ reject(cmd, changeFormatter.changeClosed(new ChangeReportFormatter.Input(change)));
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index 7323c2e..61a0cd6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -320,6 +320,7 @@
}
private final FileBasedConfig gerritConfig;
+ private final FileBasedConfig noteDbConfig;
private final SchemaFactory<ReviewDb> schemaFactory;
private final GitRepositoryManager repoManager;
private final AllProjectsName allProjects;
@@ -374,7 +375,6 @@
this.userFactory = userFactory;
this.globalNotesMigration = globalNotesMigration;
this.primaryStorageMigrator = primaryStorageMigrator;
- this.gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
this.executor = executor;
this.projects = projects;
this.changes = changes;
@@ -384,6 +384,11 @@
this.forceRebuild = forceRebuild;
this.sequenceGap = sequenceGap;
this.autoMigrate = autoMigrate;
+
+ // Stack notedb.config over gerrit.config, in the same way as GerritServerConfigProvider.
+ this.gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
+ this.noteDbConfig =
+ new FileBasedConfig(gerritConfig, sitePaths.notedb_config.toFile(), FS.detect());
}
@Override
@@ -564,9 +569,10 @@
private Optional<NotesMigrationState> loadState() throws IOException {
try {
gerritConfig.load();
- return NotesMigrationState.forConfig(gerritConfig);
+ noteDbConfig.load();
+ return NotesMigrationState.forConfig(noteDbConfig);
} catch (ConfigInvalidException | IllegalArgumentException e) {
- log.warn("error reading NoteDb migration options from " + gerritConfig.getFile(), e);
+ log.warn("error reading NoteDb migration options from " + noteDbConfig.getFile(), e);
return Optional.empty();
}
}
@@ -597,9 +603,9 @@
? "But found this state:\n" + actualOldState.get().toText()
: "But could not parse the current state"));
}
- newState.setConfigValues(gerritConfig);
- additionalUpdates.accept(gerritConfig);
- gerritConfig.save();
+ newState.setConfigValues(noteDbConfig);
+ additionalUpdates.accept(noteDbConfig);
+ noteDbConfig.save();
// Only set in-memory state once it's been persisted to storage.
globalNotesMigration.setFrom(newState);
@@ -610,9 +616,9 @@
private void enableAutoMigrate() throws MigrationException {
try {
- gerritConfig.load();
- setAutoMigrate(gerritConfig, true);
- gerritConfig.save();
+ noteDbConfig.load();
+ setAutoMigrate(noteDbConfig, true);
+ noteDbConfig.save();
} catch (ConfigInvalidException | IOException e) {
throw new MigrationException("Error saving auto-migration config", e);
}
