Merge branch 'stable-2.14'

* stable-2.14:
  Changes can't be found when project name matches Change-Id pattern
  EditScreen: Format with google-java-format
  user-review-ui: Fix spelling
  SearchSuggestOracle: Suggest "self" before other users
  DeleteTag{s}IT: Add tests for deleting tag without refs/tags prefix
  DeleteBranch{es}: Fix deletion of branch without refs/heads/ prefix

Change-Id: I6522baa4a48d678e3065ac31ecafe3b82547c9de
This commit is contained in:
David Pursehouse
2017-06-29 15:11:56 +09:00
9 changed files with 72 additions and 33 deletions

View File

@@ -1130,7 +1130,7 @@ Large files that exceed 4000 lines will not be fully rendered.
- [[line-wrapping]]`Line Wrapping`:
+
Controls weather to enable line wrapping or not.
Controls whether to enable line wrapping or not.
+
If `false` is selected then line wrapping is disabled.
This is the default option.

View File

@@ -14,11 +14,13 @@
package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -28,7 +30,6 @@ import com.google.gerrit.reviewdb.client.Branch;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class DeleteBranchIT extends AbstractDaemonTest {
private Branch.NameKey branch;
@@ -86,6 +87,15 @@ public class DeleteBranchIT extends AbstractDaemonTest {
assertDeleteSucceeds();
}
@Test
public void deleteBranchByRestWithoutRefsHeadsPrefix() throws Exception {
grantDelete();
String ref = branch.getShortName();
assertThat(ref).doesNotMatch(R_HEADS);
RestResponse r = userRestSession.delete("/projects/" + project.get() + "/branches/" + ref);
r.assertNoContent();
}
private void blockForcePush() throws Exception {
block("refs/heads/*", Permission.PUSH, ANONYMOUS_USERS).setForce(true);
}

View File

@@ -16,6 +16,8 @@ package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.rest.project.RefAssert.assertRefNames;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
@@ -37,7 +39,7 @@ import org.junit.Test;
@NoHttpd
public class DeleteBranchesIT extends AbstractDaemonTest {
private static final ImmutableList<String> BRANCHES =
ImmutableList.of("refs/heads/test-1", "refs/heads/test-2", "refs/heads/test-3");
ImmutableList.of("refs/heads/test-1", "refs/heads/test-2", "test-3");
@Before
public void setUp() throws Exception {
@@ -138,7 +140,7 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
for (String branch : branches) {
message
.append("Cannot delete ")
.append(branch)
.append(prefixRef(branch))
.append(": it doesn't exist or you do not have permission ")
.append("to delete it\n");
}
@@ -156,17 +158,22 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
private void assertRefUpdatedEvents(HashMap<String, RevCommit> revisions) throws Exception {
for (String branch : revisions.keySet()) {
RevCommit revision = revisions.get(branch);
eventRecorder.assertRefUpdatedEvents(project.get(), branch, null, revision, revision, null);
eventRecorder.assertRefUpdatedEvents(
project.get(), prefixRef(branch), null, revision, revision, null);
}
}
private String prefixRef(String ref) {
return ref.startsWith(R_HEADS) ? ref : R_HEADS + ref;
}
private ProjectApi project() throws Exception {
return gApi.projects().name(project.get());
}
private void assertBranches(List<String> branches) throws Exception {
List<String> expected = Lists.newArrayList("HEAD", RefNames.REFS_CONFIG, "refs/heads/master");
expected.addAll(branches);
expected.addAll(branches.stream().map(b -> prefixRef(b)).collect(toList()));
assertRefNames(expected, project().branches().get());
}

View File

@@ -16,9 +16,10 @@ package com.google.gerrit.acceptance.rest.project;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.TagApi;
import com.google.gerrit.extensions.api.projects.TagInput;
@@ -27,7 +28,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class DeleteTagIT extends AbstractDaemonTest {
private final String TAG = "refs/tags/test";
@@ -82,6 +82,14 @@ public class DeleteTagIT extends AbstractDaemonTest {
assertDeleteSucceeds();
}
@Test
public void deleteTagByRestWithoutRefsTagsPrefix() throws Exception {
grantDelete();
String ref = TAG.substring(R_TAGS.length());
RestResponse r = userRestSession.delete("/projects/" + project.get() + "/tags/" + ref);
r.assertNoContent();
}
private void blockForcePush() throws Exception {
block("refs/tags/*", Permission.PUSH, ANONYMOUS_USERS).setForce(true);
}

View File

@@ -15,6 +15,8 @@
package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
@@ -36,7 +38,7 @@ import org.junit.Test;
@NoHttpd
public class DeleteTagsIT extends AbstractDaemonTest {
private static final ImmutableList<String> TAGS =
ImmutableList.of("refs/tags/test-1", "refs/tags/test-2", "refs/tags/test-3");
ImmutableList.of("refs/tags/test-1", "refs/tags/test-2", "refs/tags/test-3", "test-4");
@Before
public void setUp() throws Exception {
@@ -112,7 +114,7 @@ public class DeleteTagsIT extends AbstractDaemonTest {
for (String tag : tags) {
message
.append("Cannot delete ")
.append(tag)
.append(prefixRef(tag))
.append(": it doesn't exist or you do not have permission ")
.append("to delete it\n");
}
@@ -122,18 +124,24 @@ public class DeleteTagsIT extends AbstractDaemonTest {
private HashMap<String, RevCommit> initialRevisions(List<String> tags) throws Exception {
HashMap<String, RevCommit> result = new HashMap<>();
for (String tag : tags) {
result.put(tag, getRemoteHead(project, tag));
String ref = prefixRef(tag);
result.put(ref, getRemoteHead(project, ref));
}
return result;
}
private void assertRefUpdatedEvents(HashMap<String, RevCommit> revisions) throws Exception {
for (String tag : revisions.keySet()) {
RevCommit revision = revisions.get(tag);
eventRecorder.assertRefUpdatedEvents(project.get(), tag, null, revision, revision, null);
RevCommit revision = revisions.get(prefixRef(tag));
eventRecorder.assertRefUpdatedEvents(
project.get(), prefixRef(tag), null, revision, revision, null);
}
}
private String prefixRef(String ref) {
return ref.startsWith(R_TAGS) ? ref : R_TAGS + ref;
}
private ProjectApi project() throws Exception {
return gApi.projects().name(project.get());
}
@@ -141,7 +149,9 @@ public class DeleteTagsIT extends AbstractDaemonTest {
private void assertTags(List<String> expected) throws Exception {
List<TagInfo> actualTags = project().tags().get();
Iterable<String> actualNames = Iterables.transform(actualTags, b -> b.ref);
assertThat(actualNames).containsExactlyElementsIn(expected).inOrder();
assertThat(actualNames)
.containsExactlyElementsIn(expected.stream().map(t -> prefixRef(t)).collect(toList()))
.inOrder();
}
private void assertTagsDeleted() throws Exception {

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -60,7 +62,7 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input> {
throw new ResourceConflictException("branch " + rsrc.getBranchKey() + " has open changes");
}
deleteRefFactory.create(rsrc).ref(rsrc.getRef()).delete();
deleteRefFactory.create(rsrc).ref(rsrc.getRef()).prefix(R_HEADS).delete();
return Response.none();
}
}

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
@@ -40,7 +42,7 @@ public class DeleteBranches implements RestModifyView<ProjectResource, DeleteBra
if (input == null || input.branches == null || input.branches.isEmpty()) {
throw new BadRequestException("branches must be specified");
}
deleteRefFactory.create(project).refs(input.branches).delete();
deleteRefFactory.create(project).refs(input.branches).prefix(R_HEADS).delete();
return Response.none();
}
}

View File

@@ -486,11 +486,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@Operator
public Predicate<ChangeData> change(String query) throws QueryParseException {
if (PAT_LEGACY_ID.matcher(query).matches()) {
return new LegacyChangeIdPredicate(Change.Id.parse(query));
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return new ChangeIdPredicate(parseChangeId(query));
}
Optional<ChangeTriplet> triplet = ChangeTriplet.parse(query);
if (triplet.isPresent()) {
return Predicate.and(
@@ -498,6 +493,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
branch(triplet.get().branch().get()),
new ChangeIdPredicate(parseChangeId(triplet.get().id().get())));
}
if (PAT_LEGACY_ID.matcher(query).matches()) {
return new LegacyChangeIdPredicate(Change.Id.parse(query));
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return new ChangeIdPredicate(parseChangeId(query));
}
throw new QueryParseException("Invalid change format");
}

View File

@@ -289,25 +289,25 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Test
public void byTriplet() throws Exception {
TestRepository<Repo> repo = createProject("repo");
TestRepository<Repo> repo = createProject("iabcde");
Change change = insert(repo, newChangeForBranch(repo, "branch"));
String k = change.getKey().get();
assertQuery("repo~branch~" + k, change);
assertQuery("change:repo~branch~" + k, change);
assertQuery("repo~refs/heads/branch~" + k, change);
assertQuery("change:repo~refs/heads/branch~" + k, change);
assertQuery("repo~branch~" + k.substring(0, 10), change);
assertQuery("change:repo~branch~" + k.substring(0, 10), change);
assertQuery("iabcde~branch~" + k, change);
assertQuery("change:iabcde~branch~" + k, change);
assertQuery("iabcde~refs/heads/branch~" + k, change);
assertQuery("change:iabcde~refs/heads/branch~" + k, change);
assertQuery("iabcde~branch~" + k.substring(0, 10), change);
assertQuery("change:iabcde~branch~" + k.substring(0, 10), change);
assertQuery("foo~bar");
assertThatQueryException("change:foo~bar").hasMessageThat().isEqualTo("Invalid change format");
assertQuery("otherrepo~branch~" + k);
assertQuery("change:otherrepo~branch~" + k);
assertQuery("repo~otherbranch~" + k);
assertQuery("change:repo~otherbranch~" + k);
assertQuery("repo~branch~I0000000000000000000000000000000000000000");
assertQuery("change:repo~branch~I0000000000000000000000000000000000000000");
assertQuery("iabcde~otherbranch~" + k);
assertQuery("change:iabcde~otherbranch~" + k);
assertQuery("iabcde~branch~I0000000000000000000000000000000000000000");
assertQuery("change:iabcde~branch~I0000000000000000000000000000000000000000");
}
@Test