Merge "Remove 'data' field from SubmitRequirement"
diff --git a/WORKSPACE b/WORKSPACE
index d07ab5b..015675a 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -213,24 +213,24 @@
sha1 = "42aa5155a54a87d70af32d4b0d06bf43779de0e2",
)
-FLOGGER_VERS = "0.4"
+FLOGGER_VERS = "0.5"
maven_jar(
name = "flogger",
artifact = "com.google.flogger:flogger:" + FLOGGER_VERS,
- sha1 = "9c8863dcc913b56291c0c88e6d4ca9715b43df98",
+ sha1 = "dd7449aa4aba72969ed409b96996d19906d534d7",
)
maven_jar(
name = "flogger-log4j-backend",
artifact = "com.google.flogger:flogger-log4j-backend:" + FLOGGER_VERS,
- sha1 = "17aa5e31daa1354187e14b6978597d630391c028",
+ sha1 = "5e2794b75c88223f263f1c1a9d7ea51e2dc45732",
)
maven_jar(
name = "flogger-system-backend",
artifact = "com.google.flogger:flogger-system-backend:" + FLOGGER_VERS,
- sha1 = "287b569d76abcd82f9de87fe41829fbc7ebd8ac9",
+ sha1 = "b66d3bedb14da604828a8693bb24fd78e36b0e9e",
)
maven_jar(
diff --git a/e2e-tests/load-tests/.gitignore b/e2e-tests/.gitignore
similarity index 100%
rename from e2e-tests/load-tests/.gitignore
rename to e2e-tests/.gitignore
diff --git a/e2e-tests/load-tests/Dockerfile b/e2e-tests/Dockerfile
similarity index 100%
rename from e2e-tests/load-tests/Dockerfile
rename to e2e-tests/Dockerfile
diff --git a/e2e-tests/load-tests/README.md b/e2e-tests/README.md
similarity index 100%
rename from e2e-tests/load-tests/README.md
rename to e2e-tests/README.md
diff --git a/e2e-tests/load-tests/build.sbt b/e2e-tests/build.sbt
similarity index 100%
rename from e2e-tests/load-tests/build.sbt
rename to e2e-tests/build.sbt
diff --git a/e2e-tests/load-tests/project/Dependencies.scala b/e2e-tests/project/Dependencies.scala
similarity index 100%
rename from e2e-tests/load-tests/project/Dependencies.scala
rename to e2e-tests/project/Dependencies.scala
diff --git a/e2e-tests/load-tests/project/build.properties b/e2e-tests/project/build.properties
similarity index 100%
rename from e2e-tests/load-tests/project/build.properties
rename to e2e-tests/project/build.properties
diff --git a/e2e-tests/load-tests/project/plugins.sbt b/e2e-tests/project/plugins.sbt
similarity index 100%
rename from e2e-tests/load-tests/project/plugins.sbt
rename to e2e-tests/project/plugins.sbt
diff --git a/e2e-tests/load-tests/src/test/resources/application.conf b/e2e-tests/src/test/resources/application.conf
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/application.conf
rename to e2e-tests/src/test/resources/application.conf
diff --git a/e2e-tests/load-tests/src/test/resources/data/CloneUsingBothProtocols.json b/e2e-tests/src/test/resources/data/CloneUsingBothProtocols.json
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/data/CloneUsingBothProtocols.json
rename to e2e-tests/src/test/resources/data/CloneUsingBothProtocols.json
diff --git a/e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json b/e2e-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
rename to e2e-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
diff --git a/e2e-tests/load-tests/src/test/resources/gatling.conf b/e2e-tests/src/test/resources/gatling.conf
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/gatling.conf
rename to e2e-tests/src/test/resources/gatling.conf
diff --git a/e2e-tests/load-tests/src/test/resources/hooks/commit-msg b/e2e-tests/src/test/resources/hooks/commit-msg
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/hooks/commit-msg
rename to e2e-tests/src/test/resources/hooks/commit-msg
diff --git a/e2e-tests/load-tests/src/test/resources/logback.xml b/e2e-tests/src/test/resources/logback.xml
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/logback.xml
rename to e2e-tests/src/test/resources/logback.xml
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
similarity index 100%
rename from e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
rename to e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala
similarity index 100%
rename from e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala
rename to e2e-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
similarity index 100%
rename from e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
rename to e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
diff --git a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
index e8f173c..d92da18 100644
--- a/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
+++ b/java/com/google/gerrit/httpd/raw/AuthorizationCheckServlet.java
@@ -43,6 +43,7 @@
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException {
CacheHeaders.setNotCacheable(res);
+ res.setContentLength(0);
if (user.get().isIdentifiedUser()) {
res.setStatus(HttpServletResponse.SC_NO_CONTENT);
} else {
diff --git a/java/com/google/gerrit/server/ExceptionHookImpl.java b/java/com/google/gerrit/server/ExceptionHookImpl.java
index d0b5f15..8884991 100644
--- a/java/com/google/gerrit/server/ExceptionHookImpl.java
+++ b/java/com/google/gerrit/server/ExceptionHookImpl.java
@@ -21,7 +21,6 @@
import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.server.project.ProjectConfig;
import java.util.Optional;
-import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.RefUpdate;
@@ -35,7 +34,9 @@
+ "This may be a temporary issue due to concurrent updates.\n"
+ "Please retry later.";
private static final String INVALID_PROJECT_CONFIG_USER_MESSAGE =
- "Invalid " + ProjectConfig.PROJECT_CONFIG + " file.\n" + "Please contact the project owner.";
+ "Invalid " + ProjectConfig.PROJECT_CONFIG + " file.";
+ private static final String CONTACT_PROJECT_OWNER_USER_MESSAGE =
+ "Please contact the project owner.";
@Override
public boolean shouldRetry(String actionType, String actionName, Throwable throwable) {
@@ -67,7 +68,10 @@
return ImmutableList.of(LOCK_FAILURE_USER_MESSAGE);
}
if (isInvalidProjectConfig(throwable)) {
- return ImmutableList.of(INVALID_PROJECT_CONFIG_USER_MESSAGE);
+ return ImmutableList.of(
+ getInvalidConfigMessage(throwable).orElse(INVALID_PROJECT_CONFIG_USER_MESSAGE)
+ + "\n"
+ + CONTACT_PROJECT_OWNER_USER_MESSAGE);
}
return ImmutableList.of();
}
@@ -95,9 +99,16 @@
return isMatching(
throwable,
t ->
- t instanceof ConfigInvalidException
- && t.getMessage()
- .startsWith("Invalid config file " + ProjectConfig.PROJECT_CONFIG));
+ t instanceof InvalidConfigFileException
+ && ProjectConfig.PROJECT_CONFIG.equals(
+ ((InvalidConfigFileException) t).getFileName()));
+ }
+
+ private Optional<String> getInvalidConfigMessage(Throwable throwable) {
+ return Throwables.getCausalChain(throwable).stream()
+ .filter(InvalidConfigFileException.class::isInstance)
+ .map(ex -> ex.getMessage())
+ .findFirst();
}
/**
diff --git a/java/com/google/gerrit/server/InvalidConfigFileException.java b/java/com/google/gerrit/server/InvalidConfigFileException.java
new file mode 100644
index 0000000..f10b618
--- /dev/null
+++ b/java/com/google/gerrit/server/InvalidConfigFileException.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2020 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;
+
+import com.google.gerrit.entities.Project;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
+
+/** Exception that is thrown if an invalid config file causes an error. */
+public class InvalidConfigFileException extends ConfigInvalidException {
+ private static final long serialVersionUID = 1L;
+
+ private final String fileName;
+
+ public InvalidConfigFileException(
+ Project.NameKey projectName,
+ String branchName,
+ ObjectId revision,
+ String fileName,
+ ConfigInvalidException cause) {
+ super(createMessage(projectName, branchName, revision, fileName, cause), cause);
+ this.fileName = fileName;
+ }
+
+ public String getFileName() {
+ return fileName;
+ }
+
+ private static String createMessage(
+ Project.NameKey projectName,
+ String branchName,
+ ObjectId revision,
+ String fileName,
+ ConfigInvalidException cause) {
+ StringBuilder msg =
+ new StringBuilder("Invalid config file ")
+ .append(fileName)
+ .append(" in project ")
+ .append(projectName.get())
+ .append(" in branch ")
+ .append(branchName)
+ .append(" in commit ")
+ .append(revision.name());
+ if (cause != null) {
+ msg.append(": ").append(cause.getMessage());
+ }
+ return msg.toString();
+ }
+}
diff --git a/java/com/google/gerrit/server/git/UsersSelfAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/UsersSelfAdvertiseRefsHook.java
index 169ae32..6c1879e 100644
--- a/java/com/google/gerrit/server/git/UsersSelfAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/UsersSelfAdvertiseRefsHook.java
@@ -18,11 +18,11 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.CurrentUser;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Map;
-import javax.inject.Inject;
-import javax.inject.Provider;
-import javax.inject.Singleton;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.SymbolicRef;
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index 8ab2779..feb038a 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -23,6 +23,7 @@
import com.google.gerrit.git.GitUpdateFailureException;
import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.git.ObjectIds;
+import com.google.gerrit.server.InvalidConfigFileException;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
@@ -471,15 +472,7 @@
try {
rc.fromText(text);
} catch (ConfigInvalidException err) {
- StringBuilder msg =
- new StringBuilder("Invalid config file ")
- .append(fileName)
- .append(" in commit ")
- .append(revision.name());
- if (err.getCause() != null) {
- msg.append(": ").append(err.getCause());
- }
- throw new ConfigInvalidException(msg.toString(), err);
+ throw new InvalidConfigFileException(projectName, getRefName(), revision, fileName, err);
}
}
return rc;
diff --git a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
index 030cfb2..5417494 100644
--- a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
@@ -25,11 +25,11 @@
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Inject;
import java.io.IOException;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
-import javax.inject.Inject;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.transport.PushCertificate;
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 102da71..9d09eeb 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -153,7 +153,8 @@
logger.atFine().log("Cannot find project %s", projectName.get());
return Optional.empty();
}
- throw new StorageException("project state not available", e);
+ throw new StorageException(
+ String.format("project state of project %s not available", projectName.get()), e);
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
index ea29fb3..17fba35 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
@@ -34,9 +34,9 @@
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.PluginPermissionsUtil;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.RefPattern;
-import com.google.gerrit.server.restapi.config.ListCapabilities;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -51,18 +51,18 @@
private final GroupResolver groupResolver;
private final AllProjectsName allProjects;
private final Provider<SetParent> setParent;
- private final ListCapabilities listCapabilities;
+ private final PluginPermissionsUtil pluginPermissionsUtil;
@Inject
private SetAccessUtil(
GroupResolver groupResolver,
AllProjectsName allProjects,
Provider<SetParent> setParent,
- ListCapabilities listCapabilities) {
+ PluginPermissionsUtil pluginPermissionsUtil) {
this.groupResolver = groupResolver;
this.allProjects = allProjects;
this.setParent = setParent;
- this.listCapabilities = listCapabilities;
+ this.pluginPermissionsUtil = pluginPermissionsUtil;
}
List<AccessSection> getAccessSections(Map<String, AccessSectionInfo> sectionInfos)
@@ -154,8 +154,7 @@
// Check all permissions for soundness
for (Permission p : section.getPermissions()) {
if (!isCapability(p.getName())) {
- throw new BadRequestException(
- "Cannot add non-global capability " + p.getName() + " to global capabilities");
+ throw new BadRequestException("Unknown global capability: " + p.getName());
}
}
}
@@ -244,7 +243,7 @@
if (GlobalCapability.isGlobalCapability(name)) {
return true;
}
- Set<String> pluginCapabilities = listCapabilities.collectPluginCapabilities().keySet();
+ Set<String> pluginCapabilities = pluginPermissionsUtil.collectPluginCapabilities().keySet();
return pluginCapabilities.contains(name);
}
}
diff --git a/java/com/google/gerrit/sshd/commands/SetTopicCommand.java b/java/com/google/gerrit/sshd/commands/SetTopicCommand.java
index 2345d17..70700f1 100644
--- a/java/com/google/gerrit/sshd/commands/SetTopicCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetTopicCommand.java
@@ -18,10 +18,8 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.TopicInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.extensions.events.TopicEdited;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.restapi.change.SetTopicOp;
import com.google.gerrit.server.update.BatchUpdate;
@@ -38,8 +36,6 @@
@CommandMetaData(name = "set-topic", description = "Set the topic for one or more changes")
public class SetTopicCommand extends SshCommand {
private final BatchUpdate.Factory updateFactory;
- private final ChangeMessagesUtil cmUtil;
- private final TopicEdited topicEdited;
private final ChangeArgumentParser changeArgumentParser;
private final SetTopicOp.Factory topicOpFactory;
@@ -69,13 +65,9 @@
@Inject
SetTopicCommand(
BatchUpdate.Factory updateFactory,
- ChangeMessagesUtil cmUtil,
- TopicEdited topicEdited,
ChangeArgumentParser changeArgumentParser,
SetTopicOp.Factory topicOpFactory) {
this.updateFactory = updateFactory;
- this.cmUtil = cmUtil;
- this.topicEdited = topicEdited;
this.changeArgumentParser = changeArgumentParser;
this.topicOpFactory = topicOpFactory;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
index 2c972a3..40dd70e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
@@ -27,9 +27,13 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
+import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import java.util.ArrayList;
+import java.util.List;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Test;
public class ChangeSubmitRequirementIT extends AbstractDaemonTest {
@@ -50,22 +54,63 @@
};
}
+ @Inject CustomSubmitRule rule;
+
@Test
- public void checkSubmitRequirementIsPropagated() throws Exception {
+ public void submitRequirementIsPropagated() throws Exception {
+ rule.block(false);
PushOneCommit.Result r = createChange();
ChangeInfo result = gApi.changes().id(r.getChangeId()).get();
+ assertThat(result.requirements).isEmpty();
+
+ rule.block(true);
+ result = gApi.changes().id(r.getChangeId()).get();
assertThat(result.requirements).containsExactly(reqInfo);
}
+ @Test
+ public void submitRequirementIsPropagatedInQuery() throws Exception {
+ rule.block(false);
+ PushOneCommit.Result r = createChange();
+
+ String query = "status:open project:" + project.get();
+ List<ChangeInfo> result = gApi.changes().query(query).get();
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).requirements).isEmpty();
+
+ // Submit rule behavior is changed, but the query still returns
+ // the previous result from the index
+ rule.block(true);
+ result = gApi.changes().query(query).get();
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).requirements).isEmpty();
+
+ // The submit rule result is updated after the change is reindexed
+ gApi.changes().id(r.getChangeId()).index();
+ result = gApi.changes().query(query).get();
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).requirements).containsExactly(reqInfo);
+ }
+
+ @Singleton
private static class CustomSubmitRule implements SubmitRule {
+ private final AtomicBoolean block = new AtomicBoolean(true);
+
+ public void block(boolean block) {
+ this.block.set(block);
+ }
+
@Override
public Optional<SubmitRecord> evaluate(ChangeData changeData) {
- SubmitRecord record = new SubmitRecord();
- record.labels = new ArrayList<>();
- record.status = SubmitRecord.Status.NOT_READY;
- record.requirements = ImmutableList.of(req);
- return Optional.of(record);
+ if (block.get()) {
+ SubmitRecord record = new SubmitRecord();
+ record.labels = new ArrayList<>();
+ record.status = SubmitRecord.Status.NOT_READY;
+ record.requirements = ImmutableList.of(req);
+ return Optional.of(record);
+ }
+ return Optional.empty();
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
similarity index 99%
rename from javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
rename to javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
index 92ef02c..448f347 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
@@ -46,7 +46,7 @@
import org.junit.Test;
@NoHttpd
-public class QueryChangeIT extends AbstractDaemonTest {
+public class QueryChangesIT extends AbstractDaemonTest {
@Inject private AccountOperations accountOperations;
@Inject private ProjectOperations projectOperations;
@Inject private Provider<QueryChanges> queryChangesProvider;
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index 5e4beedf..cad20b6 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -65,6 +65,7 @@
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.CommentLinkInfoImpl;
+import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Module;
@@ -906,7 +907,16 @@
// We must test this via the REST API since ExceptionHook is not invoked from the Java API.
RestResponse r = adminRestSession.get("/projects/" + allProjects.get());
r.assertConflict();
- assertThat(r.getEntityContent()).contains("Invalid project.config file");
+ assertThat(r.getEntityContent())
+ .contains(
+ String.format(
+ "Invalid config file %s in project %s in branch %s in commit %s: "
+ + "Bad entry delimiter\n"
+ + "Please contact the project owner.",
+ ProjectConfig.PROJECT_CONFIG,
+ allProjects.get(),
+ RefNames.REFS_CONFIG,
+ projectOperations.project(allProjects).getHead(RefNames.REFS_CONFIG).name()));
}
private CommentLinkInfo commentLinkInfo(String name, String match, String link) {
diff --git a/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java b/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
index 87f3c98..6f7a4c3 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
@@ -254,11 +254,13 @@
.contains(
String.format(
"invalid account configuration: commit '%s' has an invalid '%s' file for account"
- + " '%s': Invalid config file %s in commit %s",
+ + " '%s': Invalid config file %s in project %s in branch %s in commit %s",
r.getCommit().name(),
AccountProperties.ACCOUNT_CONFIG,
admin.id(),
AccountProperties.ACCOUNT_CONFIG,
+ allUsers.get(),
+ userRef,
r.getCommit().name()));
}
}
@@ -480,7 +482,8 @@
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ String userRef = RefNames.refsUsers(admin.id());
+ fetch(allUsersRepo, userRef + ":userRef");
allUsersRepo.reset("userRef");
PushOneCommit.Result r =
@@ -496,11 +499,13 @@
r.assertMessage(
String.format(
"commit '%s' has an invalid '%s' file for account '%s':"
- + " Invalid config file %s in commit %s",
+ + " Invalid config file %s in project %s in branch %s in commit %s",
r.getCommit().name(),
AccountProperties.ACCOUNT_CONFIG,
admin.id(),
AccountProperties.ACCOUNT_CONFIG,
+ allUsers.get(),
+ userRef,
r.getCommit().name()));
accountIndexedCounter.assertNoReindex();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
index b6ef5a3..9298b43 100644
--- a/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/auth/AuthenticationCheckIT.java
@@ -14,6 +14,8 @@
package com.google.gerrit.acceptance.rest.auth;
+import static com.google.common.truth.Truth.assertThat;
+
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession;
@@ -24,6 +26,7 @@
public void authCheck_loggedInUser_returnsOk() throws Exception {
RestResponse r = adminRestSession.get("/auth-check");
r.assertNoContent();
+ // Jetty strips Content-Length when status is NO_CONTENT
}
@Test
@@ -31,5 +34,6 @@
RestSession anonymous = new RestSession(server, null);
RestResponse r = anonymous.get("/auth-check");
r.assertForbidden();
+ assertThat(r.getHeader("Content-Length")).isEqualTo("0");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 3e9b1f6..08c20ec 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -456,6 +456,44 @@
}
@Test
+ public void addPermissionAsGlobalCapability() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = newAccessSectionInfo();
+
+ PermissionInfo push = newPermissionInfo();
+ PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ push.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
+ accessSectionInfo.permissions.put(Permission.PUSH, push);
+
+ accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(allProjects.get()).access(accessInput));
+ assertThat(ex).hasMessageThat().isEqualTo("Unknown global capability: " + Permission.PUSH);
+ }
+
+ @Test
+ public void addInvalidGlobalCapability() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultGlobalCapabilitiesAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
+ accessSectionInfo.permissions.put("Invalid Global Capability", permissionInfo);
+
+ accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.projects().name(allProjects.get()).access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo("Unknown global capability: Invalid Global Capability");
+ }
+
+ @Test
public void addGlobalCapabilityForNonRootProject() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultGlobalCapabilitiesAccessSectionInfo();
diff --git a/plugins/download-commands b/plugins/download-commands
index 1b98be8..5c50f9b 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 1b98be8d371e68237182df0f04361017f2be2ea8
+Subproject commit 5c50f9b17e616fd84d2c561822161fff46bbf902
diff --git a/plugins/replication b/plugins/replication
index 4590b53..041f55c 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4590b53e5ef3726ed12b8a3054ae5d4d8363f90d
+Subproject commit 041f55c0ff92e14bea9badacc617a8abb7f37b71
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 53e852f..289e3f4 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -335,10 +335,11 @@
aria-label$="[[_computeFileStatusLabel(file.status)]]">
[[_computeFileStatus(file.status)]]
</div>
+ <!-- TODO: Remove data-url as it appears its not used -->
<span
- data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path, editMode)]]"
+ data-url="[[_computeDiffURL(change, patchRange, file.__path, editMode)]]"
class="path">
- <a class="pathLink" href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path, editMode)]]">
+ <a class="pathLink" href$="[[_computeDiffURL(change, patchRange, file.__path, editMode)]]">
<span title$="[[computeDisplayPath(file.__path)]]"
class="fullFileName">
[[computeDisplayPath(file.__path)]]
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index cdc56b2..d2f11c2 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -798,19 +798,20 @@
return status || 'M';
}
- _computeDiffURL(change, patchNum, basePatchNum, path, editMode) {
+ _computeDiffURL(change, patchRange, path, editMode) {
// Polymer 2: check for undefined
- if ([change, patchNum, basePatchNum, path, editMode]
+ if ([change, patchRange, path, editMode]
.some(arg => arg === undefined)) {
return;
}
// TODO(kaspern): Fix editing for commit messages and merge lists.
if (editMode && path !== this.COMMIT_MESSAGE_PATH &&
path !== this.MERGE_LIST_PATH) {
- return Gerrit.Nav.getEditUrlForDiff(change, path, patchNum,
- basePatchNum);
+ return Gerrit.Nav.getEditUrlForDiff(change, path, patchRange.patchNum,
+ patchRange.basePatchNum);
}
- return Gerrit.Nav.getUrlForDiff(change, path, patchNum, basePatchNum);
+ return Gerrit.Nav.getUrlForDiff(change, path, patchRange.patchNum,
+ patchRange.basePatchNum);
}
_formatBytes(bytes) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 0a1c8ec..af80ca8 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1267,6 +1267,42 @@
});
});
+ suite('diff url file list', () => {
+ test('diff url', () => {
+ const diffStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiff')
+ .returns('/c/gerrit/+/1/1/index.php');
+ const change = {
+ _number: 1,
+ project: 'gerrit',
+ };
+ const path = 'index.php';
+ const patchRange = {
+ patchNum: 1,
+ };
+ assert.equal(
+ element._computeDiffURL(change, patchRange, path, false),
+ '/c/gerrit/+/1/1/index.php');
+ diffStub.restore();
+ });
+
+ test('diff url commit msg', () => {
+ const diffStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiff')
+ .returns('/c/gerrit/+/1/1//COMMIT_MSG');
+ const change = {
+ _number: 1,
+ project: 'gerrit',
+ };
+ const path = '/COMMIT_MSG';
+ const patchRange = {
+ patchNum: 1,
+ };
+ assert.equal(
+ element._computeDiffURL(change, patchRange, path, false),
+ '/c/gerrit/+/1/1//COMMIT_MSG');
+ diffStub.restore();
+ });
+ });
+
suite('size bars', () => {
test('_computeSizeBarLayout', () => {
assert.isUndefined(element._computeSizeBarLayout(null));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
index efacb97..9e1ec9e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
@@ -24,6 +24,10 @@
const TRAILING_WHITESPACE_PATTERN = /\s+$/;
+ // https://gerrit.googlesource.com/gerrit/+/234616a8627334686769f1de989d286039f4d6a5/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js#740
+ const COMMIT_MSG_PATH = '/COMMIT_MSG';
+ const COMMIT_MSG_LINE_LENGTH = 72;
+
/**
* @appliesMixin Gerrit.FireMixin
*/
@@ -271,18 +275,39 @@
return;
}
+ const localPrefs = Object.assign({}, prefs);
+ if (this.path === COMMIT_MSG_PATH) {
+ // override line_length for commit msg the same way as
+ // in gr-diff
+ localPrefs.line_length = COMMIT_MSG_LINE_LENGTH;
+ }
+
let builder = null;
if (this.isImageDiff) {
- builder = new GrDiffBuilderImage(diff, prefs, this.diffElement,
- this.baseImage, this.revisionImage);
+ builder = new GrDiffBuilderImage(
+ diff,
+ localPrefs,
+ this.diffElement,
+ this.baseImage,
+ this.revisionImage);
} else if (diff.binary) {
// If the diff is binary, but not an image.
- return new GrDiffBuilderBinary(diff, prefs, this.diffElement);
+ return new GrDiffBuilderBinary(
+ diff,
+ localPrefs,
+ this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
- builder = new GrDiffBuilderSideBySide(diff, prefs, this.diffElement,
- this._layers);
+ builder = new GrDiffBuilderSideBySide(
+ diff,
+ localPrefs,
+ this.diffElement,
+ this._layers
+ );
} else if (this.viewMode === DiffViewMode.UNIFIED) {
- builder = new GrDiffBuilderUnified(diff, prefs, this.diffElement,
+ builder = new GrDiffBuilderUnified(
+ diff,
+ localPrefs,
+ this.diffElement,
this._layers);
}
if (!builder) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
index d9d515f..3af5522 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
@@ -60,6 +60,11 @@
</test-fixture>
<script>
+ const DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
+
suite('gr-diff-builder tests', async () => {
await readyToTest();
let prefs;
@@ -192,6 +197,27 @@
assert.equal(result, expected);
});
+ [DiffViewMode.UNIFIED, DiffViewMode.SIDE_BY_SIDE]
+ .forEach(mode => {
+ test(`line_length used for regular files under ${mode}`, () => {
+ element.path = '/a.txt';
+ element.viewMode = mode;
+ builder = element._getDiffBuilder(
+ {}, {tab_size: 4, line_length: 50}
+ );
+ assert.equal(builder._prefs.line_length, 50);
+ });
+
+ test(`line_length ignored for commit msg under ${mode}`, () => {
+ element.path = '/COMMIT_MSG';
+ element.viewMode = mode;
+ builder = element._getDiffBuilder(
+ {}, {tab_size: 4, line_length: 50}
+ );
+ assert.equal(builder._prefs.line_length, 72);
+ });
+ });
+
test('_createTextEl linewrap with tabs', () => {
const text = '\t'.repeat(7) + '!';
const line = {text, highlights: []};
diff --git a/polygerrit-ui/app/embed/README.md b/polygerrit-ui/app/embed/README.md
new file mode 100644
index 0000000..e147007
--- /dev/null
+++ b/polygerrit-ui/app/embed/README.md
@@ -0,0 +1,13 @@
+This folder contains shared components that can be used independently from Gerrit.
+
+### gr-diff
+
+`gr-diff.html` is the `gr-diff` component used in gerrit to render diff. If you want to use it, feel free to import it and use it in your project as:
+
+```
+<gr-diff></gr-diff>
+```
+
+All supported attributes defined in `polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js`, you can pass them by just assigning them to the `gr-app` element.
+
+To customize the style of the diff, you can use `css variables`, all supported varibled defined in `polygerrit-ui/app/styles/themes/app-theme.html` and `polygerrit-ui/app/styles/themes/dark-theme.html`.
\ No newline at end of file
diff --git a/polygerrit-ui/app/scripts/util.js b/polygerrit-ui/app/scripts/util.js
index 565b9b3..6a8a116 100644
--- a/polygerrit-ui/app/scripts/util.js
+++ b/polygerrit-ui/app/scripts/util.js
@@ -124,6 +124,12 @@
.filter(child => !!child.shadowRoot)
.map(child => child.shadowRoot);
nodes = nodes.concat(allShadowNodes);
+
+ // Add shadowRoot of current node if has one
+ // as its not included in node.querySelectorAll('*')
+ if (node.shadowRoot) {
+ nodes.push(node.shadowRoot);
+ }
}
return result;
};
@@ -154,6 +160,12 @@
.filter(child => !!child.shadowRoot)
.map(child => child.shadowRoot);
nodes = nodes.concat(allShadowNodes);
+
+ // Add shadowRoot of current node if has one
+ // as its not included in node.querySelectorAll('*')
+ if (node.shadowRoot) {
+ nodes.push(node.shadowRoot);
+ }
}
return [...results];
};
