opendev/gerrit
Code Issues Proposed changes

SubmitWholeTopic: Use exact topic

Browse Source 
Searching by topic is prefix based, which breaks the submitWholeTopic
feature as that relies on on searching by the exact topic.

This patch introduces 2 new fields, EXACT_TOPIC and FUZZY_TOPIC, which
will be used for these two different use cases. For grouping by topic
in the server (to show related changes in the same topic and for submitting
the whole topic) EXACT_TOPIC will be used, while FUZZY_TOPIC will be used
for the user search queries.

Renamed LEGACY_TOPIC to LEGACY_TOPIC2, and the current TOPIC
to LEGACY_TOPIC3, so it matches the numbers in ChangeField.java

Change-Id: Ie44608703aa28e1cf0423ac13dccf8e037d300b4
This commit is contained in:
Stefan Beller
2015-05-29 11:37:37 -07:00
parent a030a1beac
commit f89a959ad2
10 changed files with 205 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
Documentation/user-search.txt
View File

@@ -156,15 +156,20 @@ library] is used for evaluation of such patterns.
[[topic]]
topic:'TOPIC'::
+
Changes whose designated topic at upload was 'TOPIC'. This is
often combined with 'branch:' and 'project:' operators to select
all related changes in a series.
Changes whose designated topic contains 'TOPIC', using a full-text search.
+
If 'TOPIC' starts with `^` it matches topic names by regular
expression patterns. The
link:http://www.brics.dk/automaton/[dk.brics.automaton
library] is used for evaluation of such patterns.
[[exacttopic]]
exacttopic:'TOPIC'::
+
Changes whose designated topic matches 'TOPIC' exactly. This is
often combined with 'branch:' and 'project:' operators to select
all related changes in a series.
[[ref]]
ref:'REF'::
+

2
gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
View File

