Merge "Fix intraline highlights"
diff --git a/WORKSPACE b/WORKSPACE
index 350dba1..6a86c19 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1251,8 +1251,8 @@
bower_archive(
name = "page",
package = "visionmedia/page.js",
- sha1 = "51a05428dd4f68fae1df5f12d0e2b61ba67f7757",
- version = "1.7.1",
+ sha1 = "4a31889cd75cc5e7f68a4c7f256eecaf27102eee",
+ version = "1.11.4",
)
bower_archive(
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 1b1869c..96eff88 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -112,21 +112,14 @@
// We evaluate all the plugin-defined evaluators,
// and then we collect the results in one list.
return Streams.stream(submitRules)
- .map(c -> c.call(s -> s.evaluate(cd, opts)))
+ .map(c -> c.call(s -> s.evaluate(cd)))
.flatMap(Collection::stream)
.collect(Collectors.toList());
}
private List<SubmitRecord> ruleError(String err, Exception e) {
- if (opts.logErrors()) {
- if (e == null) {
- logger.atSevere().log(err);
- } else {
- logger.atSevere().withCause(e).log(err);
- }
- return defaultRuleError();
- }
- return createRuleError(err);
+ logger.atSevere().withCause(e).log(err);
+ return defaultRuleError();
}
/**
@@ -146,14 +139,11 @@
return typeError("Error looking up change " + cd.getId(), e);
}
- return prologRule.getSubmitType(cd, opts);
+ return prologRule.getSubmitType(cd);
}
private SubmitTypeRecord typeError(String err, Exception e) {
- if (opts.logErrors()) {
- logger.atSevere().withCause(e).log(err);
- return defaultTypeError();
- }
- return SubmitTypeRecord.error(err);
+ logger.atSevere().withCause(e).log(err);
+ return defaultTypeError();
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRuleOptions.java b/java/com/google/gerrit/server/project/SubmitRuleOptions.java
index a4340b2..ad077c0 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleOptions.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleOptions.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.project;
import com.google.auto.value.AutoValue;
-import com.google.gerrit.common.Nullable;
/**
* Stable identifier for options passed to a particular submit rule evaluator.
@@ -26,12 +25,7 @@
@AutoValue
public abstract class SubmitRuleOptions {
private static final SubmitRuleOptions defaults =
- new AutoValue_SubmitRuleOptions.Builder()
- .allowClosed(false)
- .skipFilters(false)
- .logErrors(true)
- .rule(null)
- .build();
+ new AutoValue_SubmitRuleOptions.Builder().allowClosed(false).build();
public static SubmitRuleOptions defaults() {
return defaults;
@@ -43,25 +37,12 @@
public abstract boolean allowClosed();
- public abstract boolean skipFilters();
-
- public abstract boolean logErrors();
-
- @Nullable
- public abstract String rule();
-
public abstract Builder toBuilder();
@AutoValue.Builder
public abstract static class Builder {
public abstract SubmitRuleOptions.Builder allowClosed(boolean allowClosed);
- public abstract SubmitRuleOptions.Builder skipFilters(boolean skipFilters);
-
- public abstract SubmitRuleOptions.Builder rule(@Nullable String rule);
-
- public abstract SubmitRuleOptions.Builder logErrors(boolean logErrors);
-
public abstract SubmitRuleOptions build();
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
index afd02a9..d5ed9a4 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
@@ -31,9 +31,8 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.rules.DefaultSubmitRule;
+import com.google.gerrit.server.rules.PrologOptions;
import com.google.gerrit.server.rules.PrologRule;
import com.google.gerrit.server.rules.RulesCache;
import com.google.inject.Inject;
@@ -46,7 +45,6 @@
private final RulesCache rules;
private final AccountLoader.Factory accountInfoFactory;
private final ProjectCache projectCache;
- private final DefaultSubmitRule defaultSubmitRule;
private final PrologRule prologRule;
@Option(name = "--filters", usage = "impact of filters in parent projects")
@@ -58,13 +56,11 @@
RulesCache rules,
AccountLoader.Factory infoFactory,
ProjectCache projectCache,
- DefaultSubmitRule defaultSubmitRule,
PrologRule prologRule) {
this.changeDataFactory = changeDataFactory;
this.rules = rules;
this.accountInfoFactory = infoFactory;
this.projectCache = projectCache;
- this.defaultSubmitRule = defaultSubmitRule;
this.prologRule = prologRule;
}
@@ -74,33 +70,23 @@
if (input == null) {
input = new TestSubmitRuleInput();
}
- if (input.rule != null && !rules.isProjectRulesEnabled()) {
+ if (input.rule == null) {
+ throw new BadRequestException("rule is required");
+ }
+ if (!rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
- SubmitRuleOptions opts =
- SubmitRuleOptions.builder()
- .skipFilters(input.filters == Filters.SKIP)
- .rule(input.rule)
- .logErrors(false)
- .build();
-
ProjectState projectState = projectCache.get(rsrc.getProject());
if (projectState == null) {
throw new BadRequestException("project not found");
}
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- List<SubmitRecord> records;
- if (projectState.hasPrologRules() || input.rule != null) {
- records = ImmutableList.copyOf(prologRule.evaluate(cd, opts));
- } else {
- // No rules were provided as input and we have no rules.pl. This means we are supposed to run
- // the default rules. Nowadays, the default rules are implemented in Java, not Prolog.
- // Therefore, we call the DefaultRuleEvaluator instead.
- records = ImmutableList.copyOf(defaultSubmitRule.evaluate(cd, opts));
- }
-
+ List<SubmitRecord> records =
+ ImmutableList.copyOf(
+ prologRule.evaluate(
+ cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP)));
List<TestSubmitRuleInfo> out = Lists.newArrayListWithCapacity(records.size());
AccountLoader accounts = accountInfoFactory.create(true);
for (SubmitRecord r : records) {
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
index 9e8ee67..cb52fcb 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -28,6 +29,8 @@
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.rules.PrologOptions;
+import com.google.gerrit.server.rules.PrologRule;
import com.google.gerrit.server.rules.RulesCache;
import com.google.inject.Inject;
import org.kohsuke.args4j.Option;
@@ -35,19 +38,16 @@
public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
- private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
+ private final PrologRule prologRule;
@Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN;
@Inject
- TestSubmitType(
- ChangeData.Factory changeDataFactory,
- RulesCache rules,
- SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
+ TestSubmitType(ChangeData.Factory changeDataFactory, RulesCache rules, PrologRule prologRule) {
this.changeDataFactory = changeDataFactory;
this.rules = rules;
- this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
+ this.prologRule = prologRule;
}
@Override
@@ -56,21 +56,18 @@
if (input == null) {
input = new TestSubmitRuleInput();
}
- if (input.rule != null && !rules.isProjectRulesEnabled()) {
+ if (input.rule == null) {
+ throw new BadRequestException("rule is required");
+ }
+ if (!rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
- SubmitRuleOptions opts =
- SubmitRuleOptions.builder()
- .logErrors(false)
- .skipFilters(input.filters == Filters.SKIP)
- .rule(input.rule)
- .build();
-
- SubmitRuleEvaluator evaluator = submitRuleEvaluatorFactory.create(opts);
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- SubmitTypeRecord rec = evaluator.getSubmitType(cd);
+ SubmitTypeRecord rec =
+ prologRule.getSubmitType(
+ cd, PrologOptions.dryRunOptions(input.rule, input.filters == Filters.SKIP));
if (rec.status != SubmitTypeRecord.Status.OK) {
throw new BadRequestException(String.format("rule produced invalid result: %s", rec));
@@ -80,17 +77,30 @@
}
public static class Get implements RestReadView<RevisionResource> {
- private final TestSubmitType test;
+ private final ChangeData.Factory changeDataFactory;
+ private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject
- Get(TestSubmitType test) {
- this.test = test;
+ Get(
+ ChangeData.Factory changeDataFactory,
+ SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
+ this.changeDataFactory = changeDataFactory;
+ this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
}
@Override
public Response<SubmitType> apply(RevisionResource resource)
- throws AuthException, BadRequestException {
- return test.apply(resource, null);
+ throws AuthException, ResourceConflictException {
+ SubmitRuleEvaluator evaluator =
+ submitRuleEvaluatorFactory.create(SubmitRuleOptions.defaults());
+ ChangeData cd = changeDataFactory.create(resource.getNotes());
+ SubmitTypeRecord rec = evaluator.getSubmitType(cd);
+
+ if (rec.status != SubmitTypeRecord.Status.OK) {
+ throw new ResourceConflictException(String.format("rule produced invalid result: %s", rec));
+ }
+
+ return Response.ok(rec.type);
}
}
}
diff --git a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
index 8401c1d..ee997b2 100644
--- a/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
+++ b/java/com/google/gerrit/server/rules/DefaultSubmitRule.java
@@ -26,7 +26,6 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -63,7 +62,7 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// In case at least one project has a rules.pl file, we let Prolog handle it.
diff --git a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
index 4695800..ff5d99e 100644
--- a/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
+++ b/java/com/google/gerrit/server/rules/IgnoreSelfApprovalRule.java
@@ -28,7 +28,6 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -60,7 +59,7 @@
IgnoreSelfApprovalRule() {}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
List<LabelType> labelTypes;
List<PatchSetApproval> approvals;
try {
diff --git a/java/com/google/gerrit/server/rules/PrologOptions.java b/java/com/google/gerrit/server/rules/PrologOptions.java
new file mode 100644
index 0000000..da9b3ab
--- /dev/null
+++ b/java/com/google/gerrit/server/rules/PrologOptions.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.rules;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import java.util.Optional;
+
+@AutoValue
+public abstract class PrologOptions {
+ public static PrologOptions defaultOptions() {
+ return new AutoValue_PrologOptions.Builder().logErrors(true).skipFilters(false).build();
+ }
+
+ public static PrologOptions dryRunOptions(String ruleToTest, boolean skipFilters) {
+ return new AutoValue_PrologOptions.Builder()
+ .logErrors(false)
+ .skipFilters(skipFilters)
+ .rule(ruleToTest)
+ .build();
+ }
+
+ /** Whether errors should be logged. */
+ abstract boolean logErrors();
+
+ /** Whether Prolog filters from parent projects should be skipped. */
+ abstract boolean skipFilters();
+
+ /**
+ * Prolog rule that should be run. If not given, the Prolog rule that is configured for the
+ * project is used (the rule from rules.pl in refs/meta/config).
+ */
+ abstract Optional<String> rule();
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract PrologOptions.Builder logErrors(boolean logErrors);
+
+ abstract PrologOptions.Builder skipFilters(boolean skipFilters);
+
+ abstract PrologOptions.Builder rule(@Nullable String rule);
+
+ abstract PrologOptions build();
+ }
+}
diff --git a/java/com/google/gerrit/server/rules/PrologRule.java b/java/com/google/gerrit/server/rules/PrologRule.java
index 0c54f40..e15b4b5 100644
--- a/java/com/google/gerrit/server/rules/PrologRule.java
+++ b/java/com/google/gerrit/server/rules/PrologRule.java
@@ -18,7 +18,6 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -37,21 +36,29 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions opts) {
+ public Collection<SubmitRecord> evaluate(ChangeData cd) {
ProjectState projectState = projectCache.get(cd.project());
// We only want to run the Prolog engine if we have at least one rules.pl file to use.
- if ((projectState == null || !projectState.hasPrologRules()) && opts.rule() == null) {
+ if ((projectState == null || !projectState.hasPrologRules())) {
return Collections.emptyList();
}
+ return evaluate(cd, PrologOptions.defaultOptions());
+ }
+
+ public Collection<SubmitRecord> evaluate(ChangeData cd, PrologOptions opts) {
return getEvaluator(cd, opts).evaluate();
}
- private PrologRuleEvaluator getEvaluator(ChangeData cd, SubmitRuleOptions opts) {
- return factory.create(cd, opts);
+ public SubmitTypeRecord getSubmitType(ChangeData cd) {
+ return getSubmitType(cd, PrologOptions.defaultOptions());
}
- public SubmitTypeRecord getSubmitType(ChangeData cd, SubmitRuleOptions opts) {
+ public SubmitTypeRecord getSubmitType(ChangeData cd, PrologOptions opts) {
return getEvaluator(cd, opts).getSubmitType();
}
+
+ private PrologRuleEvaluator getEvaluator(ChangeData cd, PrologOptions opts) {
+ return factory.create(cd, opts);
+ }
}
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index c036c86..7f6450d 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RuleEvalException;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -73,7 +72,7 @@
public interface Factory {
/** Returns a new {@link PrologRuleEvaluator} with the specified options */
- PrologRuleEvaluator create(ChangeData cd, SubmitRuleOptions options);
+ PrologRuleEvaluator create(ChangeData cd, PrologOptions options);
}
/**
@@ -95,7 +94,7 @@
private final PrologEnvironment.Factory envFactory;
private final ChangeData cd;
private final ProjectState projectState;
- private final SubmitRuleOptions opts;
+ private final PrologOptions opts;
private Term submitRule;
@AssistedInject
@@ -107,7 +106,7 @@
PrologEnvironment.Factory envFactory,
ProjectCache projectCache,
@Assisted ChangeData cd,
- @Assisted SubmitRuleOptions options) {
+ @Assisted PrologOptions options) {
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
@@ -159,12 +158,6 @@
return ruleError("Error looking up change " + cd.getId(), e);
}
- if (!opts.allowClosed() && change.isClosed()) {
- SubmitRecord rec = new SubmitRecord();
- rec.status = SubmitRecord.Status.CLOSED;
- return Collections.singletonList(rec);
- }
-
List<Term> results;
try {
results =
@@ -465,22 +458,22 @@
PrologEnvironment env;
try {
PrologMachineCopy pmc;
- if (opts.rule() == null) {
+ if (opts.rule().isPresent()) {
+ pmc = rulesCache.loadMachine("stdin", new StringReader(opts.rule().get()));
+ } else {
pmc =
rulesCache.loadMachine(
projectState.getNameKey(), projectState.getConfig().getRulesId());
- } else {
- pmc = rulesCache.loadMachine("stdin", new StringReader(opts.rule()));
}
env = envFactory.create(pmc);
} catch (CompileException err) {
String msg;
- if (opts.rule() == null) {
+ if (opts.rule().isPresent()) {
+ msg = err.getMessage();
+ } else {
msg =
String.format(
"Cannot load rules.pl for %s: %s", projectState.getName(), err.getMessage());
- } else {
- msg = err.getMessage();
}
throw new RuleEvalException(msg, err);
}
diff --git a/java/com/google/gerrit/server/rules/SubmitRule.java b/java/com/google/gerrit/server/rules/SubmitRule.java
index 2a68683..20cb8fb 100644
--- a/java/com/google/gerrit/server/rules/SubmitRule.java
+++ b/java/com/google/gerrit/server/rules/SubmitRule.java
@@ -15,7 +15,6 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import java.util.Collection;
@@ -40,5 +39,5 @@
@ExtensionPoint
public interface SubmitRule {
/** Returns a {@link Collection} of {@link SubmitRecord} status for the change. */
- Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options);
+ Collection<SubmitRecord> evaluate(ChangeData changeData);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
index f087b78..1842a9ec 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
@@ -26,7 +26,6 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.SubmitRequirementInfo;
import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.Module;
@@ -67,7 +66,7 @@
private static class CustomSubmitRule implements SubmitRule {
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData changeData) {
SubmitRecord record = new SubmitRecord();
record.labels = new ArrayList<>();
record.status = SubmitRecord.Status.NOT_READY;
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 56a9b69..15b9a93 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -46,7 +46,6 @@
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.project.CreateProjectArgs;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
@@ -680,7 +679,7 @@
boolean failOnce;
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options) {
+ public Collection<SubmitRecord> evaluate(ChangeData changeData) {
if (failOnce) {
failOnce = false;
throw new IllegalStateException("forced failure from test");
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
index 83782c9..37237c6 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/IgnoreSelfApprovalRuleIT.java
@@ -22,7 +22,6 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.rules.IgnoreSelfApprovalRule;
import com.google.inject.Inject;
import java.util.Collection;
@@ -42,8 +41,7 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).hasSize(1);
SubmitRecord result = submitRecords.iterator().next();
@@ -69,8 +67,7 @@
// Approve as admin
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).isEmpty();
}
@@ -81,8 +78,7 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
- Collection<SubmitRecord> submitRecords =
- rule.evaluate(r.getChange(), SubmitRuleOptions.defaults());
+ Collection<SubmitRecord> submitRecords = rule.evaluate(r.getChange());
assertThat(submitRecords).isEmpty();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
index c6f2024..efc3b5b 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/PrologRuleEvaluatorIT.java
@@ -21,8 +21,8 @@
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.rules.PrologOptions;
import com.google.gerrit.server.rules.PrologRuleEvaluator;
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
@@ -156,6 +156,6 @@
}
private PrologRuleEvaluator makeEvaluator() {
- return evaluatorFactory.create(makeChangeData(), SubmitRuleOptions.defaults());
+ return evaluatorFactory.create(makeChangeData(), PrologOptions.defaultOptions());
}
}
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 841e3d8..05bbb5b 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
@@ -60,6 +60,7 @@
};
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
+ const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
Polymer({
@@ -1388,15 +1389,17 @@
/**
* Reload the change.
- * @param {boolean=} opt_reloadRelatedChanges Reloads the related chanegs
- * when true.
+ * @param {boolean=} opt_isLocationChange Reloads the related changes
+ * when true and ends reporting events that started on location change.
* @return {Promise} A promise that resolves when the core data has loaded.
* Some non-core data loading may still be in-flight when the core data
* promise resolves.
*/
- _reload(opt_reloadRelatedChanges) {
+ _reload(opt_isLocationChange) {
this._loading = true;
this._relatedChangesCollapsed = true;
+ this.$.reporting.time(CHANGE_RELOAD_TIMING_LABEL);
+ this.$.reporting.time(CHANGE_DATA_TIMING_LABEL);
// Array to house all promises related to data requests.
const allDataPromises = [];
@@ -1410,7 +1413,12 @@
// change content may start appearing.
const loadingFlagSet = detailCompletes
.then(() => { this._loading = false; })
- .then(() => { this.$.reporting.changeDisplayed(); });
+ .then(() => {
+ this.$.reporting.timeEnd(CHANGE_RELOAD_TIMING_LABEL);
+ if (opt_isLocationChange) {
+ this.$.reporting.changeDisplayed();
+ }
+ });
// Resolves when the project config has loaded.
const projectConfigLoaded = detailCompletes
@@ -1470,16 +1478,17 @@
coreDataPromise = mergeabilityLoaded;
}
- if (opt_reloadRelatedChanges) {
+ if (opt_isLocationChange) {
const relatedChangesLoaded = coreDataPromise
.then(() => this.$.relatedChanges.reload());
allDataPromises.push(relatedChangesLoaded);
}
- this.$.reporting.time(CHANGE_DATA_TIMING_LABEL);
Promise.all(allDataPromises).then(() => {
this.$.reporting.timeEnd(CHANGE_DATA_TIMING_LABEL);
- this.$.reporting.changeFullyLoaded();
+ if (opt_isLocationChange) {
+ this.$.reporting.changeFullyLoaded();
+ }
});
return coreDataPromise;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index e8de93b..79d8d93 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1844,5 +1844,50 @@
MockInteractions.tap(element.$.changeStar.$$('button'));
assert.isTrue(stub.called);
});
+
+ suite('gr-reporting tests', () => {
+ setup(() => {
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 1,
+ };
+ sandbox.stub(element, '_getChangeDetail').returns(Promise.resolve());
+ sandbox.stub(element, '_getProjectConfig').returns(Promise.resolve());
+ sandbox.stub(element, '_reloadComments').returns(Promise.resolve());
+ sandbox.stub(element, '_getMergeability').returns(Promise.resolve());
+ sandbox.stub(element, '_getLatestCommitMessage')
+ .returns(Promise.resolve());
+ });
+
+ test('don\'t report changedDisplayed on reply', done => {
+ const changeDisplayStub =
+ sandbox.stub(element.$.reporting, 'changeDisplayed');
+ const changeFullyLoadedStub =
+ sandbox.stub(element.$.reporting, 'changeFullyLoaded');
+ element._handleReplySent();
+ flush(() => {
+ assert.isFalse(changeDisplayStub.called);
+ assert.isFalse(changeFullyLoadedStub.called);
+ done();
+ });
+ });
+
+ test('report changedDisplayed on _paramsChanged', done => {
+ const changeDisplayStub =
+ sandbox.stub(element.$.reporting, 'changeDisplayed');
+ const changeFullyLoadedStub =
+ sandbox.stub(element.$.reporting, 'changeFullyLoaded');
+ element._paramsChanged({
+ view: Gerrit.Nav.View.CHANGE,
+ changeNum: 101,
+ project: 'test-project',
+ });
+ flush(() => {
+ assert.isTrue(changeDisplayStub.called);
+ assert.isTrue(changeFullyLoadedStub.called);
+ done();
+ });
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
index 7f71c89..2f2c7ba 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
@@ -40,14 +40,8 @@
:host([show-avatar]) .container {
padding-left: 0;
}
- gr-button.remove:hover,
- gr-button.remove:focus {
- --gr-button: {
- color: #333;
- }
- }
gr-button.remove {
- --gr-button: {
+ --gr-remove-button-style: {
border: 0;
color: var(--deemphasized-text-color);
font-size: 1.7rem;
@@ -59,6 +53,19 @@
text-decoration: none;
}
}
+
+ gr-button.remove:hover,
+ gr-button.remove:focus {
+ --gr-button: {
+ @apply --gr-remove-button-style
+ color: #333;
+ }
+ }
+ gr-button.remove {
+ --gr-button: {
+ @apply --gr-remove-button-style
+ }
+ }
:host:focus {
border-color: transparent;
box-shadow: none;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
index b74e48c..46877d7 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
@@ -76,14 +76,31 @@
on-focus="_onInputFocus"
on-blur="_onInputBlur"
autocomplete="off">
- <!-- slot is for future use (2.x) while prefix attribute is for 1.x
- (current) -->
- <iron-icon
- icon="gr-icons:search"
- slot="prefix"
+
+ <template is="dom-if" if="[[_isPolymer2()]]">
+ <!-- Content uses vertical-align:baseline. If iron-icon is placed
+ directly in the slot and is not visible, vertical-align doesn't
+ work, because display:none convert it from inline-block element to
+ block element. To fix this problem, iron-icon is wrapped in div
+ which never changes display type.
+ The problem doesn't exist in Polymer1 because DOM is different.
+ Applying the same fix to Polymer1 breaks vertical-align.
+ -->
+ <div slot="prefix">
+ <iron-icon
+ icon="gr-icons:search"
+ class$="searchIcon [[_computeShowSearchIconClass(showSearchIcon)]]">
+ </iron-icon>
+ </div>
+ </template>
+ <template is="dom-if" if="[[!_isPolymer2()]]">
+ <iron-icon
prefix
+ icon="gr-icons:search"
class$="searchIcon [[_computeShowSearchIconClass(showSearchIcon)]]">
- </iron-icon>
+ </iron-icon>
+ </template>
+
</paper-input>
<gr-autocomplete-dropdown
vertical-align="top"
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 34f1ff9..af31473 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -425,5 +425,9 @@
_computeShowSearchIconClass(showSearchIcon) {
return showSearchIcon ? 'showSearchIcon' : '';
},
+
+ _isPolymer2() {
+ return window.POLYMER2;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 217321f..cff35b4 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -320,12 +320,15 @@
});
});
- test('search icon shows with showSearchIcon property', () => {
- assert.equal(getComputedStyle(element.$$('iron-icon')).display,
- 'none');
- element.showSearchIcon = true;
- assert.notEqual(getComputedStyle(element.$$('iron-icon')).display,
- 'none');
+ test('search icon shows with showSearchIcon property', done => {
+ flush(() => {
+ assert.equal(getComputedStyle(element.$$('iron-icon')).display,
+ 'none');
+ element.showSearchIcon = true;
+ assert.notEqual(getComputedStyle(element.$$('iron-icon')).display,
+ 'none');
+ done();
+ });
});
test('vertical offset overridden by param if it exists', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
index 754c3c0..340297f 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -39,6 +39,37 @@
text-transform: none;
}
paper-button {
+ /* The next lines contains a copy of paper-button style.
+ Without a copy, the @apply works incorrectly with Polymer 2.
+ @apply is deprecated and is not recommended to use. It is expected
+ that @apply will be replaced with the ::part CSS pseudo-element.
+ After replacecment copied lines can be removed.
+ */
+ @apply --layout-inline;
+ @apply --layout-center-center;
+ position: relative;
+ box-sizing: border-box;
+ min-width: 5.14em;
+ margin: 0 0.29em;
+ background: transparent;
+ -webkit-tap-highlight-color: rgba(0, 0, 0, 0);
+ -webkit-tap-highlight-color: transparent;
+ font: inherit;
+ text-transform: uppercase;
+ outline-width: 0;
+ border-radius: 3px;
+ -moz-user-select: none;
+ -ms-user-select: none;
+ -webkit-user-select: none;
+ user-select: none;
+ cursor: pointer;
+ z-index: 0;
+ padding: 0.7em 0.57em;
+
+ @apply --paper-font-common-base;
+ @apply --paper-button;
+ /* End of copy*/
+
/* paper-button sets this to anti-aliased, which appears different than
bold font elsewhere on macOS. */
-webkit-font-smoothing: initial;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.html b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.html
index f6332f4..939d582 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.html
@@ -37,14 +37,8 @@
display: inline-flex;
padding: 0 .5em;
}
- gr-button.remove:hover,
- gr-button.remove:focus {
- --gr-button: {
- color: #333;
- }
- }
gr-button.remove {
- --gr-button: {
+ --gr-remove-button-style: {
border: 0;
color: var(--deemphasized-text-color);
font-size: 1.7rem;
@@ -56,6 +50,19 @@
text-decoration: none;
}
}
+
+ gr-button.remove:hover,
+ gr-button.remove:focus {
+ --gr-button: {
+ @apply --gr-remove-button-style
+ color: #333;
+ }
+ }
+ gr-button.remove {
+ --gr-button: {
+ @apply --gr-remove-button-style
+ }
+ }
.transparentBackground,
gr-button.transparentBackground {
background-color: transparent;
diff --git a/polygerrit-ui/app/styles/gr-voting-styles.html b/polygerrit-ui/app/styles/gr-voting-styles.html
index 3b1ee64..eec79be 100644
--- a/polygerrit-ui/app/styles/gr-voting-styles.html
+++ b/polygerrit-ui/app/styles/gr-voting-styles.html
@@ -23,6 +23,7 @@
border: 1px solid rgba(0,0,0,.12);
border-radius: 1em;
box-shadow: none;
+ box-sizing: border-box;
min-width: 3em;
}
}
