From d00515e898f8f3d78b549de1fab02e2ede559648 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 10 Nov 2017 15:57:41 +0900 Subject: [PATCH] List{Branches,Tags}: "can_delete" should not be set when false The ListBranches implementation already does not set "can_delete" when it's false. Fix the example in the documentation. Fix the TagInfo constructors to use Java's Boolean class, rather than the primitive boolean, and explicitly set it to null when false. Also update the documentation. In DeleteBranchIT and DeleteTagIT, add assertions that canDelete is true or null. Change-Id: Ia139f0939f1ca6cb7bdd8b097d69e7a3f6543c25 --- Documentation/rest-api-projects.txt | 6 ++---- .../gerrit/acceptance/rest/project/DeleteBranchIT.java | 2 ++ .../google/gerrit/acceptance/rest/project/DeleteTagIT.java | 7 ++++++- .../com/google/gerrit/acceptance/rest/project/TagsIT.java | 2 +- .../com/google/gerrit/extensions/api/projects/TagInfo.java | 4 ++-- .../java/com/google/gerrit/server/project/ListTags.java | 2 +- 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 9d76d34e51..c455830d76 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -1305,7 +1305,6 @@ Limit the number of branches to be included in the results. { "ref": "HEAD", "revision": "master", - "can_delete": false } ] ---- @@ -1329,7 +1328,6 @@ Skip the given number of branches from the beginning of the list. { "ref": "HEAD", "revision": "master", - "can_delete": false } ] ---- @@ -2661,7 +2659,7 @@ The `BranchInfo` entity contains information about a branch. |Field Name ||Description |`ref` ||The ref of the branch. |`revision` ||The revision to which the branch points. -|`can_delete`|`false` if not set| +|`can_delete`|not set if `false`| Whether the calling user can delete this branch. |`web_links` |optional| Links to the branch in external sites as a list of @@ -3220,7 +3218,7 @@ tag points. the signature. |`tagger`|Only set for annotated tags, if present in the tag.|The tagger as a link:rest-api-changes.html#git-person-info[GitPersonInfo] entity. -|`can_delete`|`false` if not set| +|`can_delete`|not set if `false`| Whether the calling user can delete this tag. |`web_links` |optional| Links to the tag in external sites as a list of diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java index d5bb2fdfa1..c8f2fef23f 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java @@ -139,6 +139,7 @@ public class DeleteBranchIT extends AbstractDaemonTest { } private void assertDeleteSucceeds() throws Exception { + assertThat(branch().get().canDelete).isTrue(); String branchRev = branch().get().revision; branch().delete(); eventRecorder.assertRefUpdatedEvents( @@ -148,6 +149,7 @@ public class DeleteBranchIT extends AbstractDaemonTest { } private void assertDeleteForbidden() throws Exception { + assertThat(branch().get().canDelete).isNull(); exception.expect(AuthException.class); exception.expectMessage("delete not permitted"); branch().delete(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java index dd65f7cb29..0cbbe44206 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteTagIT.java @@ -14,6 +14,7 @@ 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_TAGS; @@ -22,6 +23,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; 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.TagInfo; import com.google.gerrit.extensions.api.projects.TagInput; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; @@ -111,7 +113,9 @@ public class DeleteTagIT extends AbstractDaemonTest { } private void assertDeleteSucceeds() throws Exception { - String tagRev = tag().get().revision; + TagInfo tagInfo = tag().get(); + assertThat(tagInfo.canDelete).isTrue(); + String tagRev = tagInfo.revision; tag().delete(); eventRecorder.assertRefUpdatedEvents(project.get(), TAG, null, tagRev, tagRev, null); exception.expect(ResourceNotFoundException.class); @@ -119,6 +123,7 @@ public class DeleteTagIT extends AbstractDaemonTest { } private void assertDeleteForbidden() throws Exception { + assertThat(tag().get().canDelete).isNull(); exception.expect(AuthException.class); exception.expectMessage("delete not permitted"); tag().delete(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java index 94271f9c59..ed791a24a2 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java @@ -187,7 +187,7 @@ public class TagsIT extends AbstractDaemonTest { setApiUser(user); result = tag(input.ref).get(); - assertThat(result.canDelete).isFalse(); + assertThat(result.canDelete).isNull(); eventRecorder.assertRefUpdatedEvents(project.get(), result.ref, null, result.revision); } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/TagInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/TagInfo.java index c7b1b94041..99fc6ecf38 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/TagInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/TagInfo.java @@ -24,7 +24,7 @@ public class TagInfo extends RefInfo { public GitPerson tagger; public List webLinks; - public TagInfo(String ref, String revision, boolean canDelete, List webLinks) { + public TagInfo(String ref, String revision, Boolean canDelete, List webLinks) { this.ref = ref; this.revision = revision; this.canDelete = canDelete; @@ -37,7 +37,7 @@ public class TagInfo extends RefInfo { String object, String message, GitPerson tagger, - boolean canDelete, + Boolean canDelete, List webLinks) { this(ref, revision, canDelete, webLinks); this.object = object; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java index a58f31697a..d57234aa8f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java @@ -190,7 +190,7 @@ public class ListTags implements RestReadView { WebLinks links) throws MissingObjectException, IOException { RevObject object = rw.parseAny(ref.getObjectId()); - boolean canDelete = perm.testOrFalse(RefPermission.DELETE); + Boolean canDelete = perm.testOrFalse(RefPermission.DELETE) ? true : null; List webLinks = links.getTagLinks(projectName.get(), ref.getName()); if (object instanceof RevTag) { // Annotated or signed tag