diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index de73930b7a..cc485d5154 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2441,6 +2441,21 @@ to permit fast restarts. + By default, true. +[[httpd.gracefulStopTimeout]]httpd.gracefulStopTimeout:: ++ +Set a graceful stop time. If set, the daemon ensures that all incoming +calls are preserved for a maximum period of time, before starting +the graceful shutdown process. Sites behind a workload balancer such as +HAProxy would need this to be set for avoiding serving errors during +rolling restarts. ++ +Values should use common unit suffixes to express their setting: ++ +* s, sec, second, seconds +* m, min, minute, minutes ++ +By default, 0 seconds (immediate shutdown). + [[httpd.inheritChannel]]httpd.inheritChannel:: + If true, permits the daemon to inherit its server socket channel @@ -2760,7 +2775,7 @@ immediately after indexing it. If false, there is a race condition during two simultaneous writes that may cause one of the writes to not be reflected in the index. The check to avoid this does consume some resources. + -Defaults to true. +Defaults to false. [[index.scheduledIndexer]] ==== Subsection index.scheduledIndexer @@ -4032,7 +4047,7 @@ By default, MIXED. Only used when `sendemail.from` is set to `USER`. List of allowed domains. If user's email matches one of the domains, emails will be sent as USER, otherwise as MIXED mode. Wildcards may be specified by -including `*` to match any number of characters, for example `*.example.com` +including `\*` to match any number of characters, for example `*.example.com` matches any subdomain of `example.com`. + By default, `*`. diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt index 2d0812ea7a..de5171c140 100644 --- a/Documentation/intro-project-owner.txt +++ b/Documentation/intro-project-owner.txt @@ -424,10 +424,13 @@ messages conform to line length limits. As project owner you can administrate the branches of your project in the Gerrit Web UI under `Projects` > `List` > > -`Branches`. In the Web UI both link:project-configuration.html#branch-creation[ -branch creation] and link:project-configuration.html#branch-deletion[branch -deletion] are allowed for project owners without requiring any -additional access rights. +`Branches`. In the Web UI link:project-configuration.html#branch-creation[ +branch creation] is allowed if you have +link:access-control.html#category_create[Create Reference] access right and +link:project-configuration.html#branch-deletion[branch deletion] is allowed if +you have the link:access-control.html#category_delete[Delete Reference] or the +link:access-control.html#category_push[Push] access right with the `force` +option. By setting `HEAD` on the project you can define its link:project-configuration.html#default-branch[default branch]. For convenience diff --git a/Documentation/json.txt b/Documentation/json.txt index 2b341857f7..7360bd4573 100644 --- a/Documentation/json.txt +++ b/Documentation/json.txt @@ -157,7 +157,8 @@ Information about a ref that was updated. oldRev:: The old value of the ref, prior to the update. -newRev:: The new value the ref was updated to. +newRev:: The new value the ref was updated to. Zero value (`0000000000000000000000000000000000000000`) +indicates that the ref was deleted. refName:: Full ref name within project. diff --git a/Documentation/project-configuration.txt b/Documentation/project-configuration.txt index 5c79c1bfd4..f76b5e42d9 100644 --- a/Documentation/project-configuration.txt +++ b/Documentation/project-configuration.txt @@ -257,9 +257,7 @@ There are several ways to create a new branch in a project: To be able to create new branches the user must have the link:access-control.html#category_create[Create Reference] access -right. In addition, project owners and Gerrit administrators can create -new branches from the Web UI or via REST even without having the -`Create Reference` access right. +right. When using the Web UI, the REST endpoint or the SSH command it is only possible to create branches on commits that already exist in the @@ -295,9 +293,7 @@ another method, by force pushing nothing to an existing branch: To be able to delete branches, the user must have the link:access-control.html#category_delete[Delete Reference] or the link:access-control.html#category_push[Push] access right with the -`force` option. In addition, project owners and Gerrit administrators -can delete branches from the Web UI or via REST even without having the -`Force Push` access right. +`force` option. [[default-branch]] === Default Branch diff --git a/Documentation/rest-api-groups.txt b/Documentation/rest-api-groups.txt index 49d5ee5e39..00fd81f62a 100644 --- a/Documentation/rest-api-groups.txt +++ b/Documentation/rest-api-groups.txt @@ -1549,8 +1549,11 @@ a Gerrit internal group, or an external group that is known to Gerrit. |Field Name ||Description |`id` ||The URL encoded UUID of the group. |`name` | -not set if returned in a map where the group name is used as map key| -The name of the group. +optional, not set if returned in a map where the group name is used as map key| +The name of the group. + +For external groups the group name is missing if there is no group +backend that can resolve the group UUID. E.g. this can happen when a +plugin that provided a group backend was uninstalled. |`url` |optional| URL to information about the group. Typically a URL to a web page that permits users to apply to join the group, or manage their membership. diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index 0f030a6527..2ad8cbd74c 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt @@ -4,9 +4,7 @@ Most basic searches can be viewed by clicking on a link along the top menu bar. The link will prefill the search box with a common search -query, execute it, and present the results. If exactly one change -matches the search, the change will be presented instead of a list. - +query, execute it, and present the results. [options="header"] |================================================= @@ -14,7 +12,7 @@ matches the search, the change will be presented instead of a list. |All > Open | status:open '(or is:open)' |All > Merged | status:merged |All > Abandoned | status:abandoned -|My > Watched Changes | status:open is:watched +|My > Watched Changes | is:watched is:open |My > Starred Changes | is:starred |My > Draft Comments | has:draft |Open changes in Foo | status:open project:Foo @@ -36,6 +34,9 @@ text and let Gerrit figure out the meaning: |Approval requirement | Code-Review>=+2, Verified=1 |============================================================= +For change searches (i.e. those using a numerical id, Change-Id, or commit +SHA1), if the search results in a single change that change will be +presented instead of a list. [[search-operators]] == Search Operators @@ -601,7 +602,7 @@ label configuration. (For a more general check, use link:#submittable[submittable:ok].) `is:open (label:Verified-1 OR label:Code-Review-2)`:: -`is:open (label:Verified=reject OR label:Code-Review:reject)`:: +`is:open (label:Verified=reject OR label:Code-Review=reject)`:: + Changes that are blocked from submission due to a blocking score. diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java index b056afa9e2..5e38a1414f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupAuditLogScreen.java @@ -113,17 +113,19 @@ public class AccountGroupAuditLogScreen extends AccountGroupScreen { GroupInfo member = auditEvent.memberAsGroup(); if (AccountGroup.isInternalGroup(member.getGroupUUID())) { table.setWidget( - row, 3, new Hyperlink(member.name(), Dispatcher.toGroup(member.getGroupUUID()))); + row, + 3, + new Hyperlink(formatGroup(member), Dispatcher.toGroup(member.getGroupUUID()))); fmt.getElement(row, 3).setTitle(null); } else if (member.url() != null) { Anchor a = new Anchor(); - a.setText(member.name()); + a.setText(formatGroup(member)); a.setHref(member.url()); a.setTitle("UUID " + member.getGroupUUID().get()); table.setWidget(row, 3, a); fmt.getElement(row, 3).setTitle(null); } else { - table.setText(row, 3, member.name()); + table.setText(row, 3, formatGroup(member)); fmt.getElement(row, 3).setTitle("UUID " + member.getGroupUUID().get()); } break; @@ -148,4 +150,10 @@ public class AccountGroupAuditLogScreen extends AccountGroupScreen { b.append(")"); return b.toString(); } + + private static String formatGroup(GroupInfo group) { + return group.name() != null && !group.name().isEmpty() + ? group.name() + : group.getGroupUUID().get(); + } } diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java index 09ffe9d696..b75435118f 100644 --- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java +++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java @@ -35,9 +35,13 @@ import com.google.gerrit.testing.ConfigSuite; import com.google.inject.Injector; import com.google.inject.Module; import com.google.inject.Provider; +import java.io.File; import java.util.Arrays; import java.util.Collections; import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.SystemReader; import org.junit.Rule; import org.junit.rules.RuleChain; import org.junit.rules.TemporaryFolder; @@ -109,8 +113,12 @@ public abstract class StandaloneSiteTest { return new Statement() { @Override public void evaluate() throws Throwable { - beforeTest(description); - base.evaluate(); + try { + beforeTest(description); + base.evaluate(); + } finally { + afterTest(); + } } }; } @@ -122,13 +130,65 @@ public abstract class StandaloneSiteTest { protected Account.Id adminId; private GerritServer.Description serverDesc; + private SystemReader oldSystemReader; private void beforeTest(Description description) throws Exception { + // SystemReader must be overridden before creating any repos, since they read the user/system + // configs at initialization time, and are then stored in the RepositoryCache forever. + oldSystemReader = setFakeSystemReader(tempSiteDir.getRoot()); + serverDesc = GerritServer.Description.forTestMethod(description, configName); sitePaths = new SitePaths(tempSiteDir.getRoot().toPath()); GerritServer.init(serverDesc, baseConfig, sitePaths.site_path); } + private static SystemReader setFakeSystemReader(File tempDir) { + SystemReader oldSystemReader = SystemReader.getInstance(); + SystemReader.setInstance( + new SystemReader() { + @Override + public String getHostname() { + return oldSystemReader.getHostname(); + } + + @Override + public String getenv(String variable) { + return oldSystemReader.getenv(variable); + } + + @Override + public String getProperty(String key) { + return oldSystemReader.getProperty(key); + } + + @Override + public FileBasedConfig openUserConfig(Config parent, FS fs) { + return new FileBasedConfig(parent, new File(tempDir, "user.config"), FS.detect()); + } + + @Override + public FileBasedConfig openSystemConfig(Config parent, FS fs) { + return new FileBasedConfig(parent, new File(tempDir, "system.config"), FS.detect()); + } + + @Override + public long getCurrentTime() { + return oldSystemReader.getCurrentTime(); + } + + @Override + public int getTimezone(long when) { + return oldSystemReader.getTimezone(when); + } + }); + return oldSystemReader; + } + + private void afterTest() throws Exception { + SystemReader.setInstance(oldSystemReader); + oldSystemReader = null; + } + protected ServerContext startServer() throws Exception { return startServer(null); } @@ -153,7 +213,10 @@ public abstract class StandaloneSiteTest { } protected static void runGerrit(String... args) throws Exception { - assertThat(GerritLauncher.mainImpl(args)) + // Use invokeProgram with the current classloader, rather than mainImpl, which would create a + // new classloader. This is necessary so that static state, particularly the SystemReader, is + // shared with the test method. + assertThat(GerritLauncher.invokeProgram(StandaloneSiteTest.class.getClassLoader(), args)) .named("gerrit.war " + Arrays.stream(args).collect(joining(" "))) .isEqualTo(0); } diff --git a/java/com/google/gerrit/launcher/GerritLauncher.java b/java/com/google/gerrit/launcher/GerritLauncher.java index e8892bedd3..b7d232d29c 100644 --- a/java/com/google/gerrit/launcher/GerritLauncher.java +++ b/java/com/google/gerrit/launcher/GerritLauncher.java @@ -64,6 +64,17 @@ public final class GerritLauncher { System.exit(mainImpl(argv)); } + /** + * Invokes a proram. + * + *

