Merge "Update mocha minor version"
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index a4e27b3..723b45a 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3586,7 +3586,7 @@
|`plugin_config` |optional|
Plugin configuration as map which maps the plugin name to a map of
parameter names to link:#config-parameter-info[ConfigParameterInfo]
-entities.
+entities. Only filled for users who have read access to `refs/meta/config`.
|`actions` |optional|
Actions the caller might be able to perform on this project. The
information is a map of view names to
diff --git a/WORKSPACE b/WORKSPACE
index 47c3e9c..41d6ef8 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -49,49 +49,25 @@
# otherwise refer to RBE docs.
rbe_autoconfig(name = "rbe_default")
-# TODO(davido): Switch to upstream again, when this PR is merged:
-# https://github.com/bazelbuild/rules_closure/pull/478
http_archive(
- name = "io_bazel_rules_closure",
- sha256 = "b9c2bc6ba377aa497eb7c31681d34404febf9d4e3c9c7d98ce0d78238a0af20f",
- strip_prefix = "rules_closure-0.31",
+ name = "com_google_protobuf",
+ sha256 = "71030a04aedf9f612d2991c1c552317038c3c5a2b578ac4745267a45e7037c29",
+ strip_prefix = "protobuf-3.12.3",
urls = [
- "https://github.com/davido/rules_closure/archive/V0.31.tar.gz",
- "https://gerrit-ci.gerritforge.com/lib/V0.31.tar.gz",
+ "https://github.com/protocolbuffers/protobuf/archive/v3.12.3.tar.gz",
],
)
+load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
+
+protobuf_deps()
+
http_archive(
name = "build_bazel_rules_nodejs",
sha256 = "84abf7ac4234a70924628baa9a73a5a5cbad944c4358cf9abdb4aab29c9a5b77",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/1.7.0/rules_nodejs-1.7.0.tar.gz"],
)
-# File is specific to Polymer and copied from the Closure Github -- should be
-# synced any time there are major changes to Polymer.
-# https://github.com/google/closure-compiler/blob/master/contrib/externs/polymer-1.0.js
-http_file(
- name = "polymer_closure",
- downloaded_file_path = "polymer_closure.js",
- sha256 = "4d63a36dcca040475bd6deb815b9a600bd686e1413ac1ebd4b04516edd675020",
- urls = ["https://raw.githubusercontent.com/google/closure-compiler/35d2b3340ff23a69441f10fa3bc820691c2942f2/contrib/externs/polymer-1.0.js"],
-)
-
-load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_dependencies", "rules_closure_toolchains")
-
-# Prevent redundant loading of dependencies.
-# TODO(davido): Omit re-fetching ancient args4j version when these PRs are merged:
-# https://github.com/bazelbuild/rules_closure/pull/262
-# https://github.com/google/closure-templates/pull/155
-rules_closure_dependencies(
- omit_aopalliance = True,
- omit_bazel_skylib = True,
- omit_javax_inject = True,
- omit_rules_cc = True,
-)
-
-rules_closure_toolchains()
-
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
@@ -328,7 +304,7 @@
)
maven_jar(
- name = "args4j-intern",
+ name = "args4j",
artifact = "args4j:args4j:2.33",
sha1 = "bd87a75374a6d6523de82fef51fc3cfe9baf9fc9",
)
@@ -993,6 +969,45 @@
sha1 = "639033469776fd37c08358c6b92a4761feb2af4b",
)
+load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
+
+yarn_install(
+ name = "npm",
+ package_json = "//:package.json",
+ yarn_lock = "//:yarn.lock",
+)
+
+yarn_install(
+ name = "ui_npm",
+ args = ["--prod"],
+ package_json = "//:polygerrit-ui/app/package.json",
+ yarn_lock = "//:polygerrit-ui/app/yarn.lock",
+)
+
+yarn_install(
+ name = "ui_dev_npm",
+ package_json = "//:polygerrit-ui/package.json",
+ yarn_lock = "//:polygerrit-ui/yarn.lock",
+)
+
+yarn_install(
+ name = "tools_npm",
+ package_json = "//:tools/node_tools/package.json",
+ yarn_lock = "//:tools/node_tools/yarn.lock",
+)
+
+yarn_install(
+ name = "plugins_npm",
+ args = ["--prod"],
+ package_json = "//:plugins/package.json",
+ yarn_lock = "//:plugins/yarn.lock",
+)
+
+# Install all Bazel dependencies needed for npm packages that supply Bazel rules
+load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")
+
+install_bazel_dependencies()
+
load("//tools/bzl:js.bzl", "bower_archive", "npm_binary")
# NPM binaries bundled along with their dependencies.
@@ -1184,45 +1199,6 @@
version = "6.5.1",
)
-load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
-
-yarn_install(
- name = "npm",
- package_json = "//:package.json",
- yarn_lock = "//:yarn.lock",
-)
-
-yarn_install(
- name = "ui_npm",
- args = ["--prod"],
- package_json = "//:polygerrit-ui/app/package.json",
- yarn_lock = "//:polygerrit-ui/app/yarn.lock",
-)
-
-yarn_install(
- name = "ui_dev_npm",
- package_json = "//:polygerrit-ui/package.json",
- yarn_lock = "//:polygerrit-ui/yarn.lock",
-)
-
-yarn_install(
- name = "tools_npm",
- package_json = "//:tools/node_tools/package.json",
- yarn_lock = "//:tools/node_tools/yarn.lock",
-)
-
-yarn_install(
- name = "plugins_npm",
- args = ["--prod"],
- package_json = "//:plugins/package.json",
- yarn_lock = "//:plugins/yarn.lock",
-)
-
-# Install all Bazel dependencies needed for npm packages that supply Bazel rules
-load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")
-
-install_bazel_dependencies()
-
load("@npm_bazel_typescript//:index.bzl", "ts_setup_workspace")
ts_setup_workspace()
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 10a8852..dfb7a55 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -983,7 +983,7 @@
protected void setUseSignedOffBy(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = projectConfigFactory.read(md);
- config.getProject().setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, value);
+ config.updateProject(p -> p.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, value));
config.commit(md);
projectCache.evict(config.getProject());
}
@@ -992,7 +992,7 @@
protected void setRequireChangeId(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = projectConfigFactory.read(md);
- config.getProject().setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value);
+ config.updateProject(p -> p.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value));
config.commit(md);
projectCache.evict(config.getProject());
}
@@ -1442,10 +1442,10 @@
LabelValue... value)
throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = label(label, value);
+ LabelType.Builder labelType = label(label, value).toBuilder();
labelType.setFunction(func);
- labelType.setRefPatterns(refPatterns);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ labelType.setRefPatterns(ImmutableList.copyOf(refPatterns));
+ u.getConfig().upsertLabelType(labelType.build());
u.save();
}
}
@@ -1453,10 +1453,11 @@
protected void enableCreateNewChangeForAllNotInTarget() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
+ .updateProject(
+ p ->
+ p.setBooleanConfig(
+ BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
+ InheritableBoolean.TRUE));
u.save();
}
}
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index cfe7964..a5d8d19 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeETagComputation;
+import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
@@ -79,6 +80,7 @@
private final DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners;
private final DynamicMap<CapabilityDefinition> capabilityDefinitions;
private final DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions;
+ private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
@Inject
ExtensionRegistry(
@@ -107,7 +109,8 @@
DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners,
DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners,
DynamicMap<CapabilityDefinition> capabilityDefinitions,
- DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions) {
+ DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions,
+ DynamicMap<ProjectConfigEntry> pluginConfigEntries) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -134,6 +137,7 @@
this.workInProgressStateChangedListeners = workInProgressStateChangedListeners;
this.capabilityDefinitions = capabilityDefinitions;
this.pluginProjectPermissionDefinitions = pluginProjectPermissionDefinitions;
+ this.pluginConfigEntries = pluginConfigEntries;
}
public Registration newRegistration() {
@@ -254,6 +258,10 @@
return add(pluginProjectPermissionDefinitions, pluginProjectPermissionDefinition, exportName);
}
+ public Registration add(ProjectConfigEntry pluginConfigEntry, String exportName) {
+ return add(pluginConfigEntries, pluginConfigEntry, exportName);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 3a68414..9c1423d 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -14,23 +14,21 @@
package com.google.gerrit.common.data;
-import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Comparator.comparing;
-import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toList;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSetApproval;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
-public class LabelType {
+@AutoValue
+public abstract class LabelType {
public static final boolean DEF_ALLOW_POST_SUBMIT = true;
public static final boolean DEF_CAN_OVERRIDE = true;
public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CHANGE = true;
@@ -46,12 +44,12 @@
public static LabelType withDefaultValues(String name) {
checkName(name);
List<LabelValue> values = new ArrayList<>(2);
- values.add(new LabelValue((short) 0, "Rejected"));
- values.add(new LabelValue((short) 1, "Approved"));
- return new LabelType(name, values);
+ values.add(LabelValue.create((short) 0, "Rejected"));
+ values.add(LabelValue.create((short) 1, "Approved"));
+ return create(name, values);
}
- public static String checkName(String name) {
+ public static String checkName(String name) throws IllegalArgumentException {
checkNameInternal(name);
if ("SUBM".equals(name)) {
throw new IllegalArgumentException("Reserved label name \"" + name + "\"");
@@ -59,7 +57,7 @@
return name;
}
- public static String checkNameInternal(String name) {
+ public static String checkNameInternal(String name) throws IllegalArgumentException {
if (name == null || name.isEmpty()) {
throw new IllegalArgumentException("Empty label name");
}
@@ -76,270 +74,135 @@
return name;
}
- private static List<LabelValue> sortValues(List<LabelValue> values) {
- values = new ArrayList<>(values);
+ private static ImmutableList<LabelValue> sortValues(List<LabelValue> values) {
if (values.isEmpty()) {
- return Collections.emptyList();
+ return ImmutableList.of();
}
values = values.stream().sorted(comparing(LabelValue::getValue)).collect(toList());
short v = values.get(0).getValue();
short i = 0;
- ArrayList<LabelValue> result = new ArrayList<>();
+ ImmutableList.Builder<LabelValue> result = ImmutableList.builder();
// Fill in any missing values with empty text.
while (i < values.size()) {
while (v < values.get(i).getValue()) {
- result.add(new LabelValue(v++, ""));
+ result.add(LabelValue.create(v++, ""));
}
v++;
result.add(values.get(i++));
}
- result.trimToSize();
- return Collections.unmodifiableList(result);
+ return result.build();
}
- protected String name;
+ public abstract String getName();
- protected LabelFunction function;
+ public abstract LabelFunction getFunction();
- protected boolean copyAnyScore;
- protected boolean copyMinScore;
- protected boolean copyMaxScore;
- protected boolean copyAllScoresOnMergeFirstParentUpdate;
- protected boolean copyAllScoresOnTrivialRebase;
- protected boolean copyAllScoresIfNoCodeChange;
- protected boolean copyAllScoresIfNoChange;
- protected ImmutableList<Short> copyValues;
- protected boolean allowPostSubmit;
- protected boolean ignoreSelfApproval;
- protected short defaultValue;
+ public abstract boolean isCopyAnyScore();
- protected List<LabelValue> values;
- protected short maxNegative;
- protected short maxPositive;
+ public abstract boolean isCopyMinScore();
- private transient boolean canOverride;
- private transient List<String> refPatterns;
- private transient Map<Short, LabelValue> byValue;
+ public abstract boolean isCopyMaxScore();
- protected LabelType() {}
+ public abstract boolean isCopyAllScoresOnMergeFirstParentUpdate();
- public LabelType(String name, List<LabelValue> valueList) {
- this.name = checkName(name);
- canOverride = true;
- values = sortValues(valueList);
- defaultValue = 0;
+ public abstract boolean isCopyAllScoresOnTrivialRebase();
- function = LabelFunction.MAX_WITH_BLOCK;
+ public abstract boolean isCopyAllScoresIfNoCodeChange();
- maxNegative = Short.MIN_VALUE;
- maxPositive = Short.MAX_VALUE;
- if (!values.isEmpty()) {
- if (values.get(0).getValue() < 0) {
- maxNegative = values.get(0).getValue();
- }
- if (values.get(values.size() - 1).getValue() > 0) {
- maxPositive = values.get(values.size() - 1).getValue();
- }
- }
- setCanOverride(DEF_CAN_OVERRIDE);
- setCopyAllScoresIfNoChange(DEF_COPY_ALL_SCORES_IF_NO_CHANGE);
- setCopyAllScoresIfNoCodeChange(DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE);
- setCopyAllScoresOnTrivialRebase(DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE);
- setCopyAllScoresOnMergeFirstParentUpdate(DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
- setCopyAnyScore(DEF_COPY_ANY_SCORE);
- setCopyMaxScore(DEF_COPY_MAX_SCORE);
- setCopyMinScore(DEF_COPY_MIN_SCORE);
- setCopyValues(DEF_COPY_VALUES);
- setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
- setIgnoreSelfApproval(DEF_IGNORE_SELF_APPROVAL);
+ public abstract boolean isCopyAllScoresIfNoChange();
- byValue = new HashMap<>();
- for (LabelValue v : values) {
- byValue.put(v.getValue(), v);
- }
+ public abstract ImmutableList<Short> getCopyValues();
+
+ public abstract boolean isAllowPostSubmit();
+
+ public abstract boolean isIgnoreSelfApproval();
+
+ public abstract short getDefaultValue();
+
+ public abstract ImmutableList<LabelValue> getValues();
+
+ public abstract short getMaxNegative();
+
+ public abstract short getMaxPositive();
+
+ public abstract boolean isCanOverride();
+
+ @Nullable
+ public abstract ImmutableList<String> getRefPatterns();
+
+ public abstract ImmutableMap<Short, LabelValue> getByValue();
+
+ public static LabelType create(String name, List<LabelValue> valueList) {
+ return LabelType.builder(name, valueList).build();
}
- public String getName() {
- return name;
- }
-
- public void setName(String name) {
- this.name = checkName(name);
+ public static LabelType.Builder builder(String name, List<LabelValue> valueList) {
+ return (new AutoValue_LabelType.Builder())
+ .setName(name)
+ .setValues(valueList)
+ .setDefaultValue((short) 0)
+ .setFunction(LabelFunction.MAX_WITH_BLOCK)
+ .setMaxNegative(Short.MIN_VALUE)
+ .setMaxPositive(Short.MAX_VALUE)
+ .setCanOverride(DEF_CAN_OVERRIDE)
+ .setCopyAllScoresIfNoChange(DEF_COPY_ALL_SCORES_IF_NO_CHANGE)
+ .setCopyAllScoresIfNoCodeChange(DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE)
+ .setCopyAllScoresOnTrivialRebase(DEF_COPY_ALL_SCORES_ON_TRIVIAL_REBASE)
+ .setCopyAllScoresOnMergeFirstParentUpdate(DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE)
+ .setCopyAnyScore(DEF_COPY_ANY_SCORE)
+ .setCopyMaxScore(DEF_COPY_MAX_SCORE)
+ .setCopyMinScore(DEF_COPY_MIN_SCORE)
+ .setCopyValues(DEF_COPY_VALUES)
+ .setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT)
+ .setIgnoreSelfApproval(DEF_IGNORE_SELF_APPROVAL);
}
public boolean matches(PatchSetApproval psa) {
- return psa.labelId().get().equalsIgnoreCase(name);
- }
-
- public LabelFunction getFunction() {
- return function;
- }
-
- public void setFunction(@Nullable LabelFunction function) {
- this.function = function;
- }
-
- public boolean canOverride() {
- return canOverride;
- }
-
- @Nullable
- public List<String> getRefPatterns() {
- return refPatterns;
- }
-
- public void setCanOverride(boolean canOverride) {
- this.canOverride = canOverride;
- }
-
- public boolean allowPostSubmit() {
- return allowPostSubmit;
- }
-
- public void setAllowPostSubmit(boolean allowPostSubmit) {
- this.allowPostSubmit = allowPostSubmit;
- }
-
- public boolean ignoreSelfApproval() {
- return ignoreSelfApproval;
- }
-
- public void setIgnoreSelfApproval(boolean ignoreSelfApproval) {
- this.ignoreSelfApproval = ignoreSelfApproval;
- }
-
- public void setRefPatterns(List<String> refPatterns) {
- if (refPatterns != null && !refPatterns.isEmpty()) {
- this.refPatterns =
- refPatterns.stream().collect(collectingAndThen(toList(), Collections::unmodifiableList));
- } else {
- this.refPatterns = null;
- }
- }
-
- public List<LabelValue> getValues() {
- return values;
- }
-
- public void setValues(List<LabelValue> values) {
- this.values = sortValues(values);
+ return psa.labelId().get().equalsIgnoreCase(getName());
}
public LabelValue getMin() {
- if (values.isEmpty()) {
+ if (getValues().isEmpty()) {
return null;
}
- return values.get(0);
+ return getValues().get(0);
}
public LabelValue getMax() {
- if (values.isEmpty()) {
+ if (getValues().isEmpty()) {
return null;
}
- return values.get(values.size() - 1);
- }
-
- public short getDefaultValue() {
- return defaultValue;
- }
-
- public void setDefaultValue(short defaultValue) {
- this.defaultValue = defaultValue;
- }
-
- public boolean isCopyAnyScore() {
- return copyAnyScore;
- }
-
- public void setCopyAnyScore(boolean copyAnyScore) {
- this.copyAnyScore = copyAnyScore;
- }
-
- public boolean isCopyMinScore() {
- return copyMinScore;
- }
-
- public void setCopyMinScore(boolean copyMinScore) {
- this.copyMinScore = copyMinScore;
- }
-
- public boolean isCopyMaxScore() {
- return copyMaxScore;
- }
-
- public void setCopyMaxScore(boolean copyMaxScore) {
- this.copyMaxScore = copyMaxScore;
- }
-
- public boolean isCopyAllScoresOnMergeFirstParentUpdate() {
- return copyAllScoresOnMergeFirstParentUpdate;
- }
-
- public void setCopyAllScoresOnMergeFirstParentUpdate(
- boolean copyAllScoresOnMergeFirstParentUpdate) {
- this.copyAllScoresOnMergeFirstParentUpdate = copyAllScoresOnMergeFirstParentUpdate;
- }
-
- public boolean isCopyAllScoresOnTrivialRebase() {
- return copyAllScoresOnTrivialRebase;
- }
-
- public void setCopyAllScoresOnTrivialRebase(boolean copyAllScoresOnTrivialRebase) {
- this.copyAllScoresOnTrivialRebase = copyAllScoresOnTrivialRebase;
- }
-
- public boolean isCopyAllScoresIfNoCodeChange() {
- return copyAllScoresIfNoCodeChange;
- }
-
- public void setCopyAllScoresIfNoCodeChange(boolean copyAllScoresIfNoCodeChange) {
- this.copyAllScoresIfNoCodeChange = copyAllScoresIfNoCodeChange;
- }
-
- public boolean isCopyAllScoresIfNoChange() {
- return copyAllScoresIfNoChange;
- }
-
- public void setCopyAllScoresIfNoChange(boolean copyAllScoresIfNoChange) {
- this.copyAllScoresIfNoChange = copyAllScoresIfNoChange;
- }
-
- public ImmutableList<Short> getCopyValues() {
- return copyValues;
- }
-
- public void setCopyValues(Collection<Short> copyValues) {
- this.copyValues = copyValues.stream().sorted().collect(toImmutableList());
+ return getValues().get(getValues().size() - 1);
}
public boolean isMaxNegative(PatchSetApproval ca) {
- return maxNegative == ca.value();
+ return getMaxNegative() == ca.value();
}
public boolean isMaxPositive(PatchSetApproval ca) {
- return maxPositive == ca.value();
+ return getMaxPositive() == ca.value();
}
public LabelValue getValue(short value) {
- return byValue.get(value);
+ return getByValue().get(value);
}
public LabelValue getValue(PatchSetApproval ca) {
- return byValue.get(ca.value());
+ return getByValue().get(ca.value());
}
public LabelId getLabelId() {
- return LabelId.create(name);
+ return LabelId.create(getName());
}
@Override
- public String toString() {
- StringBuilder sb = new StringBuilder(name).append('[');
+ public final String toString() {
+ StringBuilder sb = new StringBuilder(getName()).append('[');
LabelValue min = getMin();
LabelValue max = getMax();
if (min != null && max != null) {
sb.append(
- new PermissionRange(Permission.forLabel(name), min.getValue(), max.getValue())
+ new PermissionRange(Permission.forLabel(getName()), min.getValue(), max.getValue())
.toString()
.trim());
} else if (min != null) {
@@ -350,4 +213,84 @@
sb.append(']');
return sb.toString();
}
+
+ public abstract Builder toBuilder();
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setName(String name);
+
+ public abstract Builder setFunction(LabelFunction function);
+
+ public abstract Builder setCanOverride(boolean canOverride);
+
+ public abstract Builder setAllowPostSubmit(boolean allowPostSubmit);
+
+ public abstract Builder setIgnoreSelfApproval(boolean ignoreSelfApproval);
+
+ public abstract Builder setRefPatterns(@Nullable ImmutableList<String> refPatterns);
+
+ public abstract Builder setValues(List<LabelValue> values);
+
+ public abstract Builder setDefaultValue(short defaultValue);
+
+ public abstract Builder setCopyAnyScore(boolean copyAnyScore);
+
+ public abstract Builder setCopyMinScore(boolean copyMinScore);
+
+ public abstract Builder setCopyMaxScore(boolean copyMaxScore);
+
+ public abstract Builder setCopyAllScoresOnMergeFirstParentUpdate(
+ boolean copyAllScoresOnMergeFirstParentUpdate);
+
+ public abstract Builder setCopyAllScoresOnTrivialRebase(boolean copyAllScoresOnTrivialRebase);
+
+ public abstract Builder setCopyAllScoresIfNoCodeChange(boolean copyAllScoresIfNoCodeChange);
+
+ public abstract Builder setCopyAllScoresIfNoChange(boolean copyAllScoresIfNoChange);
+
+ public abstract Builder setCopyValues(Collection<Short> copyValues);
+
+ public abstract Builder setMaxNegative(short maxNegative);
+
+ public abstract Builder setMaxPositive(short maxPositive);
+
+ public abstract ImmutableList<LabelValue> getValues();
+
+ protected abstract String getName();
+
+ protected abstract Builder setByValue(ImmutableMap<Short, LabelValue> byValue);
+
+ @Nullable
+ protected abstract ImmutableList<String> getRefPatterns();
+
+ protected abstract LabelType autoBuild();
+
+ public LabelType build() throws IllegalArgumentException {
+ setName(checkName(getName()));
+ if (getRefPatterns() == null || getRefPatterns().isEmpty()) {
+ // Empty to null
+ setRefPatterns(null);
+ }
+
+ List<LabelValue> valueList = sortValues(getValues());
+ setValues(valueList);
+ if (!valueList.isEmpty()) {
+ if (valueList.get(0).getValue() < 0) {
+ setMaxNegative(valueList.get(0).getValue());
+ }
+ if (valueList.get(valueList.size() - 1).getValue() > 0) {
+ setMaxPositive(valueList.get(valueList.size() - 1).getValue());
+ }
+ }
+
+ ImmutableMap.Builder<Short, LabelValue> byValue = ImmutableMap.builder();
+ for (LabelValue v : valueList) {
+ byValue.put(v.getValue(), v);
+ }
+ setByValue(byValue.build());
+
+ return autoBuild();
+ }
+ }
}
diff --git a/java/com/google/gerrit/common/data/LabelValue.java b/java/com/google/gerrit/common/data/LabelValue.java
index c0ba781..ec16fb2 100644
--- a/java/com/google/gerrit/common/data/LabelValue.java
+++ b/java/com/google/gerrit/common/data/LabelValue.java
@@ -14,65 +14,42 @@
package com.google.gerrit.common.data;
-import java.util.Objects;
+import com.google.auto.value.AutoValue;
-public class LabelValue {
+@AutoValue
+public abstract class LabelValue {
public static String formatValue(short value) {
if (value < 0) {
return Short.toString(value);
} else if (value == 0) {
return " 0";
} else {
- return "+" + Short.toString(value);
+ return "+" + value;
}
}
- protected short value;
- protected String text;
+ public abstract short getValue();
- public LabelValue(short value, String text) {
- this.value = value;
- this.text = text;
- }
+ public abstract String getText();
- protected LabelValue() {}
-
- public short getValue() {
- return value;
- }
-
- public String getText() {
- return text;
+ public static LabelValue create(short value, String text) {
+ return new AutoValue_LabelValue(value, text);
}
public String formatValue() {
- return formatValue(value);
+ return formatValue(getValue());
}
public String format() {
StringBuilder sb = new StringBuilder(formatValue());
- if (!text.isEmpty()) {
- sb.append(' ').append(text);
+ if (!getText().isEmpty()) {
+ sb.append(' ').append(getText());
}
return sb.toString();
}
@Override
- public boolean equals(Object o) {
- if (!(o instanceof LabelValue)) {
- return false;
- }
- LabelValue v = (LabelValue) o;
- return value == v.value && Objects.equals(text, v.text);
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(value, text);
- }
-
- @Override
- public String toString() {
+ public final String toString() {
return format();
}
}
diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java
index 1300b9d..759d50a 100644
--- a/java/com/google/gerrit/entities/Project.java
+++ b/java/com/google/gerrit/entities/Project.java
@@ -16,7 +16,10 @@
import static java.util.Objects.requireNonNull;
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableMap;
import com.google.errorprone.annotations.Immutable;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
@@ -27,7 +30,8 @@
import java.util.Optional;
/** Projects match a source code repository managed by Gerrit */
-public final class Project {
+@AutoValue
+public abstract class Project {
/** Default submit type for new projects. */
public static final SubmitType DEFAULT_SUBMIT_TYPE = SubmitType.MERGE_IF_NECESSARY;
@@ -60,10 +64,6 @@
return nameKey(KeyUtil.decode(str));
}
- public static String asStringOrNull(NameKey key) {
- return key == null ? null : key.get();
- }
-
private final String name;
protected NameKey(String name) {
@@ -98,118 +98,64 @@
}
}
- protected NameKey name;
+ public abstract NameKey getNameKey();
- protected String description;
+ @Nullable
+ public abstract String getDescription();
- protected Map<BooleanProjectConfig, InheritableBoolean> booleanConfigs;
-
- protected SubmitType submitType;
-
- protected ProjectState state;
-
- protected NameKey parent;
-
- protected String maxObjectSizeLimit;
-
- protected String defaultDashboardId;
-
- protected String localDefaultDashboardId;
-
- protected String configRefState;
-
- protected Project() {}
-
- public Project(Project.NameKey nameKey) {
- name = nameKey;
- submitType = SubmitType.MERGE_IF_NECESSARY;
- state = ProjectState.ACTIVE;
-
- booleanConfigs = new HashMap<>();
- Arrays.stream(BooleanProjectConfig.values())
- .forEach(c -> booleanConfigs.put(c, InheritableBoolean.INHERIT));
- }
-
- public Project.NameKey getNameKey() {
- return name;
- }
-
- public String getName() {
- return name != null ? name.get() : null;
- }
-
- public String getDescription() {
- return description;
- }
-
- public void setDescription(String d) {
- description = d;
- }
-
- public String getMaxObjectSizeLimit() {
- return maxObjectSizeLimit;
- }
-
- public InheritableBoolean getBooleanConfig(BooleanProjectConfig config) {
- return booleanConfigs.get(config);
- }
-
- public void setBooleanConfig(BooleanProjectConfig config, InheritableBoolean val) {
- booleanConfigs.replace(config, val);
- }
-
- public void setMaxObjectSizeLimit(String limit) {
- maxObjectSizeLimit = limit;
- }
+ protected abstract ImmutableMap<BooleanProjectConfig, InheritableBoolean> getBooleanConfigs();
/**
* Submit type as configured in {@code project.config}.
*
* <p>Does not take inheritance into account, i.e. may return {@link SubmitType#INHERIT}.
- *
- * @return submit type.
*/
- public SubmitType getConfiguredSubmitType() {
- return submitType;
- }
+ public abstract SubmitType getSubmitType();
- public void setSubmitType(SubmitType type) {
- submitType = type;
- }
-
- public ProjectState getState() {
- return state;
- }
-
- public void setState(ProjectState newState) {
- state = newState;
- }
-
- public String getDefaultDashboard() {
- return defaultDashboardId;
- }
-
- public void setDefaultDashboard(String defaultDashboardId) {
- this.defaultDashboardId = defaultDashboardId;
- }
-
- public String getLocalDefaultDashboard() {
- return localDefaultDashboardId;
- }
-
- public void setLocalDefaultDashboard(String localDefaultDashboardId) {
- this.localDefaultDashboardId = localDefaultDashboardId;
- }
+ public abstract ProjectState getState();
/**
- * Returns the name key of the parent project.
+ * Name key of the parent project.
*
- * @return name key of the parent project, {@code null} if this project is the wild project,
- * {@code null} or the name key of the wild project if this project is a direct child of the
- * wild project
+ * <p>{@code null} if this project is the wild project, {@code null} or the name key of the wild
+ * project if this project is a direct child of the wild project.
*/
- public Project.NameKey getParent() {
- return parent;
+ @Nullable
+ public abstract NameKey getParent();
+
+ @Nullable
+ public abstract String getMaxObjectSizeLimit();
+
+ @Nullable
+ public abstract String getDefaultDashboard();
+
+ @Nullable
+ public abstract String getLocalDefaultDashboard();
+
+ /** The {@code ObjectId} as 40 digit hex of {@code refs/meta/config}'s HEAD. */
+ @Nullable
+ public abstract String getConfigRefState();
+
+ public static Builder builder(Project.NameKey nameKey) {
+ Builder builder =
+ new AutoValue_Project.Builder()
+ .setNameKey(nameKey)
+ .setSubmitType(SubmitType.MERGE_IF_NECESSARY)
+ .setState(ProjectState.ACTIVE);
+ ImmutableMap.Builder<BooleanProjectConfig, InheritableBoolean> booleans =
+ ImmutableMap.builder();
+ Arrays.stream(BooleanProjectConfig.values())
+ .forEach(b -> booleans.put(b, InheritableBoolean.INHERIT));
+ builder.setBooleanConfigs(booleans.build());
+ return builder;
+ }
+
+ public String getName() {
+ return getNameKey() != null ? getNameKey().get() : null;
+ }
+
+ public InheritableBoolean getBooleanConfig(BooleanProjectConfig config) {
+ return getBooleanConfigs().get(config);
}
/**
@@ -220,11 +166,11 @@
* project
*/
public Project.NameKey getParent(Project.NameKey allProjectsName) {
- if (parent != null) {
- return parent;
+ if (getParent() != null) {
+ return getParent();
}
- if (name.equals(allProjectsName)) {
+ if (getNameKey().equals(allProjectsName)) {
return null;
}
@@ -232,29 +178,53 @@
}
public String getParentName() {
- return parent != null ? parent.get() : null;
- }
-
- public void setParentName(String n) {
- parent = n != null ? nameKey(n) : null;
- }
-
- public void setParentName(NameKey n) {
- parent = n;
- }
-
- /** Returns the {@code ObjectId} as 40 digit hex of {@code refs/meta/config}'s HEAD. */
- public String getConfigRefState() {
- return configRefState;
- }
-
- /** Sets the {@code ObjectId} as 40 digit hex of {@code refs/meta/config}'s HEAD. */
- public void setConfigRefState(String state) {
- configRefState = state;
+ return getParent() != null ? getParent().get() : null;
}
@Override
- public String toString() {
+ public final String toString() {
return Optional.of(getName()).orElse("<null>");
}
+
+ public abstract Builder toBuilder();
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setDescription(String description);
+
+ public Builder setBooleanConfig(BooleanProjectConfig config, InheritableBoolean val) {
+ Map<BooleanProjectConfig, InheritableBoolean> map = new HashMap<>(getBooleanConfigs());
+ map.replace(config, val);
+ setBooleanConfigs(ImmutableMap.copyOf(map));
+ return this;
+ }
+
+ public abstract Builder setMaxObjectSizeLimit(String limit);
+
+ public abstract Builder setSubmitType(SubmitType type);
+
+ public abstract Builder setState(ProjectState newState);
+
+ public abstract Builder setDefaultDashboard(String defaultDashboardId);
+
+ public abstract Builder setLocalDefaultDashboard(String localDefaultDashboard);
+
+ public abstract Builder setParent(NameKey n);
+
+ public Builder setParent(String n) {
+ return setParent(n != null ? nameKey(n) : null);
+ }
+
+ /** Sets the {@code ObjectId} as 40 digit hex of {@code refs/meta/config}'s HEAD. */
+ public abstract Builder setConfigRefState(String state);
+
+ public abstract Project build();
+
+ protected abstract Builder setNameKey(Project.NameKey nameKey);
+
+ protected abstract ImmutableMap<BooleanProjectConfig, InheritableBoolean> getBooleanConfigs();
+
+ protected abstract Builder setBooleanConfigs(
+ ImmutableMap<BooleanProjectConfig, InheritableBoolean> booleanConfigs);
+ }
}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index 0c3b7b0..c5f97a3 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -96,6 +96,7 @@
import com.google.gerrit.server.ssh.NoSshModule;
import com.google.gerrit.server.ssh.SshAddressesModule;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.submit.SubscriptionGraph;
import com.google.gerrit.sshd.SshHostKeyModule;
import com.google.gerrit.sshd.SshKeyCacheImpl;
import com.google.gerrit.sshd.SshModule;
@@ -322,6 +323,7 @@
}
modules.add(new RestApiModule());
+ modules.add(new SubscriptionGraph.Module());
modules.add(new WorkQueue.Module());
modules.add(new GerritInstanceNameModule());
modules.add(
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 57bec71..63278c1 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -107,6 +107,7 @@
import com.google.gerrit.server.ssh.NoSshModule;
import com.google.gerrit.server.ssh.SshAddressesModule;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.submit.SubscriptionGraph;
import com.google.gerrit.sshd.SshHostKeyModule;
import com.google.gerrit.sshd.SshKeyCacheImpl;
import com.google.gerrit.sshd.SshModule;
@@ -411,6 +412,7 @@
// work queue can get stuck waiting on index futures that will never return.
modules.add(createIndexModule());
+ modules.add(new SubscriptionGraph.Module());
modules.add(new WorkQueue.Module());
modules.add(new StreamEventsApiListener.Module());
modules.add(new EventBroker.Module());
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
index 2db17d6..739e263 100644
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -122,7 +122,7 @@
if (rec.labels != null) {
for (SubmitRecord.Label r : rec.labels) {
LabelType type = labelTypes.byLabel(r.label);
- if (type != null && (!isMerged || type.allowPostSubmit())) {
+ if (type != null && (!isMerged || type.isAllowPostSubmit())) {
toCheck.put(type.getName(), type);
}
}
@@ -139,7 +139,7 @@
}
for (SubmitRecord.Label r : rec.labels) {
LabelType type = labelTypes.byLabel(r.label);
- if (type == null || (isMerged && !type.allowPostSubmit())) {
+ if (type == null || (isMerged && !type.isAllowPostSubmit())) {
continue;
}
diff --git a/java/com/google/gerrit/server/git/NotifyConfig.java b/java/com/google/gerrit/server/git/NotifyConfig.java
index 429f15a..1a1bbb6 100644
--- a/java/com/google/gerrit/server/git/NotifyConfig.java
+++ b/java/com/google/gerrit/server/git/NotifyConfig.java
@@ -14,113 +14,101 @@
package com.google.gerrit.server.git;
-import com.google.common.base.MoreObjects;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import java.util.EnumSet;
-import java.util.HashSet;
import java.util.Set;
+import org.eclipse.jgit.annotations.Nullable;
-public class NotifyConfig implements Comparable<NotifyConfig> {
+@AutoValue
+public abstract class NotifyConfig implements Comparable<NotifyConfig> {
public enum Header {
TO,
CC,
BCC
}
- private String name;
- private EnumSet<NotifyType> types = EnumSet.of(NotifyType.ALL);
- private String filter;
+ @Nullable
+ public abstract String getName();
- private Header header;
- private Set<GroupReference> groups = new HashSet<>();
- private Set<Address> addresses = new HashSet<>();
+ public abstract ImmutableSet<NotifyType> getNotify();
- public String getName() {
- return name;
- }
+ @Nullable
+ public abstract String getFilter();
- public void setName(String name) {
- this.name = name;
- }
+ @Nullable
+ public abstract Header getHeader();
+
+ public abstract ImmutableSet<GroupReference> getGroups();
+
+ public abstract ImmutableSet<Address> getAddresses();
public boolean isNotify(NotifyType type) {
- return types.contains(type) || types.contains(NotifyType.ALL);
+ return getNotify().contains(type) || getNotify().contains(NotifyType.ALL);
}
- public Set<NotifyType> getNotify() {
- return types;
+ public static Builder builder() {
+ return new AutoValue_NotifyConfig.Builder()
+ .setNotify(ImmutableSet.copyOf(EnumSet.of(NotifyType.ALL)));
}
- public void setTypes(Set<NotifyType> newTypes) {
- types = EnumSet.copyOf(newTypes);
- }
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setName(String name);
- public String getFilter() {
- return filter;
- }
+ public abstract Builder setNotify(Set<NotifyType> newTypes);
- public void setFilter(String filter) {
- if ("*".equals(filter)) {
- this.filter = null;
- } else {
- this.filter = Strings.emptyToNull(filter);
+ public abstract Builder setFilter(@Nullable String filter);
+
+ public abstract Builder setHeader(Header hdr);
+
+ public Builder addGroup(GroupReference group) {
+ groupsBuilder().add(group);
+ return this;
+ }
+
+ public Builder addAddress(Address address) {
+ addressesBuilder().add(address);
+ return this;
+ }
+
+ protected abstract ImmutableSet.Builder<GroupReference> groupsBuilder();
+
+ protected abstract ImmutableSet.Builder<Address> addressesBuilder();
+
+ protected abstract NotifyConfig autoBuild();
+
+ protected abstract String getFilter();
+
+ public NotifyConfig build() {
+ if ("*".equals(getFilter())) {
+ setFilter(null);
+ } else {
+ setFilter(Strings.emptyToNull(getFilter()));
+ }
+ return autoBuild();
}
}
- public Header getHeader() {
- return header;
- }
-
- public void setHeader(Header hdr) {
- header = hdr;
- }
-
- public Set<GroupReference> getGroups() {
- return groups;
- }
-
- public Set<Address> getAddresses() {
- return addresses;
- }
-
- public void addEmail(GroupReference group) {
- groups.add(group);
- }
-
- public void addEmail(Address address) {
- addresses.add(address);
+ @Override
+ public final int compareTo(NotifyConfig o) {
+ return getName().compareTo(o.getName());
}
@Override
- public int compareTo(NotifyConfig o) {
- return name.compareTo(o.name);
+ public final int hashCode() {
+ return getName().hashCode();
}
@Override
- public int hashCode() {
- return name.hashCode();
- }
-
- @Override
- public boolean equals(Object obj) {
+ public final boolean equals(Object obj) {
if (obj instanceof NotifyConfig) {
return compareTo((NotifyConfig) obj) == 0;
}
return false;
}
-
- @Override
- public String toString() {
- return MoreObjects.toStringHelper(this)
- .add("name", name)
- .add("addresses", addresses)
- .add("groups", groups)
- .add("header", header)
- .add("types", types)
- .add("filter", filter)
- .toString();
- }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 348ab7d..1c81694 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -659,7 +659,9 @@
addFooter(msg, e.getValue().getFooterKey());
noteUtil.appendAccountIdIdentString(msg, e.getKey()).append('\n');
}
+
applyReviewerUpdatesToAttentionSet();
+
for (Map.Entry<Address, ReviewerStateInternal> e : reviewersByEmail.entrySet()) {
addFooter(msg, e.getValue().getByEmailFooterKey(), e.getKey().toString());
}
@@ -766,8 +768,10 @@
private void applyReviewerUpdatesToAttentionSet() {
if ((workInProgress != null && workInProgress == true)
- || getNotes().getChange().isWorkInProgress()) {
- // Users shouldn't be added to the attention set if the change is work in progress.
+ || getNotes().getChange().isWorkInProgress()
+ || status == Change.Status.MERGED) {
+ // Attention set shouldn't change here for changes that are work in progress or are about to
+ // be submitted.
return;
}
Set<Account.Id> currentReviewers =
diff --git a/java/com/google/gerrit/server/project/LabelDefinitionJson.java b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
index 0452d0b..9ff079f 100644
--- a/java/com/google/gerrit/server/project/LabelDefinitionJson.java
+++ b/java/com/google/gerrit/server/project/LabelDefinitionJson.java
@@ -31,7 +31,7 @@
labelType.getValues().stream().collect(toMap(LabelValue::formatValue, LabelValue::getText));
label.defaultValue = labelType.getDefaultValue();
label.branches = labelType.getRefPatterns() != null ? labelType.getRefPatterns() : null;
- label.canOverride = toBoolean(labelType.canOverride());
+ label.canOverride = toBoolean(labelType.isCanOverride());
label.copyAnyScore = toBoolean(labelType.isCopyAnyScore());
label.copyMinScore = toBoolean(labelType.isCopyMinScore());
label.copyMaxScore = toBoolean(labelType.isCopyMaxScore());
@@ -41,8 +41,8 @@
label.copyAllScoresOnMergeFirstParentUpdate =
toBoolean(labelType.isCopyAllScoresOnMergeFirstParentUpdate());
label.copyValues = labelType.getCopyValues().isEmpty() ? null : labelType.getCopyValues();
- label.allowPostSubmit = toBoolean(labelType.allowPostSubmit());
- label.ignoreSelfApproval = toBoolean(labelType.ignoreSelfApproval());
+ label.allowPostSubmit = toBoolean(labelType.isAllowPostSubmit());
+ label.ignoreSelfApproval = toBoolean(labelType.isIgnoreSelfApproval());
return label;
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 2d4928a..35257ef 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.common.data.Permission.isPermission;
import static com.google.gerrit.entities.Project.DEFAULT_SUBMIT_TYPE;
@@ -81,6 +82,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Consumer;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -329,6 +331,16 @@
return project;
}
+ public void setProject(Project.Builder project) {
+ this.project = project.build();
+ }
+
+ public void updateProject(Consumer<Project.Builder> update) {
+ Project.Builder builder = project.toBuilder();
+ update.accept(builder);
+ project = builder.build();
+ }
+
public AccountsSection getAccountsSection() {
return accountsSection;
}
@@ -484,6 +496,20 @@
return labelSections;
}
+ /** Adds or replaces the given {@link LabelType} in this config. */
+ public void upsertLabelType(LabelType labelType) {
+ labelSections.put(labelType.getName(), labelType);
+ }
+
+ /** Allows a mutation of an existing {@link LabelType}. */
+ public void updateLabelType(String name, Consumer<LabelType.Builder> update) {
+ LabelType labelType = labelSections.get(name);
+ checkState(labelType != null, "labelType must not be null");
+ LabelType.Builder builder = labelSections.get(name).toBuilder();
+ update.accept(builder);
+ upsertLabelType(builder.build());
+ }
+
public Collection<StoredCommentLinkInfo> getCommentLinkSections() {
return commentLinkSections.values();
}
@@ -579,13 +605,8 @@
rulesId = getObjectId("rules.pl");
Config rc = readConfig(PROJECT_CONFIG, baseConfig);
- project = new Project(projectName);
-
- Project p = project;
- p.setDescription(rc.getString(PROJECT, null, KEY_DESCRIPTION));
- if (p.getDescription() == null) {
- p.setDescription("");
- }
+ Project.Builder p = Project.builder(projectName);
+ p.setDescription(Strings.nullToEmpty(rc.getString(PROJECT, null, KEY_DESCRIPTION)));
if (revision != null) {
p.setConfigRefState(revision.toObjectId().name());
}
@@ -595,7 +616,7 @@
// as there is no guarantee which of the parents would be used then.
error(ValidationError.create(PROJECT_CONFIG, "Cannot inherit from multiple projects"));
}
- p.setParentName(rc.getString(ACCESS, null, KEY_INHERIT_FROM));
+ p.setParent(rc.getString(ACCESS, null, KEY_INHERIT_FROM));
for (BooleanProjectConfig config : BooleanProjectConfig.values()) {
p.setBooleanConfig(
@@ -615,6 +636,7 @@
p.setDefaultDashboard(rc.getString(DASHBOARD, null, KEY_DEFAULT));
p.setLocalDefaultDashboard(rc.getString(DASHBOARD, null, KEY_LOCAL_DEFAULT));
+ this.project = p.build();
loadAccountsSection(rc);
loadContributorAgreements(rc);
@@ -719,13 +741,13 @@
private void loadNotifySections(Config rc) {
notifySections = new HashMap<>();
for (String sectionName : rc.getSubsections(NOTIFY)) {
- NotifyConfig n = new NotifyConfig();
+ NotifyConfig.Builder n = NotifyConfig.builder();
n.setName(sectionName);
n.setFilter(rc.getString(NOTIFY, sectionName, KEY_FILTER));
EnumSet<NotifyType> types = EnumSet.noneOf(NotifyType.class);
types.addAll(ConfigUtil.getEnumList(rc, NOTIFY, sectionName, KEY_TYPE, NotifyType.ALL));
- n.setTypes(types);
+ n.setNotify(types);
n.setHeader(rc.getEnum(NOTIFY, sectionName, KEY_HEADER, NotifyConfig.Header.BCC));
for (String dst : rc.getStringList(NOTIFY, sectionName, KEY_EMAIL)) {
@@ -736,7 +758,7 @@
ref = groupList.resolve(GroupReference.create(groupName));
}
if (ref.getUUID() != null) {
- n.addEmail(ref);
+ n.addGroup(ref);
} else {
error(
ValidationError.create(
@@ -747,7 +769,7 @@
error(ValidationError.create(PROJECT_CONFIG, dst + " not supported"));
} else {
try {
- n.addEmail(Address.parse(dst));
+ n.addAddress(Address.parse(dst));
} catch (IllegalArgumentException err) {
error(
ValidationError.create(
@@ -756,7 +778,7 @@
}
}
}
- notifySections.put(sectionName, n);
+ notifySections.put(sectionName, n.build());
}
}
@@ -904,7 +926,7 @@
throw new IllegalArgumentException("empty value");
}
String valueText = parts.size() > 1 ? parts.get(1) : "";
- return new LabelValue(Shorts.checkedCast(PermissionRule.parseInt(parts.get(0))), valueText);
+ return LabelValue.create(Shorts.checkedCast(PermissionRule.parseInt(parts.get(0))), valueText);
}
private void loadLabelSections(Config rc) {
@@ -943,9 +965,9 @@
}
}
- LabelType label;
+ LabelType.Builder label;
try {
- label = new LabelType(name, values);
+ label = LabelType.builder(name, values);
} catch (IllegalArgumentException badName) {
error(ValidationError.create(PROJECT_CONFIG, String.format("Invalid label \"%s\"", name)));
continue;
@@ -1035,8 +1057,9 @@
label.setCopyValues(copyValues);
label.setCanOverride(
rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, LabelType.DEF_CAN_OVERRIDE));
- label.setRefPatterns(getStringListOrNull(rc, LABEL, name, KEY_BRANCH));
- labelSections.put(name, label);
+ List<String> refPatterns = getStringListOrNull(rc, LABEL, name, KEY_BRANCH);
+ label.setRefPatterns(refPatterns == null ? null : ImmutableList.copyOf(refPatterns));
+ labelSections.put(name, label.build());
}
}
@@ -1174,7 +1197,7 @@
KEY_MAX_OBJECT_SIZE_LIMIT,
validMaxObjectSizeLimit(p.getMaxObjectSizeLimit()));
- set(rc, SUBMIT, null, KEY_ACTION, p.getConfiguredSubmitType(), DEFAULT_SUBMIT_TYPE);
+ set(rc, SUBMIT, null, KEY_ACTION, p.getSubmitType(), DEFAULT_SUBMIT_TYPE);
set(rc, PROJECT, null, KEY_STATE, p.getState(), DEFAULT_STATE_VALUE);
@@ -1444,14 +1467,14 @@
LABEL,
name,
KEY_ALLOW_POST_SUBMIT,
- label.allowPostSubmit(),
+ label.isAllowPostSubmit(),
LabelType.DEF_ALLOW_POST_SUBMIT);
setBooleanConfigKey(
rc,
LABEL,
name,
KEY_IGNORE_SELF_APPROVAL,
- label.ignoreSelfApproval(),
+ label.isIgnoreSelfApproval(),
LabelType.DEF_IGNORE_SELF_APPROVAL);
setBooleanConfigKey(
rc,
@@ -1508,7 +1531,7 @@
KEY_COPY_VALUE,
label.getCopyValues().stream().map(LabelValue::formatValue).collect(toList()));
setBooleanConfigKey(
- rc, LABEL, name, KEY_CAN_OVERRIDE, label.canOverride(), LabelType.DEF_CAN_OVERRIDE);
+ rc, LABEL, name, KEY_CAN_OVERRIDE, label.isCanOverride(), LabelType.DEF_CAN_OVERRIDE);
List<String> values = new ArrayList<>(label.getValues().size());
for (LabelValue value : label.getValues()) {
values.add(value.format().trim());
diff --git a/java/com/google/gerrit/server/project/ProjectCreator.java b/java/com/google/gerrit/server/project/ProjectCreator.java
index cc10f27..6ffbdef 100644
--- a/java/com/google/gerrit/server/project/ProjectCreator.java
+++ b/java/com/google/gerrit/server/project/ProjectCreator.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import com.google.common.base.MoreObjects;
+import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GroupDescription;
@@ -150,26 +151,32 @@
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);
- }
+ config.updateProject(
+ newProject -> {
+ newProject.setDescription(Strings.nullToEmpty(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.setParent(args.newParent);
+ }
+ });
if (!args.ownerIds.isEmpty()) {
AccessSection all = config.getAccessSection(AccessSection.ALL, true);
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 1c7c7d6..6353103 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -402,7 +402,7 @@
for (LabelType type : s.getConfig().getLabelSections().values()) {
String lower = type.getName().toLowerCase();
LabelType old = types.get(lower);
- if (old == null || old.canOverride()) {
+ if (old == null || old.isCanOverride()) {
types.put(lower, type);
}
}
@@ -501,7 +501,7 @@
public SubmitType getSubmitType() {
for (ProjectState s : tree()) {
- SubmitType t = s.getProject().getConfiguredSubmitType();
+ SubmitType t = s.getProject().getSubmitType();
if (t != SubmitType.INHERIT) {
return t;
}
diff --git a/java/com/google/gerrit/server/project/RefValidationHelper.java b/java/com/google/gerrit/server/project/RefValidationHelper.java
index 9b297f9..1912660 100644
--- a/java/com/google/gerrit/server/project/RefValidationHelper.java
+++ b/java/com/google/gerrit/server/project/RefValidationHelper.java
@@ -43,7 +43,7 @@
throws ResourceConflictException {
RefOperationValidators refValidators =
refValidatorsFactory.create(
- new Project(Project.nameKey(projectName)),
+ Project.builder(Project.nameKey(projectName)).build(),
user,
RefOperationValidators.getCommand(update, operationType));
try {
diff --git a/java/com/google/gerrit/server/project/testing/TestLabels.java b/java/com/google/gerrit/server/project/testing/TestLabels.java
index 6c2ddde..2c0b23c 100644
--- a/java/com/google/gerrit/server/project/testing/TestLabels.java
+++ b/java/com/google/gerrit/server/project/testing/TestLabels.java
@@ -35,18 +35,23 @@
}
public static LabelType patchSetLock() {
- LabelType label =
- label("Patch-Set-Lock", value(1, "Patch Set Locked"), value(0, "Patch Set Unlocked"));
+ LabelType.Builder label =
+ labelBuilder(
+ "Patch-Set-Lock", value(1, "Patch Set Locked"), value(0, "Patch Set Unlocked"));
label.setFunction(LabelFunction.PATCH_SET_LOCK);
- return label;
+ return label.build();
}
public static LabelValue value(int value, String text) {
- return new LabelValue((short) value, text);
+ return LabelValue.create((short) value, text);
}
public static LabelType label(String name, LabelValue... values) {
- return new LabelType(name, Arrays.asList(values));
+ return labelBuilder(name, values).build();
+ }
+
+ public static LabelType.Builder labelBuilder(String name, LabelValue... values) {
+ return LabelType.builder(name, Arrays.asList(values));
}
private TestLabels() {}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 6edbdb0..85079e2 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -1319,7 +1319,7 @@
for (PatchSetApproval psa : del) {
LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
String normName = lt.getName();
- if (!lt.allowPostSubmit()) {
+ if (!lt.isAllowPostSubmit()) {
disallowed.add(normName);
}
Short prev = previous.get(normName);
@@ -1331,7 +1331,7 @@
for (PatchSetApproval psa : ups) {
LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
String normName = lt.getName();
- if (!lt.allowPostSubmit()) {
+ if (!lt.isAllowPostSubmit()) {
disallowed.add(normName);
}
Short prev = previous.get(normName);
diff --git a/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java b/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
index 5deace9..783b39b 100644
--- a/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
+++ b/java/com/google/gerrit/server/restapi/project/ConfigInfoImpl.java
@@ -77,8 +77,7 @@
this.defaultSubmitType.value = projectState.getSubmitType();
this.defaultSubmitType.configuredValue =
MoreObjects.firstNonNull(
- projectState.getConfig().getProject().getConfiguredSubmitType(),
- Project.DEFAULT_SUBMIT_TYPE);
+ projectState.getConfig().getProject().getSubmitType(), Project.DEFAULT_SUBMIT_TYPE);
ProjectState parent =
projectState.isAllProjects() ? projectState : projectState.parents().get(0);
this.defaultSubmitType.inheritedValue = parent.getSubmitType();
diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
index a85ad39..1c19eb0 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java
@@ -134,15 +134,15 @@
throw new BadRequestException("values are required");
}
- List<LabelValue> values = LabelDefinitionInputParser.parseValues(input.values);
-
- LabelType labelType;
try {
- labelType = new LabelType(label, values);
+ LabelType.checkName(label);
} catch (IllegalArgumentException e) {
throw new BadRequestException("invalid name: " + label, e);
}
+ List<LabelValue> values = LabelDefinitionInputParser.parseValues(input.values);
+ LabelType.Builder labelType = LabelType.builder(LabelType.checkName(label), values);
+
if (input.function != null && !input.function.trim().isEmpty()) {
labelType.setFunction(LabelDefinitionInputParser.parseFunction(input.function));
} else {
@@ -203,8 +203,9 @@
labelType.setIgnoreSelfApproval(input.ignoreSelfApproval);
}
- config.getLabelSections().put(labelType.getName(), labelType);
+ LabelType lt = labelType.build();
+ config.upsertLabelType(lt);
- return labelType;
+ return lt;
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/GetConfig.java b/java/com/google/gerrit/server/restapi/project/GetConfig.java
index ce45e7d..ad66587 100644
--- a/java/com/google/gerrit/server/restapi/project/GetConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/GetConfig.java
@@ -24,6 +24,9 @@
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.extensions.webui.UiActions;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -34,6 +37,7 @@
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
private final PluginConfigFactory cfgFactory;
private final AllProjectsName allProjects;
+ private final PermissionBackend permissionBackend;
private final UiActions uiActions;
private final DynamicMap<RestView<ProjectResource>> views;
@@ -43,24 +47,31 @@
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
PluginConfigFactory cfgFactory,
AllProjectsName allProjects,
+ PermissionBackend permissionBackend,
UiActions uiActions,
DynamicMap<RestView<ProjectResource>> views) {
this.serverEnableSignedPush = serverEnableSignedPush;
this.pluginConfigEntries = pluginConfigEntries;
this.allProjects = allProjects;
this.cfgFactory = cfgFactory;
+ this.permissionBackend = permissionBackend;
this.uiActions = uiActions;
this.views = views;
}
@Override
- public Response<ConfigInfo> apply(ProjectResource resource) {
+ public Response<ConfigInfo> apply(ProjectResource resource) throws PermissionBackendException {
+ boolean readConfigAllowed =
+ permissionBackend
+ .currentUser()
+ .project(resource.getNameKey())
+ .test(ProjectPermission.READ_CONFIG);
return Response.ok(
new ConfigInfoImpl(
serverEnableSignedPush,
resource.getProjectState(),
resource.getUser(),
- pluginConfigEntries,
+ readConfigAllowed ? pluginConfigEntries : DynamicMap.emptyMap(),
cfgFactory,
allProjects,
uiActions,
diff --git a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
index 1e288f4..ccc216d 100644
--- a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
+++ b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.project;
+import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Shorts;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -56,21 +57,22 @@
if (valueDescription.isEmpty()) {
throw new BadRequestException("description for value '" + e.getKey() + "' cannot be empty");
}
- valueList.add(new LabelValue(value, valueDescription));
+ valueList.add(LabelValue.create(value, valueDescription));
}
return valueList;
}
- public static short parseDefaultValue(LabelType labelType, short defaultValue)
+ public static short parseDefaultValue(LabelType.Builder labelType, short defaultValue)
throws BadRequestException {
- if (labelType.getValue(defaultValue) == null) {
+ if (!labelType.getValues().stream().anyMatch(v -> v.getValue() == defaultValue)) {
throw new BadRequestException("invalid default value: " + defaultValue);
}
return defaultValue;
}
- public static List<String> parseBranches(List<String> branches) throws BadRequestException {
- List<String> validBranches = new ArrayList<>();
+ public static ImmutableList<String> parseBranches(List<String> branches)
+ throws BadRequestException {
+ ImmutableList.Builder<String> validBranches = ImmutableList.builder();
for (String branch : branches) {
String newBranch = branch.trim();
if (newBranch.isEmpty()) {
@@ -86,7 +88,7 @@
}
validBranches.add(newBranch);
}
- return validBranches;
+ return validBranches.build();
}
private LabelDefinitionInputParser() {}
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java
index 9f9433b..658f57e 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java
@@ -134,28 +134,25 @@
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
ProjectConfig projectConfig = projectConfigFactory.read(md);
- Project p = projectConfig.getProject();
-
- p.setDescription(Strings.emptyToNull(input.description));
-
- for (BooleanProjectConfig cfg : BooleanProjectConfig.values()) {
- InheritableBoolean val = BooleanProjectConfigTransformations.get(cfg, input);
- if (val != null) {
- p.setBooleanConfig(cfg, val);
- }
- }
-
- if (input.maxObjectSizeLimit != null) {
- p.setMaxObjectSizeLimit(input.maxObjectSizeLimit);
- }
-
- if (input.submitType != null) {
- p.setSubmitType(input.submitType);
- }
-
- if (input.state != null) {
- p.setState(input.state);
- }
+ projectConfig.updateProject(
+ p -> {
+ p.setDescription(Strings.emptyToNull(input.description));
+ for (BooleanProjectConfig cfg : BooleanProjectConfig.values()) {
+ InheritableBoolean val = BooleanProjectConfigTransformations.get(cfg, input);
+ if (val != null) {
+ p.setBooleanConfig(cfg, val);
+ }
+ }
+ if (input.maxObjectSizeLimit != null) {
+ p.setMaxObjectSizeLimit(input.maxObjectSizeLimit);
+ }
+ if (input.submitType != null) {
+ p.setSubmitType(input.submitType);
+ }
+ if (input.state != null) {
+ p.setState(input.state);
+ }
+ });
if (input.pluginConfigValues != null) {
setPluginConfigValues(projectState, projectConfig, input.pluginConfigValues);
@@ -169,7 +166,7 @@
try {
projectConfig.commit(md);
projectCache.evict(projectConfig.getProject());
- md.getRepository().setGitwebDescription(p.getDescription());
+ md.getRepository().setGitwebDescription(projectConfig.getProject().getDescription());
} catch (IOException e) {
if (e.getCause() instanceof ConfigInvalidException) {
throw new ResourceConflictException(
diff --git a/java/com/google/gerrit/server/restapi/project/PutDescription.java b/java/com/google/gerrit/server/restapi/project/PutDescription.java
index a0b9feb..a65c626 100644
--- a/java/com/google/gerrit/server/restapi/project/PutDescription.java
+++ b/java/com/google/gerrit/server/restapi/project/PutDescription.java
@@ -16,7 +16,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.projects.DescriptionInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -73,8 +72,8 @@
try (MetaDataUpdate md = updateFactory.get().create(resource.getNameKey())) {
ProjectConfig config = projectConfigFactory.read(md);
- Project project = config.getProject();
- project.setDescription(Strings.emptyToNull(input.description));
+ String desc = input.description;
+ config.updateProject(p -> p.setDescription(Strings.emptyToNull(desc)));
String msg =
MoreObjects.firstNonNull(
@@ -86,11 +85,11 @@
md.setMessage(msg);
config.commit(md);
cache.evict(resource.getProjectState().getProject());
- md.getRepository().setGitwebDescription(project.getDescription());
+ md.getRepository().setGitwebDescription(config.getProject().getDescription());
- return Strings.isNullOrEmpty(project.getDescription())
+ return Strings.isNullOrEmpty(config.getProject().getDescription())
? Response.none()
- : Response.ok(project.getDescription());
+ : Response.ok(config.getProject().getDescription());
} catch (RepositoryNotFoundException notFound) {
throw new ResourceNotFoundException(resource.getName(), notFound);
} catch (ConfigInvalidException e) {
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
index 5d5e779..390dea9 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
@@ -243,7 +243,7 @@
} catch (UnprocessableEntityException e) {
throw new ResourceConflictException(e.getMessage(), e);
}
- config.getProject().setParentName(newParentProjectName);
+ config.updateProject(p -> p.setParent(newParentProjectName));
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
index 9920be0..5aef76a 100644
--- a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
+++ b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java
@@ -16,7 +16,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
import com.google.gerrit.extensions.api.projects.SetDashboardInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -97,11 +96,11 @@
try (MetaDataUpdate md = updateFactory.create(rsrc.getProjectState().getNameKey())) {
ProjectConfig config = projectConfigFactory.read(md);
- Project project = config.getProject();
+ String id = input.id;
if (inherited) {
- project.setDefaultDashboard(input.id);
+ config.updateProject(p -> p.setDefaultDashboard(id));
} else {
- project.setLocalDefaultDashboard(input.id);
+ config.updateProject(p -> p.setLocalDefaultDashboard(id));
}
String msg =
diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java
index 0a35865..ade274a 100644
--- a/java/com/google/gerrit/server/restapi/project/SetLabel.java
+++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java
@@ -88,6 +88,9 @@
} else {
md.setMessage("Update label");
}
+ String newName = Strings.nullToEmpty(input.name).trim();
+ labelType =
+ config.getLabelSections().get(newName.isEmpty() ? labelType.getName() : newName);
config.commit(md);
projectCache.evict(rsrc.getProject().getProjectState().getProject());
@@ -109,8 +112,7 @@
public boolean updateLabel(ProjectConfig config, LabelType labelType, LabelDefinitionInput input)
throws BadRequestException, ResourceConflictException {
boolean dirty = false;
-
- config.getLabelSections().remove(labelType.getName());
+ LabelType.Builder labelTypeBuilder = labelType.toBuilder();
if (input.name != null) {
String newName = input.name.trim();
@@ -130,10 +132,12 @@
}
try {
- labelType.setName(newName);
+ LabelType.checkName(newName);
} catch (IllegalArgumentException e) {
throw new BadRequestException("invalid name: " + input.name, e);
}
+
+ labelTypeBuilder.setName(newName);
dirty = true;
}
}
@@ -142,7 +146,7 @@
if (input.function.trim().isEmpty()) {
throw new BadRequestException("function cannot be empty");
}
- labelType.setFunction(LabelDefinitionInputParser.parseFunction(input.function));
+ labelTypeBuilder.setFunction(LabelDefinitionInputParser.parseFunction(input.function));
dirty = true;
}
@@ -150,77 +154,79 @@
if (input.values.isEmpty()) {
throw new BadRequestException("values cannot be empty");
}
- labelType.setValues(LabelDefinitionInputParser.parseValues(input.values));
+ labelTypeBuilder.setValues(LabelDefinitionInputParser.parseValues(input.values));
dirty = true;
}
if (input.defaultValue != null) {
- labelType.setDefaultValue(
- LabelDefinitionInputParser.parseDefaultValue(labelType, input.defaultValue));
+ labelTypeBuilder.setDefaultValue(
+ LabelDefinitionInputParser.parseDefaultValue(labelTypeBuilder, input.defaultValue));
dirty = true;
}
if (input.branches != null) {
- labelType.setRefPatterns(LabelDefinitionInputParser.parseBranches(input.branches));
+ labelTypeBuilder.setRefPatterns(LabelDefinitionInputParser.parseBranches(input.branches));
dirty = true;
}
if (input.canOverride != null) {
- labelType.setCanOverride(input.canOverride);
+ labelTypeBuilder.setCanOverride(input.canOverride);
dirty = true;
}
if (input.copyAnyScore != null) {
- labelType.setCopyAnyScore(input.copyAnyScore);
+ labelTypeBuilder.setCopyAnyScore(input.copyAnyScore);
dirty = true;
}
if (input.copyMinScore != null) {
- labelType.setCopyMinScore(input.copyMinScore);
+ labelTypeBuilder.setCopyMinScore(input.copyMinScore);
dirty = true;
}
if (input.copyMaxScore != null) {
- labelType.setCopyMaxScore(input.copyMaxScore);
+ labelTypeBuilder.setCopyMaxScore(input.copyMaxScore);
dirty = true;
}
if (input.copyAllScoresIfNoChange != null) {
- labelType.setCopyAllScoresIfNoChange(input.copyAllScoresIfNoChange);
+ labelTypeBuilder.setCopyAllScoresIfNoChange(input.copyAllScoresIfNoChange);
+ dirty = true;
}
if (input.copyAllScoresIfNoCodeChange != null) {
- labelType.setCopyAllScoresIfNoCodeChange(input.copyAllScoresIfNoCodeChange);
+ labelTypeBuilder.setCopyAllScoresIfNoCodeChange(input.copyAllScoresIfNoCodeChange);
dirty = true;
}
if (input.copyAllScoresOnTrivialRebase != null) {
- labelType.setCopyAllScoresOnTrivialRebase(input.copyAllScoresOnTrivialRebase);
+ labelTypeBuilder.setCopyAllScoresOnTrivialRebase(input.copyAllScoresOnTrivialRebase);
dirty = true;
}
if (input.copyAllScoresOnMergeFirstParentUpdate != null) {
- labelType.setCopyAllScoresOnMergeFirstParentUpdate(
+ labelTypeBuilder.setCopyAllScoresOnMergeFirstParentUpdate(
input.copyAllScoresOnMergeFirstParentUpdate);
dirty = true;
}
if (input.copyValues != null) {
- labelType.setCopyValues(input.copyValues);
+ labelTypeBuilder.setCopyValues(input.copyValues);
dirty = true;
}
if (input.allowPostSubmit != null) {
- labelType.setAllowPostSubmit(input.allowPostSubmit);
+ labelTypeBuilder.setAllowPostSubmit(input.allowPostSubmit);
dirty = true;
}
if (input.ignoreSelfApproval != null) {
- labelType.setIgnoreSelfApproval(input.ignoreSelfApproval);
+ labelTypeBuilder.setIgnoreSelfApproval(input.ignoreSelfApproval);
dirty = true;
}
- config.getLabelSections().put(labelType.getName(), labelType);
+ config.getLabelSections().remove(labelType.getName());
+ config.upsertLabelType(labelTypeBuilder.build());
return dirty;
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java
index 42790aa..91c29f5 100644
--- a/java/com/google/gerrit/server/restapi/project/SetParent.java
+++ b/java/com/google/gerrit/server/restapi/project/SetParent.java
@@ -103,8 +103,7 @@
validateParentUpdate(rsrc.getProjectState().getNameKey(), user, parentName, checkIfAdmin);
try (MetaDataUpdate md = updateFactory.get().create(rsrc.getNameKey())) {
ProjectConfig config = projectConfigFactory.read(md);
- Project project = config.getProject();
- project.setParentName(parentName);
+ config.updateProject(p -> p.setParent(parentName));
String msg = Strings.emptyToNull(input.commitMessage);
if (msg == null) {
@@ -117,7 +116,7 @@
config.commit(md);
cache.evict(rsrc.getProjectState().getProject());
- Project.NameKey parent = project.getParent(allProjects);
+ Project.NameKey parent = config.getProject().getParent(allProjects);
requireNonNull(parent);
return parent.get();
} catch (RepositoryNotFoundException notFound) {
diff --git a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
index 54915fb..132747d 100644
--- a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
+++ b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
@@ -66,7 +66,8 @@
return ruleError(E_UNABLE_TO_FETCH_LABELS);
}
- boolean shouldIgnoreSelfApproval = labelTypes.stream().anyMatch(LabelType::ignoreSelfApproval);
+ boolean shouldIgnoreSelfApproval =
+ labelTypes.stream().anyMatch(LabelType::isIgnoreSelfApproval);
if (!shouldIgnoreSelfApproval) {
// Shortcut to avoid further processing if no label should ignore uploader approvals
return Optional.empty();
@@ -86,7 +87,7 @@
submitRecord.requirements = new ArrayList<>();
for (LabelType t : labelTypes) {
- if (!t.ignoreSelfApproval()) {
+ if (!t.isIgnoreSelfApproval()) {
// The default rules are enough in this case.
continue;
}
diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index cfa5825..018a96a 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -30,7 +30,6 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllProjectsName;
@@ -116,19 +115,16 @@
// init basic project configs.
ProjectConfig config = projectConfigFactory.read(md);
- Project p = config.getProject();
- p.setDescription(
- input.projectDescription().orElse("Access inherited by all other projects."));
-
- // init boolean project configs.
- input.booleanProjectConfigs().forEach(p::setBooleanConfig);
+ config.updateProject(
+ p -> {
+ p.setDescription(
+ input.projectDescription().orElse("Access inherited by all other projects."));
+ // init boolean project configs.
+ input.booleanProjectConfigs().forEach(p::setBooleanConfig);
+ });
// init labels.
- input
- .codeReviewLabel()
- .ifPresent(
- codeReviewLabel ->
- config.getLabelSections().put(codeReviewLabel.getName(), codeReviewLabel));
+ input.codeReviewLabel().ifPresent(codeReviewLabel -> config.upsertLabelType(codeReviewLabel));
if (input.initDefaultAcls()) {
// init access sections.
diff --git a/java/com/google/gerrit/server/schema/AllProjectsInput.java b/java/com/google/gerrit/server/schema/AllProjectsInput.java
index 6e11a5d..c91695f 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsInput.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsInput.java
@@ -46,18 +46,17 @@
@UsedAt(UsedAt.Project.GOOGLE)
public static LabelType getDefaultCodeReviewLabel() {
- LabelType type =
- new LabelType(
+ return LabelType.builder(
"Code-Review",
ImmutableList.of(
- new LabelValue((short) 2, "Looks good to me, approved"),
- new LabelValue((short) 1, "Looks good to me, but someone else must approve"),
- new LabelValue((short) 0, "No score"),
- new LabelValue((short) -1, "I would prefer this is not merged as is"),
- new LabelValue((short) -2, "This shall not be merged")));
- type.setCopyMinScore(true);
- type.setCopyAllScoresOnTrivialRebase(true);
- return type;
+ LabelValue.create((short) 2, "Looks good to me, approved"),
+ LabelValue.create((short) 1, "Looks good to me, but someone else must approve"),
+ LabelValue.create((short) 0, "No score"),
+ LabelValue.create((short) -1, "I would prefer this is not merged as is"),
+ LabelValue.create((short) -2, "This shall not be merged")))
+ .setCopyMinScore(true)
+ .setCopyAllScoresOnTrivialRebase(true)
+ .build();
}
/** The administrator group which gets default permissions granted. */
diff --git a/java/com/google/gerrit/server/schema/AllUsersCreator.java b/java/com/google/gerrit/server/schema/AllUsersCreator.java
index 4904028..10d7070 100644
--- a/java/com/google/gerrit/server/schema/AllUsersCreator.java
+++ b/java/com/google/gerrit/server/schema/AllUsersCreator.java
@@ -26,7 +26,6 @@
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllUsersName;
@@ -112,15 +111,14 @@
md.setMessage("Initialized Gerrit Code Review " + Version.getVersion());
ProjectConfig config = projectConfigFactory.read(md);
- Project project = config.getProject();
- project.setDescription("Individual user settings and preferences.");
+ config.updateProject(p -> p.setDescription("Individual user settings and preferences."));
AccessSection users =
config.getAccessSection(
RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true);
// Initialize "Code-Review" label.
- config.getLabelSections().put(codeReviewLabel.getName(), codeReviewLabel);
+ config.upsertLabelType(codeReviewLabel);
grant(config, users, Permission.READ, false, true, registered);
grant(config, users, Permission.PUSH, false, true, registered);
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 5adda2c..a1ed373 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.config.VerboseSuperprojectUpdate;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateListener;
@@ -89,27 +88,31 @@
@Singleton
public static class Factory {
- private final GitModules.Factory gitmodulesFactory;
+ private final SubscriptionGraph.Factory subscriptionGraphFactory;
private final Provider<PersonIdent> serverIdent;
private final Config cfg;
- private final ProjectCache projectCache;
@Inject
Factory(
- GitModules.Factory gitmodulesFactory,
+ SubscriptionGraph.Factory subscriptionGraphFactory,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
- @GerritServerConfig Config cfg,
- ProjectCache projectCache) {
- this.gitmodulesFactory = gitmodulesFactory;
+ @GerritServerConfig Config cfg) {
+ this.subscriptionGraphFactory = subscriptionGraphFactory;
this.serverIdent = serverIdent;
this.cfg = cfg;
- this.projectCache = projectCache;
}
public SubmoduleOp create(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleConflictException {
- return new SubmoduleOp(
- gitmodulesFactory, serverIdent.get(), cfg, projectCache, updatedBranches, orm);
+ SubscriptionGraph subscriptionGraph;
+ if (cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true)) {
+ subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches, orm);
+ } else {
+ logger.atFine().log("Updating superprojects disabled");
+ subscriptionGraph =
+ SubscriptionGraph.createEmptyGraph(ImmutableSet.copyOf(updatedBranches));
+ }
+ return new SubmoduleOp(serverIdent.get(), cfg, orm, subscriptionGraph);
}
}
@@ -118,19 +121,15 @@
private final long maxCombinedCommitMessageSize;
private final long maxCommitMessages;
private final MergeOpRepoManager orm;
- private final SubscriptionGraph.Factory subscriptionGraphFactory;
private final SubscriptionGraph subscriptionGraph;
private final BranchTips branchTips = new BranchTips();
private SubmoduleOp(
- GitModules.Factory gitmodulesFactory,
PersonIdent myIdent,
Config cfg,
- ProjectCache projectCache,
- Set<BranchNameKey> updatedBranches,
- MergeOpRepoManager orm)
- throws SubmoduleConflictException {
+ MergeOpRepoManager orm,
+ SubscriptionGraph subscriptionGraph) {
this.myIdent = myIdent;
this.verboseSuperProject =
cfg.getEnum("submodule", null, "verboseSuperprojectUpdate", VerboseSuperprojectUpdate.TRUE);
@@ -138,15 +137,7 @@
cfg.getLong("submodule", "maxCombinedCommitMessageSize", 256 << 10);
this.maxCommitMessages = cfg.getLong("submodule", "maxCommitMessages", 1000);
this.orm = orm;
- this.subscriptionGraphFactory =
- new SubscriptionGraph.DefaultFactory(gitmodulesFactory, projectCache, orm);
- if (cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true)) {
- this.subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches);
- } else {
- logger.atFine().log("Updating superprojects disabled");
- this.subscriptionGraph =
- SubscriptionGraph.createEmptyGraph(ImmutableSet.copyOf(updatedBranches));
- }
+ this.subscriptionGraph = subscriptionGraph;
}
@UsedAt(UsedAt.Project.PLUGIN_DELETE_PROJECT)
diff --git a/java/com/google/gerrit/server/submit/SubscriptionGraph.java b/java/com/google/gerrit/server/submit/SubscriptionGraph.java
index eac6d2c..f037261 100644
--- a/java/com/google/gerrit/server/submit/SubscriptionGraph.java
+++ b/java/com/google/gerrit/server/submit/SubscriptionGraph.java
@@ -31,6 +31,8 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -102,12 +104,12 @@
}
/** Get branches updated as part of the enclosing submit or push batch. */
- ImmutableSet<BranchNameKey> getUpdatedBranches() {
+ public ImmutableSet<BranchNameKey> getUpdatedBranches() {
return updatedBranches;
}
/** Get all superprojects affected. */
- ImmutableSet<Project.NameKey> getAffectedSuperProjects() {
+ public ImmutableSet<Project.NameKey> getAffectedSuperProjects() {
return branchesByProject.keySet();
}
@@ -120,7 +122,7 @@
* Returns all branches within the superproject {@code project} which have submodule
* subscriptions.
*/
- ImmutableSet<BranchNameKey> getAffectedSuperBranches(Project.NameKey project) {
+ public ImmutableSet<BranchNameKey> getAffectedSuperBranches(Project.NameKey project) {
return branchesByProject.get(project);
}
@@ -130,71 +132,78 @@
*
* @see SubscriptionGraph#sortedBranches
*/
- ImmutableSet<BranchNameKey> getSortedSuperprojectAndSubmoduleBranches() {
+ public ImmutableSet<BranchNameKey> getSortedSuperprojectAndSubmoduleBranches() {
return sortedBranches;
}
/** Check if a {@code branch} is a submodule of a superproject. */
- boolean hasSuperproject(BranchNameKey branch) {
+ public boolean hasSuperproject(BranchNameKey branch) {
return subscribedBranches.contains(branch);
}
/** See if a {@code branch} is a superproject branch affected. */
- boolean hasSubscription(BranchNameKey branch) {
+ public boolean hasSubscription(BranchNameKey branch) {
return targets.containsKey(branch);
}
/** Get all related {@code SubmoduleSubscription}s whose super branch is {@code branch}. */
- ImmutableSet<SubmoduleSubscription> getSubscriptions(BranchNameKey branch) {
+ public ImmutableSet<SubmoduleSubscription> getSubscriptions(BranchNameKey branch) {
return targets.get(branch);
}
public interface Factory {
- SubscriptionGraph compute(Set<BranchNameKey> updatedBranches) throws SubmoduleConflictException;
+ SubscriptionGraph compute(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
+ throws SubmoduleConflictException;
+ }
+
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(Factory.class).to(DefaultFactory.class);
+ }
}
static class DefaultFactory implements Factory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ProjectCache projectCache;
private final GitModules.Factory gitmodulesFactory;
- private final Map<BranchNameKey, GitModules> branchGitModules;
- private final MergeOpRepoManager orm;
- // Fields required to the constructor of SubscriptionGraph.
- /** All affected branches, including those in superprojects and submodules. */
- private final Set<BranchNameKey> affectedBranches;
-
- /** @see SubscriptionGraph#targets */
- private final SetMultimap<BranchNameKey, SubmoduleSubscription> targets;
-
- /** @see SubscriptionGraph#branchesByProject */
- private final SetMultimap<Project.NameKey, BranchNameKey> branchesByProject;
-
- /** @see SubscriptionGraph#subscribedBranches */
- private final Set<BranchNameKey> subscribedBranches;
-
- DefaultFactory(
- GitModules.Factory gitmodulesFactory, ProjectCache projectCache, MergeOpRepoManager orm) {
+ @Inject
+ DefaultFactory(GitModules.Factory gitmodulesFactory, ProjectCache projectCache) {
this.gitmodulesFactory = gitmodulesFactory;
this.projectCache = projectCache;
- this.orm = orm;
- this.branchGitModules = new HashMap<>();
-
- this.affectedBranches = new HashSet<>();
- this.targets = MultimapBuilder.hashKeys().hashSetValues().build();
- this.branchesByProject = MultimapBuilder.hashKeys().hashSetValues().build();
- this.subscribedBranches = new HashSet<>();
}
@Override
- public SubscriptionGraph compute(Set<BranchNameKey> updatedBranches)
+ public SubscriptionGraph compute(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleConflictException {
+ Map<BranchNameKey, GitModules> branchGitModules = new HashMap<>();
+ // All affected branches, including those in superprojects and submodules.
+ Set<BranchNameKey> affectedBranches = new HashSet<>();
+
+ // See SubscriptionGraph#targets.
+ SetMultimap<BranchNameKey, SubmoduleSubscription> targets =
+ MultimapBuilder.hashKeys().hashSetValues().build();
+
+ // See SubscriptionGraph#branchesByProject.
+ SetMultimap<Project.NameKey, BranchNameKey> branchesByProject =
+ MultimapBuilder.hashKeys().hashSetValues().build();
+
+ // See SubscriptionGraph#subscribedBranches.
+ Set<BranchNameKey> subscribedBranches = new HashSet<>();
+
+ Set<BranchNameKey> sortedBranches =
+ calculateSubscriptionMaps(
+ updatedBranches,
+ affectedBranches,
+ targets,
+ branchesByProject,
+ subscribedBranches,
+ branchGitModules,
+ orm);
+
return new SubscriptionGraph(
- updatedBranches,
- targets,
- branchesByProject,
- subscribedBranches,
- calculateSubscriptionMaps(updatedBranches));
+ updatedBranches, targets, branchesByProject, subscribedBranches, sortedBranches);
}
/**
@@ -203,15 +212,22 @@
* <p>In addition to the return value, the following fields are populated as a side effect:
*
* <ul>
- * <li>{@link #affectedBranches}
- * <li>{@link #targets}
- * <li>{@link #branchesByProject}
- * <li>{@link #subscribedBranches}
+ * <li>{@code affectedBranches}
+ * <li>{@code targets}
+ * <li>{@code branchesByProject}
+ * <li>{@code subscribedBranches}
* </ul>
*
* @return the ordered set to be stored in {@link #sortedBranches}.
*/
- private Set<BranchNameKey> calculateSubscriptionMaps(Set<BranchNameKey> updatedBranches)
+ private Set<BranchNameKey> calculateSubscriptionMaps(
+ Set<BranchNameKey> updatedBranches,
+ Set<BranchNameKey> affectedBranches,
+ SetMultimap<BranchNameKey, SubmoduleSubscription> targets,
+ SetMultimap<Project.NameKey, BranchNameKey> branchesByProject,
+ Set<BranchNameKey> subscribedBranches,
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
throws SubmoduleConflictException {
logger.atFine().log("Calculating superprojects - submodules map");
LinkedHashSet<BranchNameKey> allVisited = new LinkedHashSet<>();
@@ -220,7 +236,16 @@
continue;
}
- searchForSuperprojects(updatedBranch, new LinkedHashSet<>(), allVisited);
+ searchForSuperprojects(
+ updatedBranch,
+ new LinkedHashSet<>(),
+ allVisited,
+ affectedBranches,
+ targets,
+ branchesByProject,
+ subscribedBranches,
+ branchGitModules,
+ orm);
}
// Since the searchForSuperprojects will add all branches (related or
@@ -235,7 +260,13 @@
private void searchForSuperprojects(
BranchNameKey current,
LinkedHashSet<BranchNameKey> currentVisited,
- LinkedHashSet<BranchNameKey> allVisited)
+ LinkedHashSet<BranchNameKey> allVisited,
+ Set<BranchNameKey> affectedBranches,
+ SetMultimap<BranchNameKey, SubmoduleSubscription> targets,
+ SetMultimap<Project.NameKey, BranchNameKey> branchesByProject,
+ Set<BranchNameKey> subscribedBranches,
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
throws SubmoduleConflictException {
logger.atFine().log("Now processing %s", current);
@@ -252,10 +283,19 @@
currentVisited.add(current);
try {
Collection<SubmoduleSubscription> subscriptions =
- superProjectSubscriptionsForSubmoduleBranch(current);
+ superProjectSubscriptionsForSubmoduleBranch(current, branchGitModules, orm);
for (SubmoduleSubscription sub : subscriptions) {
BranchNameKey superBranch = sub.getSuperProject();
- searchForSuperprojects(superBranch, currentVisited, allVisited);
+ searchForSuperprojects(
+ superBranch,
+ currentVisited,
+ allVisited,
+ affectedBranches,
+ targets,
+ branchesByProject,
+ subscribedBranches,
+ branchGitModules,
+ orm);
targets.put(superBranch, sub);
branchesByProject.put(superBranch.project(), superBranch);
affectedBranches.add(superBranch);
@@ -269,8 +309,8 @@
allVisited.add(current);
}
- private Collection<BranchNameKey> getDestinationBranches(BranchNameKey src, SubscribeSection s)
- throws IOException {
+ private Collection<BranchNameKey> getDestinationBranches(
+ BranchNameKey src, SubscribeSection s, MergeOpRepoManager orm) throws IOException {
OpenRepo or;
try {
or = orm.getRepo(s.project());
@@ -286,7 +326,10 @@
}
private Collection<SubmoduleSubscription> superProjectSubscriptionsForSubmoduleBranch(
- BranchNameKey srcBranch) throws IOException {
+ BranchNameKey srcBranch,
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
+ throws IOException {
logger.atFine().log("Calculating possible superprojects for %s", srcBranch);
Collection<SubmoduleSubscription> ret = new ArrayList<>();
Project.NameKey srcProject = srcBranch.project();
@@ -296,7 +339,7 @@
.orElseThrow(illegalState(srcProject))
.getSubscribeSections(srcBranch)) {
logger.atFine().log("Checking subscribe section %s", s);
- Collection<BranchNameKey> branches = getDestinationBranches(srcBranch, s);
+ Collection<BranchNameKey> branches = getDestinationBranches(srcBranch, s, orm);
for (BranchNameKey targetBranch : branches) {
Project.NameKey targetProject = targetBranch.project();
try {
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index a682d33..6c9fbed 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -87,6 +87,7 @@
import com.google.gerrit.server.securestore.SecureStore;
import com.google.gerrit.server.ssh.NoSshKeyCache;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.submit.SubscriptionGraph;
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
@@ -175,6 +176,7 @@
install(new SearchingChangeCacheImpl.Module());
factory(GarbageCollection.Factory.class);
install(new AuditModule());
+ install(new SubscriptionGraph.Module());
bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 11cc82f..c0a9da6 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -67,7 +67,8 @@
protected void setUseContributorAgreements(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = projectConfigFactory.read(md);
- config.getProject().setBooleanConfig(BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, value);
+ config.updateProject(
+ p -> p.setBooleanConfig(BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, value));
config.commit(md);
projectCache.evict(config.getProject());
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 39a860e..42c09c7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2231,7 +2231,7 @@
LabelType verified =
label("Verified", value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified);
u.save();
}
projectOperations
@@ -2518,7 +2518,7 @@
label("Verified", value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
String heads = "refs/heads/*";
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified);
u.save();
}
projectOperations
@@ -2863,9 +2863,9 @@
LabelType custom2 =
label("Custom2", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(verified.getName(), verified);
- u.getConfig().getLabelSections().put(custom1.getName(), custom1);
- u.getConfig().getLabelSections().put(custom2.getName(), custom2);
+ u.getConfig().upsertLabelType(verified);
+ u.getConfig().upsertLabelType(custom1);
+ u.getConfig().upsertLabelType(custom2);
u.save();
}
projectOperations
@@ -3603,7 +3603,7 @@
String heads = RefNames.REFS_HEADS + "*";
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified);
u.save();
}
projectOperations
@@ -3671,7 +3671,7 @@
// add new label and assert that it's returned for existing changes
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified);
u.save();
}
projectOperations
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 0ac7e20..b855e72 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -398,7 +398,7 @@
.review(ReviewInput.approve());
gApi.changes().id(result.getChangeId()).revision(result.getCommit().name()).submit();
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setState(ProjectState.READ_ONLY);
+ u.getConfig().updateProject(p -> p.setState(ProjectState.READ_ONLY));
u.save();
}
@@ -481,7 +481,7 @@
// revoke write permissions for the first repository.
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setState(ProjectState.READ_ONLY);
+ u.getConfig().updateProject(p -> p.setState(ProjectState.READ_ONLY));
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 923b66f..3d8a034 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -26,7 +26,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.testing.TestLabels.label;
+import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
import static org.eclipse.jgit.lib.Constants.HEAD;
@@ -78,8 +78,8 @@
try (ProjectConfigUpdate u = updateProject(project)) {
// Overwrite "Code-Review" label that is inherited from All-Projects.
// This way changes to the "Code Review" label don't affect other tests.
- LabelType codeReview =
- label(
+ LabelType.Builder codeReview =
+ labelBuilder(
"Code-Review",
value(2, "Looks good to me, approved"),
value(1, "Looks good to me, but someone else must approve"),
@@ -87,12 +87,12 @@
value(-1, "I would prefer that you didn't submit this"),
value(-2, "Do not submit"));
codeReview.setCopyAllScoresIfNoChange(false);
- u.getConfig().getLabelSections().put(codeReview.getName(), codeReview);
+ u.getConfig().upsertLabelType(codeReview.build());
- LabelType verified =
- label("Verified", value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
+ LabelType.Builder verified =
+ labelBuilder("Verified", value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
verified.setCopyAllScoresIfNoChange(false);
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified.build());
u.save();
}
@@ -121,7 +121,7 @@
@Test
public void stickyOnAnyScore() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyAnyScore(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyAnyScore(true));
u.save();
}
@@ -143,7 +143,7 @@
@Test
public void stickyOnMinScore() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyMinScore(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMinScore(true));
u.save();
}
@@ -165,7 +165,7 @@
@Test
public void stickyOnMaxScore() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMaxScore(true));
u.save();
}
@@ -190,9 +190,8 @@
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getLabelSections()
- .get("Code-Review")
- .setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ .updateLabelType(
+ "Code-Review", b -> b.setCopyValues(ImmutableList.of((short) -1, (short) 1)));
u.save();
}
@@ -216,7 +215,7 @@
@Test
public void stickyOnTrivialRebase() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresOnTrivialRebase(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyAllScoresOnTrivialRebase(true));
u.save();
}
@@ -262,7 +261,7 @@
@Test
public void stickyOnNoCodeChange() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Verified").setCopyAllScoresIfNoCodeChange(true);
+ u.getConfig().updateLabelType("Verified", b -> b.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
@@ -287,9 +286,7 @@
public void stickyOnMergeFirstParentUpdate() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getLabelSections()
- .get("Code-Review")
- .setCopyAllScoresOnMergeFirstParentUpdate(true);
+ .updateLabelType("Code-Review", b -> b.setCopyAllScoresOnMergeFirstParentUpdate(true));
u.save();
}
@@ -313,7 +310,7 @@
@Test
public void notStickyWithCopyOnNoChangeWhenSecondParentIsUpdated() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresIfNoChange(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyAllScoresIfNoChange(true));
u.save();
}
@@ -330,8 +327,8 @@
@Test
public void removedVotesNotSticky() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresOnTrivialRebase(true);
- u.getConfig().getLabelSections().get("Verified").setCopyAllScoresIfNoCodeChange(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyAllScoresOnTrivialRebase(true));
+ u.getConfig().updateLabelType("Verified", b -> b.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
@@ -360,8 +357,8 @@
@Test
public void stickyAcrossMultiplePatchSets() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
- u.getConfig().getLabelSections().get("Verified").setCopyAllScoresIfNoCodeChange(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMaxScore(true));
+ u.getConfig().updateLabelType("Verified", b -> b.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
@@ -386,8 +383,8 @@
// change kind against all prior patch sets. This is a regression that made Gerrit do expensive
// work in O(num-patch-sets). This test ensures that we aren't regressing.
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
- u.getConfig().getLabelSections().get("Verified").setCopyAllScoresIfNoCodeChange(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMaxScore(true));
+ u.getConfig().updateLabelType("Verified", b -> b.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
@@ -418,8 +415,8 @@
@Test
public void copyMinMaxAcrossMultiplePatchSets() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get("Code-Review").setCopyMaxScore(true);
- u.getConfig().getLabelSections().get("Code-Review").setCopyMinScore(true);
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMaxScore(true));
+ u.getConfig().updateLabelType("Code-Review", b -> b.setCopyMinScore(true));
u.save();
}
@@ -459,7 +456,7 @@
public void deleteStickyVote() throws Exception {
String label = "Code-Review";
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().get(label).setCopyMaxScore(true);
+ u.getConfig().updateLabelType(label, b -> b.setCopyMaxScore(true));
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index cb22849..840d3e0 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -696,6 +696,31 @@
}
@Test
+ public void pluginConfigsReturnedWhenRefsMetaConfigReadable() throws Exception {
+ ProjectConfigEntry entry = new ProjectConfigEntry("enabled", "true");
+ try (Registration ignored =
+ extensionRegistry.newRegistration().add(entry, "test-config-entry")) {
+ // The admin can see refs/meta/config and hence has the READ_CONFIG permission.
+ requestScopeOperations.setApiUser(admin.id());
+ ConfigInfo configInfo = getConfig();
+ assertThat(configInfo.pluginConfig).isNotNull();
+ assertThat(configInfo.pluginConfig).isNotEmpty();
+ }
+ }
+
+ @Test
+ public void pluginConfigsNotReturnedWhenRefsMetaConfigNotReadable() throws Exception {
+ ProjectConfigEntry entry = new ProjectConfigEntry("enabled", "true");
+ try (Registration ignored =
+ extensionRegistry.newRegistration().add(entry, "test-config-entry")) {
+ // This user cannot see refs/meta/config and hence does not have the READ_CONFIG permission.
+ requestScopeOperations.setApiUser(user.id());
+ ConfigInfo configInfo = getConfig();
+ assertThat(configInfo.pluginConfig).isNull();
+ }
+ }
+
+ @Test
public void noCommentlinksByDefault() throws Exception {
assertThat(getConfig().commentlinks).isEmpty();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
index dad09f9..e45d95c 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
@@ -104,18 +104,18 @@
Project.NameKey p1 = projectOperations.newProject().create();
Project.NameKey p2 = projectOperations.newProject().create();
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setParentName(p1);
+ u.getConfig().updateProject(p -> p.setParent(p1));
u.save();
}
assertThat(stalenessChecker.check(project).isStale()).isFalse();
- updateProjectConfigWithoutIndexUpdate(p1, c -> c.getProject().setParentName(p2));
+ updateProjectConfigWithoutIndexUpdate(p1, c -> c.updateProject(p -> p.setParent(p2)));
assertThat(stalenessChecker.check(project).isStale()).isTrue();
}
private void updateProjectConfigWithoutIndexUpdate(Project.NameKey project) throws Exception {
updateProjectConfigWithoutIndexUpdate(
- project, c -> c.getProject().setDescription("making it stale"));
+ project, c -> c.updateProject(p -> p.setDescription("making it stale")));
}
private void updateProjectConfigWithoutIndexUpdate(
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index f0bb201..d361247 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -772,8 +772,9 @@
String cr = "Code-Review";
try (ProjectConfigUpdate u = updateProject(project)) {
LabelType codeReview = TestLabels.codeReview();
- codeReview.setCopyAllScoresIfNoCodeChange(true);
- u.getConfig().getLabelSections().put(cr, codeReview);
+ u.getConfig().upsertLabelType(codeReview);
+ u.getConfig()
+ .updateLabelType(codeReview.getName(), lt -> lt.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 7213a9f..b01b195 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -161,7 +161,7 @@
public void setUpPatchSetLock() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
patchSetLock = TestLabels.patchSetLock();
- u.getConfig().getLabelSections().put(patchSetLock.getName(), patchSetLock);
+ u.getConfig().upsertLabelType(patchSetLock);
u.save();
}
projectOperations
@@ -1200,7 +1200,7 @@
label("Custom-Label", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
String heads = "refs/heads/*";
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(Q.getName(), Q);
+ u.getConfig().upsertLabelType(Q);
u.save();
}
projectOperations
@@ -1686,8 +1686,10 @@
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getProject()
- .setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
+ .updateProject(
+ p ->
+ p.setBooleanConfig(
+ BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE));
u.save();
}
@@ -1712,8 +1714,10 @@
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getProject()
- .setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
+ .updateProject(
+ p ->
+ p.setBooleanConfig(
+ BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE));
u.save();
}
@@ -1863,9 +1867,8 @@
@Test
public void pushNewPatchsetOverridingStickyLabel() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType codeReview = TestLabels.codeReview();
- codeReview.setCopyMaxScore(true);
- u.getConfig().getLabelSections().put(codeReview.getName(), codeReview);
+ LabelType codeReview = TestLabels.codeReview().toBuilder().setCopyMaxScore(true).build();
+ u.getConfig().upsertLabelType(codeReview);
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java b/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
index b51263e..80cc508 100644
--- a/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/ImplicitMergeCheckIT.java
@@ -85,8 +85,10 @@
private void setRejectImplicitMerges() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
- .getProject()
- .setBooleanConfig(BooleanProjectConfig.REJECT_IMPLICIT_MERGES, InheritableBoolean.TRUE);
+ .updateProject(
+ p ->
+ p.setBooleanConfig(
+ BooleanProjectConfig.REJECT_IMPLICIT_MERGES, InheritableBoolean.TRUE));
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index f9c751f..98b93a8 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -175,7 +175,7 @@
public void readOnlyProjectRejectedBeforeTestingPermissions() throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setState(ProjectState.READ_ONLY);
+ u.getConfig().updateProject(p -> p.setState(ProjectState.READ_ONLY));
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index 840853c..00c7fb8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -174,7 +174,7 @@
public void voteOnBehalfOfLabelNotPermitted() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
LabelType verified = TestLabels.verified();
- u.getConfig().getLabelSections().put(verified.getName(), verified);
+ u.getConfig().upsertLabelType(verified);
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
index 55eeaf4..1027938 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
@@ -277,7 +277,7 @@
}
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setLocalDefaultDashboard(dashboardRef + ":overview");
+ u.getConfig().updateProject(p -> p.setLocalDefaultDashboard(dashboardRef + ":overview"));
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index a8149ce..1532b33 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -240,6 +240,50 @@
assertThat(attentionSet.reason()).isEqualTo("Change was submitted");
}
+ /**
+ * There is currently a bug that adds the person who submitted the change as reviewer, which in
+ * turn adds them to the attention set. This test ensures this doesn't happen.
+ */
+ @Test
+ public void submitDoesNotAddReviewersToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange("refs/heads/master", "file1", "content");
+
+ // Someone else approves, because if admin reviews, they will be added to the reviewers (and the
+ // bug won't be reproduced).
+ requestScopeOperations.setApiUser(accountCreator.admin2().id());
+ change(r).current().review(ReviewInput.approve().addUserToAttentionSet(user.email(), "reason"));
+
+ requestScopeOperations.setApiUser(admin.id());
+
+ change(r).attention(admin.email()).remove(new AttentionSetInput("remove"));
+ change(r).current().submit();
+
+ // Attention set updates that relate to the admin (the person who replied) are filtered out.
+ AttentionSetUpdate attentionSet =
+ Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, admin));
+
+ assertThat(attentionSet.account()).isEqualTo(admin.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("remove");
+
+ change(r).addReviewer(user.email());
+ }
+
+ @Test
+ public void addedReviewersAreAddedToAttentionSetOnMergedChanges() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).current().review(ReviewInput.approve());
+ change(r).current().submit();
+
+ change(r).addReviewer(user.email());
+ AttentionSetUpdate attentionSet =
+ Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, user));
+
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was added");
+ }
+
@Test
public void reviewersAddedAndRemovedFromAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
index 37b1713..542085c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
@@ -245,7 +245,7 @@
LabelType patchSetLock = TestLabels.patchSetLock();
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(patchSetLock.getName(), patchSetLock);
+ u.getConfig().upsertLabelType(patchSetLock);
u.save();
}
projectOperations
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
index 874f07a..10fd65f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
@@ -224,7 +224,7 @@
Project project = projectCache.get(Project.nameKey(newProjectName)).get().getProject();
assertProjectInfo(project, p);
assertThat(project.getDescription()).isEqualTo(in.description);
- assertThat(project.getConfiguredSubmitType()).isEqualTo(in.submitType);
+ assertThat(project.getSubmitType()).isEqualTo(in.submitType);
assertThat(project.getBooleanConfig(BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS))
.isEqualTo(in.useContributorAgreements);
assertThat(project.getBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY))
@@ -368,8 +368,11 @@
@Test
public void createProjectWithCreateProjectCapabilityAndParentNotVisible() throws Exception {
- Project parent = projectCache.get(allProjects).get().getProject();
- parent.setState(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
+ try (ProjectConfigUpdate u = updateProject(allProjects)) {
+ u.getConfig()
+ .updateProject(p -> p.setState(com.google.gerrit.extensions.client.ProjectState.HIDDEN));
+ u.save();
+ }
projectOperations
.allProjectsForUpdate()
.add(
@@ -383,7 +386,12 @@
ProjectInfo p = gApi.projects().create(in).get();
assertThat(p.name).isEqualTo(in.name);
} finally {
- parent.setState(com.google.gerrit.extensions.client.ProjectState.ACTIVE);
+ try (ProjectConfigUpdate u = updateProject(allProjects)) {
+ u.getConfig()
+ .updateProject(
+ p -> p.setState(com.google.gerrit.extensions.client.ProjectState.ACTIVE));
+ u.save();
+ }
projectOperations
.allProjectsForUpdate()
.remove(
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
index 940fae5..3e35f04 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetLabelIT.java
@@ -22,7 +22,6 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.LabelFunction;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.common.LabelDefinitionInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -75,9 +74,7 @@
// set default value
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setDefaultValue((short) 1);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", labelType -> labelType.setDefaultValue((short) 1));
u.save();
}
@@ -100,11 +97,14 @@
// unset rules which are enabled by default
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCanOverride(false);
- labelType.setCopyAllScoresIfNoChange(false);
- labelType.setAllowPostSubmit(false);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig()
+ .updateLabelType(
+ "foo",
+ labelType -> {
+ labelType.setCanOverride(false);
+ labelType.setCopyAllScoresIfNoChange(false);
+ labelType.setAllowPostSubmit(false);
+ });
u.save();
}
@@ -128,16 +128,19 @@
// set rules which are not enabled by default
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAnyScore(true);
- labelType.setCopyMinScore(true);
- labelType.setCopyMaxScore(true);
- labelType.setCopyAllScoresIfNoCodeChange(true);
- labelType.setCopyAllScoresOnTrivialRebase(true);
- labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
- labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
- labelType.setIgnoreSelfApproval(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig()
+ .updateLabelType(
+ "foo",
+ labelType -> {
+ labelType.setCopyAnyScore(true);
+ labelType.setCopyMinScore(true);
+ labelType.setCopyMaxScore(true);
+ labelType.setCopyAllScoresIfNoCodeChange(true);
+ labelType.setCopyAllScoresOnTrivialRebase(true);
+ labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ labelType.setIgnoreSelfApproval(true);
+ });
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
index ef08079..33a0654 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListLabelsIT.java
@@ -27,7 +27,6 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.LabelFunction;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
@@ -88,9 +87,7 @@
// set default value
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setDefaultValue((short) 1);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", labelType -> labelType.setDefaultValue((short) 1));
u.save();
}
@@ -119,11 +116,14 @@
// unset rules which are enabled by default
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCanOverride(false);
- labelType.setCopyAllScoresIfNoChange(false);
- labelType.setAllowPostSubmit(false);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig()
+ .updateLabelType(
+ "foo",
+ labelType -> {
+ labelType.setCanOverride(false);
+ labelType.setCopyAllScoresIfNoChange(false);
+ labelType.setAllowPostSubmit(false);
+ });
u.save();
}
@@ -150,16 +150,19 @@
// set rules which are not enabled by default
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAnyScore(true);
- labelType.setCopyMinScore(true);
- labelType.setCopyMaxScore(true);
- labelType.setCopyAllScoresIfNoCodeChange(true);
- labelType.setCopyAllScoresOnTrivialRebase(true);
- labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
- labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
- labelType.setIgnoreSelfApproval(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig()
+ .updateLabelType(
+ "foo",
+ labelType -> {
+ labelType.setCopyAnyScore(true);
+ labelType.setCopyMinScore(true);
+ labelType.setCopyMaxScore(true);
+ labelType.setCopyAllScoresIfNoCodeChange(true);
+ labelType.setCopyAllScoresOnTrivialRebase(true);
+ labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
+ labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
+ labelType.setIgnoreSelfApproval(true);
+ });
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
index b08c72b..a1817d9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java
@@ -26,7 +26,6 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.LabelFunction;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.common.LabelDefinitionInfo;
@@ -450,9 +449,7 @@
public void setCanOverride() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCanOverride(false);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCanOverride(false));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().canOverride).isNull();
@@ -501,9 +498,7 @@
public void unsetCopyAnyScore() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAnyScore(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyAnyScore(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyAnyScore).isTrue();
@@ -537,9 +532,7 @@
public void unsetCopyMinScore() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyMinScore(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyMinScore(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyMinScore).isTrue();
@@ -573,9 +566,7 @@
public void unsetCopyMaxScore() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyMaxScore(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyMaxScore(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyMaxScore).isTrue();
@@ -594,9 +585,7 @@
public void setCopyAllScoresIfNoChange() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAllScoresIfNoChange(false);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyAllScoresIfNoChange(false));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyAllScoresIfNoChange)
@@ -651,9 +640,7 @@
public void unsetCopyAllScoresIfNoCodeChange() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAllScoresIfNoCodeChange(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyAllScoresIfNoCodeChange(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyAllScoresIfNoCodeChange)
@@ -691,9 +678,7 @@
public void unsetCopyAllScoresOnTrivialRebase() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAllScoresOnTrivialRebase(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyAllScoresOnTrivialRebase(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyAllScoresOnTrivialRebase)
@@ -741,9 +726,7 @@
public void unsetCopyAllScoresOnMergeFirstParentUpdate() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyAllScoresOnMergeFirstParentUpdate(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setCopyAllScoresOnMergeFirstParentUpdate(true));
u.save();
}
assertThat(
@@ -791,9 +774,8 @@
public void unsetCopyValues() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setCopyValues(ImmutableList.of((short) -1, (short) 1));
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig()
+ .updateLabelType("foo", lt -> lt.setCopyValues(ImmutableList.of((short) -1, (short) 1)));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().copyValues).isNotEmpty();
@@ -812,9 +794,7 @@
public void setAllowPostSubmit() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setAllowPostSubmit(false);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setAllowPostSubmit(false));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().allowPostSubmit).isNull();
@@ -863,9 +843,7 @@
public void unsetIgnoreSelfApproval() throws Exception {
configLabel("foo", LabelFunction.NO_OP);
try (ProjectConfigUpdate u = updateProject(project)) {
- LabelType labelType = u.getConfig().getLabelSections().get("foo");
- labelType.setIgnoreSelfApproval(true);
- u.getConfig().getLabelSections().put(labelType.getName(), labelType);
+ u.getConfig().updateLabelType("foo", lt -> lt.setIgnoreSelfApproval(true));
u.save();
}
assertThat(gApi.projects().name(project.get()).label("foo").get().ignoreSelfApproval).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
index 8469fff..17eb534 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
@@ -61,8 +61,8 @@
private void saveLabelConfig() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(label.getName(), label);
- u.getConfig().getLabelSections().put(pLabel.getName(), pLabel);
+ u.getConfig().upsertLabelType(label);
+ u.getConfig().upsertLabelType(pLabel);
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index d74cd71..76514ec 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -1500,7 +1500,7 @@
throws Exception {
for (SubmitType submitType : SubmitType.values()) {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getProject().setSubmitType(submitType);
+ u.getConfig().updateProject(p -> p.setSubmitType(submitType));
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 1d5204b..813a715 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -32,6 +32,7 @@
import static com.google.gerrit.server.project.testing.TestLabels.value;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
@@ -48,39 +49,38 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
-import java.util.Arrays;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class CustomLabelIT extends AbstractDaemonTest {
+ private static final String LABEL_NAME = "CustomLabel";
+ private static final LabelType LABEL =
+ label("CustomLabel", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
+ private static final String P_LABEL_NAME = "CustomLabel2";
+ private static final LabelType P =
+ label("CustomLabel2", value(1, "Positive"), value(0, "No score"));
@Inject private ProjectOperations projectOperations;
@Inject private ExtensionRegistry extensionRegistry;
- private final LabelType label =
- label("CustomLabel", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
-
- private final LabelType P = label("CustomLabel2", value(1, "Positive"), value(0, "No score"));
-
@Before
public void setUp() throws Exception {
projectOperations
.project(project)
.forUpdate()
- .add(allowLabel(label.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(-1, 1))
- .add(allowLabel(P.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(0, 1))
+ .add(allowLabel(LABEL_NAME).ref("refs/heads/*").group(ANONYMOUS_USERS).range(-1, 1))
+ .add(allowLabel(P_LABEL_NAME).ref("refs/heads/*").group(ANONYMOUS_USERS).range(0, 1))
.update();
}
@Test
public void customLabelNoOp_NegativeVoteNotBlock() throws Exception {
- label.setFunction(NO_OP);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(NO_OP));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -91,12 +91,11 @@
@Test
public void customLabelNoBlock_NegativeVoteNotBlock() throws Exception {
- label.setFunction(NO_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(NO_BLOCK));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -107,12 +106,11 @@
@Test
public void customLabelMaxNoBlock_NegativeVoteNotBlock() throws Exception {
- label.setFunction(MAX_NO_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(MAX_NO_BLOCK));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -123,16 +121,14 @@
@Test
public void customLabelMaxNoBlock_MaxVoteSubmittable() throws Exception {
- label.setFunction(MAX_NO_BLOCK);
- P.setFunction(NO_OP);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(MAX_NO_BLOCK), P.toBuilder().setFunction(NO_OP));
PushOneCommit.Result r = createChange();
assertThat(info(r.getChangeId()).submittable).isNull();
- revision(r).review(ReviewInput.approve().label(label.getName(), 1));
+ revision(r).review(ReviewInput.approve().label(LABEL_NAME, 1));
ChangeInfo c = getWithLabels(r);
assertThat(c.submittable).isTrue();
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNotNull();
assertThat(q.recommended).isNull();
@@ -143,12 +139,11 @@
@Test
public void customLabelAnyWithBlock_NegativeVoteBlock() throws Exception {
- label.setFunction(ANY_WITH_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(ANY_WITH_BLOCK));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -170,19 +165,18 @@
public void customLabelAnyWithBlock_Addreviewer_ZeroVote() throws Exception {
TestListener testListener = new TestListener();
try (Registration registration = extensionRegistry.newRegistration().add(testListener)) {
- P.setFunction(ANY_WITH_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(P.toBuilder().setFunction(ANY_WITH_BLOCK));
PushOneCommit.Result r = createChange();
AddReviewerInput in = new AddReviewerInput();
in.reviewer = user.email();
gApi.changes().id(r.getChangeId()).addReviewer(in);
- ReviewInput input = new ReviewInput().label(P.getName(), 0);
+ ReviewInput input = new ReviewInput().label(P_LABEL_NAME, 0);
input.message = "foo";
revision(r).review(input);
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(P.getName());
+ LabelInfo q = c.labels.get(P_LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -196,12 +190,11 @@
@Test
public void customLabelMaxWithBlock_NegativeVoteBlock() throws Exception {
- label.setFunction(MAX_WITH_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(MAX_WITH_BLOCK));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -212,16 +205,15 @@
@Test
public void customLabelMaxWithBlock_MaxVoteSubmittable() throws Exception {
- label.setFunction(MAX_WITH_BLOCK);
- P.setFunction(NO_OP);
- saveLabelConfig();
+ saveLabelConfig(
+ LABEL.toBuilder().setFunction(MAX_WITH_BLOCK), P.toBuilder().setFunction(NO_OP));
PushOneCommit.Result r = createChange();
assertThat(info(r.getChangeId()).submittable).isNull();
- revision(r).review(ReviewInput.approve().label(label.getName(), 1));
+ revision(r).review(ReviewInput.approve().label(LABEL_NAME, 1));
ChangeInfo c = getWithLabels(r);
assertThat(c.submittable).isTrue();
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNotNull();
assertThat(q.recommended).isNull();
@@ -232,13 +224,12 @@
@Test
public void customLabelMaxWithBlock_MaxVoteNegativeVoteBlock() throws Exception {
- label.setFunction(MAX_WITH_BLOCK);
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setFunction(MAX_WITH_BLOCK));
PushOneCommit.Result r = createChange();
- revision(r).review(new ReviewInput().label(label.getName(), 1));
- revision(r).review(new ReviewInput().label(label.getName(), -1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, 1));
+ revision(r).review(new ReviewInput().label(LABEL_NAME, -1));
ChangeInfo c = getWithLabels(r);
- LabelInfo q = c.labels.get(label.getName());
+ LabelInfo q = c.labels.get(LABEL_NAME);
assertThat(q.all).hasSize(1);
assertThat(q.approved).isNull();
assertThat(q.recommended).isNull();
@@ -249,10 +240,9 @@
@Test
public void customLabel_DisallowPostSubmit() throws Exception {
- label.setFunction(NO_OP);
- label.setAllowPostSubmit(false);
- P.setFunction(NO_OP);
- saveLabelConfig();
+ saveLabelConfig(
+ LABEL.toBuilder().setFunction(NO_OP).setAllowPostSubmit(false),
+ P.toBuilder().setFunction(NO_OP));
PushOneCommit.Result r = createChange();
revision(r).review(ReviewInput.approve());
@@ -260,20 +250,20 @@
ChangeInfo info = getWithLabels(r);
assertPermitted(info, "Code-Review", 2);
- assertPermitted(info, P.getName(), 0, 1);
- assertPermitted(info, label.getName());
+ assertPermitted(info, P_LABEL_NAME, 0, 1);
+ assertPermitted(info, LABEL_NAME);
ReviewInput postSubmitReview1 = new ReviewInput();
postSubmitReview1.label(P.getName(), P.getMax().getValue());
revision(r).review(postSubmitReview1);
ReviewInput postSubmitReview2 = new ReviewInput();
- postSubmitReview2.label(label.getName(), label.getMax().getValue());
+ postSubmitReview2.label(LABEL.getName(), LABEL.getMax().getValue());
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> revision(r).review(postSubmitReview2));
assertThat(thrown)
.hasMessageThat()
- .contains("Voting on labels disallowed after submit: " + label.getName());
+ .contains("Voting on labels disallowed after submit: " + LABEL_NAME);
}
@Test
@@ -331,10 +321,9 @@
@Test
public void customLabel_withBranch() throws Exception {
- label.setRefPatterns(Arrays.asList("master"));
- saveLabelConfig();
+ saveLabelConfig(LABEL.toBuilder().setRefPatterns(ImmutableList.of("master")));
ProjectConfig cfg = projectCache.get(project).orElseThrow(illegalState(project)).getConfig();
- assertThat(cfg.getLabelSections().get(label.getName()).getRefPatterns()).contains("master");
+ assertThat(cfg.getLabelSections().get(LABEL_NAME).getRefPatterns()).contains("master");
}
private void assertLabelStatus(String changeId, String testLabel) throws Exception {
@@ -348,10 +337,11 @@
assertThat(labelInfo.blocking).isNull();
}
- private void saveLabelConfig() throws Exception {
+ private void saveLabelConfig(LabelType.Builder... builders) throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getLabelSections().put(label.getName(), label);
- u.getConfig().getLabelSections().put(P.getName(), P);
+ for (LabelType.Builder b : builders) {
+ u.getConfig().upsertLabelType(b.build());
+ }
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index a9114b9..ff26fec 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -50,15 +50,15 @@
@Test
public void newPatchSetsNotifyConfig() throws Exception {
Address addr = Address.create("Watcher", "watcher@example.com");
- NotifyConfig nc = new NotifyConfig();
- nc.addEmail(addr);
+ NotifyConfig.Builder nc = NotifyConfig.builder();
+ nc.addAddress(addr);
nc.setName("new-patch-set");
nc.setHeader(NotifyConfig.Header.CC);
- nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS));
+ nc.setNotify(EnumSet.of(NotifyType.NEW_PATCHSETS));
nc.setFilter("message:sekret");
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().putNotifyConfig("watch", nc);
+ u.getConfig().putNotifyConfig("watch", nc.build());
u.save();
}
@@ -91,14 +91,14 @@
@Test
public void noNotificationForPrivateChangesForWatchersInNotifyConfig() throws Exception {
Address addr = Address.create("Watcher", "watcher@example.com");
- NotifyConfig nc = new NotifyConfig();
- nc.addEmail(addr);
+ NotifyConfig.Builder nc = NotifyConfig.builder();
+ nc.addAddress(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
- nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
+ nc.setNotify(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().putNotifyConfig("team", nc);
+ u.getConfig().putNotifyConfig("team", nc.build());
u.save();
}
@@ -123,14 +123,14 @@
public void noNotificationForChangeThatIsTurnedPrivateForWatchersInNotifyConfig()
throws Exception {
Address addr = Address.create("Watcher", "watcher@example.com");
- NotifyConfig nc = new NotifyConfig();
- nc.addEmail(addr);
+ NotifyConfig.Builder nc = NotifyConfig.builder();
+ nc.addAddress(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
- nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS));
+ nc.setNotify(EnumSet.of(NotifyType.NEW_PATCHSETS));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().putNotifyConfig("team", nc);
+ u.getConfig().putNotifyConfig("team", nc.build());
u.save();
}
@@ -152,14 +152,14 @@
@Test
public void noNotificationForWipChangesForWatchersInNotifyConfig() throws Exception {
Address addr = Address.create("Watcher", "watcher@example.com");
- NotifyConfig nc = new NotifyConfig();
- nc.addEmail(addr);
+ NotifyConfig.Builder nc = NotifyConfig.builder();
+ nc.addAddress(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
- nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
+ nc.setNotify(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().putNotifyConfig("team", nc);
+ u.getConfig().putNotifyConfig("team", nc.build());
u.save();
}
@@ -183,14 +183,14 @@
@Test
public void noNotificationForChangeThatIsTurnedWipForWatchersInNotifyConfig() throws Exception {
Address addr = Address.create("Watcher", "watcher@example.com");
- NotifyConfig nc = new NotifyConfig();
- nc.addEmail(addr);
+ NotifyConfig.Builder nc = NotifyConfig.builder();
+ nc.addAddress(addr);
nc.setName("team");
nc.setHeader(NotifyConfig.Header.TO);
- nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS));
+ nc.setNotify(EnumSet.of(NotifyType.NEW_PATCHSETS));
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().putNotifyConfig("team", nc);
+ u.getConfig().putNotifyConfig("team", nc.build());
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
index 1c820af..90d4e09 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
@@ -89,7 +89,7 @@
if (localLabelSections.isEmpty()) {
localLabelSections.putAll(projectCache.getAllProjects().getConfig().getLabelSections());
}
- localLabelSections.get(labelName).setIgnoreSelfApproval(newState);
+ u.getConfig().updateLabelType(labelName, lt -> lt.setIgnoreSelfApproval(newState));
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
index 65c7b5c..c8899b9 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
@@ -106,7 +106,7 @@
assertThat(cachedProjectConfig1).isNotSameInstanceAs(projectConfig);
assertThat(cachedProjectConfig1.getProject().getDescription()).isEmpty();
assertThat(projectConfig.getProject().getDescription()).isEmpty();
- projectConfig.getProject().setDescription("my fancy project");
+ projectConfig.updateProject(p -> p.setDescription("my fancy project"));
ProjectConfig cachedProjectConfig2 =
projectCache.get(key).orElseThrow(illegalState(project)).getConfig();
diff --git a/javatests/com/google/gerrit/common/data/LabelFunctionTest.java b/javatests/com/google/gerrit/common/data/LabelFunctionTest.java
index 6f5232b..8fea072 100644
--- a/javatests/com/google/gerrit/common/data/LabelFunctionTest.java
+++ b/javatests/com/google/gerrit/common/data/LabelFunctionTest.java
@@ -87,12 +87,12 @@
private static LabelType makeLabel() {
List<LabelValue> values = new ArrayList<>();
// The label text is irrelevant here, only the numerical value is used
- values.add(new LabelValue((short) -2, "Great job, please fix compilation."));
- values.add(new LabelValue((short) -1, "Really good, please make some minor changes."));
- values.add(new LabelValue((short) 0, "No vote."));
- values.add(new LabelValue((short) 1, "Closest thing perfection."));
- values.add(new LabelValue((short) 2, "Perfect!"));
- return new LabelType(LABEL_NAME, values);
+ values.add(LabelValue.create((short) -2, "Great job, please fix compilation."));
+ values.add(LabelValue.create((short) -1, "Really good, please make some minor changes."));
+ values.add(LabelValue.create((short) 0, "No vote."));
+ values.add(LabelValue.create((short) 1, "Closest thing perfection."));
+ values.add(LabelValue.create((short) 2, "Perfect!"));
+ return LabelType.create(LABEL_NAME, values);
}
private static PatchSetApproval makeApproval(int value) {
diff --git a/javatests/com/google/gerrit/common/data/LabelTypeTest.java b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
index 6c3befb..76ea6e1 100644
--- a/javatests/com/google/gerrit/common/data/LabelTypeTest.java
+++ b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
@@ -22,26 +22,26 @@
public class LabelTypeTest {
@Test
public void sortLabelValues() {
- LabelValue v0 = new LabelValue((short) 0, "Zero");
- LabelValue v1 = new LabelValue((short) 1, "One");
- LabelValue v2 = new LabelValue((short) 2, "Two");
- LabelType types = new LabelType("Label", ImmutableList.of(v2, v0, v1));
+ LabelValue v0 = LabelValue.create((short) 0, "Zero");
+ LabelValue v1 = LabelValue.create((short) 1, "One");
+ LabelValue v2 = LabelValue.create((short) 2, "Two");
+ LabelType types = LabelType.create("Label", ImmutableList.of(v2, v0, v1));
assertThat(types.getValues()).containsExactly(v0, v1, v2).inOrder();
}
@Test
public void insertMissingLabelValues() {
- LabelValue v0 = new LabelValue((short) 0, "Zero");
- LabelValue v2 = new LabelValue((short) 2, "Two");
- LabelValue v5 = new LabelValue((short) 5, "Five");
- LabelType types = new LabelType("Label", ImmutableList.of(v2, v5, v0));
+ LabelValue v0 = LabelValue.create((short) 0, "Zero");
+ LabelValue v2 = LabelValue.create((short) 2, "Two");
+ LabelValue v5 = LabelValue.create((short) 5, "Five");
+ LabelType types = LabelType.create("Label", ImmutableList.of(v2, v5, v0));
assertThat(types.getValues())
.containsExactly(
v0,
- new LabelValue((short) 1, ""),
+ LabelValue.create((short) 1, ""),
v2,
- new LabelValue((short) 3, ""),
- new LabelValue((short) 4, ""),
+ LabelValue.create((short) 3, ""),
+ LabelValue.create((short) 4, ""),
v5)
.inOrder();
}
diff --git a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
index c259e60..ba8485b 100644
--- a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
+++ b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
@@ -105,7 +105,7 @@
}
LabelType lt =
label("Verified", value(1, "Verified"), value(0, "No score"), value(-1, "Fails"));
- pc.getLabelSections().put(lt.getName(), lt);
+ pc.upsertLabelType(lt);
save(pc);
}
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index 33446e4..9029301 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -207,7 +207,7 @@
ProjectConfig allProjectsConfig = projectConfigFactory.create(allProjectsName);
allProjectsConfig.load(md);
LabelType cr = TestLabels.codeReview();
- allProjectsConfig.getLabelSections().put(cr.getName(), cr);
+ allProjectsConfig.upsertLabelType(cr);
allProjectsConfig.commit(md);
}
}
@@ -217,7 +217,7 @@
try (MetaDataUpdate md = metaDataUpdateFactory.create(localKey)) {
ProjectConfig newLocal = projectConfigFactory.create(localKey);
newLocal.load(md);
- newLocal.getProject().setParentName(parentKey);
+ newLocal.updateProject(p -> p.setParent(parentKey));
newLocal.commit(md);
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 331bb4c..4104017 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1026,7 +1026,7 @@
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig cfg = projectConfigFactory.create(project);
cfg.load(md);
- cfg.getLabelSections().put(verified.getName(), verified);
+ cfg.upsertLabelType(verified);
cfg.commit(md);
}
projectCache.evict(project);
diff --git a/javatests/com/google/gerrit/server/rules/IgnoreSelfApprovalRuleTest.java b/javatests/com/google/gerrit/server/rules/IgnoreSelfApprovalRuleTest.java
index d8af0e5..85a3207 100644
--- a/javatests/com/google/gerrit/server/rules/IgnoreSelfApprovalRuleTest.java
+++ b/javatests/com/google/gerrit/server/rules/IgnoreSelfApprovalRuleTest.java
@@ -72,12 +72,12 @@
private static LabelType makeLabel(String labelName) {
List<LabelValue> values = new ArrayList<>();
// The label text is irrelevant here, only the numerical value is used
- values.add(new LabelValue((short) -2, "-2"));
- values.add(new LabelValue((short) -1, "-1"));
- values.add(new LabelValue((short) 0, "No vote."));
- values.add(new LabelValue((short) 1, "+1"));
- values.add(new LabelValue((short) 2, "+2"));
- return new LabelType(labelName, values);
+ values.add(LabelValue.create((short) -2, "-2"));
+ values.add(LabelValue.create((short) -1, "-1"));
+ values.add(LabelValue.create((short) 0, "No vote."));
+ values.add(LabelValue.create((short) 1, "+1"));
+ values.add(LabelValue.create((short) 2, "+2"));
+ return LabelType.create(labelName, values);
}
private static PatchSetApproval makeApproval(LabelId labelId, Account.Id accountId, int value) {
diff --git a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java
index eceec8b..9cf4896 100644
--- a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java
+++ b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java
@@ -44,12 +44,12 @@
public class AllProjectsCreatorTest {
private static final LabelType TEST_LABEL =
- new LabelType(
+ LabelType.create(
"Test-Label",
ImmutableList.of(
- new LabelValue((short) 2, "Two"),
- new LabelValue((short) 0, "Zero"),
- new LabelValue((short) 1, "One")));
+ LabelValue.create((short) 2, "Two"),
+ LabelValue.create((short) 0, "Zero"),
+ LabelValue.create((short) 1, "One")));
private static final String TEST_LABEL_STRING =
String.join(
diff --git a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
index 489e1b6..5f71544 100644
--- a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
+++ b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
@@ -81,9 +81,9 @@
@Test
public void oneSuperprojectOneSubmodule() throws Exception {
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
- SubscriptionGraph subscriptionGraph = factory.compute(ImmutableSet.of(SUB_BRANCH));
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
+ SubscriptionGraph subscriptionGraph =
+ factory.compute(ImmutableSet.of(SUB_BRANCH), mergeOpRepoManager);
assertThat(subscriptionGraph.getAffectedSuperProjects()).containsExactly(SUPER_PROJECT);
assertThat(subscriptionGraph.getAffectedSuperBranches(SUPER_PROJECT))
@@ -98,12 +98,12 @@
@Test
public void circularSubscription() throws Exception {
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
setSubscription(SUPER_BRANCH, ImmutableList.of(SUB_BRANCH));
SubmoduleConflictException e =
assertThrows(
- SubmoduleConflictException.class, () -> factory.compute(ImmutableSet.of(SUB_BRANCH)));
+ SubmoduleConflictException.class,
+ () -> factory.compute(ImmutableSet.of(SUB_BRANCH), mergeOpRepoManager));
String expectedErrorMessage =
"Subproject,refs/heads/one->Superproject,refs/heads/one->Subproject,refs/heads/one";
@@ -154,10 +154,9 @@
setSubscription(submoduleBranch2, ImmutableList.of(superBranch1));
setSubscription(submoduleBranch3, ImmutableList.of(superBranch1, superBranch2));
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
SubscriptionGraph subscriptionGraph =
- factory.compute(ImmutableSet.of(submoduleBranch1, submoduleBranch2));
+ factory.compute(ImmutableSet.of(submoduleBranch1, submoduleBranch2), mergeOpRepoManager);
assertThat(subscriptionGraph.getAffectedSuperProjects())
.containsExactly(superProject1, superProject2);
diff --git a/lib/BUILD b/lib/BUILD
index d3ef4b9..0110047 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -160,7 +160,7 @@
name = "args4j",
data = ["//lib:LICENSE-args4j"],
visibility = ["//visibility:public"],
- exports = ["@args4j-intern//jar"],
+ exports = ["@args4j//jar"],
)
java_library(
diff --git a/lib/polymer_externs/BUILD b/lib/polymer_externs/BUILD
deleted file mode 100644
index f07aa2f..0000000
--- a/lib/polymer_externs/BUILD
+++ /dev/null
@@ -1,24 +0,0 @@
-# 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.
-
-load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library")
-
-package(default_visibility = ["//visibility:public"])
-
-closure_js_library(
- name = "polymer_closure",
- srcs = ["@polymer_closure//file"],
- data = ["//lib:LICENSE-Apache2.0"],
- no_closure_library = True,
-)
diff --git a/package.json b/package.json
index 0331079..d051c41 100644
--- a/package.json
+++ b/package.json
@@ -5,6 +5,7 @@
"dependencies": {},
"devDependencies": {
"@bazel/rollup": "^1.6.1",
+ "@bazel/terser": "^1.7.0",
"@bazel/typescript": "^1.6.1",
"eslint": "^6.6.0",
"eslint-config-google": "^0.13.0",
@@ -16,6 +17,7 @@
"gts": "^2.0.2",
"polymer-cli": "^1.9.11",
"prettier": "2.0.5",
+ "terser": "^4.8.0",
"typescript": "3.8.2"
},
"scripts": {
diff --git a/plugins/BUILD b/plugins/BUILD
index a071bde..943471a 100644
--- a/plugins/BUILD
+++ b/plugins/BUILD
@@ -49,6 +49,7 @@
"//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/cache/mem",
"//java/com/google/gerrit/server/cache/serialize",
+ "//java/com/google/gerrit/server/data",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/util/time",
diff --git a/plugins/delete-project b/plugins/delete-project
index 7671def..f420d06 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 7671def07882aab89b19eb7496418588ea7375d9
+Subproject commit f420d06562b97eab26a627baa7722c7f84d95763
diff --git a/plugins/replication b/plugins/replication
index b0ecbd3..ced7fc3 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit b0ecbd3c88fd0d8b19112e33049564e25ac5fc39
+Subproject commit ced7fc318feb76e2fc6d549669c5f5d8d905add5
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
index 95a1554..6bb7f0c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -40,6 +40,7 @@
import {GrDisplayNameUtils} from '../../../scripts/gr-display-name-utils/gr-display-name-utils.js';
import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {appContext} from '../../../services/app-context.js';
const CHANGE_SIZE = {
XS: 10,
@@ -83,6 +84,8 @@
/** @type {?} */
change: Object,
config: Object,
+ /** Name of the section in the change-list. Used for reporting. */
+ sectionName: String,
changeURL: {
type: String,
computed: '_computeChangeURL(change)',
@@ -106,6 +109,11 @@
};
}
+ constructor() {
+ super();
+ this.reporting = appContext.reportingService;
+ }
+
/** @override */
attached() {
super.attached();
@@ -238,7 +246,7 @@
}
if (this._hasAttention(r1) && !this._hasAttention(r2)) return -1;
if (this._hasAttention(r2) && !this._hasAttention(r1)) return 1;
- return r1.name.localeCompare(r2.name);
+ return (r1.name || '').localeCompare(r2.name || '');
});
return reviewers;
}
@@ -298,6 +306,20 @@
detail: {change: this.change, reviewed: newVal},
}));
}
+
+ _handleChangeClick(e) {
+ // Don't prevent the default and neither stop bubbling. We just want to
+ // report the click, but then let the browser handle the click on the link.
+
+ const selfId = (this.account && this.account._account_id) || -1;
+ const ownerId = (this.change && this.change.owner
+ && this.change.owner._account_id) || -1;
+
+ this.reporting.reportInteraction('change-row-clicked', {
+ section: this.sectionName,
+ isOwner: selfId === ownerId,
+ });
+ }
}
customElements.define(GrChangeListItem.is, GrChangeListItem);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
index cc1bf62..48aab4c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
@@ -133,7 +133,11 @@
>
<div class="container">
<div class="content">
- <a title$="[[change.subject]]" href$="[[changeURL]]">
+ <a
+ title$="[[change.subject]]"
+ href$="[[changeURL]]"
+ on-click="_handleChangeClick"
+ >
[[change.subject]]
</a>
</div>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
index 7a66100..e49da7e 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
@@ -183,6 +183,8 @@
checkComputeReviewers(null, [], [], [], []);
checkComputeReviewers(1, [], [], [], []);
checkComputeReviewers(1, [2], ['a'], [], [2]);
+ checkComputeReviewers(1, [2, 3], [undefined, 'a'], [], [2, 3]);
+ checkComputeReviewers(1, [2, 3], ['a', undefined], [], [3, 2]);
checkComputeReviewers(1, [99], ['owner'], [], []);
checkComputeReviewers(
1, [2, 3, 4, 5], ['b', 'a', 'd', 'c'], [3, 4], [3, 4, 2, 5]);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.js
index a18ffd0..623838b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_html.js
@@ -137,6 +137,7 @@
needs-review$="[[_computeItemNeedsReview(account, change, showReviewedState, _config)]]"
change="[[change]]"
config="[[_config]]"
+ section-name="[[changeSection.name]]"
visible-change-table-columns="[[visibleChangeTableColumns]]"
show-number="[[showNumber]]"
show-star="[[showStar]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
index 50c2355..87e100c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
@@ -21,7 +21,6 @@
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {resetPlugins} from '../../../test/test-utils.js';
import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const testHtmlPlugin = document.createElement('dom-module');
@@ -119,8 +118,6 @@
plugin.registerStyleModule('change-metadata', 'my-plugin-style');
}, undefined, 'http://test.com/plugins/style.js');
element = createElement();
- sinon.stub(pluginEndpoints, 'importUrl')
- .callsFake( url => Promise.resolve());
pluginLoader.loadPlugins([]);
pluginLoader.awaitPluginsLoaded().then(() => {
flush(done);
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
index 26baf3c..8ec3121 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
@@ -20,7 +20,6 @@
import './gr-change-metadata.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const basicFixture = fixtureFromElement('gr-change-metadata');
@@ -37,8 +36,6 @@
});
element = basicFixture.instantiate();
- sinon.stub(pluginEndpoints, 'importUrl')
- .callsFake( url => Promise.resolve());
});
test('computed fields', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 072708e..af542e0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -40,7 +40,6 @@
import '../gr-file-list/gr-file-list.js';
import '../gr-included-in-dialog/gr-included-in-dialog.js';
import '../gr-messages-list/gr-messages-list.js';
-import '../gr-messages-list/gr-messages-list-experimental.js';
import '../gr-related-changes-list/gr-related-changes-list.js';
import '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js';
import '../gr-reply-dialog/gr-reply-dialog.js';
@@ -66,7 +65,6 @@
import {PrimaryTab, SecondaryTab} from '../../../constants/constants.js';
import {NO_ROBOT_COMMENTS_THREADS_MSG} from '../../../constants/messages.js';
import {appContext} from '../../../services/app-context.js';
-import {ExperimentIds} from '../../../services/flags.js';
import {ChangeStatus} from '../../../constants/constants.js';
const CHANGE_ID_ERROR = {
@@ -439,6 +437,7 @@
[this.Shortcut.UP_TO_DASHBOARD]: '_handleUpToDashboard',
[this.Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
[this.Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
+ [this.Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_expandAllDiffs',
[this.Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
[this.Shortcut.EDIT_TOPIC]: '_handleEditTopic',
[this.Shortcut.DIFF_AGAINST_BASE]: '_handleDiffAgainstBase',
@@ -453,7 +452,6 @@
constructor() {
super();
- this.flagsService = appContext.flagsService;
this.reporting = appContext.reportingService;
}
@@ -544,14 +542,8 @@
}
}
- _isChangeLogExperimentEnabled() {
- return this.flagsService.isEnabled(ExperimentIds.CLEANER_CHANGELOG);
- }
-
get messagesList() {
- const tagName = this._isChangeLogExperimentEnabled()
- ? 'gr-messages-list-experimental' : 'gr-messages-list';
- return this.shadowRoot.querySelector(tagName);
+ return this.shadowRoot.querySelector('gr-messages-list');
}
get threadList() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
index c9fdb78..5b89c15 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
@@ -213,8 +213,7 @@
--paper-tab-ink: var(--link-color);
}
gr-thread-list,
- gr-messages-list,
- gr-messages-list-experimental {
+ gr-messages-list {
display: block;
}
gr-thread-list {
@@ -701,35 +700,19 @@
is="dom-if"
if="[[_isTabActive(_constants.SecondaryTab.CHANGE_LOG, _activeTabs)]]"
>
- <template is="dom-if" if="[[!_isChangeLogExperimentEnabled()]]">
- <gr-messages-list
- class="hideOnMobileOverlay"
- change-num="[[_changeNum]]"
- labels="[[_change.labels]]"
- messages="[[_change.messages]]"
- reviewer-updates="[[_change.reviewer_updates]]"
- change-comments="[[_changeComments]]"
- project-name="[[_change.project]]"
- show-reply-buttons="[[_loggedIn]]"
- on-message-anchor-tap="_handleMessageAnchorTap"
- on-reply="_handleMessageReply"
- ></gr-messages-list>
- </template>
- <template is="dom-if" if="[[_isChangeLogExperimentEnabled()]]">
- <gr-messages-list-experimental
- class="hideOnMobileOverlay"
- change="[[_change]]"
- change-num="[[_changeNum]]"
- labels="[[_change.labels]]"
- messages="[[_change.messages]]"
- reviewer-updates="[[_change.reviewer_updates]]"
- change-comments="[[_changeComments]]"
- project-name="[[_change.project]]"
- show-reply-buttons="[[_loggedIn]]"
- on-message-anchor-tap="_handleMessageAnchorTap"
- on-reply="_handleMessageReply"
- ></gr-messages-list-experimental>
- </template>
+ <gr-messages-list
+ class="hideOnMobileOverlay"
+ change="[[_change]]"
+ change-num="[[_changeNum]]"
+ labels="[[_change.labels]]"
+ messages="[[_change.messages]]"
+ reviewer-updates="[[_change.reviewer_updates]]"
+ change-comments="[[_changeComments]]"
+ project-name="[[_change.project]]"
+ show-reply-buttons="[[_loggedIn]]"
+ on-message-anchor-tap="_handleMessageAnchorTap"
+ on-reply="_handleMessageReply"
+ ></gr-messages-list>
</template>
</section>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
deleted file mode 100644
index 3554dff..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ /dev/null
@@ -1,108 +0,0 @@
-/**
- * @license
- * Copyright (C) 2015 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.
- */
-/*
- The custom CSS property `--gr-formatted-text-prose-max-width` controls the max
- width of formatted text blocks that are not code.
-*/
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
-import '../../shared/gr-formatted-text/gr-formatted-text.js';
-import '../../../styles/shared-styles.js';
-import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-comment-list_html.js';
-import {BaseUrlBehavior} from '../../../behaviors/base-url-behavior/base-url-behavior.js';
-import {PathListBehavior} from '../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.js';
-import {URLEncodingBehavior} from '../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-/**
- * @extends PolymerElement
- */
-class GrCommentList extends mixinBehaviors( [
- BaseUrlBehavior,
- PathListBehavior,
- URLEncodingBehavior,
-], GestureEventListeners(
- LegacyElementMixin(
- PolymerElement))) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-comment-list'; }
-
- static get properties() {
- return {
- changeNum: Number,
- comments: Object,
- patchNum: Number,
- projectName: String,
- /** @type {?} */
- projectConfig: Object,
- };
- }
-
- _computeFilesFromComments(comments) {
- const arr = Object.keys(comments || {});
- return arr.sort(this.specialFilePathCompare);
- }
-
- _isOnParent(comment) {
- return comment.side === 'PARENT';
- }
-
- _computeDiffURL(filePath, changeNum, allComments) {
- if ([filePath, changeNum, allComments].includes(undefined)) {
- return;
- }
- const fileComments = this._computeCommentsForFile(allComments, filePath);
- // This can happen for files that don't exist anymore in the current ps.
- if (fileComments.length === 0) return;
- return GerritNav.getUrlForDiffById(changeNum, this.projectName,
- filePath, fileComments[0].patch_set);
- }
-
- _computeDiffLineURL(filePath, changeNum, patchNum, comment) {
- const basePatchNum = comment.hasOwnProperty('parent') ?
- -comment.parent : null;
- return GerritNav.getUrlForDiffById(changeNum, this.projectName,
- filePath, patchNum, basePatchNum, comment.line,
- this._isOnParent(comment));
- }
-
- _computeCommentsForFile(comments, filePath) {
- // Changes are not picked up by the dom-repeat due to the array instance
- // identity not changing even when it has elements added/removed from it.
- return (comments[filePath] || []).slice();
- }
-
- _computePatchDisplayName(comment) {
- if (this._isOnParent(comment)) {
- return 'Base, ';
- }
- if (comment.patch_set != this.patchNum) {
- return `PS${comment.patch_set}, `;
- }
- return '';
- }
-}
-
-customElements.define(GrCommentList.is, GrCommentList);
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js
deleted file mode 100644
index 2811828..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js
+++ /dev/null
@@ -1,103 +0,0 @@
-/**
- * @license
- * 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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- :host {
- display: block;
- word-wrap: break-word;
- }
- .file {
- padding: var(--spacing-s) 0;
- }
- .container {
- display: flex;
- padding: var(--spacing-s) 0;
- }
- .lineNum {
- margin-right: var(--spacing-s);
- min-width: 135px;
- text-align: right;
- }
- .patchset-level-comment-text {
- margin-right: var(--spacing-m);
- }
- .message {
- flex: 1;
- --gr-formatted-text-prose-max-width: 80ch;
- }
- @media screen and (max-width: 50em) {
- .container {
- flex-direction: column;
- }
- .lineNum {
- margin-right: 0;
- min-width: initial;
- text-align: left;
- }
- }
- </style>
- <template
- is="dom-repeat"
- items="[[_computeFilesFromComments(comments)]]"
- as="file"
- >
- <div class="file">
- <template is="dom-if" if="[[!shouldHideFile(file)]]">
- <a
- class="fileLink"
- href="[[_computeDiffURL(file, changeNum, comments)]]"
- >[[computeDisplayPath(file)]]
- </a>
- </template>
- </div>
- <template
- is="dom-repeat"
- items="[[_computeCommentsForFile(comments, file)]]"
- as="comment"
- >
- <div class="container">
- <template is="dom-if" if="[[shouldHideFile(file)]]">
- <span class="patchset-level-comment-text">
- Patchset Comment:
- </span>
- </template>
- <template is="dom-if" if="[[!shouldHideFile(file)]]">
- <a
- class="lineNum"
- href$="[[_computeDiffLineURL(file, changeNum, comment.patch_set, comment)]]"
- >
- <span hidden$="[[!comment.line]]">
- <span>[[_computePatchDisplayName(comment)]]</span>
- Line <span>[[comment.line]]</span>
- </span>
- <span hidden$="[[comment.line]]">
- File comment:
- </span>
- </a>
- </template>
- <gr-formatted-text
- class="message"
- no-trailing-margin=""
- content="[[comment.message]]"
- config="[[projectConfig.commentlinks]]"
- ></gr-formatted-text>
- </div>
- </template>
- </template>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js
deleted file mode 100644
index 704d83f..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js
+++ /dev/null
@@ -1,114 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 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.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-comment-list.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-const basicFixture = fixtureFromElement('gr-comment-list');
-
-suite('gr-comment-list tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
-
- sinon.stub(GerritNav, 'mapCommentlinks').callsFake( x => x);
- });
-
- test('_computeFilesFromComments w/ special file path sorting', () => {
- const comments = {
- 'file_b.html': [],
- 'file_c.css': [],
- 'file_a.js': [],
- 'test.cc': [],
- 'test.h': [],
- };
- const expected = [
- 'file_a.js',
- 'file_b.html',
- 'file_c.css',
- 'test.h',
- 'test.cc',
- ];
- const actual = element._computeFilesFromComments(comments);
- assert.deepEqual(actual, expected);
-
- assert.deepEqual(element._computeFilesFromComments(null), []);
- });
-
- test('_computePatchDisplayName', () => {
- const comment = {line: 123, side: 'REVISION', patch_set: 10};
-
- element.patchNum = 10;
- assert.equal(element._computePatchDisplayName(comment), '');
-
- element.patchNum = 9;
- assert.equal(element._computePatchDisplayName(comment), 'PS10, ');
-
- comment.side = 'PARENT';
- assert.equal(element._computePatchDisplayName(comment), 'Base, ');
- });
-
- test('config commentlinks propagate to formatted text', () => {
- element.comments = {
- 'test.h': [{
- author: {name: 'foo'},
- patch_set: 4,
- line: 10,
- updated: '2017-10-30 20:48:40.000000000',
- message: 'Ideadbeefdeadbeef',
- unresolved: true,
- }],
- };
- element.projectConfig = {
- commentlinks: {foo: {link: '#/q/$1', match: '(I[0-9a-f]{8,40})'}},
- };
- flushAsynchronousOperations();
- const formattedText = dom(element.root).querySelector(
- 'gr-formatted-text.message');
- assert.isOk(formattedText.config);
- assert.deepEqual(formattedText.config,
- element.projectConfig.commentlinks);
- });
-
- test('_computeDiffLineURL', () => {
- const getUrlStub = sinon.stub(GerritNav, 'getUrlForDiffById');
- element.projectName = 'proj';
- element.changeNum = 123;
-
- const comment = {line: 456};
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledOnce);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, null, 456, false]);
-
- comment.side = 'PARENT';
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledTwice);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, null, 456, true]);
-
- comment.parent = 12;
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledThrice);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, -12, 456, true]);
- });
-});
-
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 7163952..0da687f 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -24,9 +24,6 @@
import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
import '../../../styles/shared-styles.js';
import '../../../styles/gr-voting-styles.js';
-import '../gr-comment-list/gr-comment-list.js';
-import {appContext} from '../../../services/app-context.js';
-import {ExperimentIds} from '../../../services/flags.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
@@ -136,8 +133,7 @@
},
_commentCountText: {
type: Number,
- computed: '_computeCommentCountText(comments,'
- + ' message.commentThreads.length, _isCleanerLogExperimentEnabled)',
+ computed: '_computeCommentCountText(message.commentThreads.length)',
},
_loggedIn: {
type: Boolean,
@@ -151,7 +147,6 @@
type: Boolean,
value: false,
},
- _isCleanerLogExperimentEnabled: Boolean,
};
}
@@ -163,7 +158,6 @@
constructor() {
super();
- this.flagsService = appContext.flagsService;
}
/** @override */
@@ -176,8 +170,6 @@
/** @override */
ready() {
super.ready();
- this._isCleanerLogExperimentEnabled = this.flagsService
- .isEnabled(ExperimentIds.CLEANER_CHANGELOG);
this.$.restAPI.getConfig().then(config => {
this.config = config;
});
@@ -197,33 +189,13 @@
}
}
- _computeCommentCountText(
- comments, threadsLength, isCleanerLogExperimentEnabled) {
- // TODO(taoalpha): clean up after cleaner-changelog experiment launched
- if (isCleanerLogExperimentEnabled) {
- if (threadsLength === 0) {
- return undefined;
- } else if (threadsLength === 1) {
- return '1 comment';
- } else {
- return `${threadsLength} comments`;
- }
+ _computeCommentCountText(threadsLength) {
+ if (threadsLength === 0) {
+ return undefined;
+ } else if (threadsLength === 1) {
+ return '1 comment';
} else {
- if (!comments) return undefined;
- let count = 0;
- for (const file in comments) {
- if (comments.hasOwnProperty(file)) {
- const commentArray = comments[file] || [];
- count += commentArray.length;
- }
- }
- if (count === 0) {
- return undefined;
- } else if (count === 1) {
- return '1 comment';
- } else {
- return `${count} comments`;
- }
+ return `${threadsLength} comments`;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
index de4a72a..af57782 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
@@ -65,7 +65,6 @@
gr-button {
margin: 0 -4px;
}
- .collapsed gr-comment-list,
.collapsed gr-thread-list,
.collapsed .replyBtn,
.collapsed .deleteBtn,
@@ -246,27 +245,16 @@
</gr-button>
</div>
</template>
- <template is="dom-if" if="[[!_isCleanerLogExperimentEnabled]]">
- <gr-comment-list
- comments="[[comments]]"
- change-num="[[changeNum]]"
- patch-num="[[message._revision_number]]"
- project-name="[[projectName]]"
- project-config="[[_projectConfig]]"
- ></gr-comment-list>
- </template>
- <template is="dom-if" if="[[_isCleanerLogExperimentEnabled]]">
- <gr-thread-list
- change="[[change]]"
- hidden$="[[!message.commentThreads.length]]"
- threads="[[message.commentThreads]]"
- change-num="[[changeNum]]"
- logged-in="[[_loggedIn]]"
- hide-toggle-buttons
- on-thread-list-modified="_onThreadListModified"
- >
- </gr-thread-list>
- </template>
+ <gr-thread-list
+ change="[[change]]"
+ hidden$="[[!message.commentThreads.length]]"
+ threads="[[message.commentThreads]]"
+ change-num="[[changeNum]]"
+ logged-in="[[_loggedIn]]"
+ hide-toggle-buttons
+ on-thread-list-modified="_onThreadListModified"
+ >
+ </gr-thread-list>
</template>
</div>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
deleted file mode 100644
index ee5f0b9..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
+++ /dev/null
@@ -1,433 +0,0 @@
-/**
- * @license
- * 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.
- */
-import '@polymer/paper-toggle-button/paper-toggle-button.js';
-import '../../shared/gr-button/gr-button.js';
-import '../../shared/gr-icons/gr-icons.js';
-import '../gr-message/gr-message.js';
-import '../../../styles/shared-styles.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-messages-list-experimental_html.js';
-import {KeyboardShortcutBehavior} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js';
-import {parseDate} from '../../../utils/date-util.js';
-import {MessageTag} from '../../../constants/constants.js';
-import {appContext} from '../../../services/app-context.js';
-
-/**
- * The content of the enum is also used in the UI for the button text.
- *
- * @enum {string}
- */
-const ExpandAllState = {
- EXPAND_ALL: 'Expand All',
- COLLAPSE_ALL: 'Collapse All',
-};
-
-/**
- * Computes message author's comments for this change message. The backend
- * sets comment.change_message_id for matching, so this computation is fairly
- * straightforward.
- */
-function computeThreads(message, allMessages, changeComments) {
- if ([message, allMessages, changeComments].includes(undefined)) {
- return [];
- }
- if (message._index === undefined) {
- return [];
- }
-
- return changeComments.getAllThreadsForChange().filter(
- thread => thread.comments.map(comment => {
- // collapse all by default
- comment.collapsed = true;
- return comment;
- }).some(comment => {
- const condition = comment.change_message_id === message.id;
- // Since getAllThreadsForChange() always returns a new copy of
- // all comments we can modify them here without worrying about
- // polluting other threads.
- comment.collapsed = !condition;
- return condition;
- })
- );
-}
-
-/**
- * If messages have the same tag, then that influences grouping and whether
- * a message is initally hidden or not, see isImportant(). So we are applying
- * some "magic" rules here in order to hide exactly the right messages.
- *
- * 1. If a message does not have a tag, but is associated with robot comments,
- * then it gets a tag.
- *
- * 2. Use the same tag for some of Gerrit's standard events, if they should be
- * considered one group, e.g. normal and wip patchset uploads.
- *
- * 3. Everything beyond the ~ character is cut off from the tag. That gives
- * tools control over which messages will be hidden.
- */
-function computeTag(message) {
- if (!message.tag) {
- const threads = message.commentThreads || [];
- const comments = threads.map(
- t => t.comments.find(c => c.change_message_id === message.id));
- const isRobot = comments.some(c => c && !!c.robot_id);
- return isRobot ? 'autogenerated:has-robot-comments' : undefined;
- }
-
- if (message.tag === MessageTag.TAG_NEW_WIP_PATCHSET) {
- return MessageTag.TAG_NEW_PATCHSET;
- }
- if (message.tag === MessageTag.TAG_UNSET_ASSIGNEE) {
- return MessageTag.TAG_SET_ASSIGNEE;
- }
- if (message.tag === MessageTag.TAG_UNSET_PRIVATE) {
- return MessageTag.TAG_SET_PRIVATE;
- }
- if (message.tag === MessageTag.TAG_SET_WIP) {
- return MessageTag.TAG_SET_READY;
- }
-
- return message.tag.replace(/~.*/, '');
-}
-
-/**
- * Try to set a revision number that makes sense, if none is set. Just copy
- * over the revision number of the next older message. This is mostly relevant
- * for reviewer updates. Other messages should typically have the revision
- * number already set.
- */
-function computeRevision(message, allMessages) {
- if (message._revision_number > 0) return message._revision_number;
- let revision = 0;
- for (const m of allMessages) {
- if (m.date > message.date) break;
- if (m._revision_number > revision) revision = m._revision_number;
- }
- return revision > 0 ? revision : undefined;
-}
-
-/**
- * Unimportant messages are initially hidden.
- *
- * Human messages are always important. They have an undefined tag.
- *
- * Autogenerated messages are unimportant, if there is a message with the same
- * tag and a higher revision number.
- */
-function computeIsImportant(message, allMessages) {
- if (!message.tag) return true;
-
- const hasSameTag = m => m.tag === message.tag;
- const revNumber = message._revision_number || 0;
- const hasHigherRevisionNumber = m => m._revision_number > revNumber;
- return !allMessages.filter(hasSameTag).some(hasHigherRevisionNumber);
-}
-
-export const TEST_ONLY = {
- computeThreads,
- computeTag,
- computeRevision,
- computeIsImportant,
-};
-
-/**
- * @extends PolymerElement
- */
-class GrMessagesListExperimental extends mixinBehaviors( [
- KeyboardShortcutBehavior,
-], GestureEventListeners(
- LegacyElementMixin(
- PolymerElement))) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-messages-list-experimental'; }
-
- static get properties() {
- return {
- /** @type {?} */
- change: Object,
- changeNum: Number,
- /**
- * These are just the change messages. They are combined with reviewer
- * updates below. So _combinedMessages is the more important property.
- */
- messages: {
- type: Array,
- value() { return []; },
- },
- /**
- * These are just the reviewer updates. They are combined with change
- * messages above. So _combinedMessages is the more important property.
- */
- reviewerUpdates: {
- type: Array,
- value() { return []; },
- },
- changeComments: Object,
- projectName: String,
- showReplyButtons: {
- type: Boolean,
- value: false,
- },
- labels: Object,
-
- /**
- * Keeps track of the state of the "Expand All" toggle button. Note that
- * you can individually expand/collapse some messages without affecting
- * the toggle button's state.
- *
- * @type {ExpandAllState}
- */
- _expandAllState: {
- type: String,
- value: ExpandAllState.EXPAND_ALL,
- },
- _expandAllTitle: {
- type: String,
- computed: '_computeExpandAllTitle(_expandAllState)',
- },
-
- _showAllActivity: {
- type: Boolean,
- value: false,
- observer: '_observeShowAllActivity',
- },
- /**
- * The merged array of change messages and reviewer updates.
- */
- _combinedMessages: {
- type: Array,
- computed: '_computeCombinedMessages(messages, reviewerUpdates, '
- + 'changeComments)',
- observer: '_combinedMessagesChanged',
- },
-
- _labelExtremes: {
- type: Object,
- computed: '_computeLabelExtremes(labels.*)',
- },
- };
- }
-
- constructor() {
- super();
- this.reporting = appContext.reportingService;
- }
-
- scrollToMessage(messageID) {
- const selector = `[data-message-id="${messageID}"]`;
- const el = this.shadowRoot.querySelector(selector);
-
- if (!el && this._showAllActivity) {
- console.warn(`Failed to scroll to message: ${messageID}`);
- return;
- }
- if (!el) {
- this._showAllActivity = true;
- setTimeout(() => this.scrollToMessage(messageID));
- return;
- }
-
- el.set('message.expanded', true);
- let top = el.offsetTop;
- for (let offsetParent = el.offsetParent;
- offsetParent;
- offsetParent = offsetParent.offsetParent) {
- top += offsetParent.offsetTop;
- }
- window.scrollTo(0, top);
- this._highlightEl(el);
- }
-
- _observeShowAllActivity(showAllActivity) {
- // We have to call render() such that the dom-repeat filter picks up the
- // change.
- this.$.messageRepeat.render();
- }
-
- /**
- * Filter for the dom-repeat of combinedMessages.
- */
- _isMessageVisible(message) {
- return this._showAllActivity || message.isImportant;
- }
-
- /**
- * Merges change messages and reviewer updates into one array. Also processes
- * all messages and updates, aligns or massages some of the properties.
- */
- _computeCombinedMessages(messages, reviewerUpdates, changeComments) {
- const params = [messages, reviewerUpdates, changeComments];
- if (params.some(o => o === undefined)) return [];
-
- let mi = 0;
- let ri = 0;
- let combinedMessages = [];
- let mDate;
- let rDate;
- for (let i = 0; i < messages.length; i++) {
- messages[i]._index = i;
- }
-
- while (mi < messages.length || ri < reviewerUpdates.length) {
- if (mi >= messages.length) {
- combinedMessages = combinedMessages.concat(reviewerUpdates.slice(ri));
- break;
- }
- if (ri >= reviewerUpdates.length) {
- combinedMessages = combinedMessages.concat(messages.slice(mi));
- break;
- }
- mDate = mDate || parseDate(messages[mi].date);
- rDate = rDate || parseDate(reviewerUpdates[ri].date);
- if (rDate < mDate) {
- combinedMessages.push(reviewerUpdates[ri++]);
- rDate = null;
- } else {
- combinedMessages.push(messages[mi++]);
- mDate = null;
- }
- }
- combinedMessages.forEach(m => {
- if (m.expanded === undefined) {
- m.expanded = false;
- }
- m.commentThreads = computeThreads(m, combinedMessages, changeComments);
- m._revision_number = computeRevision(m, combinedMessages);
- m.tag = computeTag(m);
- });
- // computeIsImportant() depends on tags and revision numbers already being
- // updated for all messages, so we have to compute this in its own forEach
- // loop.
- combinedMessages.forEach(m => {
- m.isImportant = computeIsImportant(m, combinedMessages);
- });
- return combinedMessages;
- }
-
- _updateExpandedStateOfAllMessages(exp) {
- if (this._combinedMessages) {
- for (let i = 0; i < this._combinedMessages.length; i++) {
- this._combinedMessages[i].expanded = exp;
- this.notifyPath(`_combinedMessages.${i}.expanded`);
- }
- }
- }
-
- _computeExpandAllTitle(_expandAllState) {
- if (_expandAllState === ExpandAllState.COLLAPSED_ALL) {
- return this.createTitle(
- this.Shortcut.COLLAPSE_ALL_MESSAGES, this.ShortcutSection.ACTIONS);
- }
- if (_expandAllState === ExpandAllState.EXPAND_ALL) {
- return this.createTitle(
- this.Shortcut.EXPAND_ALL_MESSAGES, this.ShortcutSection.ACTIONS);
- }
- return '';
- }
-
- _highlightEl(el) {
- const highlightedEls =
- dom(this.root).querySelectorAll('.highlighted');
- for (const highlightedEl of highlightedEls) {
- highlightedEl.classList.remove('highlighted');
- }
- function handleAnimationEnd() {
- el.removeEventListener('animationend', handleAnimationEnd);
- el.classList.remove('highlighted');
- }
- el.addEventListener('animationend', handleAnimationEnd);
- el.classList.add('highlighted');
- }
-
- /**
- * @param {boolean} expand
- */
- handleExpandCollapse(expand) {
- this._expandAllState = expand ? ExpandAllState.COLLAPSE_ALL
- : ExpandAllState.EXPAND_ALL;
- this._updateExpandedStateOfAllMessages(expand);
- }
-
- _handleExpandCollapseTap(e) {
- e.preventDefault();
- this.handleExpandCollapse(
- this._expandAllState === ExpandAllState.EXPAND_ALL);
- }
-
- _handleAnchorClick(e) {
- this.scrollToMessage(e.detail.id);
- }
-
- _isVisibleShowAllActivityToggle(messages = []) {
- return messages.some(m => !m.isImportant);
- }
-
- _computeHiddenEntriesCount(messages = []) {
- return messages.filter(m => !m.isImportant).length;
- }
-
- /**
- * This method is for reporting stats only.
- */
- _combinedMessagesChanged(combinedMessages) {
- if (combinedMessages) {
- if (combinedMessages.length === 0) return;
- const tags = combinedMessages.map(
- message => message.tag || message.type ||
- (message.comments ? 'comments' : 'none'));
- const tagsCounted = tags.reduce((acc, val) => {
- acc[val] = (acc[val] || 0) + 1;
- return acc;
- }, {all: combinedMessages.length});
- this.reporting.reportInteraction('messages-count', tagsCounted);
- }
- }
-
- /**
- * Compute a mapping from label name to objects representing the minimum and
- * maximum possible values for that label.
- */
- _computeLabelExtremes(labelRecord) {
- const extremes = {};
- const labels = labelRecord.base;
- if (!labels) { return extremes; }
- for (const key of Object.keys(labels)) {
- if (!labels[key] || !labels[key].values) { continue; }
- const values = Object.keys(labels[key].values)
- .map(v => parseInt(v, 10));
- values.sort((a, b) => a - b);
- if (!values.length) { continue; }
- extremes[key] = {min: values[0], max: values[values.length - 1]};
- }
- return extremes;
- }
-
- /**
- * Work around a issue on iOS when clicking turns into double tap
- */
- _onTapShowAllActivityToggle(e) {
- e.preventDefault();
- }
-}
-
-customElements.define(GrMessagesListExperimental.is,
- GrMessagesListExperimental);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
deleted file mode 100644
index 212de59..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
+++ /dev/null
@@ -1,115 +0,0 @@
-/**
- * @license
- * 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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- :host {
- display: flex;
- justify-content: space-between;
- }
- .header {
- align-items: center;
- border-top: 1px solid var(--border-color);
- border-bottom: 1px solid var(--border-color);
- display: flex;
- justify-content: space-between;
- padding: var(--spacing-s) var(--spacing-l);
- }
- .highlighted {
- animation: 3s fadeOut;
- }
- @keyframes fadeOut {
- 0% {
- background-color: var(--emphasis-color);
- }
- 100% {
- background-color: var(--view-background-color);
- }
- }
- .container {
- align-items: center;
- display: flex;
- }
- .hiddenEntries {
- color: var(--deemphasized-text-color);
- }
- gr-message:not(:last-of-type) {
- border-bottom: 1px solid var(--border-color);
- }
- gr-message {
- background-color: var(--background-color-secondary);
- }
- .experimentMessage {
- padding: var(--spacing-s) var(--spacing-m);
- background-color: var(--emphasis-color);
- border-radius: var(--border-radius);
- }
- .experimentMessage iron-icon {
- vertical-align: top;
- }
- </style>
- <div class="header">
- <div id="showAllActivityToggleContainer" class="container">
- <template
- is="dom-if"
- if="[[_isVisibleShowAllActivityToggle(_combinedMessages)]]"
- >
- <paper-toggle-button
- class="showAllActivityToggle"
- checked="{{_showAllActivity}}"
- aria-labelledby="showAllEntriesLabel"
- role="switch"
- on-tap="_onTapShowAllActivityToggle"
- ></paper-toggle-button>
- <div id="showAllEntriesLabel">
- <span>Show all entries</span>
- <span class="hiddenEntries" hidden$="[[_showAllActivity]]">
- ([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
- </span>
- </div>
- <span class="transparent separator"></span>
- </template>
- </div>
- <gr-button
- id="collapse-messages"
- link=""
- title="[[_expandAllTitle]]"
- on-click="_handleExpandCollapseTap"
- >
- [[_expandAllState]]
- </gr-button>
- </div>
- <template
- id="messageRepeat"
- is="dom-repeat"
- items="[[_combinedMessages]]"
- as="message"
- filter="_isMessageVisible"
- >
- <gr-message
- change="[[change]]"
- change-num="[[changeNum]]"
- message="[[message]]"
- project-name="[[projectName]]"
- show-reply-button="[[showReplyButtons]]"
- on-message-anchor-tap="_handleAnchorClick"
- label-extremes="[[_labelExtremes]]"
- data-message-id$="[[message.id]]"
- ></gr-message>
- </template>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js
deleted file mode 100644
index 1a0969f..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js
+++ /dev/null
@@ -1,544 +0,0 @@
-/**
- * @license
- * 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.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import '../../diff/gr-comment-api/gr-comment-api.js';
-import './gr-messages-list-experimental.js';
-import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {TEST_ONLY} from './gr-messages-list-experimental.js';
-import {MessageTag} from '../../../constants/constants.js';
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-createCommentApiMockWithTemplateElement(
- 'gr-messages-list-experimental-comment-mock-api', html`
- <gr-messages-list-experimental
- id="messagesList"
- change-comments="[[_changeComments]]"></gr-messages-list-experimental>
- <gr-comment-api id="commentAPI"></gr-comment-api>
-`);
-
-const basicFixture = fixtureFromTemplate(html`
-<gr-messages-list-experimental-comment-mock-api>
- <gr-messages-list-experimental></gr-messages-list-experimental>
-</gr-messages-list-experimental-comment-mock-api>
-`);
-
-const randomMessage = function(opt_params) {
- const params = opt_params || {};
- const author1 = {
- _account_id: 1115495,
- name: 'Andrew Bonventre',
- email: 'andybons@chromium.org',
- };
- return {
- id: params.id || Math.random().toString(),
- date: params.date || '2016-01-12 20:28:33.038000',
- message: params.message || Math.random().toString(),
- _revision_number: params._revision_number || 1,
- author: params.author || author1,
- tag: params.tag,
- };
-};
-
-function generateRandomMessages(count) {
- return new Array(count).fill()
- .map(() => randomMessage());
-}
-
-suite('gr-messages-list-experimental tests', () => {
- let element;
- let messages;
-
- let commentApiWrapper;
-
- const getMessages = function() {
- return dom(element.root).querySelectorAll('gr-message');
- };
-
- const MESSAGE_ID_0 = '1234ccc949c6d482b061be6a28e10782abf0e7af';
- const MESSAGE_ID_1 = '8c19ccc949c6d482b061be6a28e10782abf0e7af';
- const MESSAGE_ID_2 = 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5';
-
- const author = {
- _account_id: 42,
- name: 'Marvin the Paranoid Android',
- email: 'marvin@sirius.org',
- };
-
- const createComment = function() {
- return {
- id: '1a2b3c4d',
- message: 'some random test text',
- change_message_id: '8a7b6c5d',
- updated: '2016-01-01 01:02:03.000000000',
- line: 1,
- patch_set: 1,
- author,
- };
- };
-
- const comments = {
- file1: [
- {
- ...createComment(),
- change_message_id: MESSAGE_ID_0,
- in_reply_to: '6505d749_f0bec0aa',
- author: {
- email: 'some@email.com',
- _account_id: 123,
- },
- },
- {
- ...createComment(),
- id: '2b3c4d5e',
- change_message_id: MESSAGE_ID_1,
- in_reply_to: 'c5912363_6b820105',
- },
- {
- ...createComment(),
- id: '2b3c4d5e',
- change_message_id: MESSAGE_ID_1,
- in_reply_to: '6505d749_f0bec0aa',
- },
- {
- ...createComment(),
- id: '34ed05d749_10ed44b2',
- change_message_id: MESSAGE_ID_2,
- },
- ],
- file2: [
- {
- ...createComment(),
- change_message_id: MESSAGE_ID_1,
- in_reply_to: 'c5912363_4b7d450a',
- id: '450a935e_4f260d25',
- },
- ],
- };
-
- suite('basic tests', () => {
- setup(() => {
- stub('gr-rest-api-interface', {
- getConfig() { return Promise.resolve({}); },
- getLoggedIn() { return Promise.resolve(false); },
- getDiffComments() { return Promise.resolve(comments); },
- getDiffRobotComments() { return Promise.resolve({}); },
- getDiffDrafts() { return Promise.resolve({}); },
- });
-
- messages = generateRandomMessages(3);
- // Element must be wrapped in an element with direct access to the
- // comment API.
- commentApiWrapper = basicFixture.instantiate();
- element = commentApiWrapper.$.messagesList;
- element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
- });
-
- test('expand/collapse all', () => {
- let allMessageEls = getMessages();
- for (const message of allMessageEls) {
- message._expanded = false;
- }
- MockInteractions.tap(allMessageEls[1]);
- assert.isTrue(allMessageEls[1]._expanded);
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages'));
- allMessageEls = getMessages();
- for (const message of allMessageEls) {
- assert.isTrue(message._expanded);
- }
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages'));
- allMessageEls = getMessages();
- for (const message of allMessageEls) {
- assert.isFalse(message._expanded);
- }
- });
-
- test('expand/collapse from external keypress', () => {
- // Start with one expanded message. -> not all collapsed
- element.scrollToMessage(messages[1].id);
- assert.isFalse([...getMessages()].filter(m => m._expanded).length == 0);
-
- // Press 'z' -> all collapsed
- element.handleExpandCollapse(false);
- assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
-
- // Press 'x' -> all expanded
- element.handleExpandCollapse(true);
- assert.isTrue([...getMessages()].filter(m => !m._expanded).length == 0);
-
- // Press 'z' -> all collapsed
- element.handleExpandCollapse(false);
- assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
- });
-
- test('showAllActivity does not appear when all msgs are important', () => {
- assert.isOk(element.shadowRoot
- .querySelector('#showAllActivityToggleContainer'));
- assert.isNotOk(element.shadowRoot
- .querySelector('.showAllActivityToggle'));
- });
-
- test('scroll to message', () => {
- const allMessageEls = getMessages();
- for (const message of allMessageEls) {
- message.set('message.expanded', false);
- }
-
- const scrollToStub = sinon.stub(window, 'scrollTo');
- const highlightStub = sinon.stub(element, '_highlightEl');
-
- element.scrollToMessage('invalid');
-
- for (const message of allMessageEls) {
- assert.isFalse(message._expanded,
- 'expected gr-message to not be expanded');
- }
-
- const messageID = messages[1].id;
- element.scrollToMessage(messageID);
- assert.isTrue(
- element.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]')
- ._expanded);
-
- assert.isTrue(scrollToStub.calledOnce);
- assert.isTrue(highlightStub.calledOnce);
- });
-
- test('scroll to message offscreen', () => {
- const scrollToStub = sinon.stub(window, 'scrollTo');
- const highlightStub = sinon.stub(element, '_highlightEl');
- element.messages = generateRandomMessages(25);
- flushAsynchronousOperations();
- assert.isFalse(scrollToStub.called);
- assert.isFalse(highlightStub.called);
-
- const messageID = element.messages[1].id;
- element.scrollToMessage(messageID);
- assert.isTrue(scrollToStub.calledOnce);
- assert.isTrue(highlightStub.calledOnce);
- assert.isTrue(
- element.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]')
- ._expanded);
- });
-
- test('associating messages with comments', () => {
- const messages = [].concat(
- randomMessage(),
- {
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- },
- {
- _index: 6,
- _revision_number: 4,
- message: 'Patch Set 4:\n\n(6 comments)',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5',
- }
- );
- element.messages = messages;
- flushAsynchronousOperations();
- const messageElements = getMessages();
- assert.equal(messageElements.length, messages.length);
- assert.deepEqual(messageElements[1].message, messages[1]);
- assert.deepEqual(messageElements[2].message, messages[2]);
- });
-
- test('threads', () => {
- const messages = [
- {
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- },
- ];
- element.messages = messages;
- flushAsynchronousOperations();
- const messageElements = getMessages();
- // threads
- assert.equal(
- messageElements[0].message.commentThreads.length,
- 3);
- // first thread contains 1 comment
- assert.equal(
- messageElements[0].message.commentThreads[0].comments.length,
- 1);
- });
-
- test('updateTag human message', () => {
- const m = randomMessage();
- assert.equal(TEST_ONLY.computeTag(m), undefined);
- });
-
- test('updateTag nothing to change', () => {
- const m = randomMessage();
- const tag = 'something-normal';
- m.tag = tag;
- assert.equal(TEST_ONLY.computeTag(m), tag);
- });
-
- test('updateTag TAG_NEW_WIP_PATCHSET', () => {
- const m = randomMessage();
- m.tag = MessageTag.TAG_NEW_WIP_PATCHSET;
- assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
- });
-
- test('updateTag remove postfix', () => {
- const m = randomMessage();
- m.tag = 'something~withpostfix';
- assert.equal(TEST_ONLY.computeTag(m), 'something');
- });
-
- test('updateTag with robot comments', () => {
- const m = randomMessage();
- m.commentThreads = [{
- comments: [{
- robot_id: 'id314',
- change_message_id: m.id,
- }],
- }];
- assert.notEqual(TEST_ONLY.computeTag(m), undefined);
- });
-
- test('setRevisionNumber nothing to change', () => {
- const m1 = randomMessage();
- const m2 = randomMessage();
- assert.equal(TEST_ONLY.computeRevision(m1, [m1, m2]), 1);
- assert.equal(TEST_ONLY.computeRevision(m2, [m1, m2]), 1);
- });
-
- test('setRevisionNumber reviewer updates', () => {
- const m1 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-01 10:00:00.000000000',
- });
- m1._revision_number = undefined;
- const m2 = randomMessage(
- {
- date: '2020-01-02 10:00:00.000000000',
- });
- m2._revision_number = 1;
- const m3 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-03 10:00:00.000000000',
- });
- m3._revision_number = undefined;
- const m4 = randomMessage(
- {
- date: '2020-01-04 10:00:00.000000000',
- });
- m4._revision_number = 2;
- const m5 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-05 10:00:00.000000000',
- });
- m5._revision_number = undefined;
- const allMessages = [m1, m2, m3, m4, m5];
- assert.equal(TEST_ONLY.computeRevision(m1, allMessages), undefined);
- assert.equal(TEST_ONLY.computeRevision(m2, allMessages), 1);
- assert.equal(TEST_ONLY.computeRevision(m3, allMessages), 1);
- assert.equal(TEST_ONLY.computeRevision(m4, allMessages), 2);
- assert.equal(TEST_ONLY.computeRevision(m5, allMessages), 2);
- });
-
- test('isImportant human message', () => {
- const m = randomMessage();
- assert.isTrue(TEST_ONLY.computeIsImportant(m, []));
- assert.isTrue(TEST_ONLY.computeIsImportant(m, [m]));
- });
-
- test('isImportant even with a tag', () => {
- const m1 = randomMessage();
- const m2 = randomMessage({tag: 'autogenerated:gerrit1'});
- const m3 = randomMessage({tag: 'autogenerated:gerrit2'});
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, []));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
- });
-
- test('isImportant filters same tag and older revision', () => {
- const m1 = randomMessage({tag: 'auto', _revision_number: 2});
- const m2 = randomMessage({tag: 'auto', _revision_number: 1});
- const m3 = randomMessage({tag: 'auto'});
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m2]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
- });
-
- test('isImportant is evaluated after tag update', () => {
- const m1 = randomMessage(
- {tag: MessageTag.TAG_NEW_PATCHSET, _revision_number: 1});
- const m2 = randomMessage(
- {tag: MessageTag.TAG_NEW_WIP_PATCHSET, _revision_number: 2});
- element.messages = [m1, m2];
- flushAsynchronousOperations();
- assert.isFalse(m1.isImportant);
- assert.isTrue(m2.isImportant);
- });
-
- test('messages without author do not throw', () => {
- const messages = [{
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- }];
- element.messages = messages;
- flushAsynchronousOperations();
- const messageEls = getMessages();
- assert.equal(messageEls.length, 1);
- assert.equal(messageEls[0].message.message, messages[0].message);
- });
- });
-
- suite('gr-messages-list-experimental automate tests', () => {
- let element;
- let messages;
-
- let commentApiWrapper;
-
- setup(() => {
- stub('gr-rest-api-interface', {
- getConfig() { return Promise.resolve({}); },
- getLoggedIn() { return Promise.resolve(false); },
- getDiffComments() { return Promise.resolve({}); },
- getDiffRobotComments() { return Promise.resolve({}); },
- getDiffDrafts() { return Promise.resolve({}); },
- });
-
- messages = [
- randomMessage(),
- randomMessage({tag: 'auto', _revision_number: 2}),
- randomMessage({tag: 'auto', _revision_number: 3}),
- ];
-
- // Element must be wrapped in an element with direct access to the
- // comment API.
- commentApiWrapper = basicFixture.instantiate();
- element = commentApiWrapper.$.messagesList;
- sinon.spy(commentApiWrapper.$.commentAPI, 'loadAll');
- element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
- });
-
- test('hide autogenerated button is not hidden', () => {
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- });
-
- test('one unimportant message is hidden initially', () => {
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 2);
- });
-
- test('unimportant messages hidden after toggle', () => {
- element._showAllActivity = true;
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- MockInteractions.tap(toggle);
- flushAsynchronousOperations();
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 2);
- });
-
- test('unimportant messages shown after toggle', () => {
- element._showAllActivity = false;
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- MockInteractions.tap(toggle);
- flushAsynchronousOperations();
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 3);
- });
-
- test('_computeLabelExtremes', () => {
- const computeSpy = sinon.spy(element, '_computeLabelExtremes');
-
- element.labels = null;
- assert.isTrue(computeSpy.calledOnce);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {};
- assert.isTrue(computeSpy.calledTwice);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {}};
- assert.isTrue(computeSpy.calledThrice);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {values: {}}};
- assert.equal(computeSpy.callCount, 4);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {values: {'-12': {}}}};
- assert.equal(computeSpy.callCount, 5);
- assert.deepEqual(computeSpy.lastCall.returnValue,
- {'my-label': {min: -12, max: -12}});
-
- element.labels = {
- 'my-label': {values: {'-2': {}, '-1': {}, '0': {}, '+1': {}, '+2': {}}},
- };
- assert.equal(computeSpy.callCount, 6);
- assert.deepEqual(computeSpy.lastCall.returnValue,
- {'my-label': {min: -2, max: 2}});
-
- element.labels = {
- 'my-label': {values: {'-12': {}}},
- 'other-label': {values: {'-1': {}, ' 0': {}, '+1': {}}},
- };
- assert.equal(computeSpy.callCount, 7);
- assert.deepEqual(computeSpy.lastCall.returnValue, {
- 'my-label': {min: -12, max: -12},
- 'other-label': {min: -1, max: 1},
- });
- });
- });
-});
-
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index 30482e2..cd61396 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -1,6 +1,6 @@
/**
* @license
- * Copyright (C) 2015 The Android Open Source Project
+ * 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.
@@ -16,9 +16,10 @@
*/
import '@polymer/paper-toggle-button/paper-toggle-button.js';
import '../../shared/gr-button/gr-button.js';
+import '../../shared/gr-icons/gr-icons.js';
import '../gr-message/gr-message.js';
import '../../../styles/shared-styles.js';
-import {flush, dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
+import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
@@ -26,16 +27,9 @@
import {htmlTemplate} from './gr-messages-list_html.js';
import {KeyboardShortcutBehavior} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js';
import {parseDate} from '../../../utils/date-util.js';
+import {MessageTag} from '../../../constants/constants.js';
import {appContext} from '../../../services/app-context.js';
-const MAX_INITIAL_SHOWN_MESSAGES = 20;
-const MESSAGES_INCREMENT = 5;
-
-const ReportingEvent = {
- SHOW_ALL: 'show-all-messages',
- SHOW_MORE: 'show-more-messages',
-};
-
/**
* The content of the enum is also used in the UI for the button text.
*
@@ -47,6 +41,114 @@
};
/**
+ * Computes message author's comments for this change message. The backend
+ * sets comment.change_message_id for matching, so this computation is fairly
+ * straightforward.
+ */
+function computeThreads(message, allMessages, changeComments) {
+ if ([message, allMessages, changeComments].includes(undefined)) {
+ return [];
+ }
+ if (message._index === undefined) {
+ return [];
+ }
+
+ return changeComments.getAllThreadsForChange().filter(
+ thread => thread.comments.map(comment => {
+ // collapse all by default
+ comment.collapsed = true;
+ return comment;
+ }).some(comment => {
+ const condition = comment.change_message_id === message.id;
+ // Since getAllThreadsForChange() always returns a new copy of
+ // all comments we can modify them here without worrying about
+ // polluting other threads.
+ comment.collapsed = !condition;
+ return condition;
+ })
+ );
+}
+
+/**
+ * If messages have the same tag, then that influences grouping and whether
+ * a message is initally hidden or not, see isImportant(). So we are applying
+ * some "magic" rules here in order to hide exactly the right messages.
+ *
+ * 1. If a message does not have a tag, but is associated with robot comments,
+ * then it gets a tag.
+ *
+ * 2. Use the same tag for some of Gerrit's standard events, if they should be
+ * considered one group, e.g. normal and wip patchset uploads.
+ *
+ * 3. Everything beyond the ~ character is cut off from the tag. That gives
+ * tools control over which messages will be hidden.
+ */
+function computeTag(message) {
+ if (!message.tag) {
+ const threads = message.commentThreads || [];
+ const comments = threads.map(
+ t => t.comments.find(c => c.change_message_id === message.id));
+ const isRobot = comments.some(c => c && !!c.robot_id);
+ return isRobot ? 'autogenerated:has-robot-comments' : undefined;
+ }
+
+ if (message.tag === MessageTag.TAG_NEW_WIP_PATCHSET) {
+ return MessageTag.TAG_NEW_PATCHSET;
+ }
+ if (message.tag === MessageTag.TAG_UNSET_ASSIGNEE) {
+ return MessageTag.TAG_SET_ASSIGNEE;
+ }
+ if (message.tag === MessageTag.TAG_UNSET_PRIVATE) {
+ return MessageTag.TAG_SET_PRIVATE;
+ }
+ if (message.tag === MessageTag.TAG_SET_WIP) {
+ return MessageTag.TAG_SET_READY;
+ }
+
+ return message.tag.replace(/~.*/, '');
+}
+
+/**
+ * Try to set a revision number that makes sense, if none is set. Just copy
+ * over the revision number of the next older message. This is mostly relevant
+ * for reviewer updates. Other messages should typically have the revision
+ * number already set.
+ */
+function computeRevision(message, allMessages) {
+ if (message._revision_number > 0) return message._revision_number;
+ let revision = 0;
+ for (const m of allMessages) {
+ if (m.date > message.date) break;
+ if (m._revision_number > revision) revision = m._revision_number;
+ }
+ return revision > 0 ? revision : undefined;
+}
+
+/**
+ * Unimportant messages are initially hidden.
+ *
+ * Human messages are always important. They have an undefined tag.
+ *
+ * Autogenerated messages are unimportant, if there is a message with the same
+ * tag and a higher revision number.
+ */
+function computeIsImportant(message, allMessages) {
+ if (!message.tag) return true;
+
+ const hasSameTag = m => m.tag === message.tag;
+ const revNumber = message._revision_number || 0;
+ const hasHigherRevisionNumber = m => m._revision_number > revNumber;
+ return !allMessages.filter(hasSameTag).some(hasHigherRevisionNumber);
+}
+
+export const TEST_ONLY = {
+ computeThreads,
+ computeTag,
+ computeRevision,
+ computeIsImportant,
+};
+
+/**
* @extends PolymerElement
*/
class GrMessagesList extends mixinBehaviors( [
@@ -60,11 +162,21 @@
static get properties() {
return {
+ /** @type {?} */
+ change: Object,
changeNum: Number,
+ /**
+ * These are just the change messages. They are combined with reviewer
+ * updates below. So _combinedMessages is the more important property.
+ */
messages: {
type: Array,
value() { return []; },
},
+ /**
+ * These are just the reviewer updates. They are combined with change
+ * messages above. So _combinedMessages is the more important property.
+ */
reviewerUpdates: {
type: Array,
value() { return []; },
@@ -93,24 +205,19 @@
computed: '_computeExpandAllTitle(_expandAllState)',
},
- _hideAutomated: {
+ _showAllActivity: {
type: Boolean,
value: false,
+ observer: '_observeShowAllActivity',
},
/**
- * The messages after processing and including merged reviewer updates.
+ * The merged array of change messages and reviewer updates.
*/
- _processedMessages: {
+ _combinedMessages: {
type: Array,
- computed: '_computeItems(messages, reviewerUpdates)',
- observer: '_processedMessagesChanged',
- },
- /**
- * The subset of _processedMessages that is visible to the user.
- */
- _visibleMessages: {
- type: Array,
- value() { return []; },
+ computed: '_computeCombinedMessages(messages, reviewerUpdates, '
+ + 'changeComments)',
+ observer: '_combinedMessagesChanged',
},
_labelExtremes: {
@@ -126,27 +233,17 @@
}
scrollToMessage(messageID) {
- let el = this.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]');
- // If the message is hidden, expand the hidden messages back to that
- // point.
- if (!el) {
- let index;
- for (index = 0; index < this._processedMessages.length; index++) {
- if (this._processedMessages[index].id === messageID) {
- break;
- }
- }
- if (index === this._processedMessages.length) { return; }
+ const selector = `[data-message-id="${messageID}"]`;
+ const el = this.shadowRoot.querySelector(selector);
- const newMessages = this._processedMessages.slice(index,
- -this._visibleMessages.length);
- // Add newMessages to the beginning of _visibleMessages.
- this.splice(...['_visibleMessages', 0, 0].concat(newMessages));
- // Allow the dom-repeat to stamp.
- flush();
- el = this.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]');
+ if (!el && this._showAllActivity) {
+ console.warn(`Failed to scroll to message: ${messageID}`);
+ return;
+ }
+ if (!el) {
+ this._showAllActivity = true;
+ setTimeout(() => this.scrollToMessage(messageID));
+ return;
}
el.set('message.expanded', true);
@@ -160,22 +257,30 @@
this._highlightEl(el);
}
- _isAutomated(message) {
- return !!(message.reviewer ||
- (message.tag && message.tag.startsWith('autogenerated')));
+ _observeShowAllActivity(showAllActivity) {
+ // We have to call render() such that the dom-repeat filter picks up the
+ // change.
+ this.$.messageRepeat.render();
}
- _computeItems(messages, reviewerUpdates) {
- // Polymer 2: check for undefined
- if ([messages, reviewerUpdates].includes(undefined)) {
- return [];
- }
+ /**
+ * Filter for the dom-repeat of combinedMessages.
+ */
+ _isMessageVisible(message) {
+ return this._showAllActivity || message.isImportant;
+ }
- messages = messages || [];
- reviewerUpdates = reviewerUpdates || [];
+ /**
+ * Merges change messages and reviewer updates into one array. Also processes
+ * all messages and updates, aligns or massages some of the properties.
+ */
+ _computeCombinedMessages(messages, reviewerUpdates, changeComments) {
+ const params = [messages, reviewerUpdates, changeComments];
+ if (params.some(o => o === undefined)) return [];
+
let mi = 0;
let ri = 0;
- let result = [];
+ let combinedMessages = [];
let mDate;
let rDate;
for (let i = 0; i < messages.length; i++) {
@@ -184,44 +289,45 @@
while (mi < messages.length || ri < reviewerUpdates.length) {
if (mi >= messages.length) {
- result = result.concat(reviewerUpdates.slice(ri));
+ combinedMessages = combinedMessages.concat(reviewerUpdates.slice(ri));
break;
}
if (ri >= reviewerUpdates.length) {
- result = result.concat(messages.slice(mi));
+ combinedMessages = combinedMessages.concat(messages.slice(mi));
break;
}
mDate = mDate || parseDate(messages[mi].date);
rDate = rDate || parseDate(reviewerUpdates[ri].date);
if (rDate < mDate) {
- result.push(reviewerUpdates[ri++]);
+ combinedMessages.push(reviewerUpdates[ri++]);
rDate = null;
} else {
- result.push(messages[mi++]);
+ combinedMessages.push(messages[mi++]);
mDate = null;
}
}
- result.forEach(m => {
+ combinedMessages.forEach(m => {
if (m.expanded === undefined) {
m.expanded = false;
}
+ m.commentThreads = computeThreads(m, combinedMessages, changeComments);
+ m._revision_number = computeRevision(m, combinedMessages);
+ m.tag = computeTag(m);
});
- return result;
+ // computeIsImportant() depends on tags and revision numbers already being
+ // updated for all messages, so we have to compute this in its own forEach
+ // loop.
+ combinedMessages.forEach(m => {
+ m.isImportant = computeIsImportant(m, combinedMessages);
+ });
+ return combinedMessages;
}
- _updateExpandedStateOfAllMessages(expanded) {
- if (this._processedMessages) {
- for (let i = 0; i < this._processedMessages.length; i++) {
- this._processedMessages[i].expanded = expanded;
- }
- }
- // _visibleMessages is a subarray of _processedMessages
- // _processedMessages contains all items from _visibleMessages
- // At this point all _visibleMessages.expanded values are set,
- // and notifyPath must be used to notify Polymer about changes.
- if (this._visibleMessages) {
- for (let i = 0; i < this._visibleMessages.length; i++) {
- this.notifyPath(`_visibleMessages.${i}.expanded`);
+ _updateExpandedStateOfAllMessages(exp) {
+ if (this._combinedMessages) {
+ for (let i = 0; i < this._combinedMessages.length; i++) {
+ this._combinedMessages[i].expanded = exp;
+ this.notifyPath(`_combinedMessages.${i}.expanded`);
}
}
}
@@ -271,181 +377,31 @@
this.scrollToMessage(e.detail.id);
}
- _hasAutomatedMessages(messages) {
- if (!messages) { return false; }
- for (const message of messages) {
- if (this._isAutomated(message)) {
- return true;
- }
- }
- return false;
+ _isVisibleShowAllActivityToggle(messages = []) {
+ return messages.some(m => !m.isImportant);
+ }
+
+ _computeHiddenEntriesCount(messages = []) {
+ return messages.filter(m => !m.isImportant).length;
}
/**
- * Computes message author's file comments for change's message.
- * Method uses this.messages to find next message and relies on messages
- * to be sorted by date field descending.
- *
- * @param {!Object} changeComments changeComment object, which includes
- * a method to get all published comments (including robot comments),
- * which returns a Hash of arrays of comments, filename as key.
- * @param {!Object} message
- * @return {!Object} Hash of arrays of comments, filename as key.
+ * This method is for reporting stats only.
*/
- _computeCommentsForMessage(changeComments, message) {
- if ([changeComments, message].includes(undefined)) {
- return {};
- }
- const comments = changeComments.getAllPublishedComments();
- if (message._index === undefined || !comments || !this.messages) {
- return {};
- }
- const messages = this.messages || [];
- const index = message._index;
- const authorId = message.author && message.author._account_id;
- const mDate = parseDate(message.date).getTime();
- // NB: Messages array has oldest messages first.
- let nextMDate;
- if (index > 0) {
- for (let i = index - 1; i >= 0; i--) {
- if (messages[i] && messages[i].author &&
- messages[i].author._account_id === authorId) {
- nextMDate = parseDate(messages[i].date).getTime();
- break;
- }
- }
- }
- const msgComments = {};
- for (const file in comments) {
- if (!comments.hasOwnProperty(file)) { continue; }
- const fileComments = comments[file];
- for (let i = 0; i < fileComments.length; i++) {
- if (fileComments[i].author &&
- fileComments[i].author._account_id !== authorId) {
- continue;
- }
- const cDate = parseDate(fileComments[i].updated).getTime();
- if (cDate <= mDate) {
- if (nextMDate && cDate <= nextMDate) {
- continue;
- }
- msgComments[file] = msgComments[file] || [];
- msgComments[file].push(fileComments[i]);
- }
- }
- }
- return msgComments;
- }
-
- /**
- * Returns the number of messages to splice to the beginning of
- * _visibleMessages. This is the minimum of the total number of messages
- * remaining in the list and the number of messages needed to display five
- * more visible messages in the list.
- */
- _getDelta(visibleMessages, messages, hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- let delta = MESSAGES_INCREMENT;
- const msgsRemaining = messages.length - visibleMessages.length;
-
- if (hideAutomated) {
- let counter = 0;
- let i;
- for (i = msgsRemaining; i > 0 && counter < MESSAGES_INCREMENT; i--) {
- if (!this._isAutomated(messages[i - 1])) { counter++; }
- }
- delta = msgsRemaining - i;
- }
- return Math.min(msgsRemaining, delta);
- }
-
- /**
- * Gets the number of messages that would be visible, but do not currently
- * exist in _visibleMessages.
- */
- _numRemaining(visibleMessages, messages, hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- if (hideAutomated) {
- return this._getHumanMessages(messages).length -
- this._getHumanMessages(visibleMessages).length;
- }
- return messages.length - visibleMessages.length;
- }
-
- _computeIncrementText(visibleMessages, messages, hideAutomated) {
- let delta = this._getDelta(visibleMessages, messages, hideAutomated);
- delta = Math.min(
- this._numRemaining(visibleMessages, messages, hideAutomated), delta);
- return 'Show ' + Math.min(MESSAGES_INCREMENT, delta) + ' more';
- }
-
- _getHumanMessages(messages) {
- return messages.filter(msg => !this._isAutomated(msg));
- }
-
- _computeShowHideTextHidden(visibleMessages, messages,
- hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- if (hideAutomated) {
- messages = this._getHumanMessages(messages);
- visibleMessages = this._getHumanMessages(visibleMessages);
- }
- return visibleMessages.length >= messages.length;
- }
-
- _handleShowAllTap() {
- this._visibleMessages = this._processedMessages;
- this.reporting.reportInteraction(ReportingEvent.SHOW_ALL);
- }
-
- _handleIncrementShownMessages() {
- const delta = this._getDelta(this._visibleMessages,
- this._processedMessages, this._hideAutomated);
- const len = this._visibleMessages.length;
- const newMessages = this._processedMessages.slice(-(len + delta), -len);
- // Add newMessages to the beginning of _visibleMessages
- this.splice(...['_visibleMessages', 0, 0].concat(newMessages));
- this.reporting.reportInteraction(ReportingEvent.SHOW_MORE);
- }
-
- _processedMessagesChanged(messages) {
- if (messages) {
- this._visibleMessages = messages.slice(-MAX_INITIAL_SHOWN_MESSAGES);
-
- if (messages.length === 0) return;
- const tags = messages.map(message => message.tag || message.type ||
- (message.comments ? 'comments' : 'none'));
+ _combinedMessagesChanged(combinedMessages) {
+ if (combinedMessages) {
+ if (combinedMessages.length === 0) return;
+ const tags = combinedMessages.map(
+ message => message.tag || message.type ||
+ (message.comments ? 'comments' : 'none'));
const tagsCounted = tags.reduce((acc, val) => {
acc[val] = (acc[val] || 0) + 1;
return acc;
- }, {all: messages.length});
+ }, {all: combinedMessages.length});
this.reporting.reportInteraction('messages-count', tagsCounted);
}
}
- _computeNumMessagesText(visibleMessages, messages,
- hideAutomated) {
- const total =
- this._numRemaining(visibleMessages, messages, hideAutomated);
- return total === 1 ? 'Show 1 message' : 'Show all ' + total + ' messages';
- }
-
- _computeIncrementHidden(visibleMessages, messages,
- hideAutomated) {
- const total =
- this._numRemaining(visibleMessages, messages, hideAutomated);
- return total <= this._getDelta(visibleMessages, messages, hideAutomated);
- }
-
/**
* Compute a mapping from label name to objects representing the minimum and
* maximum possible values for that label.
@@ -468,9 +424,10 @@
/**
* Work around a issue on iOS when clicking turns into double tap
*/
- _onTapHideAutomated(e) {
+ _onTapShowAllActivityToggle(e) {
e.preventDefault();
}
}
-customElements.define(GrMessagesList.is, GrMessagesList);
+customElements.define(GrMessagesList.is,
+ GrMessagesList);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
index 2636a54..c696c8c 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
@@ -18,8 +18,7 @@
export const htmlTemplate = html`
<style include="shared-styles">
- :host,
- .messageListControls {
+ :host {
display: flex;
justify-content: space-between;
}
@@ -31,9 +30,6 @@
justify-content: space-between;
padding: var(--spacing-s) var(--spacing-l);
}
- #messageControlsContainer {
- padding: 0 var(--spacing-l);
- }
.highlighted {
animation: 3s fadeOut;
}
@@ -45,47 +41,42 @@
background-color: var(--view-background-color);
}
}
- #messageControlsContainer {
- align-items: center;
- background-color: var(--background-color-secondary);
- border-bottom: 1px solid var(--border-color);
- display: flex;
- height: 2.25em;
- justify-content: center;
- }
- #messageControlsContainer gr-button {
- padding: var(--spacing-s) 0;
- }
.container {
align-items: center;
display: flex;
}
+ .hiddenEntries {
+ color: var(--deemphasized-text-color);
+ }
gr-message:not(:last-of-type) {
border-bottom: 1px solid var(--border-color);
}
- gr-message:nth-child(2n) {
+ gr-message {
background-color: var(--background-color-secondary);
}
- gr-message:nth-child(2n + 1) {
- background-color: var(--background-color-tertiary);
- }
</style>
<div class="header">
- <span
- id="automatedMessageToggleContainer"
- class="container"
- hidden$="[[!_hasAutomatedMessages(messages)]]"
- >
- <paper-toggle-button
- id="automatedMessageToggle"
- checked="{{_hideAutomated}}"
- aria-labelledby="onlyCommentsLabel"
- role="switch"
- on-tap="_onTapHideAutomated"
- ></paper-toggle-button>
- <span id="onlyCommentsLabel">Only comments</span>
- <span class="transparent separator"></span>
- </span>
+ <div id="showAllActivityToggleContainer" class="container">
+ <template
+ is="dom-if"
+ if="[[_isVisibleShowAllActivityToggle(_combinedMessages)]]"
+ >
+ <paper-toggle-button
+ class="showAllActivityToggle"
+ checked="{{_showAllActivity}}"
+ aria-labelledby="showAllEntriesLabel"
+ role="switch"
+ on-tap="_onTapShowAllActivityToggle"
+ ></paper-toggle-button>
+ <div id="showAllEntriesLabel">
+ <span>Show all entries</span>
+ <span class="hiddenEntries" hidden$="[[_showAllActivity]]">
+ ([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
+ </span>
+ </div>
+ <span class="transparent separator"></span>
+ </template>
+ </div>
<gr-button
id="collapse-messages"
link=""
@@ -95,35 +86,17 @@
[[_expandAllState]]
</gr-button>
</div>
- <span
- id="messageControlsContainer"
- hidden$="[[_computeShowHideTextHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]"
+ <template
+ id="messageRepeat"
+ is="dom-repeat"
+ items="[[_combinedMessages]]"
+ as="message"
+ filter="_isMessageVisible"
>
- <gr-button id="oldMessagesBtn" link="" on-click="_handleShowAllTap">
- [[_computeNumMessagesText(_visibleMessages, _processedMessages,
- _hideAutomated, _visibleMessages.length)]]
- </gr-button>
- <span
- class="container"
- hidden$="[[_computeIncrementHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]"
- >
- <span class="transparent separator"></span>
- <gr-button
- id="incrementMessagesBtn"
- link=""
- on-click="_handleIncrementShownMessages"
- >
- [[_computeIncrementText(_visibleMessages, _processedMessages,
- _hideAutomated, _visibleMessages.length)]]
- </gr-button>
- </span>
- </span>
- <template is="dom-repeat" items="[[_visibleMessages]]" as="message">
<gr-message
+ change="[[change]]"
change-num="[[changeNum]]"
message="[[message]]"
- comments="[[_computeCommentsForMessage(changeComments, message)]]"
- hide-automated="[[_hideAutomated]]"
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-message-anchor-tap="_handleAnchorClick"
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
index 4f05dfc..c3245fe 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
@@ -1,7 +1,6 @@
-import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api';
/**
* @license
- * Copyright (C) 2016 The Android Open Source Project
+ * 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.
@@ -19,8 +18,10 @@
import '../../../test/common-test-setup-karma.js';
import '../../diff/gr-comment-api/gr-comment-api.js';
import './gr-messages-list.js';
-import '../../../test/mocks/comment-api.js';
+import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
+import {TEST_ONLY} from './gr-messages-list.js';
+import {MessageTag} from '../../../constants/constants.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
createCommentApiMockWithTemplateElement(
@@ -50,40 +51,15 @@
message: params.message || Math.random().toString(),
_revision_number: params._revision_number || 1,
author: params.author || author1,
+ tag: params.tag,
};
};
-const randomAutomated = function(opt_params) {
- return Object.assign({tag: 'autogenerated:gerrit:replace'},
- randomMessage(opt_params));
-};
-
function generateRandomMessages(count) {
return new Array(count).fill()
.map(() => randomMessage());
}
-function generateRandomAutomatedMessages(count) {
- return new Array(count).fill()
- .map(() => randomAutomated());
-}
-
-// Returns a shuffled copy of array
-export function shuffle(arr) {
- const result = [];
- for (const item of arr) {
- // Random number in the interval [0..array.length]
- const j = Math.floor(Math.random() * (arr.length + 1));
- if (j < result.length) {
- result.push(result[j]);
- result[j] = item;
- } else {
- result.push(item);
- }
- }
- return result;
-}
-
suite('gr-messages-list tests', () => {
let element;
let messages;
@@ -94,54 +70,63 @@
return dom(element.root).querySelectorAll('gr-message');
};
+ const MESSAGE_ID_0 = '1234ccc949c6d482b061be6a28e10782abf0e7af';
+ const MESSAGE_ID_1 = '8c19ccc949c6d482b061be6a28e10782abf0e7af';
+ const MESSAGE_ID_2 = 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5';
+
const author = {
_account_id: 42,
name: 'Marvin the Paranoid Android',
email: 'marvin@sirius.org',
};
+ const createComment = function() {
+ return {
+ id: '1a2b3c4d',
+ message: 'some random test text',
+ change_message_id: '8a7b6c5d',
+ updated: '2016-01-01 01:02:03.000000000',
+ line: 1,
+ patch_set: 1,
+ author,
+ };
+ };
+
const comments = {
file1: [
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ change_message_id: MESSAGE_ID_0,
in_reply_to: '6505d749_f0bec0aa',
- line: 62,
- id: '6505d749_10ed44b2',
- patch_set: 2,
author: {
email: 'some@email.com',
_account_id: 123,
},
},
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ id: '2b3c4d5e',
+ change_message_id: MESSAGE_ID_1,
in_reply_to: 'c5912363_6b820105',
- line: 42,
- id: '450a935e_0f1c05db',
- patch_set: 2,
- author,
},
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ id: '2b3c4d5e',
+ change_message_id: MESSAGE_ID_1,
in_reply_to: '6505d749_f0bec0aa',
- line: 62,
- id: '6505d749_10ed44b2',
- patch_set: 2,
- author,
+ },
+ {
+ ...createComment(),
+ id: '34ed05d749_10ed44b2',
+ change_message_id: MESSAGE_ID_2,
},
],
file2: [
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ change_message_id: MESSAGE_ID_1,
in_reply_to: 'c5912363_4b7d450a',
- line: 132,
id: '450a935e_4f260d25',
- patch_set: 2,
- author,
},
],
};
@@ -168,126 +153,6 @@
return commentApiWrapper.loadComments();
});
- test('show some old messages', () => {
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- element.messages = generateRandomMessages(26);
- flushAsynchronousOperations();
-
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 20);
- assert.equal(element.$.incrementMessagesBtn.innerText.toUpperCase()
- .trim(), 'SHOW 5 MORE');
- MockInteractions.tap(element.$.incrementMessagesBtn);
- flushAsynchronousOperations();
-
- assert.equal(getMessages().length, 25);
- assert.equal(element.$.incrementMessagesBtn.innerText.toUpperCase()
- .trim(), 'SHOW 1 MORE');
- MockInteractions.tap(element.$.incrementMessagesBtn);
- flushAsynchronousOperations();
-
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 26);
- });
-
- test('show all old messages', () => {
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- element.messages = generateRandomMessages(26);
- flushAsynchronousOperations();
-
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 20);
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW ALL 6 MESSAGES');
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- assert.equal(getMessages().length, 26);
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('message count respects automated', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- MockInteractions.tap(element.$.automatedMessageToggle);
- flushAsynchronousOperations();
-
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('message count still respects non-automated on toggle', () => {
- element.messages = generateRandomMessages(10)
- .concat(generateRandomAutomatedMessages(11));
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- MockInteractions.tap(element.$.automatedMessageToggle);
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('show all messages respects expand', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Expand all.
- flushAsynchronousOperations();
-
- let messages = getMessages();
- assert.equal(messages.length, 20);
- for (const message of messages) {
- assert.isTrue(message._expanded);
- }
-
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- messages = getMessages();
- assert.equal(messages.length, 21);
- for (const message of messages) {
- assert.isTrue(message._expanded);
- }
- });
-
- test('show all messages respects collapse', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Expand all.
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Collapse all.
- flushAsynchronousOperations();
-
- let messages = getMessages();
- assert.equal(messages.length, 20);
- for (const message of messages) {
- assert.isFalse(message._expanded);
- }
-
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- messages = getMessages();
- assert.equal(messages.length, 21);
- for (const message of messages) {
- assert.isFalse(message._expanded);
- }
- });
-
test('expand/collapse all', () => {
let allMessageEls = getMessages();
for (const message of allMessageEls) {
@@ -329,9 +194,11 @@
assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
});
- test('hide messages does not appear when no automated messages', () => {
+ test('showAllActivity does not appear when all msgs are important', () => {
assert.isOk(element.shadowRoot
- .querySelector('#automatedMessageToggleContainer[hidden]'));
+ .querySelector('#showAllActivityToggleContainer'));
+ assert.isNotOk(element.shadowRoot
+ .querySelector('.showAllActivityToggle'));
});
test('scroll to message', () => {
@@ -373,14 +240,13 @@
element.scrollToMessage(messageID);
assert.isTrue(scrollToStub.calledOnce);
assert.isTrue(highlightStub.calledOnce);
- assert.equal(element._visibleMessages.length, 24);
assert.isTrue(
element.shadowRoot
.querySelector('[data-message-id="' + messageID + '"]')
._expanded);
});
- test('messages', () => {
+ test('associating messages with comments', () => {
const messages = [].concat(
randomMessage(),
{
@@ -401,26 +267,156 @@
}
);
element.messages = messages;
- const isAuthor = function(author, message) {
- return message.author._account_id === author._account_id;
- };
- const isMarvin = isAuthor.bind(null, author);
flushAsynchronousOperations();
const messageElements = getMessages();
assert.equal(messageElements.length, messages.length);
assert.deepEqual(messageElements[1].message, messages[1]);
assert.deepEqual(messageElements[2].message, messages[2]);
- assert.deepEqual(messageElements[1].comments.file1,
- comments.file1.filter(isMarvin).map(c => {
- return {...c,
- path: 'file1'};
- }));
- assert.deepEqual(messageElements[1].comments.file2,
- comments.file2.filter(isMarvin).map(c => {
- return {...c,
- path: 'file2'};
- }));
- assert.deepEqual(messageElements[2].comments, {});
+ });
+
+ test('threads', () => {
+ const messages = [
+ {
+ _index: 5,
+ _revision_number: 4,
+ message: 'Uploaded patch set 4.',
+ date: '2016-09-28 13:36:33.000000000',
+ author,
+ id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
+ },
+ ];
+ element.messages = messages;
+ flushAsynchronousOperations();
+ const messageElements = getMessages();
+ // threads
+ assert.equal(
+ messageElements[0].message.commentThreads.length,
+ 3);
+ // first thread contains 1 comment
+ assert.equal(
+ messageElements[0].message.commentThreads[0].comments.length,
+ 1);
+ });
+
+ test('updateTag human message', () => {
+ const m = randomMessage();
+ assert.equal(TEST_ONLY.computeTag(m), undefined);
+ });
+
+ test('updateTag nothing to change', () => {
+ const m = randomMessage();
+ const tag = 'something-normal';
+ m.tag = tag;
+ assert.equal(TEST_ONLY.computeTag(m), tag);
+ });
+
+ test('updateTag TAG_NEW_WIP_PATCHSET', () => {
+ const m = randomMessage();
+ m.tag = MessageTag.TAG_NEW_WIP_PATCHSET;
+ assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
+ });
+
+ test('updateTag remove postfix', () => {
+ const m = randomMessage();
+ m.tag = 'something~withpostfix';
+ assert.equal(TEST_ONLY.computeTag(m), 'something');
+ });
+
+ test('updateTag with robot comments', () => {
+ const m = randomMessage();
+ m.commentThreads = [{
+ comments: [{
+ robot_id: 'id314',
+ change_message_id: m.id,
+ }],
+ }];
+ assert.notEqual(TEST_ONLY.computeTag(m), undefined);
+ });
+
+ test('setRevisionNumber nothing to change', () => {
+ const m1 = randomMessage();
+ const m2 = randomMessage();
+ assert.equal(TEST_ONLY.computeRevision(m1, [m1, m2]), 1);
+ assert.equal(TEST_ONLY.computeRevision(m2, [m1, m2]), 1);
+ });
+
+ test('setRevisionNumber reviewer updates', () => {
+ const m1 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-01 10:00:00.000000000',
+ });
+ m1._revision_number = undefined;
+ const m2 = randomMessage(
+ {
+ date: '2020-01-02 10:00:00.000000000',
+ });
+ m2._revision_number = 1;
+ const m3 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-03 10:00:00.000000000',
+ });
+ m3._revision_number = undefined;
+ const m4 = randomMessage(
+ {
+ date: '2020-01-04 10:00:00.000000000',
+ });
+ m4._revision_number = 2;
+ const m5 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-05 10:00:00.000000000',
+ });
+ m5._revision_number = undefined;
+ const allMessages = [m1, m2, m3, m4, m5];
+ assert.equal(TEST_ONLY.computeRevision(m1, allMessages), undefined);
+ assert.equal(TEST_ONLY.computeRevision(m2, allMessages), 1);
+ assert.equal(TEST_ONLY.computeRevision(m3, allMessages), 1);
+ assert.equal(TEST_ONLY.computeRevision(m4, allMessages), 2);
+ assert.equal(TEST_ONLY.computeRevision(m5, allMessages), 2);
+ });
+
+ test('isImportant human message', () => {
+ const m = randomMessage();
+ assert.isTrue(TEST_ONLY.computeIsImportant(m, []));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m, [m]));
+ });
+
+ test('isImportant even with a tag', () => {
+ const m1 = randomMessage();
+ const m2 = randomMessage({tag: 'autogenerated:gerrit1'});
+ const m3 = randomMessage({tag: 'autogenerated:gerrit2'});
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, []));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
+ });
+
+ test('isImportant filters same tag and older revision', () => {
+ const m1 = randomMessage({tag: 'auto', _revision_number: 2});
+ const m2 = randomMessage({tag: 'auto', _revision_number: 1});
+ const m3 = randomMessage({tag: 'auto'});
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m2]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
+ });
+
+ test('isImportant is evaluated after tag update', () => {
+ const m1 = randomMessage(
+ {tag: MessageTag.TAG_NEW_PATCHSET, _revision_number: 1});
+ const m2 = randomMessage(
+ {tag: MessageTag.TAG_NEW_WIP_PATCHSET, _revision_number: 2});
+ element.messages = [m1, m2];
+ flushAsynchronousOperations();
+ assert.isFalse(m1.isImportant);
+ assert.isTrue(m2.isImportant);
});
test('messages without author do not throw', () => {
@@ -437,18 +433,6 @@
assert.equal(messageEls.length, 1);
assert.equal(messageEls[0].message.message, messages[0].message);
});
-
- test('hide increment text if increment >= total remaining', () => {
- // Test with stubbed return values, as _numRemaining and _getDelta have
- // their own tests.
- sinon.stub(element, '_getDelta').returns(5);
- const remainingStub = sinon.stub(element, '_numRemaining').returns(6);
- assert.isFalse(element._computeIncrementHidden(null, null, null));
- remainingStub.restore();
-
- sinon.stub(element, '_numRemaining').returns(4);
- assert.isTrue(element._computeIncrementHidden(null, null, null));
- });
});
suite('gr-messages-list automate tests', () => {
@@ -457,18 +441,6 @@
let commentApiWrapper;
- const getMessages = function() {
- return dom(element.root).querySelectorAll('gr-message');
- };
- const getHiddenMessages = function() {
- return dom(element.root).querySelectorAll('gr-message[hidden]');
- };
-
- const randomMessageReviewer = {
- reviewer: {},
- date: '2016-01-13 20:30:33.038000',
- };
-
setup(() => {
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({}); },
@@ -478,8 +450,11 @@
getDiffDrafts() { return Promise.resolve({}); },
});
- messages = generateRandomAutomatedMessages(2);
- messages.push(randomMessageReviewer);
+ messages = [
+ randomMessage(),
+ randomMessage({tag: 'auto', _revision_number: 2}),
+ randomMessage({tag: 'auto', _revision_number: 3}),
+ ];
// Element must be wrapped in an element with direct access to the
// comment API.
@@ -494,89 +469,33 @@
});
test('hide autogenerated button is not hidden', () => {
- assert.isNotOk(element.shadowRoot
- .querySelector('#automatedMessageToggle[hidden]'));
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
});
- test('autogenerated messages are not hidden initially', () => {
- const allHiddenMessageEls = getHiddenMessages();
-
- // There are no hidden messages.
- assert.isFalse(!!allHiddenMessageEls.length);
+ test('one unimportant message is hidden initially', () => {
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 2);
});
- test('autogenerated messages hidden after comments only toggle', () => {
- let allHiddenMessageEls = getHiddenMessages();
-
- element._hideAutomated = false;
- MockInteractions.tap(element.$.automatedMessageToggle);
+ test('unimportant messages hidden after toggle', () => {
+ element._showAllActivity = true;
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
+ MockInteractions.tap(toggle);
flushAsynchronousOperations();
- const allMessageEls = getMessages();
- allHiddenMessageEls = getHiddenMessages();
-
- // Autogenerated messages are now hidden.
- assert.equal(allHiddenMessageEls.length, allMessageEls.length);
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 2);
});
- test('autogenerated messages not hidden after comments only toggle',
- () => {
- let allHiddenMessageEls = getHiddenMessages();
-
- element._hideAutomated = true;
- MockInteractions.tap(element.$.automatedMessageToggle);
- allHiddenMessageEls = getHiddenMessages();
-
- // Autogenerated messages are now hidden.
- assert.isFalse(!!allHiddenMessageEls.length);
- });
-
- test('_getDelta', () => {
- let messages = [randomMessage()];
- assert.equal(element._getDelta([], messages, false), 1);
- assert.equal(element._getDelta([], messages, true), 1);
-
- messages = generateRandomMessages(7);
- assert.equal(element._getDelta([], messages, false), 5);
- assert.equal(element._getDelta([], messages, true), 5);
-
- messages = generateRandomMessages(4)
- .concat(generateRandomAutomatedMessages(2))
- .concat(generateRandomMessages(3));
-
- const dummyArr = generateRandomMessages(2);
- assert.equal(element._getDelta(dummyArr, messages, false), 5);
- assert.equal(element._getDelta(dummyArr, messages, true), 7);
- });
-
- test('_getHumanMessages', () => {
- assert.equal(
- element._getHumanMessages(
- generateRandomAutomatedMessages(5)).length, 0);
- assert.equal(
- element._getHumanMessages(generateRandomMessages(5)).length, 5);
-
- let messages = shuffle(generateRandomMessages(5)
- .concat(generateRandomAutomatedMessages(5)));
- messages = element._getHumanMessages(messages);
- assert.equal(messages.length, 5);
- assert.isFalse(element._hasAutomatedMessages(messages));
- });
-
- test('initially show only 20 messages', () => {
- sinon.stub(element.reporting, 'reportInteraction').callsFake(
- (eventName, details) => {
- assert.equal(typeof(eventName), 'string');
- if (details) {
- assert.equal(typeof(details), 'object');
- }
- });
- const messages = Array.from(Array(23).keys())
- .map(() => {
- return {};
- });
- element._processedMessagesChanged(messages);
-
- assert.equal(element._visibleMessages.length, 20);
+ test('unimportant messages shown after toggle', () => {
+ element._showAllActivity = false;
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
+ MockInteractions.tap(toggle);
+ flushAsynchronousOperations();
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 3);
});
test('_computeLabelExtremes', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
index 9a94d27..63ab267 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
@@ -20,7 +20,6 @@
import './gr-reply-dialog.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const basicFixture = fixtureFromElement('gr-reply-dialog');
const pluginApi = _testOnly_initGerritPluginApi();
@@ -126,8 +125,6 @@
}, null, 'http://test.com/plugins/lgtm.js');
element = basicFixture.instantiate();
setupElement(element);
- sinon.stub(pluginEndpoints, 'importUrl')
- .callsFake( url => Promise.resolve());
pluginLoader.loadPlugins([]);
pluginLoader.awaitPluginsLoaded().then(() => {
flush(() => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 24bc142..3fb8442 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -28,7 +28,6 @@
import '../gr-label-scores/gr-label-scores.js';
import '../gr-thread-list/gr-thread-list.js';
import '../../../styles/shared-styles.js';
-import '../gr-comment-list/gr-comment-list.js';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
@@ -562,6 +561,9 @@
}
}
}
+ this.reportAttentionSetChanges(this._attentionModified,
+ reviewInput.add_to_attention_set,
+ reviewInput.remove_from_attention_set);
if (this.draft != null) {
if (this._isPatchsetCommentsExperimentEnabled) {
@@ -1087,6 +1089,26 @@
composed: true, bubbles: true,
}));
}
+
+ reportAttentionSetChanges(modified, addedSet, removedSet) {
+ const actions = modified ? ['MODIFIED'] : ['NOT_MODIFIED'];
+ const ownerId = (this.change && this.change.owner
+ && this.change.owner._account_id) || -1;
+ const selfId = (this._account && this._account._account_id) || -1;
+ for (const added of (addedSet || [])) {
+ const addedId = added.user;
+ const self = addedId === selfId ? '_SELF' : '';
+ const role = addedId === ownerId ? '_OWNER' : '_REVIEWER';
+ actions.push('ADD' + self + role);
+ }
+ for (const removed of (removedSet || [])) {
+ const removedId = removed.user;
+ const self = removedId === selfId ? '_SELF' : '';
+ const role = removedId === ownerId ? '_OWNER' : '_REVIEWER';
+ actions.push('REMOVE' + self + role);
+ }
+ this.reporting.reportInteraction('attention-set-actions', {actions});
+ }
}
customElements.define(GrReplyDialog.is, GrReplyDialog);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index fa686da..eb68024 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -247,7 +247,7 @@
_computeDisplayPath(path) {
const displayPath = this.computeDisplayPath(path);
if (displayPath === SpecialFilePath.PATCHSET_LEVEL_COMMENTS) {
- return `Patchset ${this.patchNum}`;
+ return `Patchset`;
}
return displayPath;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
index 3837514..db5ade1 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
@@ -208,7 +208,7 @@
element.patchNum = '3';
path = SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
- assert.equal(element._computeDisplayPath(path), 'Patchset 3');
+ assert.equal(element._computeDisplayPath(path), 'Patchset');
});
test('_computeDisplayLine', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.js b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.js
index ac9bd62..717a28e 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.js
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.js
@@ -26,6 +26,7 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-hovercard-account_html.js';
+import {appContext} from '../../../services/app-context.js';
/** @extends PolymerElement */
class GrHovercardAccount extends GestureEventListeners(
@@ -72,6 +73,11 @@
};
}
+ constructor() {
+ super();
+ this.reporting = appContext.reportingService;
+ }
+
attached() {
super.attached();
this.$.restAPI.getConfig().then(config => {
@@ -120,6 +126,8 @@
composed: true,
bubbles: true,
}));
+ this.reporting.reportInteraction('attention-hovercard-add',
+ this._reportingDetails());
this.$.restAPI.addToAttentionSet(this.change._number,
this.account._account_id, 'manually added').then(obj => {
GerritNav.navigateToChange(this.change);
@@ -136,12 +144,34 @@
composed: true,
bubbles: true,
}));
+ this.reporting.reportInteraction('attention-hovercard-remove',
+ this._reportingDetails());
this.$.restAPI.removeFromAttentionSet(this.change._number,
this.account._account_id, 'manually removed').then(obj => {
GerritNav.navigateToChange(this.change);
});
this.hide();
}
+
+ _reportingDetails() {
+ const targetId = this.account._account_id;
+ const ownerId = (this.change && this.change.owner
+ && this.change.owner._account_id) || -1;
+ const selfId = (this._selfAccount && this._selfAccount._account_id) || -1;
+ const reviewers = (
+ this.change && this.change.reviewers && this.change.reviewers.REVIEWER ?
+ [...this.change.reviewers.REVIEWER] : []);
+ const reviewerIds = reviewers
+ .map(r => r._account_id)
+ .filter(rId => rId !== ownerId);
+ return {
+ actionByOwner: selfId === ownerId,
+ actionByReviewer: reviewerIds.includes(selfId),
+ targetIsOwner: targetId === ownerId,
+ targetIsReviewer: reviewerIds.includes(targetId),
+ targetIsSelf: targetId === selfId,
+ };
+ }
}
customElements.define(GrHovercardAccount.is, GrHovercardAccount);
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.js b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.js
index 571e49a..0d351f6 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.js
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.js
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import '../../../styles/shared-styles.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
+import {flush, dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {getRootElement} from '../../../scripts/rootElement.js';
const HOVER_CLASS = 'hovered';
@@ -261,6 +261,10 @@
// position for it.
this.classList.add(HIDE_CLASS);
this.classList.add(HOVER_CLASS);
+ // Make sure that the hovercard actually rendered and all dom-if
+ // statements processed, so that we can measure the (invisible)
+ // hovercard properly in updatePosition().
+ flush();
this.updatePosition();
this.classList.remove(HIDE_CLASS);
}
diff --git a/polygerrit-ui/app/externs/BUILD b/polygerrit-ui/app/externs/BUILD
deleted file mode 100644
index 26ead9a..0000000
--- a/polygerrit-ui/app/externs/BUILD
+++ /dev/null
@@ -1,25 +0,0 @@
-# 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.
-
-load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library")
-
-package(
- default_visibility = ["//visibility:public"],
-)
-
-closure_js_library(
- name = "plugin",
- srcs = ["plugin.js"],
- no_closure_library = True,
-)
diff --git a/polygerrit-ui/app/externs/plugin.js b/polygerrit-ui/app/externs/plugin.js
deleted file mode 100644
index c88c724..0000000
--- a/polygerrit-ui/app/externs/plugin.js
+++ /dev/null
@@ -1,30 +0,0 @@
-/**
- * @license
- * 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.
- */
-
-/**
- * @fileoverview Closure compiler externs for the Gerrit UI plugins.
- * @externs
- */
-
-/* eslint-disable no-var */
-
-var Gerrit = {};
-
-/**
- * @param {!Function} callback
- */
-Gerrit.install = function(callback) {};
diff --git a/polygerrit-ui/app/services/flags.js b/polygerrit-ui/app/services/flags.js
index 64f0115..6313255 100644
--- a/polygerrit-ui/app/services/flags.js
+++ b/polygerrit-ui/app/services/flags.js
@@ -20,7 +20,6 @@
* @desc Experiment ids used in Gerrit.
*/
export const ExperimentIds = {
- CLEANER_CHANGELOG: 'UiFeature__cleaner_changelog',
PATCHSET_COMMENTS: 'UiFeature__patchset_comments',
};
diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl
index 5440b88..eeb5e6b 100644
--- a/tools/bzl/js.bzl
+++ b/tools/bzl/js.bzl
@@ -1,4 +1,4 @@
-load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_binary", "closure_js_library")
+load("@npm_bazel_terser//:index.bzl", "terser_minified")
load("//lib/js:npm.bzl", "NPM_SHA1S", "NPM_VERSIONS")
NPMJS = "NPMJS"
@@ -439,7 +439,7 @@
"""Combine html, js, css files and optionally split into js and html bundles."""
_bundle_rule(pkg = native.package_name(), *args, **kwargs)
-def polygerrit_plugin(name, app, srcs = [], deps = [], externs = [], assets = None, plugin_name = None, **kwargs):
+def polygerrit_plugin(name, app, srcs = [], deps = [], assets = None, plugin_name = None, **kwargs):
"""Bundles plugin dependencies for deployment.
This rule bundles all Polymer elements and JS dependencies into .html and .js files.
@@ -449,7 +449,6 @@
Args:
name: String, rule name.
app: String, the main or root source file.
- externs: Fileset, external definitions that should not be bundled.
assets: Fileset, additional files to be used by plugin in runtime, exported to "plugins/${name}/static".
plugin_name: String, plugin name. ${name} is used if not provided.
"""
@@ -473,29 +472,15 @@
else:
js_srcs = srcs
- closure_js_library(
- name = name + "_closure_lib",
- srcs = js_srcs + externs,
- convention = "GOOGLE",
- no_closure_library = True,
- deps = [
- "//lib/polymer_externs:polymer_closure",
- "//polygerrit-ui/app/externs:plugin",
- ],
+ native.filegroup(
+ name = name + "-src-fg",
+ srcs = js_srcs,
)
- closure_js_binary(
- name = name + "_bin",
- compilation_level = "WHITESPACE_ONLY",
- defs = [
- "--polymer_version=2",
- "--language_out=ECMASCRIPT_2017",
- "--rewrite_polyfills=false",
- ],
- deps = [
- name + "_closure_lib",
- ],
- dependency_mode = "PRUNE_LEGACY",
+ terser_minified(
+ name = name + ".min",
+ sourcemap = False,
+ src = name + "-src-fg",
)
if html_plugin:
@@ -519,7 +504,7 @@
native.genrule(
name = name + "_rename_js",
- srcs = [name + "_bin.js"],
+ srcs = [name + ".min"],
outs = [plugin_name + ".js"],
cmd = "cp $< $@",
output_to_bindir = True,
diff --git a/yarn.lock b/yarn.lock
index 8e35c84..0c4383f 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -490,6 +490,11 @@
resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-1.6.1.tgz#7ec9d39a3fca23256fca55410339724804802616"
integrity sha512-FhblJkpd8VKl9txhAAIotSsIOHRpPd2FgJG7Op3uV7LfaCVBmUs3XDBZCgfwt5wmEpd3lwCHA1Ei+O/URS2+5w==
+"@bazel/terser@^1.7.0":
+ version "1.7.0"
+ resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-1.7.0.tgz#c43e711e13b9a71c7abd3ade04fb4650d547ad01"
+ integrity sha512-u/UXk0WUinvkk1g5xxfqGieBz3r12Bj2y2m25lC5GjHBgCpGk7DyeGGi9H3QQNO1Wmpw51QSE9gaPzKzjUVGug==
+
"@bazel/typescript@^1.6.1":
version "1.6.1"
resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-1.6.1.tgz#1bf83c20021d359bc9b532181981ac540584a30c"
@@ -2705,7 +2710,7 @@
resolved "https://registry.yarnpkg.com/commander/-/commander-2.17.1.tgz#bd77ab7de6de94205ceacc72f1716d29f20a77bf"
integrity sha512-wPMUt6FnH2yzG95SA6mzjQOEKUU3aLaDEmzs1ti+1E9h+CsrZghRlqEM/EJ4KscsQVG8uNN4uVreUeT8+drlgg==
-commander@^2.19.0:
+commander@^2.19.0, commander@^2.20.0:
version "2.20.3"
resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33"
integrity sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==
@@ -8641,6 +8646,14 @@
dependencies:
source-map "^0.5.6"
+source-map-support@~0.5.12:
+ version "0.5.19"
+ resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.19.tgz#a98b62f86dcaf4f67399648c085291ab9e8fed61"
+ integrity sha512-Wonm7zOCIJzBGQdB+thsPar0kYuCIzYvxZwlBa87yi/Mdjv7Tip2cyVbLj5o0cFPN4EVkuTwb3GDDyUx2DGnGw==
+ dependencies:
+ buffer-from "^1.0.0"
+ source-map "^0.6.0"
+
source-map-url@^0.4.0:
version "0.4.0"
resolved "https://registry.yarnpkg.com/source-map-url/-/source-map-url-0.4.0.tgz#3e935d7ddd73631b97659956d55128e87b5084a3"
@@ -9136,6 +9149,15 @@
merge-stream "^1.0.0"
through2 "^2.0.1"
+terser@^4.8.0:
+ version "4.8.0"
+ resolved "https://registry.yarnpkg.com/terser/-/terser-4.8.0.tgz#63056343d7c70bb29f3af665865a46fe03a0df17"
+ integrity sha512-EAPipTNeWsb/3wLPeup1tVPaXfIaU68xMnVdPafIL1TV05OhASArYyIfFvnvJCNrR2NIOvDVNNTFRa+Re2MWyw==
+ dependencies:
+ commander "^2.20.0"
+ source-map "~0.6.1"
+ source-map-support "~0.5.12"
+
text-encoding@0.6.4:
version "0.6.4"
resolved "https://registry.yarnpkg.com/text-encoding/-/text-encoding-0.6.4.tgz#e399a982257a276dae428bb92845cb71bdc26d19"
