Merge "A11y - add headings for comments"
diff --git a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
index 780a24f..fca1b36 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -48,9 +48,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeIdPredicate;
import com.google.gerrit.server.query.change.ChangePredicates;
-import com.google.gerrit.server.query.change.CommitPredicate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -230,11 +228,11 @@
}
// Find changes that have a matching Change-Id.
- predicates.add(new ChangeIdPredicate(changeId));
+ predicates.add(ChangePredicates.idPrefix(changeId));
});
// Find changes that have a matching commit.
- predicates.add(new CommitPredicate(commit.name()));
+ predicates.add(ChangePredicates.commitPrefix(commit.name()));
}
if (!predicates.isEmpty()) {
diff --git a/java/com/google/gerrit/server/query/change/AuthorPredicate.java b/java/com/google/gerrit/server/query/change/AuthorPredicate.java
deleted file mode 100644
index 6e362ad..0000000
--- a/java/com/google/gerrit/server/query/change/AuthorPredicate.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// 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.
-
-package com.google.gerrit.server.query.change;
-
-import static com.google.gerrit.server.index.change.ChangeField.AUTHOR;
-import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_AUTHOR;
-
-import com.google.gerrit.server.index.change.ChangeField;
-
-public class AuthorPredicate extends ChangeIndexPredicate {
- public AuthorPredicate(String value) {
- super(AUTHOR, FIELD_AUTHOR, value.toLowerCase());
- }
-
- @Override
- public boolean match(ChangeData object) {
- return ChangeField.getAuthorParts(object).contains(getValue().toLowerCase());
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java
deleted file mode 100644
index f06d1f2..0000000
--- a/java/com/google/gerrit/server/query/change/ChangeIdPredicate.java
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright (C) 2010 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.query.change;
-
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.server.index.change.ChangeField;
-
-/** Predicate over Change-Id strings (aka Change.Key). */
-public class ChangeIdPredicate extends ChangeIndexPredicate {
- public ChangeIdPredicate(String id) {
- super(ChangeField.ID, ChangeQueryBuilder.FIELD_CHANGE, id);
- }
-
- @Override
- public boolean match(ChangeData cd) {
- Change change = cd.change();
- if (change == null) {
- return false;
- }
-
- String key = change.getKey().get();
- if (key.equals(getValue()) || key.startsWith(getValue())) {
- return true;
- }
- return false;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
index cce82f4..c4ca460 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIndexPredicate.java
@@ -14,6 +14,12 @@
package com.google.gerrit.server.query.change;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.common.primitives.Ints;
import com.google.common.primitives.Longs;
import com.google.gerrit.index.FieldDef;
@@ -22,11 +28,21 @@
import com.google.gerrit.index.query.Matchable;
import com.google.gerrit.index.query.Predicate;
import java.util.Objects;
+import java.util.Set;
+import java.util.stream.StreamSupport;
/** Predicate that is mapped to a field in the change index. */
public class ChangeIndexPredicate extends IndexPredicate<ChangeData>
implements Matchable<ChangeData> {
/**
+ * Text segmentation to be applied to both the query string and the indexed field for full-text
+ * queries. This is inspired by http://unicode.org/reports/tr29/ which is what Lucene uses, but
+ * complexity was reduced to the bare minimum at the cost of small discrepancies to the Unicode
+ * spec.
+ */
+ private static final Splitter FULL_TEXT_SPLITTER = Splitter.on(CharMatcher.anyOf(" ,.-\\/_\n"));
+
+ /**
* Returns an index predicate that matches no changes in the index.
*
* <p>This predicate should be used in preference to a non-index predicate (such as {@code
@@ -46,6 +62,21 @@
super(def, name, value);
}
+ /**
+ * This method matches documents without calling an index subsystem. For primitive fields (e.g.
+ * integer, long) , the matching logic is consistent across this method and all known index
+ * implementations. For text fields (i.e. prefix and full-text) the semantics vary between this
+ * implementation and known index implementations:
+ * <li>Prefix: Lucene as well as {@link #match(ChangeData)} matches terms as true prefixes
+ * (prefix:foo -> `foo bar` matches, but `baz foo bar` does not match). The index
+ * implementation at Google tokenizes both the query and the indexed text and matches tokens
+ * individually (prefix:fo ba -> `baz foo bar` matches).
+ * <li>Full text: Lucene uses a {@code PhraseQuery} to search for terms in full text fields
+ * in-order. The index implementation at Google as well as {@link #match(ChangeData)}
+ * tokenizes both the query and the indexed text and matches tokens individually.
+ *
+ * @return true if the predicate matches the provided {@link ChangeData}.
+ */
@Override
public boolean match(ChangeData cd) {
if (getField().isRepeatable()) {
@@ -74,7 +105,26 @@
return Objects.equals(fieldValueFromObject, value);
} else if (fieldTypeName.equals(FieldType.LONG.getName())) {
return Objects.equals(fieldValueFromObject, Longs.tryParse(value));
+ } else if (fieldTypeName.equals(FieldType.PREFIX.getName())) {
+ return String.valueOf(fieldValueFromObject).startsWith(value);
+ } else if (fieldTypeName.equals(FieldType.FULL_TEXT.getName())) {
+ Set<String> tokenizedField = tokenizeString(String.valueOf(fieldValueFromObject));
+ Set<String> tokenizedValue = tokenizeString(value);
+ return Sets.intersection(tokenizedField, tokenizedValue).size() == tokenizedValue.size();
+ } else if (fieldTypeName.equals(FieldType.STORED_ONLY.getName())) {
+ throw new IllegalStateException("can't filter for storedOnly field " + getField().getName());
+ } else if (fieldTypeName.equals(FieldType.TIMESTAMP.getName())) {
+ throw new IllegalStateException("timestamp queries must be handled in subclasses");
+ } else if (fieldTypeName.equals(FieldType.INTEGER_RANGE.getName())) {
+ throw new IllegalStateException("integer range queries must be handled in subclasses");
+ } else {
+ throw new IllegalStateException("unrecognized field " + fieldTypeName);
}
- throw new UnsupportedOperationException("match function must be provided in subclass");
+ }
+
+ private static ImmutableSet<String> tokenizeString(String value) {
+ return StreamSupport.stream(FULL_TEXT_SPLITTER.split(value.toLowerCase()).spliterator(), false)
+ .filter(s -> !s.trim().isEmpty())
+ .collect(toImmutableSet());
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index 7ccc7b6..617002d 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -19,6 +19,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.change.HashtagsUtil;
import com.google.gerrit.server.index.change.ChangeField;
@@ -145,6 +146,11 @@
return new ChangeIndexPredicate(ChangeField.EXACT_TOPIC, topic);
}
+ /** Returns a predicate that matches changes in the provided {@code topic}. */
+ public static Predicate<ChangeData> fuzzyTopic(String topic) {
+ return new ChangeIndexPredicate(ChangeField.FUZZY_TOPIC, topic);
+ }
+
/** Returns a predicate that matches changes submitted in the provided {@code changeSet}. */
public static Predicate<ChangeData> submissionId(String changeSet) {
return new ChangeIndexPredicate(ChangeField.SUBMISSIONID, changeSet);
@@ -162,6 +168,13 @@
ChangeField.HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase());
}
+ /** Returns a predicate that matches changes tagged with the provided {@code hashtag}. */
+ public static Predicate<ChangeData> fuzzyHashtag(String hashtag) {
+ // Use toLowerCase without locale to match behavior in ChangeField.
+ return new ChangeIndexPredicate(
+ ChangeField.FUZZY_HASHTAG, HashtagsUtil.cleanupHashtag(hashtag).toLowerCase());
+ }
+
/** Returns a predicate that matches changes that modified the provided {@code file}. */
public static Predicate<ChangeData> file(ChangeQueryBuilder.Arguments args, String file) {
Predicate<ChangeData> eqPath = path(file);
@@ -204,6 +217,11 @@
return new ChangeIndexPredicate(ChangeField.EXACT_AUTHOR, exactAuthor.toLowerCase(Locale.US));
}
+ /** Returns a predicate that matches changes authored by the provided {@code author}. */
+ public static Predicate<ChangeData> author(String author) {
+ return new ChangeIndexPredicate(ChangeField.AUTHOR, author);
+ }
+
/**
* Returns a predicate that matches changes where the patch set was committed by {@code
* exactCommitter}.
@@ -212,4 +230,52 @@
return new ChangeIndexPredicate(
ChangeField.EXACT_COMMITTER, exactCommitter.toLowerCase(Locale.US));
}
+
+ /**
+ * Returns a predicate that matches changes where the patch set was committed by {@code
+ * committer}.
+ */
+ public static Predicate<ChangeData> committer(String comitter) {
+ return new ChangeIndexPredicate(ChangeField.COMMITTER, comitter.toLowerCase(Locale.US));
+ }
+
+ /** Returns a predicate that matches changes whose ID starts with the provided {@code id}. */
+ public static Predicate<ChangeData> idPrefix(String id) {
+ return new ChangeIndexPredicate(ChangeField.ID, id);
+ }
+
+ /**
+ * Returns a predicate that matches changes in a project that has the provided {@code prefix} in
+ * its name.
+ */
+ public static Predicate<ChangeData> projectPrefix(String prefix) {
+ return new ChangeIndexPredicate(ChangeField.PROJECTS, prefix);
+ }
+
+ /**
+ * Returns a predicate that matches changes where a patch set has the provided {@code commitId}
+ * either as prefix or as full {@link org.eclipse.jgit.lib.ObjectId}.
+ */
+ public static Predicate<ChangeData> commitPrefix(String commitId) {
+ if (commitId.length() == ObjectIds.STR_LEN) {
+ return new ChangeIndexPredicate(ChangeField.EXACT_COMMIT, commitId);
+ }
+ return new ChangeIndexPredicate(ChangeField.COMMIT, commitId);
+ }
+
+ /**
+ * Returns a predicate that matches changes where the provided {@code message} appears in the
+ * commit message. Uses full-text search semantics.
+ */
+ public static Predicate<ChangeData> message(String message) {
+ return new ChangeIndexPredicate(ChangeField.COMMIT_MESSAGE, message);
+ }
+
+ /**
+ * Returns a predicate that matches changes where the provided {@code comment} appears in any
+ * comment on any patch set of the change. Uses full-text search semantics.
+ */
+ public static Predicate<ChangeData> comment(String comment) {
+ return new ChangeIndexPredicate(ChangeField.COMMENT, comment);
+ }
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 1963ca9..94b5442 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -528,7 +528,7 @@
return Predicate.and(
project(triplet.get().project().get()),
branch(triplet.get().branch().branch()),
- new ChangeIdPredicate(parseChangeId(triplet.get().id().get())));
+ ChangePredicates.idPrefix(parseChangeId(triplet.get().id().get())));
}
if (PAT_LEGACY_ID.matcher(query).matches()) {
Integer id = Ints.tryParse(query);
@@ -538,7 +538,7 @@
: ChangePredicates.idStr(Change.id(id));
}
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
- return new ChangeIdPredicate(parseChangeId(query));
+ return ChangePredicates.idPrefix(parseChangeId(query));
}
throw new QueryParseException("Invalid change format");
@@ -546,7 +546,7 @@
@Operator
public Predicate<ChangeData> comment(String value) {
- return new CommentPredicate(args.index, value);
+ return ChangePredicates.comment(value);
}
@Operator
@@ -705,7 +705,7 @@
@Operator
public Predicate<ChangeData> commit(String id) {
- return new CommitPredicate(id);
+ return ChangePredicates.commitPrefix(id);
}
@Operator
@@ -733,7 +733,7 @@
@Operator
public Predicate<ChangeData> projects(String name) {
- return new ProjectPrefixPredicate(name);
+ return ChangePredicates.projectPrefix(name);
}
@Operator
@@ -807,7 +807,7 @@
throw new QueryParseException(
"'inhashtag' operator is not supported by change index version");
}
- return new FuzzyHashtagPredicate(hashtag, args.index);
+ return ChangePredicates.fuzzyHashtag(hashtag);
}
@Operator
@@ -823,7 +823,7 @@
if (name.isEmpty()) {
return ChangePredicates.exactTopic(name);
}
- return new FuzzyTopicPredicate(name, args.index);
+ return ChangePredicates.fuzzyTopic(name);
}
@Operator
@@ -998,7 +998,7 @@
@Operator
public Predicate<ChangeData> message(String text) {
- return new MessagePredicate(args.index, text);
+ return ChangePredicates.message(text);
}
@Operator
@@ -1390,18 +1390,18 @@
public Predicate<ChangeData> author(String who) throws QueryParseException {
if (args.getSchema().hasField(ChangeField.EXACT_AUTHOR)) {
return getAuthorOrCommitterPredicate(
- who.trim(), ChangePredicates::exactAuthor, AuthorPredicate::new);
+ who.trim(), ChangePredicates::exactAuthor, ChangePredicates::author);
}
- return getAuthorOrCommitterFullTextPredicate(who.trim(), AuthorPredicate::new);
+ return getAuthorOrCommitterFullTextPredicate(who.trim(), ChangePredicates::author);
}
@Operator
public Predicate<ChangeData> committer(String who) throws QueryParseException {
if (args.getSchema().hasField(ChangeField.EXACT_COMMITTER)) {
return getAuthorOrCommitterPredicate(
- who.trim(), ChangePredicates::exactCommitter, CommitterPredicate::new);
+ who.trim(), ChangePredicates::exactCommitter, ChangePredicates::committer);
}
- return getAuthorOrCommitterFullTextPredicate(who.trim(), CommitterPredicate::new);
+ return getAuthorOrCommitterFullTextPredicate(who.trim(), ChangePredicates::committer);
}
@Operator
diff --git a/java/com/google/gerrit/server/query/change/CommentPredicate.java b/java/com/google/gerrit/server/query/change/CommentPredicate.java
deleted file mode 100644
index 7463324..0000000
--- a/java/com/google/gerrit/server/query/change/CommentPredicate.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright (C) 2013 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.query.change;
-
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.index.query.Predicate;
-import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.index.change.ChangeIndex;
-import com.google.gerrit.server.index.change.IndexedChangeQuery;
-
-public class CommentPredicate extends ChangeIndexPredicate {
- protected final ChangeIndex index;
-
- public CommentPredicate(ChangeIndex index, String value) {
- super(ChangeField.COMMENT, value);
- this.index = index;
- }
-
- @Override
- public boolean match(ChangeData object) {
- try {
- Change.Id id = object.getId();
- Predicate<ChangeData> p =
- Predicate.and(
- index.getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(id)
- : ChangePredicates.idStr(id),
- this);
- for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
- if (cData.getId().equals(id)) {
- return true;
- }
- }
- } catch (QueryParseException e) {
- throw new StorageException(e);
- }
-
- return false;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/CommitPredicate.java b/java/com/google/gerrit/server/query/change/CommitPredicate.java
deleted file mode 100644
index 2b3b345..0000000
--- a/java/com/google/gerrit/server/query/change/CommitPredicate.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// 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.
-
-package com.google.gerrit.server.query.change;
-
-import static com.google.gerrit.git.ObjectIds.matchesAbbreviation;
-import static com.google.gerrit.server.index.change.ChangeField.COMMIT;
-import static com.google.gerrit.server.index.change.ChangeField.EXACT_COMMIT;
-
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.git.ObjectIds;
-import com.google.gerrit.index.FieldDef;
-
-public class CommitPredicate extends ChangeIndexPredicate {
- static FieldDef<ChangeData, ?> commitField(String id) {
- if (id.length() == ObjectIds.STR_LEN) {
- return EXACT_COMMIT;
- }
- return COMMIT;
- }
-
- public CommitPredicate(String id) {
- super(commitField(id), id);
- }
-
- @Override
- public boolean match(ChangeData object) {
- String id = getValue().toLowerCase();
- for (PatchSet p : object.patchSets()) {
- if (equals(p, id)) {
- return true;
- }
- }
- return false;
- }
-
- protected boolean equals(PatchSet p, String id) {
- if (getField() == EXACT_COMMIT) {
- return p.commitId().name().equals(id);
- }
- return matchesAbbreviation(p.commitId(), id);
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/CommitterPredicate.java b/java/com/google/gerrit/server/query/change/CommitterPredicate.java
deleted file mode 100644
index 65034a2..0000000
--- a/java/com/google/gerrit/server/query/change/CommitterPredicate.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// 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.
-
-package com.google.gerrit.server.query.change;
-
-import static com.google.gerrit.server.index.change.ChangeField.COMMITTER;
-import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_COMMITTER;
-
-import com.google.gerrit.server.index.change.ChangeField;
-
-public class CommitterPredicate extends ChangeIndexPredicate {
- public CommitterPredicate(String value) {
- super(COMMITTER, FIELD_COMMITTER, value.toLowerCase());
- }
-
- @Override
- public boolean match(ChangeData object) {
- return ChangeField.getCommitterParts(object).contains(getValue().toLowerCase());
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/FuzzyHashtagPredicate.java b/java/com/google/gerrit/server/query/change/FuzzyHashtagPredicate.java
deleted file mode 100644
index b4d6b5f..0000000
--- a/java/com/google/gerrit/server/query/change/FuzzyHashtagPredicate.java
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright (C) 2021 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.query.change;
-
-import static com.google.gerrit.server.index.change.ChangeField.FUZZY_HASHTAG;
-
-import com.google.gerrit.server.index.change.ChangeIndex;
-
-public class FuzzyHashtagPredicate extends ChangeIndexPredicate {
- protected final ChangeIndex index;
-
- public FuzzyHashtagPredicate(String hashtag, ChangeIndex index) {
- super(FUZZY_HASHTAG, hashtag.toLowerCase());
- this.index = index;
- }
-
- @Override
- public boolean match(ChangeData cd) {
- return cd.hashtags().stream().anyMatch(ht -> ht.toLowerCase().contains(getValue()));
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java b/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
deleted file mode 100644
index 414f85b..0000000
--- a/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright (C) 2010 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.query.change;
-
-import static com.google.gerrit.server.index.change.ChangeField.FUZZY_TOPIC;
-
-import com.google.common.collect.Iterables;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.index.query.Predicate;
-import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.index.change.ChangeIndex;
-import com.google.gerrit.server.index.change.IndexedChangeQuery;
-
-public class FuzzyTopicPredicate extends ChangeIndexPredicate {
- protected final ChangeIndex index;
-
- public FuzzyTopicPredicate(String topic, ChangeIndex index) {
- super(FUZZY_TOPIC, topic);
- this.index = index;
- }
-
- @Override
- public boolean match(ChangeData cd) {
- Change change = cd.change();
- if (change == null) {
- return false;
- }
- String t = change.getTopic();
- if (t == null) {
- return false;
- }
- try {
- Predicate<ChangeData> thisId =
- index.getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(cd.getId())
- : ChangePredicates.idStr(cd.getId());
- Iterable<ChangeData> results =
- index.getSource(and(thisId, this), IndexedChangeQuery.oneResult()).read();
- return !Iterables.isEmpty(results);
- } catch (QueryParseException e) {
- throw new StorageException(e);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 340b327..76ebd81 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -65,7 +65,7 @@
}
private static Predicate<ChangeData> change(Change.Key key) {
- return new ChangeIdPredicate(key.get());
+ return ChangePredicates.idPrefix(key.get());
}
private static Predicate<ChangeData> project(Project.NameKey project) {
@@ -77,7 +77,7 @@
}
private static Predicate<ChangeData> commit(String id) {
- return new CommitPredicate(id);
+ return ChangePredicates.commitPrefix(id);
}
private final ChangeData.Factory changeDataFactory;
@@ -108,7 +108,7 @@
}
public List<ChangeData> byKeyPrefix(String prefix) {
- return query(new ChangeIdPredicate(prefix));
+ return query(ChangePredicates.idPrefix(prefix));
}
public List<ChangeData> byLegacyChangeId(Change.Id id) {
diff --git a/java/com/google/gerrit/server/query/change/MessagePredicate.java b/java/com/google/gerrit/server/query/change/MessagePredicate.java
deleted file mode 100644
index 3228f96..0000000
--- a/java/com/google/gerrit/server/query/change/MessagePredicate.java
+++ /dev/null
@@ -1,53 +0,0 @@
-// Copyright (C) 2010 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.query.change;
-
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.index.query.Predicate;
-import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.index.change.ChangeIndex;
-import com.google.gerrit.server.index.change.IndexedChangeQuery;
-
-/** Predicate to match changes that contains specified text in commit messages body. */
-public class MessagePredicate extends ChangeIndexPredicate {
- protected final ChangeIndex index;
-
- public MessagePredicate(ChangeIndex index, String value) {
- super(ChangeField.COMMIT_MESSAGE, value);
- this.index = index;
- }
-
- @Override
- public boolean match(ChangeData object) {
- try {
- Predicate<ChangeData> p =
- Predicate.and(
- index.getSchema().useLegacyNumericFields()
- ? ChangePredicates.id(object.getId())
- : ChangePredicates.idStr(object.getId()),
- this);
- for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
- if (cData.getId().equals(object.getId())) {
- return true;
- }
- }
- } catch (QueryParseException e) {
- throw new StorageException(e);
- }
-
- return false;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/ProjectPrefixPredicate.java b/java/com/google/gerrit/server/query/change/ProjectPrefixPredicate.java
deleted file mode 100644
index b89fffe..0000000
--- a/java/com/google/gerrit/server/query/change/ProjectPrefixPredicate.java
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (C) 2014 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.query.change;
-
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.server.index.change.ChangeField;
-
-public class ProjectPrefixPredicate extends ChangeIndexPredicate {
- public ProjectPrefixPredicate(String prefix) {
- super(ChangeField.PROJECTS, prefix);
- }
-
- @Override
- public boolean match(ChangeData object) {
- Change c = object.change();
- return c != null && c.getDest().project().get().startsWith(getValue());
- }
-}
diff --git a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
index 9ba7fa3..ae7f540 100644
--- a/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/CommitsCollection.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.project.Reachable;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangePredicates;
-import com.google.gerrit.server.query.change.CommitPredicate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -152,7 +151,7 @@
ChangePredicates.project(project),
Predicate.or(
Arrays.stream(commit.getParents())
- .map(parent -> new CommitPredicate(parent.getId().getName()))
+ .map(parent -> ChangePredicates.commitPrefix(parent.getId().getName()))
.collect(toImmutableList())));
changes =
retryHelper
diff --git a/java/com/google/gerrit/server/submit/CommitMergeStatus.java b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
index bf8b840..4638bfa 100644
--- a/java/com/google/gerrit/server/submit/CommitMergeStatus.java
+++ b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
@@ -77,7 +77,14 @@
EMPTY_COMMIT(
"Change could not be merged because the commit is empty.\n"
+ "\n"
- + "Project policy requires all commits to contain modifications to at least one file.");
+ + "Project policy requires all commits to contain modifications to at least one file."),
+
+ FAST_FORWARD_INDEPENDENT_CHANGES(
+ "Change could not be merged because the submission has two independent changes "
+ + "with the same destination branch.\n"
+ + "\n"
+ + "Independent changes can't be submitted to the same destination branch with "
+ + "FAST_FORWARD_ONLY submit strategy");
private final String description;
diff --git a/java/com/google/gerrit/server/submit/FastForwardOnly.java b/java/com/google/gerrit/server/submit/FastForwardOnly.java
index 176b063..8a30898 100644
--- a/java/com/google/gerrit/server/submit/FastForwardOnly.java
+++ b/java/com/google/gerrit/server/submit/FastForwardOnly.java
@@ -14,11 +14,15 @@
package com.google.gerrit.server.submit;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.update.RepoContext;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
public class FastForwardOnly extends SubmitStrategy {
FastForwardOnly(SubmitStrategy.Arguments args) {
@@ -28,6 +32,21 @@
@Override
public List<SubmitStrategyOp> buildOps(Collection<CodeReviewCommit> toMerge) {
List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge);
+
+ Map<BranchNameKey, CodeReviewCommit> branchToCommit = new HashMap<>();
+ for (CodeReviewCommit codeReviewCommit : sorted) {
+ BranchNameKey branchNameKey = codeReviewCommit.change().getDest();
+ CodeReviewCommit otherCommitInBranch = branchToCommit.get(branchNameKey);
+ if (otherCommitInBranch == null) {
+ branchToCommit.put(branchNameKey, codeReviewCommit);
+ } else {
+ // we found another change with the same destination branch.
+ codeReviewCommit.setStatusCode(CommitMergeStatus.FAST_FORWARD_INDEPENDENT_CHANGES);
+ otherCommitInBranch.setStatusCode(CommitMergeStatus.FAST_FORWARD_INDEPENDENT_CHANGES);
+ return ImmutableList.of();
+ }
+ }
+
List<SubmitStrategyOp> ops = new ArrayList<>(sorted.size());
CodeReviewCommit newTipCommit =
args.mergeUtil.getFirstFastForward(args.mergeTip.getInitialTip(), args.rw, sorted);
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyListener.java b/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
index b533bebc..59c6b81 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
@@ -134,6 +134,7 @@
case NOT_FAST_FORWARD:
case EMPTY_COMMIT:
case MISSING_DEPENDENCY:
+ case FAST_FORWARD_INDEPENDENT_CHANGES:
// TODO(dborowitz): Reformat these messages to be more appropriate for
// short problem descriptions.
String message = s.getDescription();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
index 66eb48c..58e48e9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -107,6 +108,39 @@
}
@Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void submitTwoIndependentChangesWithFastForwardFail() throws Throwable {
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ PushOneCommit.Result change1 = createChange("subject1", "file1.txt", "content", "topic");
+
+ testRepo.reset(initialHead);
+ PushOneCommit.Result change2 = createChange("subject2", "file2.txt", "content", "topic");
+
+ approve(change1.getChangeId());
+ approve(change2.getChangeId());
+
+ String fastForwardIndependentChangesError =
+ "Change could not be merged because the submission"
+ + " has two independent changes with the same destination branch. Independent changes can't "
+ + "be submitted to the same destination branch with FAST_FORWARD_ONLY submit strategy";
+
+ submitWithConflict(
+ change2.getChangeId(),
+ String.format(
+ "Failed to submit 2 changes due to the following problems:\n"
+ + "Change %d: %s\nChange %d: %s",
+ change1.getChange().getId().get(),
+ fastForwardIndependentChangesError,
+ change2.getChange().getId().get(),
+ fastForwardIndependentChangesError));
+
+ RevCommit updatedHead = projectOperations.project(project).getHead("master");
+ assertThat(updatedHead.getId()).isEqualTo(initialHead.getId());
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
+ }
+
+ @Test
public void submitFastForwardNotPossible_Conflict() throws Throwable {
RevCommit initialHead = projectOperations.project(project).getHead("master");
PushOneCommit.Result change = createChange("Change 1", "a.txt", "content");
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index d351af6..693406c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -429,6 +429,7 @@
></gr-linked-text>
</gr-editable-content>
</div>
+ <h3 class="assistive-tech-only">Comments and Checks Summary</h3>
<gr-change-summary
change-comments="[[_changeComments]]"
comment-threads="[[_commentThreads]]"
@@ -464,7 +465,7 @@
<paper-tab
on-click="_onPaperTabClick"
data-name$="[[_constants.PrimaryTab.FILES]]"
- >Files</paper-tab
+ ><span>Files</span></paper-tab
>
<paper-tab
on-click="_onPaperTabClick"
@@ -482,7 +483,7 @@
<paper-tab
data-name$="[[_constants.PrimaryTab.CHECKS]]"
on-click="_onPaperTabClick"
- >Checks</paper-tab
+ ><span>Checks</span></paper-tab
>
</template>
<template
@@ -503,7 +504,7 @@
data-name$="[[_constants.PrimaryTab.FINDINGS]]"
on-click="_onPaperTabClick"
>
- Findings
+ <span>Findings</span>
</paper-tab>
</paper-tabs>
@@ -563,6 +564,7 @@
is="dom-if"
if="[[_isTabActive(_constants.PrimaryTab.COMMENT_THREADS, _activeTabs)]]"
>
+ <h3 class="assistive-tech-only">Comments</h3>
<gr-thread-list
threads="[[_commentThreads]]"
change="[[_change]]"
@@ -578,6 +580,7 @@
is="dom-if"
if="[[_isTabActive(_constants.PrimaryTab.CHECKS, _activeTabs)]]"
>
+ <h3 class="assistive-tech-only">Checks</h3>
<gr-checks-tab
id="checksTab"
tab-state="[[_tabState.checksTab]]"
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 2408a26..b4b4c19 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -412,16 +412,16 @@
return displayPath;
}
- _computeDisplayLine() {
- if (this.lineNum === FILE) {
+ _computeDisplayLine(lineNum?: LineNumber, range?: CommentRange) {
+ if (lineNum === FILE) {
if (this.path === SpecialFilePath.PATCHSET_LEVEL_COMMENTS) {
return '';
}
return FILE;
}
- if (this.lineNum) return `#${this.lineNum}`;
+ if (lineNum) return `#${lineNum}`;
// If range is set, then lineNum equals the end line of the range.
- if (this.range) return `#${this.range.end_line}`;
+ if (range) return `#${range.end_line}`;
return '';
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index b6e44f6..6ab82b7 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -123,7 +123,7 @@
<template is="dom-if" if="[[!_isPatchsetLevelComment(path)]]">
<a
href$="[[_getDiffUrlForComment(projectName, changeNum, path, patchNum)]]"
- >[[_computeDisplayLine()]]</a
+ >[[_computeDisplayLine(lineNum, range)]]</a
>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 64f5ad8..2eeb12d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -285,15 +285,24 @@
test('_computeDisplayLine', () => {
element.lineNum = 5;
- assert.equal(element._computeDisplayLine(), '#5');
+ assert.equal(
+ element._computeDisplayLine(element.lineNum, element.range),
+ '#5'
+ );
element.path = SpecialFilePath.COMMIT_MESSAGE;
element.lineNum = 5;
- assert.equal(element._computeDisplayLine(), '#5');
+ assert.equal(
+ element._computeDisplayLine(element.lineNum, element.range),
+ '#5'
+ );
element.lineNum = undefined;
element.path = SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
- assert.equal(element._computeDisplayLine(), '');
+ assert.equal(
+ element._computeDisplayLine(element.lineNum, element.range),
+ ''
+ );
});
});
});
