Support 'conflicts:<change>' operator to find changes with same files
The 'conflicts=<change>' search operator finds all changes that touch at least one file that was also touched by the given change. If this operator is combined with 'status:open' all changes that potentially conflict with the given change are returned. In future this result may be displayed on the change screen in some kind of change dashboard. In future we may also test for the returned changes whether they actually conflict with the given change, taking the 'automatically resolve conflicts' setting of the project into account. Then it would be possible to show all conflicting changes on the change screen. Change-Id: I637255b131466bac09338e08039d5fd171f71431 Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
@@ -74,6 +74,14 @@ change:'ID'::
|
||||
Either a legacy numerical 'ID' such as 15183, or a newer style
|
||||
Change-Id that was scraped out of the commit message.
|
||||
|
||||
[[conflicts]]
|
||||
conflicts:'ID'::
|
||||
+
|
||||
Changes that potentially conflict with change 'ID' because they touch
|
||||
at least one file that was also touched by change 'ID'. Change 'ID' can
|
||||
be specified as a legacy numerical 'ID' such as 15183, or a newer style
|
||||
Change-Id that was scraped out of the commit message.
|
||||
|
||||
[[owner]]
|
||||
owner:'USER'::
|
||||
+
|
||||
|
@@ -55,10 +55,14 @@ public abstract class AbstractDaemonTest {
|
||||
}
|
||||
|
||||
private void beforeTest(Config cfg, boolean memory) throws Exception {
|
||||
server = GerritServer.start(cfg, memory);
|
||||
server = startServer(cfg, memory);
|
||||
server.getTestInjector().injectMembers(this);
|
||||
}
|
||||
|
||||
protected GerritServer startServer(Config cfg, boolean memory) throws Exception {
|
||||
return GerritServer.start(cfg, memory);
|
||||
}
|
||||
|
||||
private void afterTest() throws Exception {
|
||||
server.stop();
|
||||
}
|
||||
|
@@ -0,0 +1,26 @@
|
||||
// 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.acceptance;
|
||||
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
|
||||
public class AbstractDaemonTestWithSecondaryIndex extends AbstractDaemonTest {
|
||||
|
||||
@Override
|
||||
protected GerritServer startServer(Config cfg, boolean memory)
|
||||
throws Exception {
|
||||
return GerritServer.start(cfg, memory, true);
|
||||
}
|
||||
}
|
@@ -15,10 +15,13 @@
|
||||
package com.google.gerrit.acceptance;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.gerrit.lucene.LuceneIndexModule;
|
||||
import com.google.gerrit.pgm.Daemon;
|
||||
import com.google.gerrit.pgm.Init;
|
||||
import com.google.gerrit.pgm.Reindex;
|
||||
import com.google.gerrit.server.config.FactoryModule;
|
||||
import com.google.gerrit.server.config.GerritServerConfig;
|
||||
import com.google.gerrit.server.index.ChangeSchemas;
|
||||
import com.google.gerrit.server.util.SocketUtil;
|
||||
import com.google.inject.Injector;
|
||||
import com.google.inject.Key;
|
||||
@@ -48,6 +51,12 @@ public class GerritServer {
|
||||
|
||||
/** Returns fully started Gerrit server */
|
||||
static GerritServer start(Config base, boolean memory) throws Exception {
|
||||
return start(base, memory, false);
|
||||
}
|
||||
|
||||
/** Returns fully started Gerrit server */
|
||||
static GerritServer start(Config base, boolean memory, boolean index)
|
||||
throws Exception {
|
||||
final CyclicBarrier serverStarted = new CyclicBarrier(2);
|
||||
final Daemon daemon = new Daemon(new Runnable() {
|
||||
public void run() {
|
||||
@@ -69,11 +78,25 @@ public class GerritServer {
|
||||
mergeTestConfig(cfg);
|
||||
cfg.setBoolean("httpd", null, "requestLog", false);
|
||||
cfg.setBoolean("sshd", null, "requestLog", false);
|
||||
if (index) {
|
||||
cfg.setString("index", null, "type", "lucene");
|
||||
cfg.setBoolean("index", "lucene", "testInmemory", true);
|
||||
daemon.setLuceneModule(new LuceneIndexModule(
|
||||
ChangeSchemas.getLatest().getVersion(),
|
||||
Runtime.getRuntime().availableProcessors(), null));
|
||||
}
|
||||
daemon.setDatabaseForTesting(ImmutableList.<Module>of(
|
||||
new InMemoryTestingDatabaseModule(cfg)));
|
||||
daemon.start();
|
||||
} else {
|
||||
site = initSite(base);
|
||||
Config cfg = base != null ? base : new Config();
|
||||
if (index) {
|
||||
cfg.setString("index", null, "type", "lucene");
|
||||
}
|
||||
site = initSite(cfg);
|
||||
if (index) {
|
||||
reindex(site);
|
||||
}
|
||||
daemonService = Executors.newSingleThreadExecutor();
|
||||
daemonService.submit(new Callable<Void>() {
|
||||
public Void call() throws Exception {
|
||||
@@ -114,6 +137,15 @@ public class GerritServer {
|
||||
return tmp;
|
||||
}
|
||||
|
||||
/** Runs the reindex command. Works only if the site is not currently running. */
|
||||
private static void reindex(File site) throws Exception {
|
||||
Reindex reindex = new Reindex();
|
||||
int rc = reindex.main(new String[] {"-d", site.getPath()});
|
||||
if (rc != 0) {
|
||||
throw new RuntimeException("Reindex failed");
|
||||
}
|
||||
}
|
||||
|
||||
private static void mergeTestConfig(Config cfg)
|
||||
throws IOException {
|
||||
InetSocketAddress http = newPort();
|
||||
|
@@ -0,0 +1,152 @@
|
||||
// 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.acceptance.rest.change;
|
||||
|
||||
import static com.google.gerrit.acceptance.git.GitUtil.checkout;
|
||||
import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
|
||||
import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import com.google.common.base.Function;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTestWithSecondaryIndex;
|
||||
import com.google.gerrit.acceptance.AccountCreator;
|
||||
import com.google.gerrit.acceptance.RestResponse;
|
||||
import com.google.gerrit.acceptance.RestSession;
|
||||
import com.google.gerrit.acceptance.SshSession;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.acceptance.git.GitUtil;
|
||||
import com.google.gerrit.acceptance.git.PushOneCommit;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gson.Gson;
|
||||
import com.google.gson.reflect.TypeToken;
|
||||
import com.google.gwtorm.server.SchemaFactory;
|
||||
import com.google.inject.Inject;
|
||||
|
||||
import com.jcraft.jsch.JSchException;
|
||||
|
||||
import org.apache.http.HttpStatus;
|
||||
import org.eclipse.jgit.api.Git;
|
||||
import org.eclipse.jgit.api.errors.GitAPIException;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Set;
|
||||
|
||||
public class ConflictsOperatorIT extends AbstractDaemonTestWithSecondaryIndex {
|
||||
|
||||
@Inject
|
||||
private AccountCreator accounts;
|
||||
|
||||
@Inject
|
||||
private SchemaFactory<ReviewDb> reviewDbProvider;
|
||||
|
||||
private TestAccount admin;
|
||||
private RestSession session;
|
||||
private Project.NameKey project;
|
||||
private ReviewDb db;
|
||||
private int count;
|
||||
|
||||
@Before
|
||||
public void setUp() throws Exception {
|
||||
admin = accounts.admin();
|
||||
session = new RestSession(server, admin);
|
||||
initSsh(admin);
|
||||
|
||||
project = new Project.NameKey("p");
|
||||
|
||||
db = reviewDbProvider.open();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void noConflictingChanges() throws JSchException, IOException,
|
||||
GitAPIException {
|
||||
Git git = createProject();
|
||||
PushOneCommit.Result change = createChange(git, true);
|
||||
createChange(git, false);
|
||||
|
||||
Set<String> changes = queryConflictingChanges(change);
|
||||
assertEquals(0, changes.size());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void conflictingChanges() throws JSchException, IOException,
|
||||
GitAPIException {
|
||||
Git git = createProject();
|
||||
PushOneCommit.Result change = createChange(git, true);
|
||||
PushOneCommit.Result conflictingChange1 = createChange(git, true);
|
||||
PushOneCommit.Result conflictingChange2 = createChange(git, true);
|
||||
createChange(git, false);
|
||||
|
||||
Set<String> changes = queryConflictingChanges(change);
|
||||
assertChanges(changes, conflictingChange1, conflictingChange2);
|
||||
}
|
||||
|
||||
private Git createProject() throws JSchException, IOException,
|
||||
GitAPIException {
|
||||
SshSession sshSession = new SshSession(server, admin);
|
||||
try {
|
||||
GitUtil.createProject(sshSession, project.get(), null, true);
|
||||
return cloneProject(sshSession.getUrl() + "/" + project.get());
|
||||
} finally {
|
||||
sshSession.close();
|
||||
}
|
||||
}
|
||||
|
||||
private PushOneCommit.Result createChange(Git git, boolean conflicting)
|
||||
throws GitAPIException, IOException {
|
||||
checkout(git, "origin/master");
|
||||
String file = conflicting ? "test.txt" : "test-" + count + ".txt";
|
||||
PushOneCommit push =
|
||||
new PushOneCommit(db, admin.getIdent(), "Change " + count, file,
|
||||
"content " + count);
|
||||
count++;
|
||||
return push.to(git, "refs/for/master");
|
||||
}
|
||||
|
||||
private Set<String> queryConflictingChanges(PushOneCommit.Result change)
|
||||
throws IOException {
|
||||
RestResponse r =
|
||||
session.get("/changes/?q=conflicts:" + change.getChangeId());
|
||||
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
|
||||
Set<ChangeInfo> changes =
|
||||
(new Gson()).fromJson(r.getReader(),
|
||||
new TypeToken<Set<ChangeInfo>>() {}.getType());
|
||||
r.consume();
|
||||
return ImmutableSet.copyOf(Iterables.transform(changes,
|
||||
new Function<ChangeInfo, String>() {
|
||||
@Override
|
||||
public String apply(ChangeInfo input) {
|
||||
return input.id;
|
||||
}
|
||||
}));
|
||||
}
|
||||
|
||||
private void assertChanges(Set<String> actualChanges,
|
||||
PushOneCommit.Result... expectedChanges) {
|
||||
assertEquals(expectedChanges.length, actualChanges.size());
|
||||
for (PushOneCommit.Result c : expectedChanges) {
|
||||
assertTrue(actualChanges.contains(id(c)));
|
||||
}
|
||||
}
|
||||
|
||||
private String id(PushOneCommit.Result change) {
|
||||
return project.get() + "~master~" + change.getChangeId();
|
||||
}
|
||||
}
|
@@ -80,6 +80,7 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
|
||||
|
||||
suggestions.add("commit:");
|
||||
suggestions.add("comment:");
|
||||
suggestions.add("conflicts:");
|
||||
suggestions.add("project:");
|
||||
suggestions.add("branch:");
|
||||
suggestions.add("topic:");
|
||||
@@ -89,7 +90,6 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
|
||||
suggestions.add("label:");
|
||||
suggestions.add("message:");
|
||||
suggestions.add("file:");
|
||||
|
||||
suggestions.add("has:");
|
||||
suggestions.add("has:draft");
|
||||
suggestions.add("has:star");
|
||||
|
@@ -138,6 +138,7 @@ public class Daemon extends SiteProgram {
|
||||
private Injector httpdInjector;
|
||||
private File runFile;
|
||||
private boolean test;
|
||||
private AbstractModule luceneModule;
|
||||
|
||||
private Runnable serverStarted;
|
||||
|
||||
@@ -249,6 +250,12 @@ public class Daemon extends SiteProgram {
|
||||
headless = true;
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
public void setLuceneModule(LuceneIndexModule m) {
|
||||
luceneModule = m;
|
||||
test = true;
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
public void start() {
|
||||
if (dbInjector == null) {
|
||||
@@ -303,7 +310,7 @@ public class Daemon extends SiteProgram {
|
||||
AbstractModule changeIndexModule;
|
||||
switch (IndexModule.getIndexType(cfgInjector)) {
|
||||
case LUCENE:
|
||||
changeIndexModule = new LuceneIndexModule();
|
||||
changeIndexModule = luceneModule != null ? luceneModule : new LuceneIndexModule();
|
||||
break;
|
||||
case SOLR:
|
||||
changeIndexModule = new SolrIndexModule();
|
||||
|
@@ -84,6 +84,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
public static final String FIELD_CHANGE = "change";
|
||||
public static final String FIELD_COMMENT = "comment";
|
||||
public static final String FIELD_COMMIT = "commit";
|
||||
public static final String FIELD_CONFLICTS = "conflicts";
|
||||
public static final String FIELD_DRAFTBY = "draftby";
|
||||
public static final String FIELD_FILE = "file";
|
||||
public static final String FIELD_IS = "is";
|
||||
@@ -218,10 +219,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
.parse(query));
|
||||
|
||||
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
|
||||
if (query.charAt(0) == 'i') {
|
||||
query = "I" + query.substring(1);
|
||||
}
|
||||
return new ChangeIdPredicate(args.dbProvider, query);
|
||||
return new ChangeIdPredicate(args.dbProvider, parseChangeId(query));
|
||||
}
|
||||
|
||||
throw new IllegalArgumentException();
|
||||
@@ -307,6 +305,14 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
.fromString(id));
|
||||
}
|
||||
|
||||
@Operator
|
||||
public Predicate<ChangeData> conflicts(String value) throws OrmException,
|
||||
QueryParseException {
|
||||
requireIndex(FIELD_CONFLICTS, value);
|
||||
return new ConflictsPredicate(args.dbProvider, args.patchListCache, value,
|
||||
parseChange(value));
|
||||
}
|
||||
|
||||
@Operator
|
||||
public Predicate<ChangeData> project(String name) {
|
||||
if (name.startsWith("^"))
|
||||
@@ -665,6 +671,31 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
|
||||
return g;
|
||||
}
|
||||
|
||||
private List<Change> parseChange(String value) throws OrmException,
|
||||
QueryParseException {
|
||||
if (PAT_LEGACY_ID.matcher(value).matches()) {
|
||||
return Collections.singletonList(args.dbProvider.get().changes()
|
||||
.get(Change.Id.parse(value)));
|
||||
} else if (PAT_CHANGE_ID.matcher(value).matches()) {
|
||||
Change.Key a = new Change.Key(parseChangeId(value));
|
||||
List<Change> changes =
|
||||
args.dbProvider.get().changes().byKeyRange(a, a.max()).toList();
|
||||
if (changes.isEmpty()) {
|
||||
throw error("Change " + value + " not found");
|
||||
}
|
||||
return changes;
|
||||
}
|
||||
|
||||
throw error("Change " + value + " not found");
|
||||
}
|
||||
|
||||
private static String parseChangeId(String value) {
|
||||
if (value.charAt(0) == 'i') {
|
||||
value = "I" + value.substring(1);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
private Account.Id self() {
|
||||
if (currentUser.isIdentifiedUser()) {
|
||||
return ((IdentifiedUser) currentUser).getAccountId();
|
||||
|
@@ -0,0 +1,64 @@
|
||||
// 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.common.collect.Lists;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.patch.PatchListCache;
|
||||
import com.google.gerrit.server.query.OrPredicate;
|
||||
import com.google.gerrit.server.query.Predicate;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Provider;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
class ConflictsPredicate extends OrPredicate<ChangeData> {
|
||||
private final String value;
|
||||
|
||||
ConflictsPredicate(Provider<ReviewDb> db, PatchListCache plc, String value,
|
||||
List<Change> changes) throws OrmException {
|
||||
super(predicates(db, plc, changes));
|
||||
this.value = value;
|
||||
}
|
||||
|
||||
private static List<Predicate<ChangeData>> predicates(Provider<ReviewDb> db,
|
||||
PatchListCache plc, List<Change> changes) throws OrmException {
|
||||
List<Predicate<ChangeData>> changePredicates =
|
||||
Lists.newArrayListWithCapacity(changes.size());
|
||||
for (Change c : changes) {
|
||||
List<String> files = new ChangeData(c).currentFilePaths(db, plc);
|
||||
List<Predicate<ChangeData>> filePredicates =
|
||||
Lists.newArrayListWithCapacity(files.size());
|
||||
for (String file : files) {
|
||||
filePredicates.add(new EqualsFilePredicate(db, plc, file));
|
||||
}
|
||||
|
||||
List<Predicate<ChangeData>> predicatesForOneChange =
|
||||
Lists.newArrayListWithCapacity(2);
|
||||
predicatesForOneChange.add(
|
||||
not(new LegacyChangeIdPredicate(db, c.getId())));
|
||||
predicatesForOneChange.add(or(filePredicates));
|
||||
|
||||
changePredicates.add(and(predicatesForOneChange));
|
||||
}
|
||||
return changePredicates;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return ChangeQueryBuilder.FIELD_CONFLICTS + ":" + value;
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user