Creates a new classloader to load and run the program class. To reuse a classloader across + * calls (e.g. from tests), use {@link #invokeProgram(ClassLoader, String[])}. + * + * @param argv arguments, as would be passed to {@code gerrit.war}. The first argument is the + * program name. + * @return program return code. + * @throws Exception if any error occurs. + */ public static int mainImpl(String[] argv) throws Exception { if (argv.length == 0) { File me; @@ -164,7 +175,16 @@ public final class GerritLauncher { } } - private static int invokeProgram(ClassLoader loader, String[] origArgv) throws Exception { + /** + * Invokes a proram in the provided {@code ClassLoader}. + * + * @param loader classloader to load program class from. + * @param origArgv arguments, as would be passed to {@code gerrit.war}. The first argument is the + * program name. + * @return program return code. + * @throws Exception if any error occurs. + */ + public static int invokeProgram(ClassLoader loader, String[] origArgv) throws Exception { String name = origArgv[0]; final String[] argv = new String[origArgv.length - 1]; System.arraycopy(origArgv, 1, argv, 0, argv.length); diff --git a/java/com/google/gerrit/pgm/MigrateToNoteDb.java b/java/com/google/gerrit/pgm/MigrateToNoteDb.java index 22fdb2ce2f..10761c7917 100644 --- a/java/com/google/gerrit/pgm/MigrateToNoteDb.java +++ b/java/com/google/gerrit/pgm/MigrateToNoteDb.java @@ -29,13 +29,16 @@ import com.google.gerrit.pgm.util.ThreadLimiter; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.index.DummyIndexModule; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; +import com.google.gerrit.server.notedb.rebuild.GcAllUsers; import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator; import com.google.gerrit.server.schema.DataSourceType; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Provider; +import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; import org.kohsuke.args4j.Option; @@ -99,6 +102,7 @@ public class MigrateToNoteDb extends SiteProgram { private LifecycleManager dbManager; private LifecycleManager sysManager; + @Inject private GcAllUsers gcAllUsers; @Inject private Provider migratorBuilderProvider; @Override @@ -137,6 +141,9 @@ public class MigrateToNoteDb extends SiteProgram { migrator.migrate(); } } + try (PrintWriter w = new PrintWriter(System.out, true)) { + gcAllUsers.run(w); + } } finally { stop(); } @@ -190,6 +197,7 @@ public class MigrateToNoteDb extends SiteProgram { install(dbInjector.getInstance(BatchProgramModule.class)); install(new DummyIndexModule()); factory(ChangeResource.Factory.class); + factory(GarbageCollection.Factory.class); } }); } diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java index 3895d169e5..b6eac05229 100644 --- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java +++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java @@ -39,6 +39,7 @@ import java.util.EnumSet; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import javax.servlet.DispatcherType; import javax.servlet.Filter; import org.eclipse.jetty.http.HttpScheme; @@ -56,6 +57,7 @@ import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.RequestLogHandler; +import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.FilterHolder; @@ -149,6 +151,15 @@ public class JettyServer { httpd.addBean(mbean); } + long gracefulStopTimeout = + cfg.getTimeUnit("httpd", null, "gracefulStopTimeout", 0L, TimeUnit.MILLISECONDS); + if (gracefulStopTimeout > 0) { + StatisticsHandler statsHandler = new StatisticsHandler(); + statsHandler.setHandler(app); + app = statsHandler; + httpd.setStopTimeout(gracefulStopTimeout); + } + httpd.setHandler(app); httpd.setStopAtShutdown(false); } diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java index cf51197281..1ea115a5a6 100644 --- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java +++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java @@ -152,7 +152,7 @@ public class ChangeIndexer { } private static boolean autoReindexIfStale(Config cfg) { - return cfg.getBoolean("index", null, "autoReindexIfStale", true); + return cfg.getBoolean("index", null, "autoReindexIfStale", false); } /** diff --git a/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java b/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java new file mode 100644 index 0000000000..0653192308 --- /dev/null +++ b/java/com/google/gerrit/server/notedb/rebuild/GcAllUsers.java @@ -0,0 +1,122 @@ +// Copyright (C) 2018 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.notedb.rebuild; + +import static com.google.common.base.Preconditions.checkNotNull; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_GC_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_AUTO; + +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableList; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.data.GarbageCollectionResult; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.git.GarbageCollection; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.LocalDiskRepositoryManager; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.function.Consumer; +import org.eclipse.jgit.lib.Repository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Singleton +public class GcAllUsers { + private static final Logger log = LoggerFactory.getLogger(GcAllUsers.class); + + private final AllUsersName allUsers; + private final GarbageCollection.Factory gcFactory; + private final GitRepositoryManager repoManager; + + @Inject + GcAllUsers( + AllUsersName allUsers, + GarbageCollection.Factory gcFactory, + GitRepositoryManager repoManager) { + this.allUsers = allUsers; + this.gcFactory = gcFactory; + this.repoManager = repoManager; + } + + public void runWithLogger() { + // Print log messages using logger, and skip progress. + run(s -> log.info(s), null); + } + + public void run(PrintWriter writer) { + // Print both log messages and progress to given writer. + run(checkNotNull(writer)::println, writer); + } + + private void run(Consumer logOneLine, @Nullable PrintWriter progressWriter) { + if (!(repoManager instanceof LocalDiskRepositoryManager)) { + logOneLine.accept("Skipping GC of " + allUsers + "; not a local disk repo"); + return; + } + if (!enableAutoGc(logOneLine)) { + logOneLine.accept( + "Skipping GC of " + + allUsers + + " due to disabling " + + CONFIG_GC_SECTION + + "." + + CONFIG_KEY_AUTO); + logOneLine.accept( + "If loading accounts is slow after the NoteDb migration, run `git gc` on " + + allUsers + + " manually"); + return; + } + + if (progressWriter == null) { + // Mimic log line from GarbageCollection. + logOneLine.accept("collecting garbage for \"" + allUsers + "\":\n"); + } + GarbageCollectionResult result = + gcFactory.create().run(ImmutableList.of(allUsers), progressWriter); + if (!result.hasErrors()) { + return; + } + for (GarbageCollectionResult.Error e : result.getErrors()) { + switch (e.getType()) { + case GC_ALREADY_SCHEDULED: + logOneLine.accept("GC already scheduled for " + e.getProjectName()); + break; + case GC_FAILED: + logOneLine.accept("GC failed for " + e.getProjectName()); + break; + case REPOSITORY_NOT_FOUND: + logOneLine.accept(e.getProjectName() + " repo not found"); + break; + default: + logOneLine.accept("GC failed for " + e.getProjectName() + ": " + e.getType()); + break; + } + } + } + + private boolean enableAutoGc(Consumer logOneLine) { + try (Repository repo = repoManager.openRepository(allUsers)) { + return repo.getConfig().getInt(CONFIG_GC_SECTION, CONFIG_KEY_AUTO, -1) != 0; + } catch (IOException e) { + logOneLine.accept( + "Error reading config for " + allUsers + ":\n" + Throwables.getStackTraceAsString(e)); + return false; + } + } +} diff --git a/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java index 535736df91..65755ed14f 100644 --- a/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java +++ b/java/com/google/gerrit/server/notedb/rebuild/OnlineNoteDbMigrator.java @@ -50,19 +50,22 @@ public class OnlineNoteDbMigrator implements LifecycleListener { } } - private Provider migratorBuilderProvider; + private final GcAllUsers gcAllUsers; private final OnlineUpgrader indexUpgrader; + private final Provider migratorBuilderProvider; private final boolean upgradeIndex; private final boolean trial; @Inject OnlineNoteDbMigrator( @GerritServerConfig Config cfg, - Provider migratorBuilderProvider, + GcAllUsers gcAllUsers, OnlineUpgrader indexUpgrader, + Provider migratorBuilderProvider, @Named(TRIAL) boolean trial) { - this.migratorBuilderProvider = migratorBuilderProvider; + this.gcAllUsers = gcAllUsers; this.indexUpgrader = indexUpgrader; + this.migratorBuilderProvider = migratorBuilderProvider; this.upgradeIndex = VersionManager.getOnlineUpgrade(cfg); this.trial = trial || NoteDbMigrator.getTrialMode(cfg); } @@ -88,6 +91,7 @@ public class OnlineNoteDbMigrator implements LifecycleListener { } catch (Exception e) { log.error("Error in online NoteDb migration", e); } + gcAllUsers.runWithLogger(); log.info("Online NoteDb migration completed in {}s", sw.elapsed(TimeUnit.SECONDS)); if (upgradeIndex) { diff --git a/javatests/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java index 35fcc949a4..1bb23fba9e 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.pgm; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth8.assertThat; import static com.google.common.truth.TruthJUnit.assume; @@ -28,7 +29,9 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.LocalDiskRepositoryManager; import com.google.gerrit.server.index.GerritIndexStatus; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; @@ -41,6 +44,11 @@ import com.google.gerrit.testing.NoteDbMode; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Key; import com.google.inject.TypeLiteral; +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; +import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -72,6 +80,16 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest { 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()); + + // Set gc.pruneExpire=now so GC prunes all unreachable objects from All-Users, which allows us + // to reliably test that it behaves as expected. + Path cfgPath = sitePaths.site_path.resolve("git").resolve("All-Users.git").resolve("config"); + assertWithMessage("Expected All-Users config at %s", cfgPath) + .that(Files.isRegularFile(cfgPath)) + .isTrue(); + FileBasedConfig cfg = new FileBasedConfig(cfgPath.toFile(), FS.detect()); + cfg.setString("gc", null, "pruneExpire", "now"); + cfg.save(); } @Test @@ -114,11 +132,17 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest { migrate(); assertNotesMigrationState(NotesMigrationState.NOTE_DB); + File allUsersDir; try (ServerContext ctx = startServer()) { GitRepositoryManager repoManager = ctx.getInjector().getInstance(GitRepositoryManager.class); try (Repository repo = repoManager.openRepository(project)) { assertThat(repo.exactRef(RefNames.changeMetaRef(changeId))).isNotNull(); } + assertThat(repoManager).isInstanceOf(LocalDiskRepositoryManager.class); + try (Repository repo = + repoManager.openRepository(ctx.getInjector().getInstance(AllUsersName.class))) { + allUsersDir = repo.getDirectory(); + } try (ReviewDb db = openUnderlyingReviewDb(ctx)) { Change c = db.changes().get(changeId); @@ -137,6 +161,15 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest { } assertNoAutoMigrateConfig(gerritConfig); assertAutoMigrateConfig(noteDbConfig, false); + + try (FileRepository repo = new FileRepository(allUsersDir)) { + try (Stream paths = Files.walk(repo.getObjectsDirectory().toPath())) { + assertThat(paths.filter(p -> !p.toString().contains("pack") && Files.isRegularFile(p))) + .named("loose object files in All-Users") + .isEmpty(); + } + assertThat(repo.getObjectDatabase().getPacks()).named("packfiles in All-Users").hasSize(1); + } } @Test diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index ddda473c30..c3b5af9d39 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -51,6 +51,7 @@ import com.google.gerrit.extensions.api.changes.StarsInput; import com.google.gerrit.extensions.api.groups.GroupInput; import com.google.gerrit.extensions.api.projects.ConfigInput; import com.google.gerrit.extensions.client.InheritableBoolean; +import com.google.gerrit.extensions.client.ProjectWatchInfo; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.ChangeInfo; @@ -228,8 +229,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { "Add Email", userId, u -> u.addExternalId(ExternalId.createEmail(userId, email)).setPreferredEmail(email)); - user = userFactory.create(userId); - requestContext.setContext(newRequestContext(userId)); + resetUser(); } protected RequestContext newRequestContext(Account.Id requestUserId) { @@ -247,6 +247,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { }; } + protected void resetUser() { + user = userFactory.create(userId); + requestContext.setContext(newRequestContext(userId)); + } + @After public void tearDownInjector() { if (lifecycle != null) { @@ -384,6 +389,20 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { assertQuery("is:closed", expected); } + @Test + public void byStatusAbandoned() throws Exception { + TestRepository repo = createProject("repo"); + ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.MERGED); + insert(repo, ins1); + ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.ABANDONED); + Change change1 = insert(repo, ins2); + insert(repo, newChangeWithStatus(repo, Change.Status.NEW)); + + assertQuery("status:abandoned", change1); + assertQuery("status:ABANDONED", change1); + assertQuery("is:abandoned", change1); + } + @Test public void byStatusPrefix() throws Exception { TestRepository repo = createProject("repo"); @@ -1502,6 +1521,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { Change change1 = insert(repo, newChange(repo)); Change change2 = insert(repo, newChange(repo)); + assertQuery("has:draft"); + DraftInput in = new DraftInput(); in.line = 1; in.message = "nit: trailing whitespace"; @@ -1517,6 +1538,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId().get(); + assertQuery("has:draft", change2, change1); assertQuery("draftby:" + userId.get(), change2, change1); assertQuery("draftby:" + user2); } @@ -2155,6 +2177,35 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { .containsExactlyElementsIn(expectedPatterns); } + @Test + public void watched() throws Exception { + TestRepository repo = createProject("repo"); + ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW); + Change change1 = insert(repo, ins1); + + TestRepository repo2 = createProject("repo2"); + + ChangeInserter ins2 = newChangeWithStatus(repo2, Change.Status.NEW); + insert(repo2, ins2); + + assertQuery("is:watched"); + assertQuery("watchedby:self"); + + List projectsToWatch = new ArrayList<>(); + ProjectWatchInfo pwi = new ProjectWatchInfo(); + pwi.project = "repo"; + pwi.filter = null; + pwi.notifyAbandonedChanges = true; + pwi.notifyNewChanges = true; + pwi.notifyAllComments = true; + projectsToWatch.add(pwi); + gApi.accounts().self().setWatchedProjects(projectsToWatch); + resetUser(); + + assertQuery("is:watched", change1); + assertQuery("watchedby:self", change1); + } + @Test public void selfAndMe() throws Exception { TestRepository repo = createProject("repo");