@@ -67,7 +67,7 @@ public class GetRevisionActions implements ETagView<RevisionResource> {
Hasher h = Hashing.md5().newHasher();
CurrentUser user = rsrc.getControl().getCurrentUser();
try {
for (ChangeData c : queryProvider.get().byTopicOpen(topic)) {
for (ChangeData c : queryProvider.get().byExactTopicOpen(topic)) {
new ChangeResource(c.changeControl(), rebaseChange).prepareETag(h, user);
}
} catch (OrmException e){

2
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -697,7 +697,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private List<ChangeData> getChangesByTopic(String topic) {
try {
return queryProvider.get().byTopicOpen(topic);
return queryProvider.get().byExactTopicOpen(topic);
} catch (OrmException e) {
throw new OrmRuntimeException(e);
}

27
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
View File

@@ -149,7 +149,7 @@ public class ChangeField {
@Deprecated
/** Topic, a short annotation on the branch. */
public static final FieldDef<ChangeData, String> LEGACY_TOPIC =
public static final FieldDef<ChangeData, String> LEGACY_TOPIC2 =
new FieldDef.Single<ChangeData, String>(
"topic2", FieldType.EXACT, false) {
@Override
@@ -159,8 +159,9 @@ public class ChangeField {
}
};
@Deprecated
/** Topic, a short annotation on the branch. */
public static final FieldDef<ChangeData, String> TOPIC =
public static final FieldDef<ChangeData, String> LEGACY_TOPIC3 =
new FieldDef.Single<ChangeData, String>(
"topic3", FieldType.PREFIX, false) {
@Override
@@ -170,6 +171,28 @@ public class ChangeField {
}
};
/** Topic, a short annotation on the branch. */
public static final FieldDef<ChangeData, String> EXACT_TOPIC =
new FieldDef.Single<ChangeData, String>(
"topic4", FieldType.EXACT, false) {
@Override
public String get(ChangeData input, FillArgs args)
throws OrmException {
return getTopic(input);
}
};
/** Topic, a short annotation on the branch. */
public static final FieldDef<ChangeData, String> FUZZY_TOPIC =
new FieldDef.Single<ChangeData, String>(
"topic5", FieldType.FULL_TEXT, false) {
@Override
public String get(ChangeData input, FillArgs args)
throws OrmException {
return getTopic(input);
}
};
/** Last update time since January 1, 1970. */
public static final FieldDef<ChangeData, Timestamp> UPDATED =
new FieldDef.Single<ChangeData, Timestamp>(

51
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
View File

@@ -38,7 +38,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.LEGACY_TOPIC,
ChangeField.LEGACY_TOPIC2,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -68,7 +68,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.LEGACY_TOPIC,
ChangeField.LEGACY_TOPIC2,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -96,7 +96,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.LEGACY_TOPIC,
ChangeField.LEGACY_TOPIC2,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -124,7 +124,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.LEGACY_TOPIC,
ChangeField.LEGACY_TOPIC2,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -153,7 +153,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.LEGACY_TOPIC3,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -182,7 +182,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.LEGACY_TOPIC3,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -212,7 +212,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.LEGACY_TOPIC3,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -243,7 +243,7 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.LEGACY_TOPIC3,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
@@ -267,6 +267,7 @@ public class ChangeSchemas {
ChangeField.GROUP,
ChangeField.EDITBY);
@SuppressWarnings("deprecation")
static final Schema<ChangeData> V20 = schema(
ChangeField.LEGACY_ID,
ChangeField.ID,
@@ -274,7 +275,39 @@ public class ChangeSchemas {
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.LEGACY_TOPIC3,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
ChangeField.TR,
ChangeField.LABEL,
ChangeField.COMMIT_MESSAGE,
ChangeField.COMMENT,
ChangeField.CHANGE,
ChangeField.APPROVAL,
ChangeField.MERGEABLE,
ChangeField.ADDED,
ChangeField.DELETED,
ChangeField.DELTA,
ChangeField.HASHTAG,
ChangeField.COMMENTBY,
ChangeField.PATCH_SET,
ChangeField.GROUP,
ChangeField.EDITBY,
ChangeField.REVIEWEDBY);
static final Schema<ChangeData> V21 = schema(
ChangeField.LEGACY_ID,
ChangeField.ID,
ChangeField.STATUS,
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.EXACT_TOPIC,
ChangeField.FUZZY_TOPIC,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,

10
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -477,12 +477,20 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return new HashtagPredicate(hashtag);
}
@Operator
public Predicate<ChangeData> exacttopic(String name) {
return new ExactTopicPredicate(args.getSchema(), name);
}
@Operator
public Predicate<ChangeData> topic(String name) {
if (name.startsWith("^")) {
return new RegexTopicPredicate(args.getSchema(), name);
}
return new TopicPredicate(args.getSchema(), name);
if (name.isEmpty()) {
return new ExactTopicPredicate(args.getSchema(), name);
}
return new TopicPredicate(args.getSchema(), name, args.index);
}
@Operator

63
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactTopicPredicate.java Normal file
View File

@@ -0,0 +1,63 @@
// 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.ChangeField.EXACT_TOPIC;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.index.Schema;
import com.google.gwtorm.server.OrmException;
class ExactTopicPredicate extends IndexPredicate<ChangeData> {
@SuppressWarnings("deprecation")
static FieldDef<ChangeData, ?> topicField(Schema<ChangeData> schema) {
if (schema == null) {
return ChangeField.LEGACY_TOPIC2;
}
if (schema.hasField(EXACT_TOPIC)) {
return schema.getFields().get(EXACT_TOPIC.getName());
}
if (schema.hasField(ChangeField.LEGACY_TOPIC2)) {
return schema.getFields().get(ChangeField.LEGACY_TOPIC2.getName());
}
// Not exact, but we cannot do any better.
return schema.getFields().get(ChangeField.LEGACY_TOPIC3.getName());
}
ExactTopicPredicate(Schema<ChangeData> schema, String topic) {
super(topicField(schema), topic);
}
@Override
public boolean match(final ChangeData object) throws OrmException {
Change change = object.change();
if (change == null) {
return false;
}
String t = change.getTopic();
if (t == null && getField() == ChangeField.LEGACY_TOPIC2) {
t = "";
}
return getValue().equals(t);
}
@Override
public int getCost() {
return 1;
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@@ -86,10 +86,6 @@ public class InternalChangeQuery {
return this;
}
private Predicate<ChangeData> topic(String topic) {
return new TopicPredicate(schema(indexes), topic);
}
public List<ChangeData> byKey(Change.Key key) throws OrmException {
return byKeyPrefix(key.get());
}
@@ -135,9 +131,9 @@ public class InternalChangeQuery {
return query(and(project(project), open()));
}
public List<ChangeData> byTopicOpen(String topic)
public List<ChangeData> byExactTopicOpen(String topic)
throws OrmException {
return query(and(topic(topic), open()));
return query(and(new ExactTopicPredicate(schema(indexes), topic), open()));
}
public List<ChangeData> byCommitPrefix(String prefix) throws OrmException {

50
gerrit-server/src/main/java/com/google/gerrit/server/query/change/TopicPredicate.java
View File

@@ -14,43 +14,67 @@
package com.google.gerrit.server.query.change;
import static com.google.gerrit.server.index.ChangeField.TOPIC;
import static com.google.gerrit.server.index.ChangeField.FUZZY_TOPIC;
import static com.google.gerrit.server.index.ChangeField.LEGACY_TOPIC2;
import static com.google.gerrit.server.index.ChangeField.LEGACY_TOPIC3;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.ChangeIndex;
import com.google.gerrit.server.index.FieldDef;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.index.Schema;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gwtorm.server.OrmException;
class TopicPredicate extends IndexPredicate<ChangeData> {
private final ChangeIndex index;
@SuppressWarnings("deprecation")
static FieldDef<ChangeData, ?> topicField(Schema<ChangeData> schema) {
if (schema == null) {
return ChangeField.LEGACY_TOPIC;
return LEGACY_TOPIC2;
}
FieldDef<ChangeData, ?> f = schema.getFields().get(TOPIC.getName());
if (f != null) {
return f;
if (schema.hasField(FUZZY_TOPIC)) {
return schema.getFields().get(FUZZY_TOPIC.getName());
}
return schema.getFields().get(ChangeField.LEGACY_TOPIC.getName());
if (schema.hasField(LEGACY_TOPIC3)) {
return schema.getFields().get(LEGACY_TOPIC3.getName());
}
return schema.getFields().get(LEGACY_TOPIC2.getName());
}
TopicPredicate(Schema<ChangeData> schema, String topic) {
TopicPredicate(Schema<ChangeData> schema, String topic, ChangeIndex index) {
super(topicField(schema), topic);
this.index = index;
}
@SuppressWarnings("deprecation")
@Override
public boolean match(final ChangeData object) throws OrmException {
Change change = object.change();
public boolean match(final ChangeData cd) throws OrmException {
Change change = cd.change();
if (change == null) {
return false;
}
String t = change.getTopic();
if (t == null && getField() == TOPIC) {
t = "";
if (t == null) {
return false;
}
return getValue().equals(t);
if (getField() == FUZZY_TOPIC || getField() == LEGACY_TOPIC3) {
try {
Predicate<ChangeData> thisId = new LegacyChangeIdPredicate(cd.getId());
Iterable<ChangeData> results = index.getSource(and(thisId, this), 0, 1).read();
return !Iterables.isEmpty(results);
} catch (QueryParseException e) {
throw new OrmException(e);
}
}
if (getField() == LEGACY_TOPIC2) {
return t.equals(getValue());
}
return false;
}
@Override

21
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -441,13 +441,26 @@ public abstract class AbstractQueryChangesTest {
change2.setTopic("feature2");
ins2.insert();
Change change3 = newChange(repo, null, null, null, null).insert();
ChangeInserter ins3 = newChange(repo, null, null, null, null);
Change change3 = ins3.getChange();
change3.setTopic("Cherrypick-feature2");
ins3.insert();
ChangeInserter ins4 = newChange(repo, null, null, null, null);
Change change4 = ins4.getChange();
change4.setTopic("feature2-fixup");
ins4.insert();
Change change5 = newChange(repo, null, null, null, null).insert();
assertQuery("topic:foo");
assertQuery("topic:feature1", change1);
assertQuery("topic:feature2", change2);
assertQuery("topic:feature", change2, change1);
assertQuery("topic:\"\"", change3, change2, change1);
assertQuery("topic:feature2", change4, change3, change2);
assertQuery("exacttopic:feature2", change2);
assertQuery("topic:feature2", change4, change3, change2);
assertQuery("topic:fixup", change4);
assertQuery("exacttopic:\"\"", change5);
assertQuery("topic:\"\"", change5);
}
@Test