Merge "Add small test for IntBlob"
diff --git a/Documentation/config-auto-site-initialization.txt b/Documentation/config-auto-site-initialization.txt
index acd03c9..1be0af9 100644
--- a/Documentation/config-auto-site-initialization.txt
+++ b/Documentation/config-auto-site-initialization.txt
@@ -27,15 +27,14 @@
run for that site. The database connectivity, in that case, is defined
in the `etc/gerrit.config`.
-If `gerrit.site_path` is not defined then Gerrit will try to find the
-`gerrit.init_path` system property. If defined this property will be
-used to determine the site path. The database connectivity, also for
-this case, is defined by the `jdbc/ReviewDb` JNDI property.
+`gerrit.site_path` system property must be defined to run the init for
+that site.
[WARNING]
Defining the `jdbc/ReviewDb` JNDI property for an H2 database under the
-path defined by either `gerrit.site_path` or `gerrit.init_path` will
-cause an incomplete auto initialization and Gerrit will fail to start.
+path defined by `gerrit.site_path` will cause an incomplete auto
+initialization and Gerrit will fail to start.
+
Opening a connection to such a database will create a subfolder under the
site path folder (in order to create the H2 database) and Gerrit will
no longer consider that site path to be new and, because of that,
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 765e56e..589071a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3798,6 +3798,20 @@
If no keys are specified, web-of-trust checks are disabled. This is the
default behavior.
+[[receive.enableProtocolV2]]receive.enableProtocolV2::
++
+Enable support for git protocol version 2.
++
+When this option is enabled, clients may send upload pack using git
+protocol version 2.
++
+The repository must also be configured on the server side to use protocol
+version 2 by setting `protocol.version = 2` either in the gerrit user's
+`~/.gitconfig` file (which will enable it for all repositories) or on
+a per repository basis by setting the option in the `.git/config` file
+of the repository.
++
+Defaults to false, git protocol version 2 is not enabled.
[[repository]]
=== Section repository
@@ -5081,16 +5095,9 @@
The format is one Base-64 encoded public key per line.
+== Configuring the Polygerrit UI
-== Database system_config
-
-Several columns in the `system_config` table within the metadata
-database may be set to control how Gerrit behaves.
-
-[NOTE]
-The contents of the `system_config` table are cached at startup
-by Gerrit. If you modify any columns in this table, Gerrit needs
-to be restarted before it will use the new values.
+Please see link:dev-polygerrit.html[UI] on configuring the Polygerrit UI.
=== Configurable Parameters
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index d5ffda2..d7bd8b3 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -801,6 +801,64 @@
}
----
+=== Calling Command Options ===
+
+Within an OptionHandler, during the processing of an option, plugins can
+provide and call extra parameters on the current command during parsing
+simulating as if they had been passed from the command line originally.
+
+To call additional parameters from within an option handler, instantiate
+the com.google.gerrit.util.cli.CmdLineParser.Parameters class with the
+existing parameters, and then call callParameters() with the additional
+parameters to be parsed. OptionHandlers may optionally pass this class to
+other methods which may then both parse/consume more parameters and call
+additional parameters.
+
+When calling command options not provided by your plugin, there is always
+a risk that the options may not exist, perhaps because the options being
+called are to be provided by another plugin, and said plugin is not
+currently installed. To protect againt this situation, it is possible to
+define an option as being dependent on other options using the
+@RequiresOptions() annotation. If the required options are not all not
+currently present, then the dependent option will not be available or
+visible in the help.
+
+The example below shows a plugin that adds a "--special" option (perhaps
+for use with the Query command) that calls (and requires) the
+"--format json" option.
+
+[source, java]
+----
+public class JsonOutputOptionHandler<T> extends OptionHandler<T> {
+ protected com.google.gerrit.util.cli.CmdLineParser.MyParser myParser;
+
+ public JsonOutputOptionHandler(CmdLineParser parser, OptionDef option, Setter<? super T> setter) {
+ super(parser, option, setter);
+ myParser = (com.google.gerrit.util.cli.CmdLineParser.MyParser) owner;
+ }
+
+ @Override
+ public int parseArguments(org.kohsuke.args4j.spi.Parameters params) throws CmdLineException {
+ new Parameters(params, myParser).callParameters("--format", "json");
+ setter.addValue(true);
+ return 0; // we didn't consume any additional args
+ }
+
+ @Override
+ public String getDefaultMetaVariable() {
+ ...
+ }
+}
+
+@RequiresOptions("--format")
+@Option(
+ name = "--special",
+ usage = "ouptut results using json",
+ handler = JsonOutputOptionHandler.class
+)
+boolean json;
+----
+
[[query_attributes]]
=== Query Attributes ===
diff --git a/Documentation/install-j2ee.txt b/Documentation/install-j2ee.txt
index f7252e0..91d73cc 100644
--- a/Documentation/install-j2ee.txt
+++ b/Documentation/install-j2ee.txt
@@ -105,9 +105,8 @@
----
[TIP]
-Under Jetty, restarting the web application (e.g. after modifying
-`system_config`) is as simple as touching the context config file:
-`'$JETTY_HOME'/contexts/gerrit.xml`
+Under Jetty, restarting the web application is as simple as
+touching the context config file: `'$JETTY_HOME'/contexts/gerrit.xml`
[[tomcat]]
== Tomcat 7.x
diff --git a/WORKSPACE b/WORKSPACE
index 50714f1..8c8102b 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1016,8 +1016,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.4.3",
- sha1 = "5c24325430971ba2fa4769eb446f026b7680d5e7",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.5.0",
+ sha1 = "241436d27cf65b84d17126dc7b6b947e8e2c173c",
)
JACKSON_VERSION = "2.9.7"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 624f12f..8c64aa9 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -126,6 +126,7 @@
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gerrit.testing.FakeGroupAuditService;
import com.google.gerrit.testing.NoteDbMode;
import com.google.gerrit.testing.SshMode;
import com.google.gerrit.testing.TempFileUtil;
@@ -242,6 +243,7 @@
@Inject protected ChangeNoteUtil changeNoteUtil;
@Inject protected ChangeResource.Factory changeResourceFactory;
@Inject protected FakeEmailSender sender;
+ @Inject protected FakeGroupAuditService auditService;
@Inject protected GerritApi gApi;
@Inject protected GitRepositoryManager repoManager;
@Inject protected GroupBackend groupBackend;
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 9f9cbf9..be8932e 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -43,6 +43,7 @@
import com.google.gerrit.server.util.SocketUtil;
import com.google.gerrit.server.util.SystemLog;
import com.google.gerrit.testing.FakeEmailSender;
+import com.google.gerrit.testing.FakeGroupAuditService;
import com.google.gerrit.testing.InMemoryDatabase;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.NoteDbChecker;
@@ -355,6 +356,7 @@
},
site);
daemon.setEmailModuleForTesting(new FakeEmailSender.Module());
+ daemon.setAuditEventModuleForTesting(new FakeGroupAuditService.Module());
daemon.setAdditionalSysModuleForTesting(testSysModule);
daemon.setEnableSshd(desc.useSsh());
daemon.setSlave(isSlave(baseConfig));
diff --git a/java/com/google/gerrit/common/audit/Audit.java b/java/com/google/gerrit/common/audit/Audit.java
index 25e4caf..a791e97 100644
--- a/java/com/google/gerrit/common/audit/Audit.java
+++ b/java/com/google/gerrit/common/audit/Audit.java
@@ -23,7 +23,7 @@
* Audit annotation for JSON/RPC interfaces.
*
* <p>Flag with @Audit all the JSON/RPC methods to be traced in audit-trail and submitted to the
- * AuditService.
+ * GroupAuditService.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD})
diff --git a/java/com/google/gerrit/common/data/AccessSection.java b/java/com/google/gerrit/common/data/AccessSection.java
index 49948f8f..c8d8d41 100644
--- a/java/com/google/gerrit/common/data/AccessSection.java
+++ b/java/com/google/gerrit/common/data/AccessSection.java
@@ -16,6 +16,7 @@
import static java.util.Objects.requireNonNull;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Project;
import java.util.ArrayList;
@@ -36,12 +37,8 @@
super(refPattern);
}
- // TODO(ekempin): Make this method return an ImmutableList once the GWT UI is gone.
public List<Permission> getPermissions() {
- if (permissions == null) {
- return new ArrayList<>();
- }
- return new ArrayList<>(permissions);
+ return permissions == null ? ImmutableList.of() : ImmutableList.copyOf(permissions);
}
public void setPermissions(List<Permission> list) {
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index de6108e..a30d412 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -14,6 +14,7 @@
package com.google.gerrit.common.data;
+import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
@@ -157,12 +158,8 @@
exclusiveGroup = newExclusiveGroup;
}
- // TODO(ekempin): Make this method return an ImmutableList once the GWT UI is gone.
public List<PermissionRule> getRules() {
- if (rules == null) {
- return new ArrayList<>();
- }
- return new ArrayList<>(rules);
+ return rules == null ? ImmutableList.of() : ImmutableList.copyOf(rules);
}
public void setRules(List<PermissionRule> list) {
diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
index 05fd7a7..65d2916 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
@@ -40,6 +40,7 @@
case V6_2:
case V6_3:
case V6_4:
+ case V6_5:
this.searchFilteringName = "_source";
this.indicesExistParam = "?allow_no_indices=false";
this.exactFieldType = "keyword";
diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
index dfa5d21..4c98df1 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java
@@ -22,7 +22,8 @@
V5_6("5.6.*"),
V6_2("6.2.*"),
V6_3("6.3.*"),
- V6_4("6.4.*");
+ V6_4("6.4.*"),
+ V6_5("6.5.*");
private final String version;
private final Pattern pattern;
diff --git a/java/com/google/gerrit/git/RefUpdateUtil.java b/java/com/google/gerrit/git/RefUpdateUtil.java
index ee74f2b..520d0f2 100644
--- a/java/com/google/gerrit/git/RefUpdateUtil.java
+++ b/java/com/google/gerrit/git/RefUpdateUtil.java
@@ -125,6 +125,11 @@
case LOCK_FAILURE:
throw new LockFailureException("Failed to update " + ru.getName() + ": " + result, ru);
default:
+ case IO_FAILURE:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
throw new IOException("Failed to update " + ru.getName() + ": " + ru.getResult());
}
}
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 77ce983..e74c4b2 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -15,6 +15,8 @@
package com.google.gerrit.httpd;
import com.google.common.cache.Cache;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -23,6 +25,7 @@
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.audit.HttpAuditEvent;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -30,12 +33,14 @@
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.group.GroupAuditService;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -44,6 +49,7 @@
import com.google.inject.name.Named;
import java.io.IOException;
import java.time.Duration;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -141,6 +147,30 @@
addReceivePackFilter(receiveFilter);
}
+ private static String extractWhat(HttpServletRequest request) {
+ StringBuilder commandName = new StringBuilder(request.getRequestURL());
+ if (request.getQueryString() != null) {
+ commandName.append("?").append(request.getQueryString());
+ }
+ return commandName.toString();
+ }
+
+ private static ListMultimap<String, String> extractParameters(HttpServletRequest request) {
+
+ ListMultimap<String, String> multiMap = ArrayListMultimap.create();
+ if (request.getQueryString() != null) {
+ request
+ .getParameterMap()
+ .forEach(
+ (k, v) -> {
+ for (int i = 0; i < v.length; i++) {
+ multiMap.put(k, v[i]);
+ }
+ });
+ }
+ return multiMap;
+ }
+
static class Resolver implements RepositoryResolver<HttpServletRequest> {
private final GitRepositoryManager manager;
private final PermissionBackend permissionBackend;
@@ -229,6 +259,13 @@
up.setTimeout(config.getTimeout());
up.setPreUploadHook(PreUploadHookChain.newChain(Lists.newArrayList(preUploadHooks)));
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));
+ if (config.enableProtocolV2()) {
+ String header = req.getHeader("Git-Protocol");
+ if (header != null) {
+ String[] params = header.split(":");
+ up.setExtraParameters(Arrays.asList(params));
+ }
+ }
ProjectState state = (ProjectState) req.getAttribute(ATT_STATE);
for (UploadPackInitializer initializer : uploadPackInitializers) {
initializer.init(state.getNameKey(), up);
@@ -240,12 +277,19 @@
static class UploadFilter implements Filter {
private final UploadValidators.Factory uploadValidatorsFactory;
private final PermissionBackend permissionBackend;
+ private final Provider<CurrentUser> userProvider;
+ private final GroupAuditService groupAuditService;
@Inject
UploadFilter(
- UploadValidators.Factory uploadValidatorsFactory, PermissionBackend permissionBackend) {
+ UploadValidators.Factory uploadValidatorsFactory,
+ PermissionBackend permissionBackend,
+ Provider<CurrentUser> userProvider,
+ GroupAuditService groupAuditService) {
this.uploadValidatorsFactory = uploadValidatorsFactory;
this.permissionBackend = permissionBackend;
+ this.userProvider = userProvider;
+ this.groupAuditService = groupAuditService;
}
@Override
@@ -268,7 +312,22 @@
return;
} catch (PermissionBackendException e) {
throw new ServletException(e);
+ } finally {
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
+ HttpServletResponse httpResponse = (HttpServletResponse) response;
+ groupAuditService.dispatch(
+ new HttpAuditEvent(
+ httpRequest.getSession().getId(),
+ userProvider.get(),
+ extractWhat(httpRequest),
+ TimeUtil.nowMs(),
+ extractParameters(httpRequest),
+ httpRequest.getMethod(),
+ httpRequest,
+ httpResponse.getStatus(),
+ httpResponse));
}
+
// We use getRemoteHost() here instead of getRemoteAddr() because REMOTE_ADDR
// may have been overridden by a proxy server -- we'll try to avoid this.
UploadValidators uploadValidators =
@@ -326,15 +385,18 @@
private final Cache<AdvertisedObjectsCacheKey, Set<ObjectId>> cache;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> userProvider;
+ private final GroupAuditService groupAuditService;
@Inject
ReceiveFilter(
@Named(ID_CACHE) Cache<AdvertisedObjectsCacheKey, Set<ObjectId>> cache,
PermissionBackend permissionBackend,
- Provider<CurrentUser> userProvider) {
+ Provider<CurrentUser> userProvider,
+ GroupAuditService groupAuditService) {
this.cache = cache;
this.permissionBackend = permissionBackend;
this.userProvider = userProvider;
+ this.groupAuditService = groupAuditService;
}
@Override
@@ -365,6 +427,20 @@
return;
} catch (PermissionBackendException e) {
throw new RuntimeException(e);
+ } finally {
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
+ HttpServletResponse httpResponse = (HttpServletResponse) response;
+ groupAuditService.dispatch(
+ new HttpAuditEvent(
+ httpRequest.getSession().getId(),
+ userProvider.get(),
+ extractWhat(httpRequest),
+ TimeUtil.nowMs(),
+ extractParameters(httpRequest),
+ httpRequest.getMethod(),
+ httpRequest,
+ httpResponse.getStatus(),
+ httpResponse));
}
if (canUpload != Capable.OK) {
diff --git a/java/com/google/gerrit/httpd/HttpLogoutServlet.java b/java/com/google/gerrit/httpd/HttpLogoutServlet.java
index abfcc22..ab7bfdf 100644
--- a/java/com/google/gerrit/httpd/HttpLogoutServlet.java
+++ b/java/com/google/gerrit/httpd/HttpLogoutServlet.java
@@ -17,8 +17,8 @@
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.AuditEvent;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.audit.AuditEvent;
import com.google.gerrit.server.audit.AuditService;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
diff --git a/java/com/google/gerrit/httpd/init/SiteInitializer.java b/java/com/google/gerrit/httpd/init/SiteInitializer.java
index de4f284..67510cd 100644
--- a/java/com/google/gerrit/httpd/init/SiteInitializer.java
+++ b/java/com/google/gerrit/httpd/init/SiteInitializer.java
@@ -14,19 +14,17 @@
package com.google.gerrit.httpd.init;
+import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.pgm.init.BaseInit;
import com.google.gerrit.pgm.init.PluginsDistribution;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.sql.Connection;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Statement;
import java.util.List;
public final class SiteInitializer {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final String GERRIT_SITE_PATH = "gerrit.site_path";
private final String sitePath;
private final String initPath;
@@ -53,42 +51,29 @@
return;
}
- try (Connection conn = connectToDb()) {
- Path site = getSiteFromReviewDb(conn);
- if (site == null && initPath != null) {
- site = Paths.get(initPath);
- }
- if (site != null) {
- logger.atInfo().log("Initializing site at %s", site.toRealPath().normalize());
- new BaseInit(
- site,
- new ReviewDbDataSourceProvider(),
- false,
- false,
- pluginsDistribution,
- pluginsToInstall)
- .run();
- }
+ String path = System.getProperty(GERRIT_SITE_PATH);
+ Path site = null;
+ if (!Strings.isNullOrEmpty(path)) {
+ site = Paths.get(path);
+ }
+
+ if (site == null && initPath != null) {
+ site = Paths.get(initPath);
+ }
+ if (site != null) {
+ logger.atInfo().log("Initializing site at %s", site.toRealPath().normalize());
+ new BaseInit(
+ site,
+ new ReviewDbDataSourceProvider(),
+ false,
+ false,
+ pluginsDistribution,
+ pluginsToInstall)
+ .run();
}
} catch (Exception e) {
logger.atSevere().withCause(e).log("Site init failed");
throw new RuntimeException(e);
}
}
-
- private Connection connectToDb() throws SQLException {
- return new ReviewDbDataSourceProvider().get().getConnection();
- }
-
- private Path getSiteFromReviewDb(Connection conn) {
- try (Statement stmt = conn.createStatement();
- ResultSet rs = stmt.executeQuery("SELECT site_path FROM system_config")) {
- if (rs.next()) {
- return Paths.get(rs.getString(1));
- }
- } catch (SQLException e) {
- return null;
- }
- return null;
- }
}
diff --git a/java/com/google/gerrit/httpd/init/SitePathFromSystemConfigProvider.java b/java/com/google/gerrit/httpd/init/SitePathFromSystemConfigProvider.java
deleted file mode 100644
index 96ba28b..0000000
--- a/java/com/google/gerrit/httpd/init/SitePathFromSystemConfigProvider.java
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright (C) 2009 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.httpd.init;
-
-import com.google.gerrit.reviewdb.client.SystemConfig;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.config.SitePath;
-import com.google.gerrit.server.schema.ReviewDbFactory;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.List;
-
-/** Provides {@link Path} annotated with {@link SitePath}. */
-class SitePathFromSystemConfigProvider implements Provider<Path> {
- private final Path path;
-
- @Inject
- SitePathFromSystemConfigProvider(@ReviewDbFactory SchemaFactory<ReviewDb> schemaFactory)
- throws OrmException {
- path = read(schemaFactory);
- }
-
- @Override
- public Path get() {
- return path;
- }
-
- private static Path read(SchemaFactory<ReviewDb> schemaFactory) throws OrmException {
- try (ReviewDb db = schemaFactory.open()) {
- List<SystemConfig> all = db.systemConfig().all().toList();
- switch (all.size()) {
- case 1:
- return Paths.get(all.get(0).sitePath);
- 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");
- }
- }
- }
-}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index b83da19..8870d1d 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -48,6 +48,7 @@
import com.google.gerrit.server.account.AccountDeactivator;
import com.google.gerrit.server.account.InternalAccountDirectory;
import com.google.gerrit.server.api.GerritApiModule;
+import com.google.gerrit.server.api.PluginApiModule;
import com.google.gerrit.server.audit.AuditModule;
import com.google.gerrit.server.cache.h2.H2CacheModule;
import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
@@ -60,6 +61,7 @@
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
+import com.google.gerrit.server.config.GerritRuntime;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerConfigModule;
import com.google.gerrit.server.config.SitePath;
@@ -107,6 +109,7 @@
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.Provider;
+import com.google.inject.ProvisionException;
import com.google.inject.name.Names;
import com.google.inject.servlet.GuiceFilter;
import com.google.inject.servlet.GuiceServletContextListener;
@@ -134,6 +137,8 @@
public class WebAppInitializer extends GuiceServletContextListener implements Filter {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private static final String GERRIT_SITE_PATH = "gerrit.site_path";
+
private Path sitePath;
private Injector dbInjector;
private Injector cfgInjector;
@@ -155,9 +160,11 @@
private synchronized void init() {
if (manager == null) {
- final String path = System.getProperty("gerrit.site_path");
+ String path = System.getProperty(GERRIT_SITE_PATH);
if (path != null) {
sitePath = Paths.get(path);
+ } else {
+ throw new ProvisionException(GERRIT_SITE_PATH + " must be defined");
}
if (System.getProperty("gerrit.init") != null) {
@@ -171,7 +178,7 @@
}
new SiteInitializer(
path,
- System.getProperty("gerrit.init_path"),
+ System.getProperty(GERRIT_SITE_PATH),
new UnzippedDistribution(servletContext),
pluginsToInstall)
.init();
@@ -292,21 +299,6 @@
listener().to(ReviewDbDataSourceProvider.class);
}
});
-
- // If we didn't get the site path from the system property
- // we need to get it from the database, as that's our old
- // method of locating the site path on disk.
- //
- modules.add(
- new AbstractModule() {
- @Override
- protected void configure() {
- bind(Path.class)
- .annotatedWith(SitePath.class)
- .toProvider(SitePathFromSystemConfigProvider.class)
- .in(SINGLETON);
- }
- });
modules.add(new GerritServerConfigModule());
}
modules.add(new DatabaseModule());
@@ -336,6 +328,7 @@
modules.add(new MimeUtil2Module());
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
+ modules.add(new PluginApiModule());
modules.add(new SearchingChangeCacheImpl.Module());
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultPermissionBackendModule());
@@ -385,6 +378,7 @@
@Override
protected void configure() {
bind(GerritOptions.class).toInstance(new GerritOptions(false, false, false));
+ bind(GerritRuntime.class).toInstance(GerritRuntime.DAEMON);
}
});
modules.add(new GarbageCollectionModule());
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index e581514..9a24867 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -199,6 +199,7 @@
private AbstractModule luceneModule;
private Module emailModule;
private Module testSysModule;
+ private Module auditEventModule;
private Runnable serverStarted;
private IndexType indexType;
@@ -320,6 +321,11 @@
}
@VisibleForTesting
+ public void setAuditEventModuleForTesting(Module module) {
+ auditEventModule = module;
+ }
+
+ @VisibleForTesting
public void setLuceneModule(LuceneIndexModule m) {
luceneModule = m;
inMemoryTest = true;
@@ -425,7 +431,6 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new PluginApiModule());
- modules.add(new AuditModule());
modules.add(new SearchingChangeCacheImpl.Module(slave));
modules.add(new InternalAccountDirectory.Module());
@@ -438,6 +443,11 @@
} else {
modules.add(new SmtpEmailSender.Module());
}
+ if (auditEventModule != null) {
+ modules.add(auditEventModule);
+ } else {
+ modules.add(new AuditModule());
+ }
modules.add(new SignedTokenEmailTokenVerifier.Module());
modules.add(new RestApiModule());
modules.add(new GpgModule(config));
diff --git a/java/com/google/gerrit/pgm/Init.java b/java/com/google/gerrit/pgm/Init.java
index 1739de9..4ea31da 100644
--- a/java/com/google/gerrit/pgm/Init.java
+++ b/java/com/google/gerrit/pgm/Init.java
@@ -245,7 +245,7 @@
}
private void verifyInstallPluginList(ConsoleUI ui, List<PluginData> plugins) {
- if (nullOrEmpty(installPlugins) || nullOrEmpty(plugins)) {
+ if (nullOrEmpty(installPlugins)) {
return;
}
Set<String> missing = Sets.newHashSet(installPlugins);
diff --git a/java/com/google/gerrit/reviewdb/client/SystemConfig.java b/java/com/google/gerrit/reviewdb/client/SystemConfig.java
deleted file mode 100644
index cd42dd1..0000000
--- a/java/com/google/gerrit/reviewdb/client/SystemConfig.java
+++ /dev/null
@@ -1,89 +0,0 @@
-// Copyright (C) 2008 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.reviewdb.client;
-
-import com.google.gwtorm.client.Column;
-import com.google.gwtorm.client.StringKey;
-
-/** Global configuration needed to serve web requests. */
-public final class SystemConfig {
- public static final class Key extends StringKey<com.google.gwtorm.client.Key<?>> {
- private static final long serialVersionUID = 1L;
-
- private static final String VALUE = "X";
-
- @Column(id = 1, length = 1)
- protected String one = VALUE;
-
- public Key() {}
-
- @Override
- public String get() {
- return VALUE;
- }
-
- @Override
- protected void set(String newValue) {
- assert get().equals(newValue);
- }
- }
-
- /** Construct a new, unconfigured instance. */
- public static SystemConfig create() {
- final SystemConfig r = new SystemConfig();
- r.singleton = new SystemConfig.Key();
- return r;
- }
-
- @Column(id = 1)
- protected Key singleton;
-
- /** Local filesystem location of header/footer/CSS configuration files */
- @Column(id = 3, notNull = false, length = Integer.MAX_VALUE)
- public transient String sitePath;
-
- // DO NOT LOOK BELOW THIS LINE. These fields have all been deleted,
- // but survive to support schema upgrade code.
-
- /** DEPRECATED DO NOT USE */
- @Column(id = 2, length = 36, notNull = false)
- public transient String registerEmailPrivateKey;
- /** DEPRECATED DO NOT USE */
- @Column(id = 4, notNull = false)
- public AccountGroup.Id adminGroupId;
- /** DEPRECATED DO NOT USE */
- @Column(id = 10, notNull = false)
- public AccountGroup.UUID adminGroupUUID;
- /** DEPRECATED DO NOT USE */
- @Column(id = 5, notNull = false)
- public AccountGroup.Id anonymousGroupId;
- /** DEPRECATED DO NOT USE */
- @Column(id = 6, notNull = false)
- public AccountGroup.Id registeredGroupId;
- /** DEPRECATED DO NOT USE */
- @Column(id = 7, notNull = false)
- public Project.NameKey wildProjectName;
- /** DEPRECATED DO NOT USE */
- @Column(id = 9, notNull = false)
- public AccountGroup.Id ownerGroupId;
- /** DEPRECATED DO NOT USE */
- @Column(id = 8, notNull = false)
- public AccountGroup.Id batchUsersGroupId;
- /** DEPRECATED DO NOT USE */
- @Column(id = 11, notNull = false)
- public AccountGroup.UUID batchUsersGroupUUID;
-
- protected SystemConfig() {}
-}
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
index 9cdc0f7..8e4292c 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDb.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
@@ -17,7 +17,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.SystemConfig;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.Relation;
import com.google.gwtorm.server.Schema;
@@ -31,7 +30,6 @@
* <ul>
* <li>{@link Account}: Per-user account registration, preferences, identity.
* <li>{@link Change}: All review information about a single proposed change.
- * <li>{@link SystemConfig}: Server-wide settings, managed by administrator.
* </ul>
*/
public interface ReviewDb extends Schema {
@@ -40,8 +38,7 @@
@Relation(id = 1)
SchemaVersionAccess schemaVersion();
- @Relation(id = 2)
- SystemConfigAccess systemConfig();
+ // Deleted @Relation(id = 2)
// Deleted @Relation(id = 3)
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
index 0deaa57..202729e 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
@@ -110,11 +110,6 @@
}
@Override
- public SystemConfigAccess systemConfig() {
- return delegate.systemConfig();
- }
-
- @Override
public ChangeAccess changes() {
return delegate.changes();
}
diff --git a/java/com/google/gerrit/reviewdb/server/SystemConfigAccess.java b/java/com/google/gerrit/reviewdb/server/SystemConfigAccess.java
deleted file mode 100644
index a2177fd..0000000
--- a/java/com/google/gerrit/reviewdb/server/SystemConfigAccess.java
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (C) 2008 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.reviewdb.server;
-
-import com.google.gerrit.reviewdb.client.SystemConfig;
-import com.google.gwtorm.server.Access;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.PrimaryKey;
-import com.google.gwtorm.server.Query;
-import com.google.gwtorm.server.ResultSet;
-
-/** Access interface for {@link SystemConfig}. */
-public interface SystemConfigAccess extends Access<SystemConfig, SystemConfig.Key> {
- @Override
- @PrimaryKey("singleton")
- SystemConfig get(SystemConfig.Key key) throws OrmException;
-
- @Query
- ResultSet<SystemConfig> all() throws OrmException;
-}
diff --git a/java/com/google/gerrit/server/audit/AuditEvent.java b/java/com/google/gerrit/server/AuditEvent.java
similarity index 97%
rename from java/com/google/gerrit/server/audit/AuditEvent.java
rename to java/com/google/gerrit/server/AuditEvent.java
index 46b2844..773a307 100644
--- a/java/com/google/gerrit/server/audit/AuditEvent.java
+++ b/java/com/google/gerrit/server/AuditEvent.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.audit;
+package com.google.gerrit.server;
import static java.util.Objects.requireNonNull;
@@ -20,7 +20,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.util.time.TimeUtil;
public class AuditEvent {
diff --git a/java/com/google/gerrit/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 3759f09..dc5a262 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -50,8 +50,18 @@
* }
* </pre>
*
- * The option will be prefixed by the plugin name. In the example above, if the plugin name was
+ * <p>The option will be prefixed by the plugin name. In the example above, if the plugin name was
* my-plugin, then the --verbose option as used by the caller would be --my-plugin--verbose.
+ *
+ * <p>Additional options can be annotated with @RequiresOption which will cause them to be ignored
+ * unless the required option is present. For example:
+ *
+ * <pre>
+ * {@literal @}RequiresOptions("--help")
+ * {@literal @}Option(name = "--help-as-json",
+ * usage = "display help text in json format")
+ * public boolean displayHelpAsJson;
+ * </pre>
*/
public interface DynamicBean {}
@@ -261,6 +271,7 @@
for (Entry<String, DynamicBean> e : beansByPlugin.entrySet()) {
clp.parseWithPrefix("--" + e.getKey(), e.getValue());
}
+ clp.drainOptionQueue();
}
public void setDynamicBeans() {
diff --git a/java/com/google/gerrit/server/audit/AuditListener.java b/java/com/google/gerrit/server/audit/AuditListener.java
index 3f8c298..f555bbd 100644
--- a/java/com/google/gerrit/server/audit/AuditListener.java
+++ b/java/com/google/gerrit/server/audit/AuditListener.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.audit;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.AuditEvent;
@ExtensionPoint
public interface AuditListener {
diff --git a/java/com/google/gerrit/server/audit/AuditService.java b/java/com/google/gerrit/server/audit/AuditService.java
index cbca65b..425e22a 100644
--- a/java/com/google/gerrit/server/audit/AuditService.java
+++ b/java/com/google/gerrit/server/audit/AuditService.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.AuditEvent;
import com.google.gerrit.server.audit.group.GroupAuditListener;
import com.google.gerrit.server.audit.group.GroupMemberAuditEvent;
import com.google.gerrit.server.audit.group.GroupSubgroupAuditEvent;
@@ -39,6 +40,7 @@
this.groupAuditListeners = groupAuditListeners;
}
+ @Override
public void dispatch(AuditEvent action) {
auditListeners.runEach(l -> l.onAuditableAction(action));
}
diff --git a/java/com/google/gerrit/server/audit/HttpAuditEvent.java b/java/com/google/gerrit/server/audit/HttpAuditEvent.java
index 11a6b63..5ea2485 100644
--- a/java/com/google/gerrit/server/audit/HttpAuditEvent.java
+++ b/java/com/google/gerrit/server/audit/HttpAuditEvent.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.audit;
import com.google.common.collect.ListMultimap;
+import com.google.gerrit.server.AuditEvent;
import com.google.gerrit.server.CurrentUser;
public class HttpAuditEvent extends AuditEvent {
diff --git a/java/com/google/gerrit/server/audit/SshAuditEvent.java b/java/com/google/gerrit/server/audit/SshAuditEvent.java
index 89f01ac..fee959e 100644
--- a/java/com/google/gerrit/server/audit/SshAuditEvent.java
+++ b/java/com/google/gerrit/server/audit/SshAuditEvent.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.audit;
import com.google.common.collect.ListMultimap;
+import com.google.gerrit.server.AuditEvent;
import com.google.gerrit.server.CurrentUser;
public class SshAuditEvent extends AuditEvent {
diff --git a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
index 66d6555..0bfe5fd 100644
--- a/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
+++ b/java/com/google/gerrit/server/config/ConfigUpdatedEvent.java
@@ -13,10 +13,11 @@
// limitations under the License.
package com.google.gerrit.server.config;
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.Multimap;
import java.util.Collections;
import java.util.LinkedHashSet;
-import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.apache.commons.lang.StringUtils;
@@ -36,6 +37,8 @@
* (+ various overloaded versions of these)
*/
public class ConfigUpdatedEvent {
+ public static final Multimap<UpdateResult, ConfigUpdateEntry> NO_UPDATES =
+ new ImmutableMultimap.Builder<UpdateResult, ConfigUpdateEntry>().build();
private final Config oldConfig;
private final Config newConfig;
@@ -52,25 +55,29 @@
return this.newConfig;
}
- public Update accept(ConfigKey entry) {
+ private String getString(ConfigKey key, Config config) {
+ return config.getString(key.section(), key.subsection(), key.name());
+ }
+
+ public Multimap<UpdateResult, ConfigUpdateEntry> accept(ConfigKey entry) {
return accept(Collections.singleton(entry));
}
- public Update accept(Set<ConfigKey> entries) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> accept(Set<ConfigKey> entries) {
return createUpdate(entries, UpdateResult.APPLIED);
}
- public Update accept(String section) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> accept(String section) {
Set<ConfigKey> entries = getEntriesFromSection(oldConfig, section);
entries.addAll(getEntriesFromSection(newConfig, section));
return createUpdate(entries, UpdateResult.APPLIED);
}
- public Update reject(ConfigKey entry) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> reject(ConfigKey entry) {
return reject(Collections.singleton(entry));
}
- public Update reject(Set<ConfigKey> entries) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> reject(Set<ConfigKey> entries) {
return createUpdate(entries, UpdateResult.REJECTED);
}
@@ -87,20 +94,15 @@
return res;
}
- private Update createUpdate(Set<ConfigKey> entries, UpdateResult updateResult) {
- Update update = new Update(updateResult);
+ private Multimap<UpdateResult, ConfigUpdateEntry> createUpdate(
+ Set<ConfigKey> entries, UpdateResult updateResult) {
+ Multimap<UpdateResult, ConfigUpdateEntry> updates = ArrayListMultimap.create();
entries
.stream()
.filter(this::isValueUpdated)
- .forEach(
- key -> {
- update.addConfigUpdate(
- new ConfigUpdateEntry(
- key,
- oldConfig.getString(key.section(), key.subsection(), key.name()),
- newConfig.getString(key.section(), key.subsection(), key.name())));
- });
- return update;
+ .map(e -> new ConfigUpdateEntry(e, getString(e, oldConfig), getString(e, newConfig)))
+ .forEach(e -> updates.put(updateResult, e));
+ return updates;
}
public boolean isSectionUpdated(String section) {
@@ -142,31 +144,6 @@
}
}
- /**
- * One Accepted/Rejected Update have one or more config updates (ConfigUpdateEntry) tied to it.
- */
- public static class Update {
- private UpdateResult result;
- private final Set<ConfigUpdateEntry> configUpdates;
-
- public Update(UpdateResult result) {
- this.configUpdates = new LinkedHashSet<>();
- this.result = result;
- }
-
- public UpdateResult getResult() {
- return result;
- }
-
- public List<ConfigUpdateEntry> getConfigUpdates() {
- return ImmutableList.copyOf(configUpdates);
- }
-
- public void addConfigUpdate(ConfigUpdateEntry entry) {
- this.configUpdates.add(entry);
- }
- }
-
public enum ConfigEntryType {
ADDED,
REMOVED,
diff --git a/java/com/google/gerrit/server/config/GerritConfigListener.java b/java/com/google/gerrit/server/config/GerritConfigListener.java
index 337a962..f5b2976 100644
--- a/java/com/google/gerrit/server/config/GerritConfigListener.java
+++ b/java/com/google/gerrit/server/config/GerritConfigListener.java
@@ -14,9 +14,11 @@
package com.google.gerrit.server.config;
+import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import java.util.EventListener;
-import java.util.List;
/**
* Implementations of the GerritConfigListener interface expects to react GerritServerConfig
@@ -24,5 +26,5 @@
*/
@ExtensionPoint
public interface GerritConfigListener extends EventListener {
- List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event);
+ Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event);
}
diff --git a/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java b/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
index 1dfa3fc..d21e1c3 100644
--- a/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
+++ b/java/com/google/gerrit/server/config/GerritConfigListenerHelper.java
@@ -15,13 +15,12 @@
package com.google.gerrit.server.config;
import com.google.common.collect.ImmutableSet;
-import java.util.Collections;
public class GerritConfigListenerHelper {
public static GerritConfigListener acceptIfChanged(ConfigKey... keys) {
return e ->
e.isEntriesUpdated(ImmutableSet.copyOf(keys))
- ? Collections.singletonList(e.accept(ImmutableSet.copyOf(keys)))
- : Collections.emptyList();
+ ? e.accept(ImmutableSet.copyOf(keys))
+ : ConfigUpdatedEvent.NO_UPDATES;
}
}
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
index 1890de8..09c10740 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
@@ -14,12 +14,14 @@
package com.google.gerrit.server.config;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import java.util.ArrayList;
-import java.util.List;
/** Issues a configuration reload from the GerritServerConfigProvider and notify all listeners. */
@Singleton
@@ -40,18 +42,20 @@
* Reloads the Gerrit Server Configuration from disk. Synchronized to ensure that one issued
* reload is fully completed before a new one starts.
*/
- public List<ConfigUpdatedEvent.Update> reloadConfig() {
+ public Multimap<UpdateResult, ConfigUpdateEntry> reloadConfig() {
logger.atInfo().log("Starting server configuration reload");
- List<ConfigUpdatedEvent.Update> updates = fireUpdatedConfigEvent(configProvider.updateConfig());
+ Multimap<UpdateResult, ConfigUpdateEntry> updates =
+ fireUpdatedConfigEvent(configProvider.updateConfig());
logger.atInfo().log("Server configuration reload completed succesfully");
return updates;
}
- public List<ConfigUpdatedEvent.Update> fireUpdatedConfigEvent(ConfigUpdatedEvent event) {
- ArrayList<ConfigUpdatedEvent.Update> result = new ArrayList<>();
+ public Multimap<UpdateResult, ConfigUpdateEntry> fireUpdatedConfigEvent(
+ ConfigUpdatedEvent event) {
+ Multimap<UpdateResult, ConfigUpdateEntry> updates = ArrayListMultimap.create();
for (GerritConfigListener configListener : configListeners) {
- result.addAll(configListener.configUpdated(event));
+ updates.putAll(configListener.configUpdated(event));
}
- return result;
+ return updates;
}
}
diff --git a/java/com/google/gerrit/server/git/TransferConfig.java b/java/com/google/gerrit/server/git/TransferConfig.java
index 55b9448..dde524f 100644
--- a/java/com/google/gerrit/server/git/TransferConfig.java
+++ b/java/com/google/gerrit/server/git/TransferConfig.java
@@ -29,6 +29,7 @@
private final long maxObjectSizeLimit;
private final String maxObjectSizeLimitFormatted;
private final boolean inheritProjectMaxObjectSizeLimit;
+ private final boolean enableProtocolV2;
@Inject
TransferConfig(@GerritServerConfig Config cfg) {
@@ -45,6 +46,7 @@
maxObjectSizeLimitFormatted = cfg.getString("receive", null, "maxObjectSizeLimit");
inheritProjectMaxObjectSizeLimit =
cfg.getBoolean("receive", "inheritProjectMaxObjectSizeLimit", false);
+ enableProtocolV2 = cfg.getBoolean("receive", "enableProtocolV2", false);
packConfig = new PackConfig();
packConfig.setDeltaCompress(false);
@@ -72,4 +74,8 @@
public boolean inheritProjectMaxObjectSizeLimit() {
return inheritProjectMaxObjectSizeLimit;
}
+
+ public boolean enableProtocolV2() {
+ return enableProtocolV2;
+ }
}
diff --git a/java/com/google/gerrit/server/group/GroupAuditService.java b/java/com/google/gerrit/server/group/GroupAuditService.java
index c543a6e..4b851ea 100644
--- a/java/com/google/gerrit/server/group/GroupAuditService.java
+++ b/java/com/google/gerrit/server/group/GroupAuditService.java
@@ -18,9 +18,11 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Account.Id;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.AuditEvent;
import java.sql.Timestamp;
public interface GroupAuditService {
+ void dispatch(AuditEvent action);
void dispatchAddMembers(
Account.Id actor,
diff --git a/java/com/google/gerrit/server/project/CommentLinkProvider.java b/java/com/google/gerrit/server/project/CommentLinkProvider.java
index 56cf51e..4987d00 100644
--- a/java/com/google/gerrit/server/project/CommentLinkProvider.java
+++ b/java/com/google/gerrit/server/project/CommentLinkProvider.java
@@ -16,15 +16,17 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.gerrit.server.config.GerritConfigListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.util.Collections;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
@@ -64,11 +66,11 @@
}
@Override
- public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
if (event.isSectionUpdated(ProjectConfig.COMMENTLINK)) {
commentLinks = parseConfig(event.getNewConfig());
- return Collections.singletonList(event.accept(ProjectConfig.COMMENTLINK));
+ return event.accept(ProjectConfig.COMMENTLINK);
}
- return Collections.emptyList();
+ return ConfigUpdatedEvent.NO_UPDATES;
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectCreator.java b/java/com/google/gerrit/server/project/ProjectCreator.java
new file mode 100644
index 0000000..b50b046
--- /dev/null
+++ b/java/com/google/gerrit/server/project/ProjectCreator.java
@@ -0,0 +1,255 @@
+// 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.project;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.GroupDescription;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.events.NewProjectCreatedListener;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.config.RepositoryConfig;
+import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+public class ProjectCreator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final GitRepositoryManager repoManager;
+ private final PluginSetContext<NewProjectCreatedListener> createdListeners;
+ private final ProjectCache projectCache;
+ private final GroupBackend groupBackend;
+ private final MetaDataUpdate.User metaDataUpdateFactory;
+ private final GitReferenceUpdated referenceUpdated;
+ private final RepositoryConfig repositoryCfg;
+ private final Provider<PersonIdent> serverIdent;
+ private final Provider<IdentifiedUser> identifiedUser;
+ private final ProjectConfig.Factory projectConfigFactory;
+
+ @Inject
+ ProjectCreator(
+ GitRepositoryManager repoManager,
+ PluginSetContext<NewProjectCreatedListener> createdListeners,
+ ProjectCache projectCache,
+ GroupBackend groupBackend,
+ MetaDataUpdate.User metaDataUpdateFactory,
+ GitReferenceUpdated referenceUpdated,
+ RepositoryConfig repositoryCfg,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
+ Provider<IdentifiedUser> identifiedUser,
+ ProjectConfig.Factory projectConfigFactory) {
+ this.repoManager = repoManager;
+ this.createdListeners = createdListeners;
+ this.projectCache = projectCache;
+ this.groupBackend = groupBackend;
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.referenceUpdated = referenceUpdated;
+ this.repositoryCfg = repositoryCfg;
+ this.serverIdent = serverIdent;
+ this.identifiedUser = identifiedUser;
+ this.projectConfigFactory = projectConfigFactory;
+ }
+
+ public ProjectState createProject(CreateProjectArgs args)
+ throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
+ final Project.NameKey nameKey = args.getProject();
+ try {
+ final String head = args.permissionsOnly ? RefNames.REFS_CONFIG : args.branch.get(0);
+ try (Repository repo = repoManager.openRepository(nameKey)) {
+ if (repo.getObjectDatabase().exists()) {
+ throw new ResourceConflictException("project \"" + nameKey + "\" exists");
+ }
+ } catch (RepositoryNotFoundException e) {
+ // It does not exist, safe to ignore.
+ }
+ try (Repository repo = repoManager.createRepository(nameKey)) {
+ RefUpdate u = repo.updateRef(Constants.HEAD);
+ u.disableRefLog();
+ u.link(head);
+
+ createProjectConfig(args);
+
+ if (!args.permissionsOnly && args.createEmptyCommit) {
+ createEmptyCommits(repo, nameKey, args.branch);
+ }
+
+ fire(nameKey, head);
+
+ return projectCache.get(nameKey);
+ }
+ } catch (RepositoryCaseMismatchException e) {
+ throw new ResourceConflictException(
+ "Cannot create "
+ + nameKey.get()
+ + " because the name is already occupied by another project."
+ + " The other project has the same name, only spelled in a"
+ + " different case.");
+ } catch (RepositoryNotFoundException badName) {
+ throw new BadRequestException("invalid project name: " + nameKey);
+ } catch (ConfigInvalidException e) {
+ String msg = "Cannot create " + nameKey;
+ logger.atSevere().withCause(e).log(msg);
+ throw e;
+ }
+ }
+
+ private void createProjectConfig(CreateProjectArgs args)
+ throws IOException, ConfigInvalidException {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
+ ProjectConfig config = projectConfigFactory.read(md);
+
+ Project newProject = config.getProject();
+ newProject.setDescription(args.projectDescription);
+ newProject.setSubmitType(
+ MoreObjects.firstNonNull(
+ args.submitType, repositoryCfg.getDefaultSubmitType(args.getProject())));
+ newProject.setBooleanConfig(
+ BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, args.contributorAgreements);
+ newProject.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, args.signedOffBy);
+ newProject.setBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE, args.contentMerge);
+ newProject.setBooleanConfig(
+ BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
+ args.newChangeForAllNotInTarget);
+ newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
+ newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
+ newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
+ newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);
+ newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_SIGNED_PUSH, args.requireSignedPush);
+ if (args.newParent != null) {
+ newProject.setParentName(args.newParent);
+ }
+
+ if (!args.ownerIds.isEmpty()) {
+ AccessSection all = config.getAccessSection(AccessSection.ALL, true);
+ for (AccountGroup.UUID ownerId : args.ownerIds) {
+ GroupDescription.Basic g = groupBackend.get(ownerId);
+ if (g != null) {
+ GroupReference group = config.resolve(GroupReference.forGroup(g));
+ all.getPermission(Permission.OWNER, true).add(new PermissionRule(group));
+ }
+ }
+ }
+
+ md.setMessage("Created project\n");
+ config.commit(md);
+ md.getRepository().setGitwebDescription(args.projectDescription);
+ }
+ projectCache.onCreateProject(args.getProject());
+ }
+
+ private void createEmptyCommits(Repository repo, Project.NameKey project, List<String> refs)
+ throws IOException {
+ try (ObjectInserter oi = repo.newObjectInserter()) {
+ CommitBuilder cb = new CommitBuilder();
+ cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
+ cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent());
+ cb.setCommitter(serverIdent.get());
+ cb.setMessage("Initial empty repository\n");
+
+ ObjectId id = oi.insert(cb);
+ oi.flush();
+
+ for (String ref : refs) {
+ RefUpdate ru = repo.updateRef(ref);
+ ru.setNewObjectId(id);
+ Result result = ru.update();
+ switch (result) {
+ case NEW:
+ referenceUpdated.fire(
+ project, ru, ReceiveCommand.Type.CREATE, identifiedUser.get().state());
+ break;
+ case FAST_FORWARD:
+ case FORCED:
+ case IO_FAILURE:
+ case LOCK_FAILURE:
+ case NOT_ATTEMPTED:
+ case NO_CHANGE:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case RENAMED:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
+ default:
+ {
+ throw new IOException(
+ String.format("Failed to create ref \"%s\": %s", ref, result.name()));
+ }
+ }
+ }
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log("Cannot create empty commit for %s", project.get());
+ throw e;
+ }
+ }
+
+ private void fire(Project.NameKey name, String head) {
+ if (createdListeners.isEmpty()) {
+ return;
+ }
+
+ ProjectCreator.Event event = new ProjectCreator.Event(name, head);
+ createdListeners.runEach(l -> l.onNewProjectCreated(event));
+ }
+
+ static class Event extends AbstractNoNotifyEvent implements NewProjectCreatedListener.Event {
+ private final Project.NameKey name;
+ private final String head;
+
+ Event(Project.NameKey name, String head) {
+ this.name = name;
+ this.head = head;
+ }
+
+ @Override
+ public String getProjectName() {
+ return name.get();
+ }
+
+ @Override
+ public String getHeadName() {
+ return head;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
index de3c3ee..cab07e3 100644
--- a/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
+++ b/java/com/google/gerrit/server/restapi/config/ReloadConfig.java
@@ -16,12 +16,12 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
+import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.api.config.ConfigUpdateEntryInfo;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.config.ConfigResource;
-import com.google.gerrit.server.config.ConfigUpdatedEvent;
import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.gerrit.server.config.GerritServerConfigReloader;
@@ -29,10 +29,11 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
-import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
public class ReloadConfig implements RestModifyView<ConfigResource, Input> {
@@ -49,25 +50,22 @@
public Map<String, List<ConfigUpdateEntryInfo>> apply(ConfigResource resource, Input input)
throws RestApiException, PermissionBackendException {
permissions.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
-
- List<ConfigUpdatedEvent.Update> updates = config.reloadConfig();
-
- Map<String, List<ConfigUpdateEntryInfo>> reply = new HashMap<>();
- for (UpdateResult result : UpdateResult.values()) {
- reply.put(result.name().toLowerCase(), new ArrayList<>());
- }
+ Multimap<UpdateResult, ConfigUpdateEntry> updates = config.reloadConfig();
if (updates.isEmpty()) {
- return reply;
+ return Collections.emptyMap();
}
- updates
+ return updates
+ .asMap()
+ .entrySet()
.stream()
- .forEach(u -> reply.get(u.getResult().name().toLowerCase()).addAll(toEntryInfos(u)));
- return reply;
+ .collect(
+ Collectors.toMap(
+ e -> e.getKey().name().toLowerCase(), e -> toEntryInfos(e.getValue())));
}
- private static List<ConfigUpdateEntryInfo> toEntryInfos(ConfigUpdatedEvent.Update update) {
- return update
- .getConfigUpdates()
+ private static List<ConfigUpdateEntryInfo> toEntryInfos(
+ Collection<ConfigUpdateEntry> updateEntries) {
+ return updateEntries
.stream()
.map(ReloadConfig::toConfigUpdateEntryInfo)
.collect(toImmutableList());
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 60a24d8..3785784 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -19,21 +19,14 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.ProjectUtil;
-import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
-import com.google.gerrit.common.data.GroupDescription;
-import com.google.gerrit.common.data.GroupReference;
-import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ProjectInfo;
-import com.google.gerrit.extensions.events.NewProjectCreatedListener;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -41,29 +34,17 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ProjectOwnerGroupsProvider;
-import com.google.gerrit.server.config.RepositoryConfig;
-import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.RepositoryCaseMismatchException;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.plugincontext.PluginItemContext;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.CreateProjectArgs;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCreator;
import com.google.gerrit.server.project.ProjectJson;
import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectResource;
@@ -79,84 +60,47 @@
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.transport.ReceiveCommand;
@RequiresCapability(GlobalCapability.CREATE_PROJECT)
@Singleton
public class CreateProject
implements RestCollectionCreateView<TopLevelResource, ProjectResource, ProjectInput> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
private final Provider<ProjectsCollection> projectsCollection;
private final Provider<GroupResolver> groupResolver;
private final PluginSetContext<ProjectCreationValidationListener>
projectCreationValidationListeners;
private final ProjectJson json;
- private final GitRepositoryManager repoManager;
- private final PluginSetContext<NewProjectCreatedListener> createdListeners;
- private final ProjectCache projectCache;
- private final GroupBackend groupBackend;
private final ProjectOwnerGroupsProvider.Factory projectOwnerGroups;
- private final MetaDataUpdate.User metaDataUpdateFactory;
- private final GitReferenceUpdated referenceUpdated;
- private final RepositoryConfig repositoryCfg;
- private final Provider<PersonIdent> serverIdent;
- private final Provider<IdentifiedUser> identifiedUser;
private final Provider<PutConfig> putConfig;
private final AllProjectsName allProjects;
private final AllUsersName allUsers;
private final PluginItemContext<ProjectNameLockManager> lockManager;
- private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectCreator projectCreator;
@Inject
CreateProject(
+ ProjectCreator projectCreator,
Provider<ProjectsCollection> projectsCollection,
Provider<GroupResolver> groupResolver,
ProjectJson json,
PluginSetContext<ProjectCreationValidationListener> projectCreationValidationListeners,
- GitRepositoryManager repoManager,
- PluginSetContext<NewProjectCreatedListener> createdListeners,
- ProjectCache projectCache,
- GroupBackend groupBackend,
ProjectOwnerGroupsProvider.Factory projectOwnerGroups,
- MetaDataUpdate.User metaDataUpdateFactory,
- GitReferenceUpdated referenceUpdated,
- RepositoryConfig repositoryCfg,
- @GerritPersonIdent Provider<PersonIdent> serverIdent,
- Provider<IdentifiedUser> identifiedUser,
Provider<PutConfig> putConfig,
AllProjectsName allProjects,
AllUsersName allUsers,
- PluginItemContext<ProjectNameLockManager> lockManager,
- ProjectConfig.Factory projectConfigFactory) {
+ PluginItemContext<ProjectNameLockManager> lockManager) {
this.projectsCollection = projectsCollection;
+ this.projectCreator = projectCreator;
this.groupResolver = groupResolver;
this.projectCreationValidationListeners = projectCreationValidationListeners;
this.json = json;
- this.repoManager = repoManager;
- this.createdListeners = createdListeners;
- this.projectCache = projectCache;
- this.groupBackend = groupBackend;
this.projectOwnerGroups = projectOwnerGroups;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
- this.referenceUpdated = referenceUpdated;
- this.repositoryCfg = repositoryCfg;
- this.serverIdent = serverIdent;
- this.identifiedUser = identifiedUser;
this.putConfig = putConfig;
this.allProjects = allProjects;
this.allUsers = allUsers;
this.lockManager = lockManager;
- this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -227,7 +171,7 @@
throw new ResourceConflictException(e.getMessage(), e);
}
- ProjectState projectState = createProject(args);
+ ProjectState projectState = projectCreator.createProject(args);
requireNonNull(
projectState,
() -> String.format("failed to create project %s", args.getProject().get()));
@@ -243,93 +187,6 @@
}
}
- private ProjectState createProject(CreateProjectArgs args)
- throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
- final Project.NameKey nameKey = args.getProject();
- try {
- final String head = args.permissionsOnly ? RefNames.REFS_CONFIG : args.branch.get(0);
- try (Repository repo = repoManager.openRepository(nameKey)) {
- if (repo.getObjectDatabase().exists()) {
- throw new ResourceConflictException("project \"" + nameKey + "\" exists");
- }
- } catch (RepositoryNotFoundException e) {
- // It does not exist, safe to ignore.
- }
- try (Repository repo = repoManager.createRepository(nameKey)) {
- RefUpdate u = repo.updateRef(Constants.HEAD);
- u.disableRefLog();
- u.link(head);
-
- createProjectConfig(args);
-
- if (!args.permissionsOnly && args.createEmptyCommit) {
- createEmptyCommits(repo, nameKey, args.branch);
- }
-
- fire(nameKey, head);
-
- return projectCache.get(nameKey);
- }
- } catch (RepositoryCaseMismatchException e) {
- throw new ResourceConflictException(
- "Cannot create "
- + nameKey.get()
- + " because the name is already occupied by another project."
- + " The other project has the same name, only spelled in a"
- + " different case.");
- } catch (RepositoryNotFoundException badName) {
- throw new BadRequestException("invalid project name: " + nameKey);
- } catch (ConfigInvalidException e) {
- String msg = "Cannot create " + nameKey;
- logger.atSevere().withCause(e).log(msg);
- throw e;
- }
- }
-
- private void createProjectConfig(CreateProjectArgs args)
- throws IOException, ConfigInvalidException {
- try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
- ProjectConfig config = projectConfigFactory.read(md);
-
- Project newProject = config.getProject();
- newProject.setDescription(args.projectDescription);
- newProject.setSubmitType(
- MoreObjects.firstNonNull(
- args.submitType, repositoryCfg.getDefaultSubmitType(args.getProject())));
- newProject.setBooleanConfig(
- BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, args.contributorAgreements);
- newProject.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, args.signedOffBy);
- newProject.setBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE, args.contentMerge);
- newProject.setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- args.newChangeForAllNotInTarget);
- newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
- newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
- newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
- newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);
- newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_SIGNED_PUSH, args.requireSignedPush);
- if (args.newParent != null) {
- newProject.setParentName(args.newParent);
- }
-
- if (!args.ownerIds.isEmpty()) {
- AccessSection all = config.getAccessSection(AccessSection.ALL, true);
- for (AccountGroup.UUID ownerId : args.ownerIds) {
- GroupDescription.Basic g = groupBackend.get(ownerId);
- if (g != null) {
- GroupReference group = config.resolve(GroupReference.forGroup(g));
- all.getPermission(Permission.OWNER, true).add(new PermissionRule(group));
- }
- }
- }
-
- md.setMessage("Created project\n");
- config.commit(md);
- md.getRepository().setGitwebDescription(args.projectDescription);
- }
- projectCache.onCreateProject(args.getProject());
- }
-
private List<String> normalizeBranchNames(List<String> branches) throws BadRequestException {
if (branches == null || branches.isEmpty()) {
return Collections.singletonList(Constants.R_HEADS + Constants.MASTER);
@@ -350,78 +207,4 @@
}
return normalizedBranches;
}
-
- private void createEmptyCommits(Repository repo, Project.NameKey project, List<String> refs)
- throws IOException {
- try (ObjectInserter oi = repo.newObjectInserter()) {
- CommitBuilder cb = new CommitBuilder();
- cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
- cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent());
- cb.setCommitter(serverIdent.get());
- cb.setMessage("Initial empty repository\n");
-
- ObjectId id = oi.insert(cb);
- oi.flush();
-
- for (String ref : refs) {
- RefUpdate ru = repo.updateRef(ref);
- ru.setNewObjectId(id);
- Result result = ru.update();
- switch (result) {
- case NEW:
- referenceUpdated.fire(
- project, ru, ReceiveCommand.Type.CREATE, identifiedUser.get().state());
- break;
- case FAST_FORWARD:
- case FORCED:
- case IO_FAILURE:
- case LOCK_FAILURE:
- case NOT_ATTEMPTED:
- case NO_CHANGE:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case RENAMED:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- default:
- {
- throw new IOException(
- String.format("Failed to create ref \"%s\": %s", ref, result.name()));
- }
- }
- }
- } catch (IOException e) {
- logger.atSevere().withCause(e).log("Cannot create empty commit for %s", project.get());
- throw e;
- }
- }
-
- private void fire(Project.NameKey name, String head) {
- if (createdListeners.isEmpty()) {
- return;
- }
-
- Event event = new Event(name, head);
- createdListeners.runEach(l -> l.onNewProjectCreated(event));
- }
-
- static class Event extends AbstractNoNotifyEvent implements NewProjectCreatedListener.Event {
- private final Project.NameKey name;
- private final String head;
-
- Event(Project.NameKey name, String head) {
- this.name = name;
- this.head = head;
- }
-
- @Override
- public String getProjectName() {
- return name.get();
- }
-
- @Override
- public String getHeadName() {
- return head;
- }
- }
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index 15cb7f8..12aaf76 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -19,6 +19,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.api.projects.ParentInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -32,6 +33,8 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ConfigKey;
import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.gerrit.server.config.GerritConfigListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
@@ -46,8 +49,6 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
-import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
@@ -175,18 +176,18 @@
}
@Override
- public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
ConfigKey receiveSetParent = ConfigKey.create("receive", "allowProjectOwnersToChangeParent");
if (!event.isValueUpdated(receiveSetParent)) {
- return Collections.emptyList();
+ return ConfigUpdatedEvent.NO_UPDATES;
}
try {
boolean enabled =
event.getNewConfig().getBoolean("receive", "allowProjectOwnersToChangeParent", false);
this.allowProjectOwnersToChangeParent = enabled;
- return Collections.singletonList(event.accept(receiveSetParent));
} catch (IllegalArgumentException iae) {
- return Collections.singletonList(event.reject(receiveSetParent));
+ return event.reject(receiveSetParent);
}
+ return event.accept(receiveSetParent);
}
}
diff --git a/java/com/google/gerrit/server/schema/GroupRebuilder.java b/java/com/google/gerrit/server/schema/GroupRebuilder.java
index 54cbb86..0157025a 100644
--- a/java/com/google/gerrit/server/schema/GroupRebuilder.java
+++ b/java/com/google/gerrit/server/schema/GroupRebuilder.java
@@ -275,8 +275,7 @@
* Distinct event types.
*
* <p>Events at the same time by the same user are batched together by type. The types should
- * correspond to the possible batch operations supported by {@link
- * com.google.gerrit.server.audit.AuditService}.
+ * correspond to the possible batch operations supported by AuditService.
*/
enum Type {
ADD_MEMBER,
diff --git a/java/com/google/gerrit/server/schema/ReviewDbSchemaCreator.java b/java/com/google/gerrit/server/schema/ReviewDbSchemaCreator.java
index 2aaf26e..3a42f07 100644
--- a/java/com/google/gerrit/server/schema/ReviewDbSchemaCreator.java
+++ b/java/com/google/gerrit/server/schema/ReviewDbSchemaCreator.java
@@ -21,7 +21,6 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
-import com.google.gerrit.reviewdb.client.SystemConfig;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.Sequences;
@@ -150,7 +149,6 @@
GroupReference admins = createGroupReference("Administrators");
GroupReference batchUsers = createGroupReference("Non-Interactive Users");
- initSystemConfig(db);
allProjectsCreator.setAdministrators(admins).setBatchUsers(batchUsers).create();
// We have to create the All-Users repository before we can use it to store the groups in it.
allUsersCreator.setAdministrators(admins).create();
@@ -274,15 +272,4 @@
.setGroupUUID(groupReference.getUUID())
.build();
}
-
- private SystemConfig initSystemConfig(ReviewDb db) throws OrmException {
- SystemConfig s = SystemConfig.create();
- try {
- s.sitePath = site_path.toRealPath().normalize().toString();
- } catch (IOException e) {
- s.sitePath = site_path.toAbsolutePath().normalize().toString();
- }
- db.systemConfig().insert(Collections.singleton(s));
- return s;
- }
}
diff --git a/java/com/google/gerrit/server/schema/ReviewDbSchemaUpdater.java b/java/com/google/gerrit/server/schema/ReviewDbSchemaUpdater.java
index aa6e9f8..86f4560 100644
--- a/java/com/google/gerrit/server/schema/ReviewDbSchemaUpdater.java
+++ b/java/com/google/gerrit/server/schema/ReviewDbSchemaUpdater.java
@@ -16,7 +16,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
-import com.google.gerrit.reviewdb.client.SystemConfig;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.GerritPersonIdent;
@@ -38,7 +37,6 @@
import com.google.inject.Stage;
import java.io.IOException;
import java.sql.SQLException;
-import java.util.Collections;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
@@ -46,18 +44,15 @@
/** Creates or updates the current database schema. */
public class ReviewDbSchemaUpdater {
private final SchemaFactory<ReviewDb> schema;
- private final SitePaths site;
private final ReviewDbSchemaCreator creator;
private final Provider<ReviewDbSchemaVersion> updater;
@Inject
ReviewDbSchemaUpdater(
@ReviewDbFactory SchemaFactory<ReviewDb> schema,
- SitePaths site,
ReviewDbSchemaCreator creator,
Injector parent) {
this.schema = schema;
- this.site = site;
this.creator = creator;
this.updater = buildInjector(parent).getProvider(ReviewDbSchemaVersion.class);
}
@@ -119,8 +114,6 @@
} catch (SQLException e) {
throw new OrmException("Cannot upgrade schema", e);
}
-
- updateSystemConfig(db);
}
}
}
@@ -137,17 +130,4 @@
return null;
}
}
-
- private void updateSystemConfig(ReviewDb db) throws OrmException {
- final SystemConfig sc = db.systemConfig().get(new SystemConfig.Key());
- if (sc == null) {
- throw new OrmException("No record in system_config table");
- }
- try {
- sc.sitePath = site.site_path.toRealPath().normalize().toString();
- } catch (IOException e) {
- sc.sitePath = site.site_path.toAbsolutePath().normalize().toString();
- }
- db.systemConfig().update(Collections.singleton(sc));
- }
}
diff --git a/java/com/google/gerrit/server/schema/ReviewDbSchemaVersion.java b/java/com/google/gerrit/server/schema/ReviewDbSchemaVersion.java
index 46dd600..10a2d39 100644
--- a/java/com/google/gerrit/server/schema/ReviewDbSchemaVersion.java
+++ b/java/com/google/gerrit/server/schema/ReviewDbSchemaVersion.java
@@ -36,7 +36,7 @@
/** A version of the database schema. */
public abstract class ReviewDbSchemaVersion {
/** The current schema version. */
- public static final Class<Schema_169> C = Schema_169.class;
+ public static final Class<Schema_170> C = Schema_170.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/java/com/google/gerrit/server/schema/Schema_151.java b/java/com/google/gerrit/server/schema/Schema_151.java
index d244018..0e8700f 100644
--- a/java/com/google/gerrit/server/schema/Schema_151.java
+++ b/java/com/google/gerrit/server/schema/Schema_151.java
@@ -60,7 +60,7 @@
PreparedStatement addedOnRetrieval, AccountGroup.Id groupId) throws SQLException {
addedOnRetrieval.setInt(1, groupId.get());
try (ResultSet resultSet = addedOnRetrieval.executeQuery()) {
- if (resultSet.first()) {
+ if (resultSet.next()) {
return Optional.of(resultSet.getTimestamp(1));
}
}
diff --git a/java/com/google/gerrit/server/schema/Schema_170.java b/java/com/google/gerrit/server/schema/Schema_170.java
new file mode 100644
index 0000000..6a86494
--- /dev/null
+++ b/java/com/google/gerrit/server/schema/Schema_170.java
@@ -0,0 +1,25 @@
+// 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.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class Schema_170 extends ReviewDbSchemaVersion {
+ @Inject
+ Schema_170(Provider<Schema_169> prior) {
+ super(prior);
+ }
+}
diff --git a/java/com/google/gerrit/server/schema/Schema_87.java b/java/com/google/gerrit/server/schema/Schema_87.java
index fe21565..79884ba 100644
--- a/java/com/google/gerrit/server/schema/Schema_87.java
+++ b/java/com/google/gerrit/server/schema/Schema_87.java
@@ -59,7 +59,7 @@
PreparedStatement uuidRetrieval, AccountGroup.Id id) throws SQLException {
uuidRetrieval.setInt(1, id.get());
try (ResultSet uuidResults = uuidRetrieval.executeQuery()) {
- if (uuidResults.first()) {
+ if (uuidResults.next()) {
Optional.of(new AccountGroup.UUID(uuidResults.getString(1)));
}
}
diff --git a/java/com/google/gerrit/sshd/AbstractGitCommand.java b/java/com/google/gerrit/sshd/AbstractGitCommand.java
index c49ae82..921b416 100644
--- a/java/com/google/gerrit/sshd/AbstractGitCommand.java
+++ b/java/com/google/gerrit/sshd/AbstractGitCommand.java
@@ -29,6 +29,8 @@
import org.kohsuke.args4j.Argument;
public abstract class AbstractGitCommand extends BaseCommand {
+ private static final String GIT_PROTOCOL = "GIT_PROTOCOL";
+
@Argument(index = 0, metaVar = "PROJECT.git", required = true, usage = "project name")
protected ProjectState projectState;
@@ -45,9 +47,15 @@
protected Repository repo;
protected Project.NameKey projectName;
protected Project project;
+ protected String[] extraParameters;
@Override
public void start(Environment env) {
+ String gitProtocol = env.getEnv().get(GIT_PROTOCOL);
+ if (gitProtocol != null) {
+ extraParameters = gitProtocol.split(":");
+ }
+
Context ctx = context.subContext(newSession(), context.getCommandLine());
final Context old = sshScope.set(ctx);
try {
diff --git a/java/com/google/gerrit/sshd/SshLog.java b/java/com/google/gerrit/sshd/SshLog.java
index 0e34889..df3242c 100644
--- a/java/com/google/gerrit/sshd/SshLog.java
+++ b/java/com/google/gerrit/sshd/SshLog.java
@@ -15,6 +15,7 @@
package com.google.gerrit.sshd;
import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.server.CurrentUser;
@@ -24,6 +25,8 @@
import com.google.gerrit.server.audit.SshAuditEvent;
import com.google.gerrit.server.config.ConfigKey;
import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.gerrit.server.config.GerritConfigListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.ioutil.HexFormat;
@@ -33,8 +36,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.util.Collections;
-import java.util.List;
import org.apache.log4j.AsyncAppender;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
@@ -318,25 +319,22 @@
}
@Override
- public List<ConfigUpdatedEvent.Update> configUpdated(ConfigUpdatedEvent event) {
+ public Multimap<UpdateResult, ConfigUpdateEntry> configUpdated(ConfigUpdatedEvent event) {
ConfigKey sshdRequestLog = ConfigKey.create("sshd", "requestLog");
if (!event.isValueUpdated(sshdRequestLog)) {
- return Collections.emptyList();
+ return ConfigUpdatedEvent.NO_UPDATES;
}
boolean stateUpdated;
try {
boolean enabled = event.getNewConfig().getBoolean("sshd", "requestLog", true);
-
if (enabled) {
stateUpdated = enableLogging();
} else {
stateUpdated = disableLogging();
}
- return stateUpdated
- ? Collections.singletonList(event.accept(sshdRequestLog))
- : Collections.emptyList();
+ return stateUpdated ? event.accept(sshdRequestLog) : ConfigUpdatedEvent.NO_UPDATES;
} catch (IllegalArgumentException iae) {
- return Collections.singletonList(event.reject(sshdRequestLog));
+ return event.reject(sshdRequestLog);
}
}
}
diff --git a/java/com/google/gerrit/sshd/commands/ReloadConfig.java b/java/com/google/gerrit/sshd/commands/ReloadConfig.java
index 1b21230..cbe3c57 100644
--- a/java/com/google/gerrit/sshd/commands/ReloadConfig.java
+++ b/java/com/google/gerrit/sshd/commands/ReloadConfig.java
@@ -16,16 +16,15 @@
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE;
+import com.google.common.collect.Multimap;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.server.config.ConfigUpdatedEvent;
+import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
import com.google.gerrit.server.config.GerritServerConfigReloader;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
-import java.util.List;
-import java.util.stream.Collectors;
/** Issues a reload of gerrit.config. */
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@@ -39,31 +38,16 @@
@Override
protected void run() throws Failure {
- List<ConfigUpdatedEvent.Update> updates = gerritServerConfigReloader.reloadConfig();
+ Multimap<UpdateResult, ConfigUpdateEntry> updates = gerritServerConfigReloader.reloadConfig();
if (updates.isEmpty()) {
stdout.println("No config entries updated!");
return;
}
// Print out UpdateResult.{ACCEPTED|REJECTED} entries grouped by their type
- for (UpdateResult updateResult : UpdateResult.values()) {
- List<ConfigUpdatedEvent.Update> filteredUpdates = filterUpdates(updates, updateResult);
- if (filteredUpdates.isEmpty()) {
- continue;
- }
- stdout.println(updateResult.toString() + " configuration changes:");
- filteredUpdates
- .stream()
- .flatMap(update -> update.getConfigUpdates().stream())
- .forEach(cfgEntry -> stdout.println(cfgEntry.toString()));
+ for (UpdateResult result : updates.keySet()) {
+ stdout.println(result.toString() + " configuration changes:");
+ updates.get(result).forEach(cfgEntry -> stdout.println(cfgEntry.toString()));
}
}
-
- public static List<ConfigUpdatedEvent.Update> filterUpdates(
- List<ConfigUpdatedEvent.Update> updates, UpdateResult result) {
- return updates
- .stream()
- .filter(update -> update.getResult() == result)
- .collect(Collectors.toList());
- }
}
diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java
index 24a6975..cb5d449 100644
--- a/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/java/com/google/gerrit/sshd/commands/Upload.java
@@ -14,6 +14,7 @@
package com.google.gerrit.sshd.commands;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -52,7 +53,6 @@
PermissionBackend.ForProject perm =
permissionBackend.user(user).project(projectState.getNameKey());
try {
-
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
} catch (AuthException e) {
throw new Failure(1, "fatal: upload-pack not permitted on this server");
@@ -65,6 +65,9 @@
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));
+ if (config.enableProtocolV2() && extraParameters != null) {
+ up.setExtraParameters(ImmutableList.copyOf(extraParameters));
+ }
List<PreUploadHook> allPreUploadHooks = Lists.newArrayList(preUploadHooks);
allPreUploadHooks.add(
diff --git a/java/com/google/gerrit/testing/DisabledReviewDb.java b/java/com/google/gerrit/testing/DisabledReviewDb.java
index d902e11..2bf95b0 100644
--- a/java/com/google/gerrit/testing/DisabledReviewDb.java
+++ b/java/com/google/gerrit/testing/DisabledReviewDb.java
@@ -21,7 +21,6 @@
import com.google.gerrit.reviewdb.server.PatchSetApprovalAccess;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.SchemaVersionAccess;
-import com.google.gerrit.reviewdb.server.SystemConfigAccess;
import com.google.gwtorm.server.Access;
import com.google.gwtorm.server.StatementExecutor;
@@ -71,11 +70,6 @@
}
@Override
- public SystemConfigAccess systemConfig() {
- throw new Disabled();
- }
-
- @Override
public ChangeAccess changes() {
throw new Disabled();
}
diff --git a/java/com/google/gerrit/testing/FakeGroupAuditService.java b/java/com/google/gerrit/testing/FakeGroupAuditService.java
new file mode 100644
index 0000000..7c6674b
--- /dev/null
+++ b/java/com/google/gerrit/testing/FakeGroupAuditService.java
@@ -0,0 +1,112 @@
+// 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.testing;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.AuditEvent;
+import com.google.gerrit.server.audit.AuditListener;
+import com.google.gerrit.server.audit.group.GroupAuditListener;
+import com.google.gerrit.server.audit.group.GroupMemberAuditEvent;
+import com.google.gerrit.server.audit.group.GroupSubgroupAuditEvent;
+import com.google.gerrit.server.group.GroupAuditService;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.List;
+
+@Singleton
+public class FakeGroupAuditService implements GroupAuditService {
+
+ private final PluginSetContext<GroupAuditListener> groupAuditListeners;
+ private final PluginSetContext<AuditListener> auditListeners;
+
+ public static class Module extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.setOf(binder(), GroupAuditListener.class);
+ DynamicSet.setOf(binder(), AuditListener.class);
+ bind(GroupAuditService.class).to(FakeGroupAuditService.class);
+ }
+ }
+
+ @Inject
+ public FakeGroupAuditService(
+ PluginSetContext<GroupAuditListener> groupAuditListeners,
+ PluginSetContext<AuditListener> auditListeners) {
+ this.groupAuditListeners = groupAuditListeners;
+ this.auditListeners = auditListeners;
+ }
+
+ public List<AuditEvent> auditEvents = new ArrayList<>();
+
+ public void clearEvents() {
+ auditEvents.clear();
+ }
+
+ @Override
+ public void dispatch(AuditEvent action) {
+ auditEvents.add(action);
+ }
+
+ @Override
+ public void dispatchAddMembers(
+ Account.Id actor,
+ AccountGroup.UUID updatedGroup,
+ ImmutableSet<Account.Id> addedMembers,
+ Timestamp addedOn) {
+ GroupMemberAuditEvent event =
+ GroupMemberAuditEvent.create(actor, updatedGroup, addedMembers, addedOn);
+ groupAuditListeners.runEach(l -> l.onAddMembers(event));
+ }
+
+ @Override
+ public void dispatchDeleteMembers(
+ Account.Id actor,
+ AccountGroup.UUID updatedGroup,
+ ImmutableSet<Account.Id> deletedMembers,
+ Timestamp deletedOn) {
+ GroupMemberAuditEvent event =
+ GroupMemberAuditEvent.create(actor, updatedGroup, deletedMembers, deletedOn);
+ groupAuditListeners.runEach(l -> l.onDeleteMembers(event));
+ }
+
+ @Override
+ public void dispatchAddSubgroups(
+ Account.Id actor,
+ AccountGroup.UUID updatedGroup,
+ ImmutableSet<AccountGroup.UUID> addedSubgroups,
+ Timestamp addedOn) {
+ GroupSubgroupAuditEvent event =
+ GroupSubgroupAuditEvent.create(actor, updatedGroup, addedSubgroups, addedOn);
+ groupAuditListeners.runEach(l -> l.onAddSubgroups(event));
+ }
+
+ @Override
+ public void dispatchDeleteSubgroups(
+ Account.Id actor,
+ AccountGroup.UUID updatedGroup,
+ ImmutableSet<AccountGroup.UUID> deletedSubgroups,
+ Timestamp deletedOn) {
+ GroupSubgroupAuditEvent event =
+ GroupSubgroupAuditEvent.create(actor, updatedGroup, deletedSubgroups, deletedOn);
+ groupAuditListeners.runEach(l -> l.onDeleteSubgroups(event));
+ }
+}
diff --git a/java/com/google/gerrit/testing/InMemoryDatabase.java b/java/com/google/gerrit/testing/InMemoryDatabase.java
index c9d6bd6..b489652 100644
--- a/java/com/google/gerrit/testing/InMemoryDatabase.java
+++ b/java/com/google/gerrit/testing/InMemoryDatabase.java
@@ -20,7 +20,6 @@
import com.google.gerrit.pgm.init.index.elasticsearch.ElasticIndexModuleOnInit;
import com.google.gerrit.pgm.init.index.lucene.LuceneIndexModuleOnInit;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
-import com.google.gerrit.reviewdb.client.SystemConfig;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.schema.ReviewDbSchemaCreator;
@@ -127,12 +126,6 @@
return this;
}
- public SystemConfig getSystemConfig() throws OrmException {
- try (ReviewDb c = open()) {
- return c.systemConfig().get(new SystemConfig.Key());
- }
- }
-
public CurrentSchemaVersion getSchemaVersion() throws OrmException {
try (ReviewDb c = open()) {
return c.schemaVersion().get(new CurrentSchemaVersion.Key());
diff --git a/java/com/google/gerrit/util/cli/CmdLineParser.java b/java/com/google/gerrit/util/cli/CmdLineParser.java
index 5b7ea3f..13447c6 100644
--- a/java/com/google/gerrit/util/cli/CmdLineParser.java
+++ b/java/com/google/gerrit/util/cli/CmdLineParser.java
@@ -50,8 +50,11 @@
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.ResourceBundle;
@@ -85,6 +88,90 @@
CmdLineParser create(Object bean);
}
+ /**
+ * This may be used by an option handler during parsing to "call" additional parameters simulating
+ * as if they had been passed from the command line originally.
+ *
+ * <p>To call additional parameters from within an option handler, instantiate this class with the
+ * parameters and then call callParameters() with the additional parameters to be parsed.
+ * OptionHandlers may optionally pass this class to other methods which may then both
+ * parse/consume more parameters and call additional parameters.
+ */
+ public static class Parameters implements org.kohsuke.args4j.spi.Parameters {
+ protected final String[] args;
+ protected MyParser parser;
+ protected int consumed = 0;
+
+ public Parameters(org.kohsuke.args4j.spi.Parameters args, MyParser parser)
+ throws CmdLineException {
+ this.args = new String[args.size()];
+ for (int i = 0; i < args.size(); i++) {
+ this.args[i] = args.getParameter(i);
+ }
+ this.parser = parser;
+ }
+
+ public Parameters(String[] args, MyParser parser) {
+ this.args = args;
+ this.parser = parser;
+ }
+
+ @Override
+ public String getParameter(int idx) throws CmdLineException {
+ return args[idx];
+ }
+
+ /**
+ * get and consume (consider parsed) a parameter
+ *
+ * @param name name
+ * @return the consumed parameter
+ */
+ public String consumeParameter() throws CmdLineException {
+ return getParameter(consumed++);
+ }
+
+ @Override
+ public int size() {
+ return args.length;
+ }
+
+ /**
+ * Add 'count' to the value of parsed parameters. May be called more than once.
+ *
+ * @param count How many parameters were just parsed.
+ */
+ public void consume(int count) {
+ consumed += count;
+ }
+
+ /**
+ * Reports handlers how many parameters were parsed
+ *
+ * @return the count of parsed parameters
+ */
+ public int getConsumed() {
+ return consumed;
+ }
+
+ /**
+ * Use during parsing to call additional parameters simulating as if they had been passed from
+ * the command line originally.
+ *
+ * @param String... args A variable amount of parameters to call immediately
+ * <p>The parameters will be parsed immediately, before the remaining parameter will be
+ * parsed.
+ * <p>Note: Since this is done outside of the arg4j parsing loop, it will not match exactly
+ * what would happen if they were actually passed from the command line, but it will be
+ * pretty close. If this were moved to args4j, the interface could be the same and it could
+ * match exactly the behavior as if passed from the command line originally.
+ */
+ public void callParameters(String... args) throws CmdLineException {
+ Parameters impl = new Parameters(Arrays.copyOfRange(args, 1, args.length), parser);
+ parser.findOptionByName(args[0]).parseArguments(impl);
+ }
+ }
+
private final OptionHandlers handlers;
private final MyParser parser;
@@ -270,6 +357,10 @@
parser.parseWithPrefix(prefix, bean);
}
+ public void drainOptionQueue() {
+ parser.addOptionsWithMetRequirements();
+ }
+
private String makeOption(String name) {
if (!name.startsWith("-")) {
if (name.length() == 1) {
@@ -404,18 +495,58 @@
}
}
- private class MyParser extends org.kohsuke.args4j.CmdLineParser {
+ public class MyParser extends org.kohsuke.args4j.CmdLineParser {
@SuppressWarnings("rawtypes")
private List<OptionHandler> optionsList;
+ private Map<String, QueuedOption> queuedOptionsByName = new LinkedHashMap<>();
private HelpOption help;
+ private class QueuedOption {
+ public final Option option;
+ public final Setter setter;
+ public final String[] requiredOptions;
+
+ private QueuedOption(Option option, Setter setter, RequiresOptions requiresOptions) {
+ this.option = option;
+ this.setter = setter;
+ this.requiredOptions = requiresOptions != null ? requiresOptions.value() : new String[0];
+ }
+ }
+
MyParser(Object bean) {
super(bean, ParserProperties.defaults().withAtSyntax(false));
parseAdditionalOptions(bean, new HashSet<>());
+ addOptionsWithMetRequirements();
ensureOptionsInitialized();
}
+ public int addOptionsWithMetRequirements() {
+ int count = 0;
+ for (Iterator<Map.Entry<String, QueuedOption>> it = queuedOptionsByName.entrySet().iterator();
+ it.hasNext(); ) {
+ QueuedOption queuedOption = it.next().getValue();
+ if (hasAllRequiredOptions(queuedOption)) {
+ addOption(queuedOption.setter, queuedOption.option);
+ it.remove();
+ count++;
+ }
+ }
+ if (count > 0) {
+ count += addOptionsWithMetRequirements();
+ }
+ return count;
+ }
+
+ private boolean hasAllRequiredOptions(QueuedOption queuedOption) {
+ for (String name : queuedOption.requiredOptions) {
+ if (findOptionByName(name) == null) {
+ return false;
+ }
+ }
+ return true;
+ }
+
// NOTE: Argument annotations on bean are ignored.
public void parseWithPrefix(String prefix, Object bean) {
parseWithPrefix(prefix, bean, new HashSet<>());
@@ -430,13 +561,19 @@
for (Method m : c.getDeclaredMethods()) {
Option o = m.getAnnotation(Option.class);
if (o != null) {
- addOption(new MethodSetter(this, bean, m), new PrefixedOption(prefix, o));
+ queueOption(
+ new PrefixedOption(prefix, o),
+ new MethodSetter(this, bean, m),
+ m.getAnnotation(RequiresOptions.class));
}
}
for (Field f : c.getDeclaredFields()) {
Option o = f.getAnnotation(Option.class);
if (o != null) {
- addOption(Setters.create(f, bean), new PrefixedOption(prefix, o));
+ queueOption(
+ new PrefixedOption(prefix, o),
+ Setters.create(f, bean),
+ f.getAnnotation(RequiresOptions.class));
}
if (f.isAnnotationPresent(Options.class)) {
try {
@@ -480,6 +617,37 @@
return add(super.createOptionHandler(option, setter));
}
+ /**
+ * Finds a registered {@code OptionHandler} by its name or its alias.
+ *
+ * @param name name
+ * @return the {@code OptionHandler} or {@code null}
+ * <p>Note: this is cut & pasted from the parent class in arg4j, it was private and it
+ * needed to be exposed.
+ */
+ public OptionHandler findOptionByName(String name) {
+ for (OptionHandler h : optionsList) {
+ NamedOptionDef option = (NamedOptionDef) h.option;
+ if (name.equals(option.name())) {
+ return h;
+ }
+ for (String alias : option.aliases()) {
+ if (name.equals(alias)) {
+ return h;
+ }
+ }
+ }
+ return null;
+ }
+
+ private void queueOption(Option option, Setter setter, RequiresOptions requiresOptions) {
+ if (queuedOptionsByName.put(option.name(), new QueuedOption(option, setter, requiresOptions))
+ != null) {
+ throw new IllegalAnnotationError(
+ "Option name " + option.name() + " is used more than once");
+ }
+ }
+
@SuppressWarnings("rawtypes")
private OptionHandler add(OptionHandler handler) {
ensureOptionsInitialized();
diff --git a/java/com/google/gerrit/util/cli/RequiresOptions.java b/java/com/google/gerrit/util/cli/RequiresOptions.java
new file mode 100644
index 0000000..de6ba44
--- /dev/null
+++ b/java/com/google/gerrit/util/cli/RequiresOptions.java
@@ -0,0 +1,45 @@
+// 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.util.cli;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a field/setter annotated with {@literal @}Option as having a dependency on multiple other
+ * command line option.
+ *
+ * <p>If any of the required command line options are not present, the {@literal @}Option will be
+ * ignored.
+ *
+ * <p>For example:
+ *
+ * <pre>
+ * {@literal @}RequiresOptions({"--help", "--usage"})
+ * {@literal @}Option(name = "--help-as-json",
+ * usage = "display help text in json format")
+ * public boolean displayHelpAsJson;
+ * </pre>
+ */
+@Retention(RUNTIME)
+@Target({FIELD, METHOD, PARAMETER})
+public @interface RequiresOptions {
+ String[] value();
+}
diff --git a/java/gerrit/PRED_commit_delta_4.java b/java/gerrit/PRED_commit_delta_4.java
index 7c26632..d2634ea 100644
--- a/java/gerrit/PRED_commit_delta_4.java
+++ b/java/gerrit/PRED_commit_delta_4.java
@@ -102,7 +102,7 @@
String oldName = patch.getOldName();
Patch.ChangeType changeType = patch.getChangeType();
- if (newName.equals("/COMMIT_MSG")) {
+ if (Patch.isMagic(newName)) {
continue;
}
diff --git a/java/gerrit/PRED_commit_edits_2.java b/java/gerrit/PRED_commit_edits_2.java
index c196026..f46a6487 100644
--- a/java/gerrit/PRED_commit_edits_2.java
+++ b/java/gerrit/PRED_commit_edits_2.java
@@ -14,6 +14,7 @@
package gerrit;
+import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.Text;
@@ -90,7 +91,7 @@
String newName = entry.getNewName();
String oldName = entry.getOldName();
- if (newName.equals("/COMMIT_MSG")) {
+ if (Patch.isMagic(newName)) {
continue;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index db96ac1..ee22141 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -1512,9 +1512,9 @@
}
private static void assertIncludes(Iterable<GroupInfo> includes, String... expectedNames) {
- assertThat(Iterables.transform(includes, i -> i.name))
- .containsExactlyElementsIn(Arrays.asList(expectedNames))
- .inOrder();
+ Iterable<String> names = Iterables.transform(includes, i -> i.name);
+ assertThat(names).containsExactlyElementsIn(Arrays.asList(expectedNames));
+ assertThat(names).isOrdered();
}
private void assertNoIncludes(String group) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index 66a09f8..f306932 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -53,6 +53,7 @@
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.junit.Before;
public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest {
@@ -95,7 +96,7 @@
return cfg;
}
- protected TestRepository<?> createProjectWithPush(
+ protected Project.NameKey createProjectForPush(
String name,
@Nullable Project.NameKey parent,
boolean createEmptyCommit,
@@ -104,24 +105,40 @@
Project.NameKey project = createProject(name, parent, createEmptyCommit, submitType);
grant(project, "refs/heads/*", Permission.PUSH);
grant(project, "refs/for/refs/heads/*", Permission.SUBMIT);
- return cloneProject(project);
- }
-
- protected TestRepository<?> createProjectWithPush(String name, @Nullable Project.NameKey parent)
- throws Exception {
- return createProjectWithPush(name, parent, true, getSubmitType());
- }
-
- protected TestRepository<?> createProjectWithPush(String name, boolean createEmptyCommit)
- throws Exception {
- return createProjectWithPush(name, null, createEmptyCommit, getSubmitType());
+ return project;
}
protected TestRepository<?> createProjectWithPush(String name) throws Exception {
return createProjectWithPush(name, null, true, getSubmitType());
}
+ protected TestRepository<?> createProjectWithPush(
+ String name,
+ @Nullable Project.NameKey parent,
+ boolean createEmptyCommit,
+ SubmitType submitType)
+ throws Exception {
+ return cloneProject(createProjectForPush(name, parent, createEmptyCommit, submitType));
+ }
+
+ protected TestRepository<?> createProjectWithPush(String name, boolean createEmptyCommit)
+ throws Exception {
+ return cloneProject(createProjectForPush(name, null, createEmptyCommit, getSubmitType()));
+ }
+
private static AtomicInteger contentCounter = new AtomicInteger(0);
+ protected TestRepository<?> superRepo;
+ protected Project.NameKey superKey;
+ protected TestRepository<?> subRepo;
+ protected Project.NameKey subKey;
+
+ @Before
+ public void setUp() throws Exception {
+ superKey = createProjectForPush("super", null, true, getSubmitType());
+ subKey = createProjectForPush("sub", null, true, getSubmitType());
+ superRepo = cloneProject(superKey);
+ subRepo = cloneProject(subKey);
+ }
protected ObjectId pushChangeTo(
TestRepository<?> repo, String ref, String file, String content, String message, String topic)
@@ -184,16 +201,31 @@
protected void allowSubmoduleSubscription(
String submodule, String subBranch, String superproject, String superBranch, boolean match)
throws Exception {
- Project.NameKey sub = new Project.NameKey(name(submodule));
- Project.NameKey superName = new Project.NameKey(name(superproject));
- try (MetaDataUpdate md = metaDataUpdateFactory.create(sub)) {
+ allowSubmoduleSubscription(
+ nameKey(submodule), subBranch, nameKey(superproject), superBranch, match);
+ }
+
+ protected void allowMatchingSubmoduleSubscription(
+ Project.NameKey submodule, String subBranch, Project.NameKey superproject, String superBranch)
+ throws Exception {
+ allowSubmoduleSubscription(submodule, subBranch, superproject, superBranch, true);
+ }
+
+ protected void allowSubmoduleSubscription(
+ Project.NameKey submodule,
+ String subBranch,
+ Project.NameKey superproject,
+ String superBranch,
+ boolean match)
+ throws Exception {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(submodule)) {
md.setMessage("Added superproject subscription");
SubscribeSection s;
ProjectConfig pc = projectConfigFactory.read(md);
- if (pc.getSubscribeSections().containsKey(superName)) {
- s = pc.getSubscribeSections().get(superName);
+ if (pc.getSubscribeSections().containsKey(superproject)) {
+ s = pc.getSubscribeSections().get(superproject);
} else {
- s = new SubscribeSection(superName);
+ s = new SubscribeSection(superproject);
}
String refspec;
if (superBranch == null) {
@@ -223,6 +255,15 @@
protected void createSubmoduleSubscription(
TestRepository<?> repo, String branch, String subscribeToRepo, String subscribeToBranch)
throws Exception {
+ createSubmoduleSubscription(repo, branch, nameKey(subscribeToRepo), subscribeToBranch);
+ }
+
+ protected void createSubmoduleSubscription(
+ TestRepository<?> repo,
+ String branch,
+ Project.NameKey subscribeToRepo,
+ String subscribeToBranch)
+ throws Exception {
Config config = new Config();
prepareSubmoduleConfigEntry(config, subscribeToRepo, subscribeToBranch);
pushSubmoduleConfig(repo, branch, config);
@@ -246,12 +287,20 @@
String subscribeToRepoPrefix,
String subscribeToRepo,
String subscribeToBranch) {
- subscribeToRepo = name(subscribeToRepo);
- String url = subscribeToRepoPrefix + subscribeToRepo;
- config.setString("submodule", subscribeToRepo, "path", subscribeToRepo);
- config.setString("submodule", subscribeToRepo, "url", url);
+ prepareRelativeSubmoduleConfigEntry(
+ config, subscribeToRepoPrefix, nameKey(subscribeToRepo), subscribeToBranch);
+ }
+
+ protected void prepareRelativeSubmoduleConfigEntry(
+ Config config,
+ String subscribeToRepoPrefix,
+ Project.NameKey subscribeToRepo,
+ String subscribeToBranch) {
+ String url = subscribeToRepoPrefix + subscribeToRepo.get();
+ config.setString("submodule", subscribeToRepo.get(), "path", subscribeToRepo.get());
+ config.setString("submodule", subscribeToRepo.get(), "url", url);
if (subscribeToBranch != null) {
- config.setString("submodule", subscribeToRepo, "branch", subscribeToBranch);
+ config.setString("submodule", subscribeToRepo.get(), "branch", subscribeToBranch);
}
}
@@ -260,21 +309,41 @@
// The submodule subscription module checks for gerrit.canonicalWebUrl to
// detect if it's configured for automatic updates. It doesn't matter if
// it serves from that URL.
+ prepareSubmoduleConfigEntry(
+ config, nameKey(subscribeToRepo), nameKey(subscribeToRepo), subscribeToBranch);
+ }
+
+ protected void prepareSubmoduleConfigEntry(
+ Config config, Project.NameKey subscribeToRepo, String subscribeToBranch) {
+ // The submodule subscription module checks for gerrit.canonicalWebUrl to
+ // detect if it's configured for automatic updates. It doesn't matter if
+ // it serves from that URL.
prepareSubmoduleConfigEntry(config, subscribeToRepo, subscribeToRepo, subscribeToBranch);
}
+ protected Project.NameKey nameKey(String s) {
+ return new Project.NameKey(name(s));
+ }
+
protected void prepareSubmoduleConfigEntry(
Config config, String subscribeToRepo, String subscribeToRepoPath, String subscribeToBranch) {
- subscribeToRepo = name(subscribeToRepo);
- subscribeToRepoPath = name(subscribeToRepoPath);
+ prepareSubmoduleConfigEntry(
+ config, nameKey(subscribeToRepo), nameKey(subscribeToRepoPath), subscribeToBranch);
+ }
+
+ protected void prepareSubmoduleConfigEntry(
+ Config config,
+ Project.NameKey subscribeToRepo,
+ Project.NameKey subscribeToRepoPath,
+ String subscribeToBranch) {
// The submodule subscription module checks for gerrit.canonicalWebUrl to
// detect if it's configured for automatic updates. It doesn't matter if
// it serves from that URL.
String url = cfg.getString("gerrit", null, "canonicalWebUrl") + "/" + subscribeToRepo;
- config.setString("submodule", subscribeToRepoPath, "path", subscribeToRepoPath);
- config.setString("submodule", subscribeToRepoPath, "url", url);
+ config.setString("submodule", subscribeToRepoPath.get(), "path", subscribeToRepoPath.get());
+ config.setString("submodule", subscribeToRepoPath.get(), "url", url);
if (subscribeToBranch != null) {
- config.setString("submodule", subscribeToRepoPath, "branch", subscribeToBranch);
+ config.setString("submodule", subscribeToRepoPath.get(), "branch", subscribeToBranch);
}
}
@@ -302,8 +371,17 @@
TestRepository<?> subRepo,
String subBranch)
throws Exception {
+ expectToHaveSubmoduleState(repo, branch, nameKey(submodule), subRepo, subBranch);
+ }
- submodule = name(submodule);
+ protected void expectToHaveSubmoduleState(
+ TestRepository<?> repo,
+ String branch,
+ Project.NameKey submodule,
+ TestRepository<?> subRepo,
+ String subBranch)
+ throws Exception {
+
ObjectId commitId =
repo.git()
.fetch()
@@ -326,7 +404,7 @@
rw.parseBody(c.getTree());
RevTree tree = c.getTree();
- RevObject actualId = repo.get(tree, submodule);
+ RevObject actualId = repo.get(tree, submodule.get());
assertThat(actualId).isEqualTo(subHead);
}
@@ -334,8 +412,12 @@
protected void expectToHaveSubmoduleState(
TestRepository<?> repo, String branch, String submodule, ObjectId expectedId)
throws Exception {
+ expectToHaveSubmoduleState(repo, branch, nameKey(submodule), expectedId);
+ }
- submodule = name(submodule);
+ protected void expectToHaveSubmoduleState(
+ TestRepository<?> repo, String branch, Project.NameKey submodule, ObjectId expectedId)
+ throws Exception {
ObjectId commitId =
repo.git()
.fetch()
@@ -349,7 +431,7 @@
rw.parseBody(c.getTree());
RevTree tree = c.getTree();
- RevObject actualId = repo.get(tree, submodule);
+ RevObject actualId = repo.get(tree, submodule.get());
assertThat(actualId).isEqualTo(expectedId);
}
@@ -410,8 +492,11 @@
protected boolean hasSubmodule(TestRepository<?> repo, String branch, String submodule)
throws Exception {
+ return hasSubmodule(repo, branch, new Project.NameKey(name(submodule)));
+ }
- submodule = name(submodule);
+ protected boolean hasSubmodule(TestRepository<?> repo, String branch, Project.NameKey submodule)
+ throws Exception {
Ref branchTip =
repo.git().fetch().setRemote("origin").call().getAdvertisedRef("refs/heads/" + branch);
if (branchTip == null) {
@@ -426,7 +511,7 @@
RevTree tree = c.getTree();
try {
- repo.get(tree, submodule);
+ repo.get(tree, submodule.get());
return true;
} catch (AssertionError e) {
return false;
@@ -446,7 +531,8 @@
RevWalk rw = repo.getRevWalk();
RevCommit c = rw.parseCommit(commitId);
- assertThat(c.getFullMessage()).isEqualTo(expectedMessage);
+ String msg = c.getFullMessage();
+ assertThat(msg).isEqualTo(expectedMessage);
}
protected PersonIdent getAuthor(TestRepository<?> repo, String branch) throws Exception {
@@ -465,8 +551,13 @@
protected void directUpdateSubmodule(String project, String refName, String path, AnyObjectId id)
throws Exception {
- path = name(path);
- try (Repository serverRepo = repoManager.openRepository(new Project.NameKey(name(project)));
+ directUpdateSubmodule(nameKey(project), refName, nameKey(path), id);
+ }
+
+ protected void directUpdateSubmodule(
+ Project.NameKey project, String refName, Project.NameKey path, AnyObjectId id)
+ throws Exception {
+ try (Repository serverRepo = repoManager.openRepository(project);
ObjectInserter ins = serverRepo.newObjectInserter();
RevWalk rw = new RevWalk(serverRepo)) {
Ref ref = serverRepo.exactRef(refName);
@@ -480,7 +571,7 @@
b.finish();
DirCacheEditor e = dc.editor();
e.add(
- new PathEdit(path) {
+ new PathEdit(path.get()) {
@Override
public void apply(DirCacheEntry ent) {
ent.setFileMode(FileMode.GITLINK);
diff --git a/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
new file mode 100644
index 0000000..42e046a
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
@@ -0,0 +1,68 @@
+// 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.acceptance.git;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.server.AuditEvent;
+import java.util.Collections;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.junit.Before;
+import org.junit.Test;
+
+public class GitOverHttpServletIT extends AbstractPushForReview {
+
+ @Before
+ public void beforeEach() throws Exception {
+ CredentialsProvider.setDefault(
+ new UsernamePasswordCredentialsProvider(admin.username, admin.httpPassword));
+ selectProtocol(AbstractPushForReview.Protocol.HTTP);
+ auditService.clearEvents();
+ }
+
+ @Test
+ public void receivePackAuditEventLog() throws Exception {
+ testRepo
+ .git()
+ .push()
+ .setRemote("origin")
+ .setRefSpecs(new RefSpec("HEAD:refs/for/master"))
+ .call();
+
+ // Git smart protocol makes two requests:
+ // https://github.com/git/git/blob/master/Documentation/technical/http-protocol.txt
+ assertThat(auditService.auditEvents.size()).isEqualTo(2);
+
+ AuditEvent e = auditService.auditEvents.get(1);
+ assertThat(e.who.getAccountId()).isEqualTo(admin.id);
+ assertThat(e.what).endsWith("/git-receive-pack");
+ assertThat(e.params).isEmpty();
+ }
+
+ @Test
+ public void uploadPackAuditEventLog() throws Exception {
+ testRepo.git().fetch().call();
+
+ assertThat(auditService.auditEvents.size()).isEqualTo(1);
+
+ AuditEvent e = auditService.auditEvents.get(0);
+ assertThat(e.who.toString()).isEqualTo("ANONYMOUS");
+ assertThat(e.params.get("service"))
+ .containsExactlyElementsIn(Collections.singletonList("git-upload-pack"));
+ assertThat(e.what).endsWith("service=git-upload-pack");
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 03b1165..d76b310 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -25,6 +25,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.TestTimeUtil;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -47,198 +48,169 @@
@Test
@GerritConfig(name = "submodule.enableSuperProjectSubscriptions", value = "false")
public void testSubscriptionWithoutGlobalServerSetting() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionWithoutSpecificSubscription() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
-
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionToEmptyRepo() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isTrue();
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD);
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isTrue();
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD);
}
@Test
public void subscriptionToExistingRepo() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isTrue();
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD);
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isTrue();
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD);
}
@Test
public void subscriptionWildcardACLForSingleBranch() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
// master is allowed to be subscribed to master branch only:
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", null);
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, null);
// create 'branch':
pushChangeTo(superRepo, "branch");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
- createSubmoduleSubscription(superRepo, "branch", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
+ createSubmoduleSubscription(superRepo, "branch", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD);
- assertThat(hasSubmodule(superRepo, "branch", "subscribed-to-project")).isFalse();
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD);
+ assertThat(hasSubmodule(superRepo, "branch", subKey)).isFalse();
}
@Test
public void subscriptionWildcardACLForMissingProject() throws Exception {
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/*", "not-existing-super-project", "refs/heads/*");
+ subKey, "refs/heads/*", new Project.NameKey("not-existing-super-project"), "refs/heads/*");
pushChangeTo(subRepo, "master");
}
@Test
public void subscriptionWildcardACLForMissingBranch() throws Exception {
- createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/*", "super-project", "refs/heads/*");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/*", superKey, "refs/heads/*");
pushChangeTo(subRepo, "foo");
}
@Test
public void subscriptionWildcardACLForMissingGitmodules() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/*", "super-project", "refs/heads/*");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/*", superKey, "refs/heads/*");
pushChangeTo(superRepo, "master");
pushChangeTo(subRepo, "master");
}
@Test
public void subscriptionWildcardACLOneOnOneMapping() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
// any branch is allowed to be subscribed to the same superprojects branch:
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/*", "super-project", "refs/heads/*");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/*", superKey, "refs/heads/*");
// create 'branch' in both repos:
pushChangeTo(superRepo, "branch");
pushChangeTo(subRepo, "branch");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
- createSubmoduleSubscription(superRepo, "branch", "subscribed-to-project", "branch");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
+ createSubmoduleSubscription(superRepo, "branch", subKey, "branch");
ObjectId subHEAD1 = pushChangeTo(subRepo, "master");
ObjectId subHEAD2 = pushChangeTo(subRepo, "branch");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD1);
- expectToHaveSubmoduleState(superRepo, "branch", "subscribed-to-project", subHEAD2);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD1);
+ expectToHaveSubmoduleState(superRepo, "branch", subKey, subHEAD2);
// Now test that cross subscriptions do not work:
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "branch");
+ createSubmoduleSubscription(superRepo, "master", subKey, "branch");
ObjectId subHEAD3 = pushChangeTo(subRepo, "branch");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD1);
- expectToHaveSubmoduleState(superRepo, "branch", "subscribed-to-project", subHEAD3);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD1);
+ expectToHaveSubmoduleState(superRepo, "branch", subKey, subHEAD3);
}
@Test
public void subscriptionWildcardACLForManyBranches() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
// Any branch is allowed to be subscribed to any superproject branch:
- allowSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/*", "super-project", null, false);
+ allowSubmoduleSubscription(subKey, "refs/heads/*", superKey, null, false);
pushChangeTo(superRepo, "branch");
pushChangeTo(subRepo, "another-branch");
- createSubmoduleSubscription(superRepo, "branch", "subscribed-to-project", "another-branch");
+ createSubmoduleSubscription(superRepo, "branch", subKey, "another-branch");
ObjectId subHEAD = pushChangeTo(subRepo, "another-branch");
- expectToHaveSubmoduleState(superRepo, "branch", "subscribed-to-project", subHEAD);
+ expectToHaveSubmoduleState(superRepo, "branch", subKey, subHEAD);
}
@Test
public void subscriptionWildcardACLOneToManyBranches() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
// Any branch is allowed to be subscribed to any superproject branch:
- allowSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/*", false);
+ allowSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/*", false);
pushChangeTo(superRepo, "branch");
- createSubmoduleSubscription(superRepo, "branch", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "branch", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "branch", "subscribed-to-project", subHEAD);
+ expectToHaveSubmoduleState(superRepo, "branch", subKey, subHEAD);
- createSubmoduleSubscription(superRepo, "branch", "subscribed-to-project", "branch");
+ createSubmoduleSubscription(superRepo, "branch", subKey, "branch");
pushChangeTo(subRepo, "branch");
// no change expected, as only master is subscribed:
- expectToHaveSubmoduleState(superRepo, "branch", "subscribed-to-project", subHEAD);
+ expectToHaveSubmoduleState(superRepo, "branch", subKey, subHEAD);
}
@Test
@GerritConfig(name = "submodule.verboseSuperprojectUpdate", value = "false")
public void testSubmoduleShortCommitMessage() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
// The first update doesn't include any commit messages
ObjectId subRepoId = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subRepoId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepoId);
expectToHaveCommitMessage(superRepo, "master", "Update git submodules\n\n");
// Any following update also has a short message
subRepoId = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subRepoId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepoId);
expectToHaveCommitMessage(superRepo, "master", "Update git submodules\n\n");
}
@Test
@GerritConfig(name = "submodule.verboseSuperprojectUpdate", value = "SUBJECT_ONLY")
public void testSubmoduleSubjectCommitMessage() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
// The first update doesn't include the rev log
@@ -248,7 +220,7 @@
"master",
"Update git submodules\n\n"
+ "* Update "
- + name("subscribed-to-project")
+ + subKey.get()
+ " from branch 'master'\n to "
+ subHEAD.getName());
@@ -261,7 +233,7 @@
"master",
"Update git submodules\n\n"
+ "* Update "
- + name("subscribed-to-project")
+ + subKey.get()
+ " from branch 'master'\n to "
+ subHEAD.getName()
+ "\n - "
@@ -270,13 +242,11 @@
@Test
public void submoduleCommitMessage() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
// The first update doesn't include the rev log
@@ -286,7 +256,7 @@
"master",
"Update git submodules\n\n"
+ "* Update "
- + name("subscribed-to-project")
+ + subKey.get()
+ " from branch 'master'\n to "
+ subHEAD.getName());
@@ -299,7 +269,7 @@
"master",
"Update git submodules\n\n"
+ "* Update "
- + name("subscribed-to-project")
+ + subKey.get()
+ " from branch 'master'\n to "
+ subHEAD.getName()
+ "\n - "
@@ -308,171 +278,151 @@
@Test
public void subscriptionUnsubscribe() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
ObjectId subHEADbeforeUnsubscribing = pushChangeTo(subRepo, "master");
deleteAllSubscriptions(superRepo, "master");
- expectToHaveSubmoduleState(
- superRepo, "master", "subscribed-to-project", subHEADbeforeUnsubscribing);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEADbeforeUnsubscribing);
pushChangeTo(superRepo, "refs/heads/master", "commit after unsubscribe", "");
pushChangeTo(subRepo, "refs/heads/master", "commit after unsubscribe", "");
- expectToHaveSubmoduleState(
- superRepo, "master", "subscribed-to-project", subHEADbeforeUnsubscribing);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEADbeforeUnsubscribing);
}
@Test
public void subscriptionUnsubscribeByDeletingGitModules() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
ObjectId subHEADbeforeUnsubscribing = pushChangeTo(subRepo, "master");
deleteGitModulesFile(superRepo, "master");
- expectToHaveSubmoduleState(
- superRepo, "master", "subscribed-to-project", subHEADbeforeUnsubscribing);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEADbeforeUnsubscribing);
pushChangeTo(superRepo, "refs/heads/master", "commit after unsubscribe", "");
pushChangeTo(subRepo, "refs/heads/master", "commit after unsubscribe", "");
- expectToHaveSubmoduleState(
- superRepo, "master", "subscribed-to-project", subHEADbeforeUnsubscribing);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEADbeforeUnsubscribing);
}
@Test
public void subscriptionToDifferentBranches() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/foo", "super-project", "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "foo");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/foo", superKey, "refs/heads/master");
+
+ createSubmoduleSubscription(superRepo, "master", subKey, "foo");
ObjectId subFoo = pushChangeTo(subRepo, "foo");
pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subFoo);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subFoo);
}
@Test
public void branchCircularSubscription() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "super-project", "refs/heads/master", "subscribed-to-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(superKey, "refs/heads/master", subKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
pushChangeTo(superRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
- createSubmoduleSubscription(subRepo, "master", "super-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
+ createSubmoduleSubscription(subRepo, "master", superKey, "master");
pushChangeTo(subRepo, "master");
pushChangeTo(superRepo, "master");
- assertThat(hasSubmodule(subRepo, "master", "super-project")).isFalse();
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(subRepo, "master", superKey)).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void projectCircularSubscription() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "super-project", "refs/heads/dev", "subscribed-to-project", "refs/heads/dev");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(superKey, "refs/heads/dev", subKey, "refs/heads/dev");
pushChangeTo(subRepo, "master");
pushChangeTo(superRepo, "master");
pushChangeTo(subRepo, "dev");
pushChangeTo(superRepo, "dev");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
- createSubmoduleSubscription(subRepo, "dev", "super-project", "dev");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
+ createSubmoduleSubscription(subRepo, "dev", superKey, "dev");
ObjectId subMasterHead = pushChangeTo(subRepo, "master");
ObjectId superDevHead = pushChangeTo(superRepo, "dev");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isTrue();
- assertThat(hasSubmodule(subRepo, "dev", "super-project")).isTrue();
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subMasterHead);
- expectToHaveSubmoduleState(subRepo, "dev", "super-project", superDevHead);
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isTrue();
+ assertThat(hasSubmodule(subRepo, "dev", superKey)).isTrue();
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subMasterHead);
+ expectToHaveSubmoduleState(subRepo, "dev", superKey, superDevHead);
}
@Test
public void subscriptionFailOnMissingACL() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionFailOnWrongProjectACL() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "wrong-super-project", "refs/heads/master");
+ subKey,
+ "refs/heads/master",
+ new Project.NameKey("wrong-super-project"),
+ "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionFailOnWrongBranchACL() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/wrong-branch");
+ subKey, "refs/heads/master", superKey, "refs/heads/wrong-branch");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
pushChangeTo(subRepo, "master");
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionInheritACL() throws Exception {
- createProjectWithPush("config-repo");
- createProjectWithPush("config-repo2", new Project.NameKey(name("config-repo")));
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo =
- createProjectWithPush("subscribed-to-project", new Project.NameKey(name("config-repo2")));
- allowMatchingSubmoduleSubscription(
- "config-repo", "refs/heads/*", "super-project", "refs/heads/*");
+ Project.NameKey configKey = createProjectForPush("config-repo", null, true, getSubmitType());
+ Project.NameKey config2Key =
+ createProjectForPush("config-repo2", configKey, true, getSubmitType());
+ cloneProject(config2Key);
+
+ subKey = createProjectForPush("subrepo", config2Key, true, getSubmitType());
+ subRepo = cloneProject(subKey);
+
+ allowMatchingSubmoduleSubscription(configKey, "refs/heads/*", superKey, "refs/heads/*");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subHEAD);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subHEAD);
}
@Test
public void allowedButNotSubscribed() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
subRepo
@@ -490,16 +440,16 @@
assertThat(r.getRemoteUpdate("refs/heads/master").getStatus())
.isEqualTo(RemoteRefUpdate.Status.OK);
- assertThat(hasSubmodule(superRepo, "master", "subscribed-to-project")).isFalse();
+ assertThat(hasSubmodule(superRepo, "master", subKey)).isFalse();
}
@Test
public void subscriptionDeepRelative() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("nested/subscribed-to-project");
+ Project.NameKey nest =
+ createProjectForPush("nested/subscribed-to-project", null, true, getSubmitType());
+ TestRepository<?> subRepo = cloneProject(nest);
// master is allowed to be subscribed to any superprojects branch:
- allowMatchingSubmoduleSubscription(
- "nested/subscribed-to-project", "refs/heads/master", "super-project", null);
+ allowMatchingSubmoduleSubscription(nest, "refs/heads/master", superKey, null);
pushChangeTo(subRepo, "master");
createRelativeSubmoduleSubscription(
@@ -519,7 +469,9 @@
@Test
@GerritConfig(name = "submodule.verboseSuperprojectUpdate", value = "SUBJECT_ONLY")
- @GerritConfig(name = "submodule.maxCombinedCommitMessageSize", value = "220")
+ // The value 195 must tuned to the test environment, and is sensitive to the
+ // length of the uniquified repository name.
+ @GerritConfig(name = "submodule.maxCombinedCommitMessageSize", value = "200")
public void submoduleSubjectCommitMessageSizeLimit() throws Exception {
assume().that(isSubmitWholeTopicEnabled()).isFalse();
testSubmoduleSubjectCommitMessageAndExpectTruncation();
@@ -530,11 +482,10 @@
// Make sure that the commit is created at an earlier timestamp than the submit timestamp.
TestTimeUtil.resetWithClockStep(1, SECONDS);
try {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ subKey, "refs/heads/master", superKey, "refs/heads/master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
PushOneCommit.Result pushResult =
createChange(subRepo, "refs/heads/master", "Change", "a.txt", "some content", null);
@@ -561,18 +512,18 @@
// is afterwards.
TestTimeUtil.resetWithClockStep(1, SECONDS);
try {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+ Project.NameKey proj2 =
+ createProjectForPush("subscribed-to-project-2", null, true, getSubmitType());
+
+ TestRepository<?> subRepo2 = cloneProject(proj2);
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+ subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
- prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
+ prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
pushSubmoduleConfig(superRepo, "master", config);
String topic = "foo";
@@ -610,21 +561,17 @@
// is afterwards.
TestTimeUtil.resetWithClockStep(1, SECONDS);
try {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
-
- createProjectWithPush("subscribed-to-project-2");
- TestRepository<?> subRepo2 =
- cloneProject(new Project.NameKey(name("subscribed-to-project-2")), user);
+ Project.NameKey proj2 =
+ createProjectForPush("subscribed-to-project-2", null, true, getSubmitType());
+ TestRepository<InMemoryRepository> repo2 = cloneProject(proj2, user);
allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+ subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
- prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
+ prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
pushSubmoduleConfig(superRepo, "master", config);
String topic = "foo";
@@ -636,7 +583,7 @@
// Create change as user.
PushOneCommit push =
- pushFactory.create(db, user.getIdent(), subRepo2, "Change 2", "b.txt", "other content");
+ pushFactory.create(db, user.getIdent(), repo2, "Change 2", "b.txt", "other content");
PushOneCommit.Result pushResult2 = push.to("refs/for/master/" + name(topic));
approve(pushResult2.getChangeId());
@@ -659,14 +606,15 @@
@Test
public void updateOnlyRelevantSubmodules() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo1 = createProjectWithPush("subscribed-to-project-1");
- TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+ Project.NameKey subkey1 =
+ createProjectForPush("subscribed-to-project-1", null, true, getSubmitType());
+ Project.NameKey subkey2 =
+ createProjectForPush("subscribed-to-project-2", null, true, getSubmitType());
+ TestRepository<?> subRepo1 = cloneProject(subkey1);
+ TestRepository<?> subRepo2 = cloneProject(subkey2);
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project-1", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subkey1, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subkey2, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
prepareSubmoduleConfigEntry(config, "subscribed-to-project-1", "master");
@@ -689,41 +637,35 @@
@Test
public void skipUpdatingBrokenGitlinkPointer() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
// Push once to initialize submodule.
ObjectId subTip = pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subTip);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subTip);
// Write an invalid SHA-1 directly to the gitlink.
ObjectId badId = ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
- directUpdateSubmodule("super-project", "refs/heads/master", "subscribed-to-project", badId);
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", badId);
+ directUpdateSubmodule(superKey, "refs/heads/master", subKey, badId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, badId);
// Push succeeds, but gitlink update is skipped.
pushChangeTo(subRepo, "master");
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", badId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, badId);
}
private ObjectId directUpdateRef(String project, String ref) throws Exception {
- try (Repository serverRepo = repoManager.openRepository(new Project.NameKey(name(project)))) {
+ try (Repository serverRepo = repoManager.openRepository(nameKey(project))) {
return new TestRepository<>(serverRepo).branch(ref).commit().create().copy();
}
}
private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
pushChangeTo(subRepo, "master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
// The first update doesn't include the rev log, so we ignore it
pushChangeTo(subRepo, "master");
@@ -736,6 +678,6 @@
"master",
String.format(
"Update git submodules\n\n* Update %s from branch 'master'\n to %s\n - %s\n\n[...]",
- name("subscribed-to-project"), subHEAD.getName(), subCommitMsg.getShortMessage()));
+ subKey.get(), subHEAD.getName(), subCommitMsg.getShortMessage()));
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java
index eef3295..0b583bf 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsWholeTopicMergeIT.java
@@ -68,12 +68,8 @@
@Test
public void subscriptionUpdateOfManyChanges() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
-
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD =
subRepo
@@ -150,15 +146,13 @@
.getAdvertisedRef("refs/heads/master")
.getObjectId();
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subRepoId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepoId);
// As the submodules have changed commits, the superproject tree will be
// different, so we cannot directly compare the trees here, so make
// assumptions only about the changed branches:
- Project.NameKey p1 = new Project.NameKey(name("super-project"));
- Project.NameKey p2 = new Project.NameKey(name("subscribed-to-project"));
- assertThat(preview).containsKey(new Branch.NameKey(p1, "refs/heads/master"));
- assertThat(preview).containsKey(new Branch.NameKey(p2, "refs/heads/master"));
+ assertThat(preview).containsKey(new Branch.NameKey(superKey, "refs/heads/master"));
+ assertThat(preview).containsKey(new Branch.NameKey(subKey, "refs/heads/master"));
if ((getSubmitType() == SubmitType.CHERRY_PICK)
|| (getSubmitType() == SubmitType.REBASE_ALWAYS)) {
@@ -176,12 +170,9 @@
@Test
public void subscriptionUpdateIncludingChangeInSuperproject() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId subHEAD =
subRepo
@@ -274,27 +265,27 @@
.getAdvertisedRef("refs/heads/master")
.getObjectId();
- expectToHaveSubmoduleState(superRepo, "master", "subscribed-to-project", subRepoId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepoId);
}
@Test
public void updateManySubmodules() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub1 = createProjectWithPush("sub1");
- TestRepository<?> sub2 = createProjectWithPush("sub2");
- TestRepository<?> sub3 = createProjectWithPush("sub3");
+ Project.NameKey subKey1 = createProjectForPush("sub1", null, true, getSubmitType());
+ Project.NameKey subKey2 = createProjectForPush("sub2", null, true, getSubmitType());
+ Project.NameKey subKey3 = createProjectForPush("sub3", null, true, getSubmitType());
- allowMatchingSubmoduleSubscription(
- "sub1", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub2", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub3", "refs/heads/master", "super-project", "refs/heads/master");
+ TestRepository<?> sub1 = cloneProject(subKey1);
+ TestRepository<?> sub2 = cloneProject(subKey2);
+ TestRepository<?> sub3 = cloneProject(subKey3);
+
+ allowMatchingSubmoduleSubscription(subKey1, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey2, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey3, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "sub1", "master");
- prepareSubmoduleConfigEntry(config, "sub2", "master");
- prepareSubmoduleConfigEntry(config, "sub3", "master");
+ prepareSubmoduleConfigEntry(config, subKey1, "master");
+ prepareSubmoduleConfigEntry(config, subKey2, "master");
+ prepareSubmoduleConfigEntry(config, subKey3, "master");
pushSubmoduleConfig(superRepo, "master", config);
ObjectId superPreviousId = pushChangeTo(superRepo, "master");
@@ -309,9 +300,9 @@
gApi.changes().id(getChangeId(sub1, sub1Id).get()).current().submit();
- expectToHaveSubmoduleState(superRepo, "master", "sub1", sub1, "master");
- expectToHaveSubmoduleState(superRepo, "master", "sub2", sub2, "master");
- expectToHaveSubmoduleState(superRepo, "master", "sub3", sub3, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey1, sub1, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey2, sub2, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey3, sub3, "master");
String sub1HEAD =
sub1.git()
@@ -346,15 +337,15 @@
"master",
"Update git submodules\n\n"
+ "* Update "
- + name("sub1")
+ + subKey1.get()
+ " from branch 'master'\n to "
+ sub1HEAD
+ "\n\n* Update "
- + name("sub2")
+ + subKey2.get()
+ " from branch 'master'\n to "
+ sub2HEAD
+ "\n\n* Update "
- + name("sub3")
+ + subKey3.get()
+ " from branch 'master'\n to "
+ sub3HEAD);
}
@@ -374,73 +365,74 @@
@Test
public void doNotUseFastForward() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project", false);
- TestRepository<?> sub = createProjectWithPush("sub", false);
+ // like setup, but without empty commit
+ superKey = createProjectForPush("super-nc", null, false, getSubmitType());
+ subKey = createProjectForPush("sub-nc", null, false, getSubmitType());
+ superRepo = cloneProject(superKey);
+ subRepo = cloneProject(subKey);
- allowMatchingSubmoduleSubscription(
- "sub", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "sub", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
- ObjectId subId = pushChangeTo(sub, "refs/for/master", "some message", "same-topic");
+ ObjectId subId = pushChangeTo(subRepo, "refs/for/master", "some message", "same-topic");
ObjectId superId = pushChangeTo(superRepo, "refs/for/master", "some message", "same-topic");
- String subChangeId = getChangeId(sub, subId).get();
+ String subChangeId = getChangeId(subRepo, subId).get();
approve(subChangeId);
approve(getChangeId(superRepo, superId).get());
gApi.changes().id(subChangeId).current().submit();
- expectToHaveSubmoduleState(superRepo, "master", "sub", sub, "master");
- RevCommit superHead = getRemoteHead(name("super-project"), "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepo, "master");
+ RevCommit superHead = getRemoteHead(superKey, "master");
assertThat(superHead.getShortMessage()).contains("some message");
assertThat(superHead.getId()).isNotEqualTo(superId);
}
@Test
public void useFastForwardWhenNoSubmodule() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project", false);
- TestRepository<?> sub = createProjectWithPush("sub", false);
+ // like setup, but without empty commit
+ superKey = createProjectForPush("super-nc", null, false, getSubmitType());
+ subKey = createProjectForPush("sub-nc", null, false, getSubmitType());
+ superRepo = cloneProject(superKey);
+ subRepo = cloneProject(subKey);
- ObjectId subId = pushChangeTo(sub, "refs/for/master", "some message", "same-topic");
-
+ ObjectId subId = pushChangeTo(subRepo, "refs/for/master", "some message", "same-topic");
ObjectId superId = pushChangeTo(superRepo, "refs/for/master", "some message", "same-topic");
- String subChangeId = getChangeId(sub, subId).get();
+ String subChangeId = getChangeId(subRepo, subId).get();
approve(subChangeId);
approve(getChangeId(superRepo, superId).get());
gApi.changes().id(subChangeId).current().submit();
- RevCommit superHead = getRemoteHead(name("super-project"), "master");
+ RevCommit superHead = getRemoteHead(superKey, "master");
assertThat(superHead.getShortMessage()).isEqualTo("some message");
assertThat(superHead.getId()).isEqualTo(superId);
}
@Test
public void sameProjectSameBranchDifferentPaths() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub = createProjectWithPush("sub");
-
- allowMatchingSubmoduleSubscription(
- "sub", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "sub", "master");
- prepareSubmoduleConfigEntry(config, "sub", "sub-copy", "master");
+ prepareSubmoduleConfigEntry(config, subKey, "master");
+ Project.NameKey copyKey = nameKey("sub-copy");
+ prepareSubmoduleConfigEntry(config, subKey, copyKey, "master");
pushSubmoduleConfig(superRepo, "master", config);
ObjectId superPreviousId = pushChangeTo(superRepo, "master");
- ObjectId subId = pushChangeTo(sub, "refs/for/master", "some message", "");
+ ObjectId subId = pushChangeTo(subRepo, "refs/for/master", "some message", "");
- approve(getChangeId(sub, subId).get());
+ approve(getChangeId(subRepo, subId).get());
- gApi.changes().id(getChangeId(sub, subId).get()).current().submit();
+ gApi.changes().id(getChangeId(subRepo, subId).get()).current().submit();
- expectToHaveSubmoduleState(superRepo, "master", "sub", sub, "master");
- expectToHaveSubmoduleState(superRepo, "master", "sub-copy", sub, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepo, "master");
+ expectToHaveSubmoduleState(superRepo, "master", copyKey, subRepo, "master");
superRepo
.git()
@@ -457,37 +449,33 @@
@Test
public void sameProjectDifferentBranchDifferentPaths() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub = createProjectWithPush("sub");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/dev", superKey, "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub", "refs/heads/dev", "super-project", "refs/heads/master");
-
- ObjectId devHead = pushChangeTo(sub, "dev");
+ ObjectId devHead = pushChangeTo(subRepo, "dev");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "sub", "sub-master", "master");
- prepareSubmoduleConfigEntry(config, "sub", "sub-dev", "dev");
+ prepareSubmoduleConfigEntry(config, subKey, nameKey("sub-master"), "master");
+ prepareSubmoduleConfigEntry(config, subKey, nameKey("sub-dev"), "dev");
pushSubmoduleConfig(superRepo, "master", config);
ObjectId subMasterId =
- pushChangeTo(sub, "refs/for/master", "some message", "b.txt", "content b", "same-topic");
+ pushChangeTo(
+ subRepo, "refs/for/master", "some message", "b.txt", "content b", "same-topic");
- sub.reset(devHead);
+ subRepo.reset(devHead);
ObjectId subDevId =
pushChangeTo(
- sub, "refs/for/dev", "some message in dev", "b.txt", "content b", "same-topic");
+ subRepo, "refs/for/dev", "some message in dev", "b.txt", "content b", "same-topic");
- approve(getChangeId(sub, subMasterId).get());
- approve(getChangeId(sub, subDevId).get());
+ approve(getChangeId(subRepo, subMasterId).get());
+ approve(getChangeId(subRepo, subDevId).get());
ObjectId superPreviousId = pushChangeTo(superRepo, "master");
- gApi.changes().id(getChangeId(sub, subMasterId).get()).current().submit();
+ gApi.changes().id(getChangeId(subRepo, subMasterId).get()).current().submit();
- expectToHaveSubmoduleState(superRepo, "master", "sub-master", sub, "master");
- expectToHaveSubmoduleState(superRepo, "master", "sub-dev", sub, "dev");
+ expectToHaveSubmoduleState(superRepo, "master", nameKey("sub-master"), subRepo, "master");
+ expectToHaveSubmoduleState(superRepo, "master", nameKey("sub-dev"), subRepo, "dev");
superRepo
.git()
@@ -504,29 +492,27 @@
@Test
public void nonSubmoduleInSameTopic() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub = createProjectWithPush("sub");
- TestRepository<?> standAlone = createProjectWithPush("standalone");
+ Project.NameKey standaloneKey = createProjectForPush("standalone", null, true, getSubmitType());
+ TestRepository<?> standAlone = cloneProject(standaloneKey);
- allowMatchingSubmoduleSubscription(
- "sub", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
- createSubmoduleSubscription(superRepo, "master", "sub", "master");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
ObjectId superPreviousId = pushChangeTo(superRepo, "master");
- ObjectId subId = pushChangeTo(sub, "refs/for/master", "some message", "same-topic");
+ ObjectId subId = pushChangeTo(subRepo, "refs/for/master", "some message", "same-topic");
ObjectId standAloneId =
pushChangeTo(standAlone, "refs/for/master", "some message", "same-topic");
- String subChangeId = getChangeId(sub, subId).get();
+ String subChangeId = getChangeId(subRepo, subId).get();
String standAloneChangeId = getChangeId(standAlone, standAloneId).get();
approve(subChangeId);
approve(standAloneChangeId);
gApi.changes().id(subChangeId).current().submit();
- expectToHaveSubmoduleState(superRepo, "master", "sub", sub, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey, subRepo, "master");
ChangeStatus status = gApi.changes().id(standAloneChangeId).info().status;
assertThat(status).isEqualTo(ChangeStatus.MERGED);
@@ -546,17 +532,18 @@
@Test
public void recursiveSubmodules() throws Exception {
- TestRepository<?> topRepo = createProjectWithPush("top-project");
- TestRepository<?> midRepo = createProjectWithPush("mid-project");
- TestRepository<?> bottomRepo = createProjectWithPush("bottom-project");
+ Project.NameKey topKey = createProjectForPush("top-project", null, true, getSubmitType());
+ Project.NameKey midKey = createProjectForPush("mid-project", null, true, getSubmitType());
+ Project.NameKey botKey = createProjectForPush("bottom-project", null, true, getSubmitType());
+ TestRepository<?> topRepo = cloneProject(topKey);
+ TestRepository<?> midRepo = cloneProject(midKey);
+ TestRepository<?> bottomRepo = cloneProject(botKey);
- allowMatchingSubmoduleSubscription(
- "mid-project", "refs/heads/master", "top-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "bottom-project", "refs/heads/master", "mid-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(midKey, "refs/heads/master", topKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(botKey, "refs/heads/master", midKey, "refs/heads/master");
- createSubmoduleSubscription(topRepo, "master", "mid-project", "master");
- createSubmoduleSubscription(midRepo, "master", "bottom-project", "master");
+ createSubmoduleSubscription(topRepo, "master", midKey, "master");
+ createSubmoduleSubscription(midRepo, "master", botKey, "master");
ObjectId bottomHead = pushChangeTo(bottomRepo, "refs/for/master", "some message", "same-topic");
ObjectId topHead = pushChangeTo(topRepo, "refs/for/master", "some message", "same-topic");
@@ -569,27 +556,27 @@
gApi.changes().id(id1).current().submit();
- expectToHaveSubmoduleState(midRepo, "master", "bottom-project", bottomRepo, "master");
- expectToHaveSubmoduleState(topRepo, "master", "mid-project", midRepo, "master");
+ expectToHaveSubmoduleState(midRepo, "master", botKey, bottomRepo, "master");
+ expectToHaveSubmoduleState(topRepo, "master", midKey, midRepo, "master");
}
@Test
public void triangleSubmodules() throws Exception {
- TestRepository<?> topRepo = createProjectWithPush("top-project");
- TestRepository<?> midRepo = createProjectWithPush("mid-project");
- TestRepository<?> bottomRepo = createProjectWithPush("bottom-project");
+ Project.NameKey topKey = createProjectForPush("top-project", null, true, getSubmitType());
+ Project.NameKey midKey = createProjectForPush("mid-project", null, true, getSubmitType());
+ Project.NameKey botKey = createProjectForPush("bottom-project", null, true, getSubmitType());
+ TestRepository<?> topRepo = cloneProject(topKey);
+ TestRepository<?> midRepo = cloneProject(midKey);
+ TestRepository<?> bottomRepo = cloneProject(botKey);
- allowMatchingSubmoduleSubscription(
- "mid-project", "refs/heads/master", "top-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "bottom-project", "refs/heads/master", "mid-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "bottom-project", "refs/heads/master", "top-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(midKey, "refs/heads/master", topKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(botKey, "refs/heads/master", midKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(botKey, "refs/heads/master", topKey, "refs/heads/master");
- createSubmoduleSubscription(midRepo, "master", "bottom-project", "master");
+ createSubmoduleSubscription(midRepo, "master", botKey, "master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "bottom-project", "master");
- prepareSubmoduleConfigEntry(config, "mid-project", "master");
+ prepareSubmoduleConfigEntry(config, botKey, "master");
+ prepareSubmoduleConfigEntry(config, midKey, "master");
pushSubmoduleConfig(topRepo, "master", config);
ObjectId bottomHead = pushChangeTo(bottomRepo, "refs/for/master", "some message", "same-topic");
@@ -603,26 +590,26 @@
gApi.changes().id(id1).current().submit();
- expectToHaveSubmoduleState(midRepo, "master", "bottom-project", bottomRepo, "master");
- expectToHaveSubmoduleState(topRepo, "master", "mid-project", midRepo, "master");
- expectToHaveSubmoduleState(topRepo, "master", "bottom-project", bottomRepo, "master");
+ expectToHaveSubmoduleState(midRepo, "master", botKey, bottomRepo, "master");
+ expectToHaveSubmoduleState(topRepo, "master", midKey, midRepo, "master");
+ expectToHaveSubmoduleState(topRepo, "master", botKey, bottomRepo, "master");
}
private String prepareBranchCircularSubscription() throws Exception {
- TestRepository<?> topRepo = createProjectWithPush("top-project");
- TestRepository<?> midRepo = createProjectWithPush("mid-project");
- TestRepository<?> bottomRepo = createProjectWithPush("bottom-project");
+ Project.NameKey topKey = createProjectForPush("top-project", null, true, getSubmitType());
+ Project.NameKey midKey = createProjectForPush("mid-project", null, true, getSubmitType());
+ Project.NameKey botKey = createProjectForPush("bottom-project", null, true, getSubmitType());
+ TestRepository<?> topRepo = cloneProject(topKey);
+ TestRepository<?> midRepo = cloneProject(midKey);
+ TestRepository<?> bottomRepo = cloneProject(botKey);
- createSubmoduleSubscription(midRepo, "master", "bottom-project", "master");
- createSubmoduleSubscription(topRepo, "master", "mid-project", "master");
- createSubmoduleSubscription(bottomRepo, "master", "top-project", "master");
+ createSubmoduleSubscription(midRepo, "master", botKey, "master");
+ createSubmoduleSubscription(topRepo, "master", midKey, "master");
+ createSubmoduleSubscription(bottomRepo, "master", topKey, "master");
- allowMatchingSubmoduleSubscription(
- "bottom-project", "refs/heads/master", "mid-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "mid-project", "refs/heads/master", "top-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "top-project", "refs/heads/master", "bottom-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(botKey, "refs/heads/master", midKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(midKey, "refs/heads/master", topKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(topKey, "refs/heads/master", botKey, "refs/heads/master");
ObjectId bottomMasterHead = pushChangeTo(bottomRepo, "refs/for/master", "some message", "");
String changeId = getChangeId(bottomRepo, bottomMasterHead).get();
@@ -649,19 +636,14 @@
@Test
public void projectCircularSubscriptionWholeTopic() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
-
- allowMatchingSubmoduleSubscription(
- "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "super-project", "refs/heads/dev", "subscribed-to-project", "refs/heads/dev");
+ allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(superKey, "refs/heads/dev", subKey, "refs/heads/dev");
pushChangeTo(subRepo, "dev");
pushChangeTo(superRepo, "dev");
- createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
- createSubmoduleSubscription(subRepo, "dev", "super-project", "dev");
+ createSubmoduleSubscription(superRepo, "master", subKey, "master");
+ createSubmoduleSubscription(subRepo, "dev", superKey, "dev");
ObjectId subMasterHead =
pushChangeTo(
@@ -672,15 +654,15 @@
approve(getChangeId(superRepo, superDevHead).get());
exception.expectMessage("Project level circular subscriptions detected");
- exception.expectMessage("subscribed-to-project");
- exception.expectMessage("super-project");
+ exception.expectMessage(subKey.get());
+ exception.expectMessage(superKey.get());
gApi.changes().id(getChangeId(subRepo, subMasterHead).get()).current().submit();
}
@Test
public void projectNoSubscriptionWholeTopic() throws Exception {
- TestRepository<?> repoA = createProjectWithPush("project-a");
- TestRepository<?> repoB = createProjectWithPush("project-b");
+ TestRepository<?> repoA = createProjectWithPush("project-a", null, true, getSubmitType());
+ TestRepository<?> repoB = createProjectWithPush("project-b", null, true, getSubmitType());
// bootstrap the dev branch
ObjectId a0 = pushChangeTo(repoA, "dev");
@@ -747,8 +729,8 @@
@Test
public void twoProjectsMultipleBranchesWholeTopic() throws Exception {
- TestRepository<?> repoA = createProjectWithPush("project-a");
- TestRepository<?> repoB = createProjectWithPush("project-b");
+ TestRepository<?> repoA = createProjectWithPush("project-a", null, true, getSubmitType());
+ TestRepository<?> repoB = createProjectWithPush("project-b", null, true, getSubmitType());
// bootstrap the dev branch
pushChangeTo(repoA, "dev");
@@ -796,18 +778,17 @@
public void retrySubmitAfterTornTopicOnLockFailure() throws Exception {
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub1 = createProjectWithPush("sub1");
- TestRepository<?> sub2 = createProjectWithPush("sub2");
+ Project.NameKey subKey1 = createProjectForPush("sub1", null, true, getSubmitType());
+ TestRepository<?> sub1 = cloneProject(subKey1);
+ Project.NameKey subKey2 = createProjectForPush("sub2", null, true, getSubmitType());
+ TestRepository<?> sub2 = cloneProject(subKey2);
- allowMatchingSubmoduleSubscription(
- "sub1", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub2", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey1, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey2, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "sub1", "master");
- prepareSubmoduleConfigEntry(config, "sub2", "master");
+ prepareSubmoduleConfigEntry(config, subKey1, "master");
+ prepareSubmoduleConfigEntry(config, subKey2, "master");
pushSubmoduleConfig(superRepo, "master", config);
ObjectId superPreviousId = pushChangeTo(superRepo, "master");
@@ -837,20 +818,20 @@
sub1.git().fetch().call();
RevWalk rw1 = sub1.getRevWalk();
- RevCommit master1 = rw1.parseCommit(getRemoteHead(name("sub1"), "master"));
+ RevCommit master1 = rw1.parseCommit(getRemoteHead(subKey1.get(), "master"));
RevCommit change1Ps = parseCurrentRevision(rw1, changeId1);
assertThat(rw1.isMergedInto(change1Ps, master1)).isTrue();
sub2.git().fetch().call();
RevWalk rw2 = sub2.getRevWalk();
- RevCommit master2 = rw2.parseCommit(getRemoteHead(name("sub2"), "master"));
+ RevCommit master2 = rw2.parseCommit(getRemoteHead(subKey2.get(), "master"));
RevCommit change2Ps = parseCurrentRevision(rw2, changeId2);
assertThat(rw2.isMergedInto(change2Ps, master2)).isTrue();
assertThat(input.generateLockFailures).containsExactly(false);
- expectToHaveSubmoduleState(superRepo, "master", "sub1", sub1, "master");
- expectToHaveSubmoduleState(superRepo, "master", "sub2", sub2, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey1, sub1, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey2, sub2, "master");
assertWithMessage("submodule subscription update should have made one commit")
.that(superRepo.getRepository().resolve("origin/master^"))
@@ -859,24 +840,23 @@
@Test
public void skipUpdatingBrokenGitlinkPointer() throws Exception {
- TestRepository<?> superRepo = createProjectWithPush("super-project");
- TestRepository<?> sub1 = createProjectWithPush("sub1");
- TestRepository<?> sub2 = createProjectWithPush("sub2");
+ Project.NameKey subKey1 = createProjectForPush("sub1", null, true, getSubmitType());
+ TestRepository<?> sub1 = cloneProject(subKey1);
+ Project.NameKey subKey2 = createProjectForPush("sub2", null, true, getSubmitType());
+ TestRepository<?> sub2 = cloneProject(subKey2);
- allowMatchingSubmoduleSubscription(
- "sub1", "refs/heads/master", "super-project", "refs/heads/master");
- allowMatchingSubmoduleSubscription(
- "sub2", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey1, "refs/heads/master", superKey, "refs/heads/master");
+ allowMatchingSubmoduleSubscription(subKey2, "refs/heads/master", superKey, "refs/heads/master");
Config config = new Config();
- prepareSubmoduleConfigEntry(config, "sub1", "master");
- prepareSubmoduleConfigEntry(config, "sub2", "master");
+ prepareSubmoduleConfigEntry(config, subKey1, "master");
+ prepareSubmoduleConfigEntry(config, subKey2, "master");
pushSubmoduleConfig(superRepo, "master", config);
// Write an invalid SHA-1 directly to one of the gitlinks.
ObjectId badId = ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
- directUpdateSubmodule("super-project", "refs/heads/master", "sub1", badId);
- expectToHaveSubmoduleState(superRepo, "master", "sub1", badId);
+ directUpdateSubmodule(superKey, "refs/heads/master", subKey1, badId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey1, badId);
String topic = "same-topic";
ObjectId sub1Id = pushChangeTo(sub1, "refs/for/master", "some message", topic);
@@ -893,7 +873,7 @@
assertThat(info(changeId2).status).isEqualTo(ChangeStatus.MERGED);
// sub1 was skipped but sub2 succeeded.
- expectToHaveSubmoduleState(superRepo, "master", "sub1", badId);
- expectToHaveSubmoduleState(superRepo, "master", "sub2", sub2, "master");
+ expectToHaveSubmoduleState(superRepo, "master", subKey1, badId);
+ expectToHaveSubmoduleState(superRepo, "master", subKey2, sub2, "master");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
index 0d5d2cd..29a5bd0 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -14,28 +14,17 @@
package com.google.gerrit.acceptance.pgm;
-import com.google.gerrit.elasticsearch.ElasticContainer;
-import com.google.gerrit.elasticsearch.ElasticTestUtils;
-import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
+import static com.google.gerrit.elasticsearch.ElasticTestUtils.createAllIndexes;
+import static com.google.gerrit.elasticsearch.ElasticTestUtils.getConfig;
+
import com.google.gerrit.elasticsearch.ElasticVersion;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Injector;
-import java.util.UUID;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
public class ElasticReindexIT extends AbstractReindexTests {
- private static Config getConfig(ElasticVersion version) {
- ElasticNodeInfo elasticNodeInfo;
- ElasticContainer<?> container = ElasticContainer.createAndStart(version);
- elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
- String indicesPrefix = UUID.randomUUID().toString();
- Config cfg = new Config();
- ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix, version);
- return cfg;
- }
-
@ConfigSuite.Default
public static Config elasticsearchV2() {
return getConfig(ElasticVersion.V2_4);
@@ -48,12 +37,12 @@
@ConfigSuite.Config
public static Config elasticsearchV6() {
- return getConfig(ElasticVersion.V6_4);
+ return getConfig(ElasticVersion.V6_5);
}
@Override
public void configureIndex(Injector injector) throws Exception {
- ElasticTestUtils.createAllIndexes(injector);
+ createAllIndexes(injector);
}
@Before
diff --git a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
index 9d69955..1e60071 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java
@@ -14,27 +14,16 @@
package com.google.gerrit.acceptance.ssh;
-import com.google.gerrit.elasticsearch.ElasticContainer;
-import com.google.gerrit.elasticsearch.ElasticTestUtils;
-import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
+import static com.google.gerrit.elasticsearch.ElasticTestUtils.createAllIndexes;
+import static com.google.gerrit.elasticsearch.ElasticTestUtils.getConfig;
+
import com.google.gerrit.elasticsearch.ElasticVersion;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Injector;
-import java.util.UUID;
import org.eclipse.jgit.lib.Config;
public class ElasticIndexIT extends AbstractIndexTests {
- private static Config getConfig(ElasticVersion version) {
- ElasticNodeInfo elasticNodeInfo;
- ElasticContainer<?> container = ElasticContainer.createAndStart(version);
- elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
- String indicesPrefix = UUID.randomUUID().toString();
- Config cfg = new Config();
- ElasticTestUtils.configure(cfg, elasticNodeInfo.port, indicesPrefix, version);
- return cfg;
- }
-
@ConfigSuite.Default
public static Config elasticsearchV2() {
return getConfig(ElasticVersion.V2_4);
@@ -47,11 +36,11 @@
@ConfigSuite.Config
public static Config elasticsearchV6() {
- return getConfig(ElasticVersion.V6_4);
+ return getConfig(ElasticVersion.V6_5);
}
@Override
public void configureIndex(Injector injector) throws Exception {
- ElasticTestUtils.createAllIndexes(injector);
+ createAllIndexes(injector);
}
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 93e97c4..c3150f1 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -52,6 +52,8 @@
return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.2";
case V6_4:
return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.3";
+ case V6_5:
+ return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.5.0";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
index b46e040..9f7b60c 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -21,6 +21,7 @@
import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.util.Collection;
+import java.util.UUID;
import org.eclipse.jgit.lib.Config;
public final class ElasticTestUtils {
@@ -55,6 +56,16 @@
}
}
+ public static Config getConfig(ElasticVersion version) {
+ ElasticNodeInfo elasticNodeInfo;
+ ElasticContainer<?> container = ElasticContainer.createAndStart(version);
+ elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
+ String indicesPrefix = UUID.randomUUID().toString();
+ Config cfg = new Config();
+ configure(cfg, elasticNodeInfo.port, indicesPrefix, version);
+ return cfg;
+ }
+
private ElasticTestUtils() {
// hide default constructor
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
index b8154ce..eeb4c09 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V6_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V6_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
index 3445b36..7525b65 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V6_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V6_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
index 851b27d..e8d5683 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
@@ -41,7 +41,7 @@
return;
}
- container = ElasticContainer.createAndStart(ElasticVersion.V6_4);
+ container = ElasticContainer.createAndStart(ElasticVersion.V6_5);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
}
diff --git a/javatests/com/google/gerrit/server/schema/ReviewDbSchemaCreatorTest.java b/javatests/com/google/gerrit/server/schema/ReviewDbSchemaCreatorTest.java
index c2bf422..bb358cc 100644
--- a/javatests/com/google/gerrit/server/schema/ReviewDbSchemaCreatorTest.java
+++ b/javatests/com/google/gerrit/server/schema/ReviewDbSchemaCreatorTest.java
@@ -31,7 +31,6 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.File;
-import java.io.IOException;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
@@ -63,7 +62,7 @@
}
@Test
- public void getCauses_CreateSchema() throws OrmException, SQLException, IOException {
+ public void getCauses_CreateSchema() throws OrmException, SQLException {
// Initially the schema should be empty.
String[] types = {"TABLE", "VIEW"};
try (JdbcSchema d = (JdbcSchema) db.open();
@@ -82,7 +81,6 @@
if (sitePath.getName().equals(".")) {
sitePath = sitePath.getParentFile();
}
- assertThat(db.getSystemConfig().sitePath).isEqualTo(sitePath.getCanonicalPath());
}
private LabelTypes getLabelTypes() throws Exception {
diff --git a/javatests/com/google/gerrit/server/schema/ReviewDbSchemaUpdaterTest.java b/javatests/com/google/gerrit/server/schema/ReviewDbSchemaUpdaterTest.java
index c0684d1..4133558 100644
--- a/javatests/com/google/gerrit/server/schema/ReviewDbSchemaUpdaterTest.java
+++ b/javatests/com/google/gerrit/server/schema/ReviewDbSchemaUpdaterTest.java
@@ -20,7 +20,6 @@
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
-import com.google.gerrit.reviewdb.client.SystemConfig;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -148,7 +147,5 @@
u.update(new TestUpdateUI());
db.assertSchemaVersion();
- final SystemConfig sc = db.getSystemConfig();
- assertThat(sc.sitePath).isEqualTo(paths.site_path.toAbsolutePath().toString());
}
}
diff --git a/plugins/download-commands b/plugins/download-commands
index cf58d79..edd7156 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit cf58d79bc034e8904aa459d8974df5796a734e1d
+Subproject commit edd715618415d9a5e03b4555d9a2d3cca8fff6e8
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
index 5fa8187..6ce1a09 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
@@ -56,7 +56,6 @@
<style include="gr-form-styles"></style>
<main class="gr-form-styles read-only">
<style include="shared-styles"></style>
- <style include="dashboard-header-styles"></style>
<div class="info">
<h1 id="Title" class$="name">
[[repo]]
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
index 6443095..77a1e2a 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
@@ -307,9 +307,9 @@
commands.push({
title,
command: commandObj[title]
- .replace('${project}', repo)
+ .replace('${project}', encodeURI(repo))
.replace('${project-base-name}',
- repo.substring(repo.lastIndexOf('/') + 1)),
+ encodeURI(repo.substring(repo.lastIndexOf('/') + 1))),
});
}
return commands;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 420a14f..6aea35b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -51,16 +51,6 @@
(function() {
'use strict';
- const Defs = {};
-
- /**
- * @typedef {{
- * number: number,
- * leftSide: {boolean}
- * }}
- */
- Defs.LineOfInterest;
-
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
@@ -115,10 +105,6 @@
parentIndex: Number,
path: String,
projectName: String,
- /**
- * @type {Defs.LineOfInterest|null}
- */
- lineOfInterest: Object,
_builder: Object,
_groups: Array,
@@ -127,6 +113,7 @@
/** @type {!Array<!Gerrit.HoveredRange>} */
commentRanges: {
type: Array,
+ value: () => [],
},
},
@@ -161,7 +148,7 @@
this._layers = layers;
},
- render(comments, prefs) {
+ render(keyLocations, prefs) {
this.$.syntaxLayer.enabled = prefs.syntax_highlighting;
this._showTabs = !!prefs.show_tabs;
this._showTrailingWhitespace = !!prefs.show_whitespace_errors;
@@ -172,8 +159,7 @@
this._builder = this._getDiffBuilder(this.diff, prefs);
this.$.processor.context = prefs.context;
- this.$.processor.keyLocations = this._getKeyLocations(comments,
- this.lineOfInterest);
+ this.$.processor.keyLocations = keyLocations;
this._clearDiffContent();
this._builder.addColumns(this.diffElement, prefs.font_size);
@@ -332,33 +318,6 @@
this.diffElement.innerHTML = null;
},
- /**
- * @param {!Object} comments
- * @param {Defs.LineOfInterest|null} lineOfInterest
- */
- _getKeyLocations(comments, lineOfInterest) {
- const result = {
- left: {},
- right: {},
- };
- for (const side in comments) {
- if (side !== GrDiffBuilder.Side.LEFT &&
- side !== GrDiffBuilder.Side.RIGHT) {
- continue;
- }
- for (const c of comments[side]) {
- result[side][c.line || GrDiffLine.FILE] = true;
- }
- }
-
- if (lineOfInterest) {
- const side = lineOfInterest.leftSide ? 'left' : 'right';
- result[side][lineOfInterest.number] = true;
- }
-
- return result;
- },
-
_groupsChanged(changeRecord) {
if (!changeRecord) { return; }
for (const splice of changeRecord.indexSplices) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index fd74d55..c7a1140 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -396,33 +396,6 @@
`Fix in diff preferences`);
});
- test('_getKeyLocations', () => {
- assert.deepEqual(element._getKeyLocations({left: [], right: []}, null),
- {left: {}, right: {}});
- const comments = {
- left: [{line: 123}, {}],
- right: [{line: 456}],
- };
- assert.deepEqual(element._getKeyLocations(comments, null), {
- left: {FILE: true, 123: true},
- right: {456: true},
- });
-
- const lineOfInterest = {number: 789, leftSide: true};
- assert.deepEqual(
- element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true, 789: true},
- right: {456: true},
- });
-
- delete lineOfInterest.leftSide;
- assert.deepEqual(
- element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true},
- right: {456: true, 789: true},
- });
- });
-
suite('_isTotal', () => {
test('is total for add', () => {
const group = new GrDiffGroup(GrDiffGroup.Type.DELTA);
@@ -852,7 +825,7 @@
suite('rendering text, images and binary files', () => {
let processStub;
- let comments;
+ let keyLocations;
let prefs;
let content;
@@ -862,7 +835,7 @@
processStub = sandbox.stub(element.$.processor, 'process')
.returns(Promise.resolve());
sandbox.stub(element, '_anyLineTooLong').returns(true);
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
prefs = {
line_length: 10,
show_tabs: true,
@@ -883,7 +856,7 @@
test('text', () => {
element.diff = {content};
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isFalse(processStub.lastCall.args[1]);
});
@@ -892,7 +865,7 @@
test('image', () => {
element.diff = {content, binary: true};
element.isImageDiff = true;
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isTrue(processStub.lastCall.args[1]);
});
@@ -900,7 +873,7 @@
test('binary', () => {
element.diff = {content, binary: true};
- return element.render(comments, prefs).then(() => {
+ return element.render(keyLocations, prefs).then(() => {
assert.isTrue(processStub.calledOnce);
assert.isTrue(processStub.lastCall.args[1]);
});
@@ -910,7 +883,7 @@
suite('rendering', () => {
let content;
let outputEl;
- let comments;
+ let keyLocations;
setup(done => {
const prefs = {
@@ -938,7 +911,7 @@
});
element = fixture('basic');
outputEl = element.queryEffectiveChildren('#diffTable');
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
sandbox.stub(element, '_getDiffBuilder', () => {
const builder = new GrDiffBuilder({content}, [], prefs, outputEl);
sandbox.stub(builder, 'addColumns');
@@ -952,7 +925,7 @@
return builder;
});
element.diff = {content};
- element.render(comments, prefs).then(done);
+ element.render(keyLocations, prefs).then(done);
});
test('reporting', done => {
@@ -977,7 +950,7 @@
});
test('addColumns is called', done => {
- element.render(comments, {}).then(done);
+ element.render(keyLocations, {}).then(done);
assert.isTrue(element._builder.addColumns.called);
});
@@ -1001,7 +974,7 @@
test('render-start and render are fired', done => {
const dispatchEventStub = sandbox.stub(element, 'dispatchEvent');
- element.render(comments, {}).then(() => {
+ element.render(keyLocations, {}).then(() => {
const firedEventTypes = dispatchEventStub.getCalls()
.map(c => { return c.args[0].type; });
assert.include(firedEventTypes, 'render-start');
@@ -1029,7 +1002,7 @@
context: -1,
syntax_highlighting: true,
};
- element.render(comments, prefs);
+ element.render(keyLocations, prefs);
});
test('cancel', () => {
@@ -1046,7 +1019,7 @@
let builder;
let diff;
let prefs;
- let comments;
+ let keyLocations;
setup(done => {
element = fixture('mock-diff');
@@ -1058,9 +1031,9 @@
show_tabs: true,
tab_size: 4,
};
- comments = {left: [], right: [], meta: {patchRange: undefined}};
+ keyLocations = {left: {}, right: {}};
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
done();
});
@@ -1170,7 +1143,7 @@
test('_getNextContentOnSide unified left', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'left',
@@ -1190,7 +1163,7 @@
test('_getNextContentOnSide unified right', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render(comments, prefs).then(() => {
+ element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'right',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index d335e7a..3eb7b37 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -37,7 +37,6 @@
commit-range="[[commitRange]]"
hidden$="[[hidden]]"
no-render-on-prefs-change="[[noRenderOnPrefsChange]]"
- comments="[[comments]]"
line-wrapping="[[lineWrapping]]"
view-mode="[[viewMode]]"
line-of-interest="[[lineOfInterest]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 814c7268..5839141 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -199,7 +199,7 @@
_threadEls: {
type: Array,
- value: [],
+ value: () => [],
},
},
@@ -208,7 +208,16 @@
],
listeners: {
+ // These are named inconsistently for a reason:
+ // The create-comment event is fired to indicate that we should
+ // create a comment.
+ // The comment-* events are just notifying that the comments did already
+ // change in some way, and that we should update any models we may want
+ // to keep in sync.
'create-comment': '_handleCreateComment',
+ 'comment-discard': '_handleCommentDiscard',
+ 'comment-update': '_handleCommentUpdate',
+ 'comment-save': '_handleCommentSave',
},
observers: [
@@ -723,5 +732,76 @@
this.getParentIndex(patchRangeRecord.base.basePatchNum) : null;
},
+ _handleCommentSave(e) {
+ const comment = e.detail.comment;
+ const side = e.detail.comment.__commentSide;
+ const idx = this._findDraftIndex(comment, side);
+ this.set(['comments', side, idx], comment);
+ this._handleCommentSaveOrDiscard();
+ },
+
+ _handleCommentDiscard(e) {
+ const comment = e.detail.comment;
+ this._removeComment(comment);
+ this._handleCommentSaveOrDiscard();
+ },
+
+ /**
+ * Closure annotation for Polymer.prototype.push is off. Submitted PR:
+ * https://github.com/Polymer/polymer/pull/4776
+ * but for not supressing annotations.
+ *
+ * @suppress {checkTypes}
+ */
+ _handleCommentUpdate(e) {
+ const comment = e.detail.comment;
+ const side = e.detail.comment.__commentSide;
+ let idx = this._findCommentIndex(comment, side);
+ if (idx === -1) {
+ idx = this._findDraftIndex(comment, side);
+ }
+ if (idx !== -1) { // Update draft or comment.
+ this.set(['comments', side, idx], comment);
+ } else { // Create new draft.
+ this.push(['comments', side], comment);
+ }
+ },
+
+ _handleCommentSaveOrDiscard() {
+ this.dispatchEvent(new CustomEvent('diff-comments-modified',
+ {bubbles: true}));
+ },
+
+ _removeComment(comment) {
+ const side = comment.__commentSide;
+ this._removeCommentFromSide(comment, side);
+ },
+
+ _removeCommentFromSide(comment, side) {
+ let idx = this._findCommentIndex(comment, side);
+ if (idx === -1) {
+ idx = this._findDraftIndex(comment, side);
+ }
+ if (idx !== -1) {
+ this.splice('comments.' + side, idx, 1);
+ }
+ },
+
+ /** @return {number} */
+ _findCommentIndex(comment, side) {
+ if (!comment.id || !this.comments[side]) {
+ return -1;
+ }
+ return this.comments[side].findIndex(item => item.id === comment.id);
+ },
+
+ /** @return {number} */
+ _findDraftIndex(comment, side) {
+ if (!comment.__draftID || !this.comments[side]) {
+ return -1;
+ }
+ return this.comments[side].findIndex(
+ item => item.__draftID === comment.__draftID);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 423bdc6..5344256 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -46,15 +46,195 @@
async getLoggedIn() { return getLoggedIn; },
});
element = fixture('basic');
- // For reasons beyond me, fixture reuses elements, cleans out some
- // stuff but not that list.
- element._threadEls = [];
});
teardown(() => {
sandbox.restore();
});
+ suite('handle comment-update', () => {
+ setup(() => {
+ sandbox.stub(element, '_commentsChanged');
+ element.comments = {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ {id: 'd2', __draft: true, __commentSide: 'right'},
+ ],
+ };
+ });
+
+ test('creating a draft', () => {
+ const comment = {__draft: true, __draftID: 'tempID', side: 'PARENT',
+ __commentSide: 'left'};
+ element.fire('comment-update', {comment});
+ assert.include(element.comments.left, comment);
+ });
+
+ test('discarding a draft', () => {
+ const draftID = 'tempID';
+ const id = 'savedID';
+ const comment = {
+ __draft: true,
+ __draftID: draftID,
+ side: 'PARENT',
+ __commentSide: 'left',
+ };
+ const diffCommentsModifiedStub = sandbox.stub();
+ element.addEventListener('diff-comments-modified',
+ diffCommentsModifiedStub);
+ element.comments.left.push(comment);
+ comment.id = id;
+ element.fire('comment-discard', {comment});
+ const drafts = element.comments.left.filter(item => {
+ return item.__draftID === draftID;
+ });
+ assert.equal(drafts.length, 0);
+ assert.isTrue(diffCommentsModifiedStub.called);
+ });
+
+ test('saving a draft', () => {
+ const draftID = 'tempID';
+ const id = 'savedID';
+ const comment = {
+ __draft: true,
+ __draftID: draftID,
+ side: 'PARENT',
+ __commentSide: 'left',
+ };
+ const diffCommentsModifiedStub = sandbox.stub();
+ element.addEventListener('diff-comments-modified',
+ diffCommentsModifiedStub);
+ element.comments.left.push(comment);
+ comment.id = id;
+ element.fire('comment-save', {comment});
+ const drafts = element.comments.left.filter(item => {
+ return item.__draftID === draftID;
+ });
+ assert.equal(drafts.length, 1);
+ assert.equal(drafts[0].id, id);
+ assert.isTrue(diffCommentsModifiedStub.called);
+ });
+ });
+
+ test('remove comment', () => {
+ sandbox.stub(element, '_commentsChanged');
+ element.comments = {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ {id: 'd2', __draft: true, __commentSide: 'right'},
+ ],
+ };
+
+ element._removeComment({});
+ // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
+ // to believe that one object deepEquals another even when they do :-/.
+ assert.equal(JSON.stringify(element.comments), JSON.stringify({
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ {id: 'd2', __draft: true, __commentSide: 'right'},
+ ],
+ }));
+
+ element._removeComment({id: 'bc2', side: 'PARENT',
+ __commentSide: 'left'});
+ assert.deepEqual(element.comments, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ {id: 'd2', __draft: true, __commentSide: 'right'},
+ ],
+ });
+
+ element._removeComment({id: 'd2', __commentSide: 'right'});
+ assert.deepEqual(element.comments, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ ],
+ });
+ });
+
test('thread-discard handling', () => {
const threads = [
{comments: [{id: 4711}]},
@@ -685,12 +865,6 @@
assert.equal(element.$.diff.noRenderOnPrefsChange, value);
});
- test('passes in comments', () => {
- const value = {left: [], right: []};
- element.comments = value;
- assert.equal(element.$.diff.comments, value);
- });
-
test('passes in lineWrapping', () => {
const value = true;
element.lineWrapping = value;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index fd54af4..5a56069 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -1040,6 +1040,7 @@
},
_handleNextUnreviewedFile(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
this._setReviewed(true);
// Ensure that the currently viewed file always appears in unreviewedFiles
// so we resolve the right "next" file.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 862db10..e587953 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -293,8 +293,7 @@
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
- revision-image="[[revisionImage]]"
- line-of-interest="[[lineOfInterest]]">
+ revision-image="[[revisionImage]]">
<slot></slot>
<table
id="diffTable"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index f87e46f..a8cf320 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -35,6 +35,18 @@
RIGHT: 'right',
};
+ const Defs = {};
+
+ /**
+ * Special line number which should not be collapsed into a shared region.
+ *
+ * @typedef {{
+ * number: number,
+ * leftSide: boolean
+ * }}
+ */
+ Defs.LineOfInterest;
+
const LARGE_DIFF_THRESHOLD_LINES = 10000;
const FULL_CONTEXT = -1;
const LIMITED_CONTEXT = 10;
@@ -62,12 +74,6 @@
* @event show-auth-required
*/
- /**
- * Fired when a comment is saved or discarded
- *
- * @event diff-comments-modified
- */
-
/**
* Fired when a comment is created
*
@@ -101,14 +107,10 @@
reflectToAttribute: true,
},
noRenderOnPrefsChange: Boolean,
- comments: {
- type: Object,
- value: {left: [], right: []},
- },
/** @type {!Array<!Gerrit.HoveredRange>} */
_commentRanges: {
type: Array,
- value: [],
+ value: () => [],
},
lineWrapping: {
type: Boolean,
@@ -121,15 +123,24 @@
observer: '_viewModeObserver',
},
- /**
- * Special line number which should not be collapsed into a shared region.
- * @type {{
- * number: number,
- * leftSide: {boolean}
- * }|null}
- */
+ /** @type ?Defs.LineOfInterest */
lineOfInterest: Object,
+ /**
+ * The key locations based on the comments and line of interests,
+ * where lines should not be collapsed.
+ *
+ * @type {{left: Object<(string|number), number>,
+ * right: Object<(string|number), number>}}
+ */
+ _keyLocations: {
+ type: Object,
+ value: () => ({
+ left: {},
+ right: {},
+ }),
+ },
+
loading: {
type: Boolean,
value: false,
@@ -216,15 +227,12 @@
],
listeners: {
- 'comment-discard': '_handleCommentDiscard',
- 'comment-update': '_handleCommentUpdate',
- 'comment-save': '_handleCommentSave',
'create-range-comment': '_handleCreateRangeComment',
'render-content': '_handleRenderContent',
},
attached() {
- this._updateRangesWhenNodesChange();
+ this._observeNodes();
},
detached() {
@@ -232,25 +240,38 @@
this._unobserveNodes();
},
- _updateRangesWhenNodesChange() {
+ _observeNodes() {
+ this._nodeObserver = Polymer.dom(this).observeNodes(info => {
+ const addedThreadEls = info.addedNodes.filter(isThreadEl);
+ // In principle we should also handle removed nodes, but I have not
+ // figured out how to do that yet without also catching all the removals
+ // caused by further redistribution. Right now, comments are never
+ // removed by no longer slotting them in, so I decided to not handle
+ // this situation until it occurs.
+ this._updateRanges(addedThreadEls);
+ this._updateKeyLocations(addedThreadEls);
+ });
+ },
+
+ _updateRanges(addedThreadEls) {
function commentRangeFromThreadEl(threadEl) {
const side = threadEl.getAttribute('comment-side');
const range = JSON.parse(threadEl.getAttribute('range'));
return {side, range, hovering: false};
}
- this._nodeObserver = Polymer.dom(this).observeNodes(info => {
- const addedThreadEls = info.addedNodes.filter(isThreadEl);
- const addedCommentRanges = addedThreadEls
- .map(commentRangeFromThreadEl)
- .filter(({range}) => range);
- this.push('_commentRanges', ...addedCommentRanges);
- // In principal we should also handle removed nodes, but I have not
- // figured out how to do that yet without also catching all the removals
- // caused by further redistribution. Right now, comments are never
- // removed by no longer slotting them in, so I decided to not handle
- // this situation until it occurs.
- });
+ const addedCommentRanges = addedThreadEls
+ .map(commentRangeFromThreadEl)
+ .filter(({range}) => range);
+ this.push('_commentRanges', ...addedCommentRanges);
+ },
+
+ _updateKeyLocations(addedThreadEls) {
+ for (const threadEl of addedThreadEls) {
+ const commentSide = threadEl.getAttribute('comment-side');
+ const lineNum = threadEl.getAttribute('line-num') || GrDiffLine.FILE;
+ this._keyLocations[commentSide][lineNum] = true;
+ }
},
/** Cancel any remaining diff builder rendering work. */
@@ -285,11 +306,6 @@
}
},
- _handleCommentSaveOrDiscard() {
- this.dispatchEvent(new CustomEvent('diff-comments-modified',
- {bubbles: true}));
- },
-
/** @return {string} */
_computeContainerClass(loggedIn, viewMode, displayLine) {
const classes = ['diffContainer'];
@@ -484,75 +500,6 @@
return side;
},
- _handleCommentDiscard(e) {
- const comment = e.detail.comment;
- this._removeComment(comment);
- this._handleCommentSaveOrDiscard();
- },
-
- _removeComment(comment) {
- const side = comment.__commentSide;
- this._removeCommentFromSide(comment, side);
- },
-
- _handleCommentSave(e) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- const idx = this._findDraftIndex(comment, side);
- this.set(['comments', side, idx], comment);
- this._handleCommentSaveOrDiscard();
- },
-
- /**
- * Closure annotation for Polymer.prototype.push is off. Submitted PR:
- * https://github.com/Polymer/polymer/pull/4776
- * but for not supressing annotations.
- *
- * @suppress {checkTypes} */
- _handleCommentUpdate(e) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) { // Update draft or comment.
- this.set(['comments', side, idx], comment);
- } else { // Create new draft.
- this.push(['comments', side], comment);
- }
- },
-
- _removeCommentFromSide(comment, side) {
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- this.splice('comments.' + side, idx, 1);
- }
- },
-
- /** @return {number} */
- _findCommentIndex(comment, side) {
- if (!comment.id || !this.comments[side]) {
- return -1;
- }
- return this.comments[side].findIndex(item => {
- return item.id === comment.id;
- });
- },
-
- /** @return {number} */
- _findDraftIndex(comment, side) {
- if (!comment.__draftID || !this.comments[side]) {
- return -1;
- }
- return this.comments[side].findIndex(item => {
- return item.__draftID === comment.__draftID;
- });
- },
-
_prefsObserver(newPrefs, oldPrefs) {
// Scan the preference objects one level deep to see if they differ.
let differ = !oldPrefs;
@@ -612,7 +559,7 @@
this.updateStyles(stylesToUpdate);
- if (this.diff && this.comments && !this.noRenderOnPrefsChange) {
+ if (this.diff && !this.noRenderOnPrefsChange) {
this._renderDiffTable();
}
},
@@ -639,13 +586,18 @@
}
this._showWarning = false;
- this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
+
+ if (this.lineOfInterest) {
+ const side = this.lineOfInterest.leftSide ? 'left' : 'right';
+ this._keyLocations[side][this.lineOfInterest.number] = true;
+ }
+ this.$.diffBuilder.render(this._keyLocations, this._getBypassPrefs());
},
_handleRenderContent() {
this._incrementalNodeObserver = Polymer.dom(this).observeNodes(info => {
const addedThreadEls = info.addedNodes.filter(isThreadEl);
- // In principal we should also handle removed nodes, but I have not
+ // In principle we should also handle removed nodes, but I have not
// figured out how to do that yet without also catching all the removals
// caused by further redistribution. Right now, comments are never
// removed by no longer slotting them in, so I decided to not handle
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 62284ad..99f2ded9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -192,107 +192,6 @@
element.$$('.diffContainer').classList.contains('displayLine'));
});
- test('remove comment', () => {
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
-
- element._removeComment({});
- // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
- // to believe that one object deepEquals another even when they do :-/.
- assert.equal(JSON.stringify(element.comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- }));
-
- element._removeComment({id: 'bc2', side: 'PARENT',
- __commentSide: 'left'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- });
-
- element._removeComment({id: 'd2', __commentSide: 'right'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- ],
- });
- });
-
test('thread groups', () => {
const contentEl = document.createElement('div');
@@ -333,11 +232,6 @@
};
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
- element.comments = {
- left: [],
- right: [],
- meta: {patchRange: undefined},
- };
element.isImageDiff = true;
element.prefs = {
auto_hide_diff_table_header: true,
@@ -663,11 +557,6 @@
const setupDiff = function() {
const mock = document.createElement('mock-diff-response');
element.diff = mock.diffResponse;
- element.comments = {
- left: [],
- right: [],
- meta: {patchRange: undefined},
- };
element.prefs = {
context: 10,
tab_size: 8,
@@ -766,29 +655,6 @@
change_type: 'MODIFIED',
content: [{skip: 66}],
};
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
});
test('change in preferences re-renders diff', () => {
@@ -807,86 +673,6 @@
assert.isFalse(element._renderDiffTable.called);
});
});
-
- suite('handle comment-update', () => {
- setup(() => {
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
- });
-
- test('creating a draft', () => {
- const comment = {__draft: true, __draftID: 'tempID', side: 'PARENT',
- __commentSide: 'left'};
- element.fire('comment-update', {comment});
- assert.include(element.comments.left, comment);
- });
-
- test('discarding a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sandbox.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.fire('comment-discard', {comment});
- const drafts = element.comments.left.filter(item => {
- return item.__draftID === draftID;
- });
- assert.equal(drafts.length, 0);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
-
- test('saving a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sandbox.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.fire('comment-save', {comment});
- const drafts = element.comments.left.filter(item => {
- return item.__draftID === draftID;
- });
- assert.equal(drafts.length, 1);
- assert.equal(drafts[0].id, id);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
- });
});
suite('diff header', () => {
@@ -946,7 +732,6 @@
const mock = document.createElement('mock-diff-response');
sandbox.stub(element.$.diffBuilder, 'getDiffLength').returns(10000);
element.diff = mock.diffResponse;
- element.comments = {left: [], right: []};
element.noRenderOnPrefsChange = true;
});
@@ -1127,6 +912,57 @@
assert.equal(element._computeNewlineWarningClass('foo', false), shown);
});
});
+
+ suite('key locations', () => {
+ let renderStub;
+
+ setup(() => {
+ element = fixture('basic');
+ element.prefs = {};
+ renderStub = sandbox.stub(element.$.diffBuilder, 'render');
+ });
+
+ test('lineOfInterest is a key location', () => {
+ element.lineOfInterest = {number: 789, leftSide: true};
+ element._renderDiffTable();
+ assert.isTrue(renderStub.called);
+ assert.deepEqual(renderStub.lastCall.args[0], {
+ left: {789: true},
+ right: {},
+ });
+ });
+
+ test('line comments are key locations', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ Polymer.dom(element).appendChild(threadEl);
+ Polymer.dom.flush();
+
+ element._renderDiffTable();
+ assert.isTrue(renderStub.called);
+ assert.deepEqual(renderStub.lastCall.args[0], {
+ left: {},
+ right: {3: true},
+ });
+ });
+
+ test('file comments are key locations', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('comment-side', 'left');
+ Polymer.dom(element).appendChild(threadEl);
+ Polymer.dom.flush();
+
+ element._renderDiffTable();
+ assert.isTrue(renderStub.called);
+ assert.deepEqual(renderStub.lastCall.args[0], {
+ left: {FILE: true},
+ right: {},
+ });
+ });
+ });
});
a11ySuite('basic');
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index c5faa49..75f632d 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -56,6 +56,9 @@
<name>Luca Milanesio</name>
</developer>
<developer>
+ <name>Marco Miller</name>
+ </developer>
+ <developer>
<name>Martin Fick</name>
</developer>
<developer>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 52b11c1..cb8494b 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -56,6 +56,9 @@
<name>Luca Milanesio</name>
</developer>
<developer>
+ <name>Marco Miller</name>
+ </developer>
+ <developer>
<name>Martin Fick</name>
</developer>
<developer>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index d22c3ee..f58a6c7 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -56,6 +56,9 @@
<name>Luca Milanesio</name>
</developer>
<developer>
+ <name>Marco Miller</name>
+ </developer>
+ <developer>
<name>Martin Fick</name>
</developer>
<developer>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index e6c04e2..9849237 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -56,6 +56,9 @@
<name>Luca Milanesio</name>
</developer>
<developer>
+ <name>Marco Miller</name>
+ </developer>
+ <developer>
<name>Martin Fick</name>
</developer>
<developer>
diff --git a/webapp/WEB-INF/web.xml b/webapp/WEB-INF/web.xml
index 386eb07..e901357 100644
--- a/webapp/WEB-INF/web.xml
+++ b/webapp/WEB-INF/web.xml
@@ -8,7 +8,7 @@
<filter>
<filter-name>guiceFilter</filter-name>
- <filter-class>com.google.gerrit.httpd.WebAppInitializer</filter-class>
+ <filter-class>com.google.gerrit.httpd.init.WebAppInitializer</filter-class>
</filter>
<filter-mapping>
<filter-name>guiceFilter</filter-name>
