opendev/gerrit
Code Issues Proposed changes

Merge "Do not require 'Create Reference' for pushing annotated tag"

Browse Source
This commit is contained in:
David Pursehouse
2017-06-27 12:03:44 +00:00
committed by Gerrit Code Review
parent 361f30cc0b 61e7176bf0
commit 0e8b730c77
5 changed files with 133 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

162
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java → gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
View File

@@ -19,8 +19,8 @@ import static com.google.gerrit.acceptance.GitUtil.createAnnotatedTag;
import static com.google.gerrit.acceptance.GitUtil.deleteRef; import static com.google.gerrit.acceptance.GitUtil.deleteRef;
import static com.google.gerrit.acceptance.GitUtil.pushHead; import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.acceptance.GitUtil.updateAnnotatedTag; import static com.google.gerrit.acceptance.GitUtil.updateAnnotatedTag;
import static com.google.gerrit.acceptance.rest.project.PushTagIT.TagType.ANNOTATED; import static com.google.gerrit.acceptance.rest.project.AbstractPushTag.TagType.ANNOTATED;
import static com.google.gerrit.acceptance.rest.project.PushTagIT.TagType.LIGHTWEIGHT; import static com.google.gerrit.acceptance.rest.project.AbstractPushTag.TagType.LIGHTWEIGHT;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
@@ -38,7 +38,7 @@ import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@NoHttpd @NoHttpd
public class PushTagIT extends AbstractDaemonTest { public abstract class AbstractPushTag extends AbstractDaemonTest {
enum TagType { enum TagType {
LIGHTWEIGHT(Permission.CREATE), LIGHTWEIGHT(Permission.CREATE),
ANNOTATED(Permission.CREATE_TAG); ANNOTATED(Permission.CREATE_TAG);
@@ -51,6 +51,7 @@ public class PushTagIT extends AbstractDaemonTest {
} }
private RevCommit initialHead; private RevCommit initialHead;
private TagType tagType;
@Before @Before
public void setup() throws Exception { public void setup() throws Exception {
@@ -58,145 +59,131 @@ public class PushTagIT extends AbstractDaemonTest {
testRepo = cloneProject(project, user); testRepo = cloneProject(project, user);
initialHead = getRemoteHead(); initialHead = getRemoteHead();
tagType = getTagType();
} }
protected abstract TagType getTagType();
@Test @Test
public void createTagForExistingCommit() throws Exception { public void createTagForExistingCommit() throws Exception {
for (TagType tagType : TagType.values()) { pushTagForExistingCommit(Status.REJECTED_OTHER_REASON);
pushTagForExistingCommit(tagType, Status.REJECTED_OTHER_REASON);
allowTagCreation(tagType); allowTagCreation();
pushTagForExistingCommit(tagType, Status.OK); pushTagForExistingCommit(Status.OK);
allowPushOnRefsTags(); allowPushOnRefsTags();
pushTagForExistingCommit(tagType, Status.OK); pushTagForExistingCommit(Status.OK);
removePushFromRefsTags(); removePushFromRefsTags();
}
} }
@Test @Test
public void createTagForNewCommit() throws Exception { public void createTagForNewCommit() throws Exception {
for (TagType tagType : TagType.values()) { pushTagForNewCommit(Status.REJECTED_OTHER_REASON);
pushTagForNewCommit(tagType, Status.REJECTED_OTHER_REASON);
allowTagCreation(tagType); allowTagCreation();
pushTagForNewCommit(tagType, Status.REJECTED_OTHER_REASON); pushTagForNewCommit(Status.REJECTED_OTHER_REASON);
allowPushOnRefsTags(); allowPushOnRefsTags();
pushTagForNewCommit(tagType, Status.OK); pushTagForNewCommit(Status.OK);
removePushFromRefsTags(); removePushFromRefsTags();
}
} }
@Test @Test
public void fastForward() throws Exception { public void fastForward() throws Exception {
for (TagType tagType : TagType.values()) { allowTagCreation();
allowTagCreation(tagType); String tagName = pushTagForExistingCommit(Status.OK);
String tagName = pushTagForExistingCommit(tagType, Status.OK);
fastForwardTagToExistingCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); fastForwardTagToExistingCommit(tagName, Status.REJECTED_OTHER_REASON);
fastForwardTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); fastForwardTagToNewCommit(tagName, Status.REJECTED_OTHER_REASON);
allowTagDeletion(); allowTagDeletion();
fastForwardTagToExistingCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); fastForwardTagToExistingCommit(tagName, Status.REJECTED_OTHER_REASON);
fastForwardTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); fastForwardTagToNewCommit(tagName, Status.REJECTED_OTHER_REASON);
allowPushOnRefsTags(); allowPushOnRefsTags();
Status expectedStatus = tagType == ANNOTATED ? Status.REJECTED_OTHER_REASON : Status.OK; Status expectedStatus = tagType == ANNOTATED ? Status.REJECTED_OTHER_REASON : Status.OK;
fastForwardTagToExistingCommit(tagType, tagName, expectedStatus); fastForwardTagToExistingCommit(tagName, expectedStatus);
fastForwardTagToNewCommit(tagType, tagName, expectedStatus); fastForwardTagToNewCommit(tagName, expectedStatus);
allowForcePushOnRefsTags(); allowForcePushOnRefsTags();
fastForwardTagToExistingCommit(tagType, tagName, Status.OK); fastForwardTagToExistingCommit(tagName, Status.OK);
fastForwardTagToNewCommit(tagType, tagName, Status.OK); fastForwardTagToNewCommit(tagName, Status.OK);
removePushFromRefsTags(); removePushFromRefsTags();
}
} }
@Test @Test
public void forceUpdate() throws Exception { public void forceUpdate() throws Exception {
for (TagType tagType : TagType.values()) { allowTagCreation();
allowTagCreation(tagType); String tagName = pushTagForExistingCommit(Status.OK);
String tagName = pushTagForExistingCommit(tagType, Status.OK);
forceUpdateTagToExistingCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToExistingCommit(tagName, Status.REJECTED_OTHER_REASON);
forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToNewCommit(tagName, Status.REJECTED_OTHER_REASON);
allowPushOnRefsTags(); allowPushOnRefsTags();
forceUpdateTagToExistingCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToExistingCommit(tagName, Status.REJECTED_OTHER_REASON);
forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToNewCommit(tagName, Status.REJECTED_OTHER_REASON);
allowTagDeletion(); allowTagDeletion();
forceUpdateTagToExistingCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToExistingCommit(tagName, Status.REJECTED_OTHER_REASON);
forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON); forceUpdateTagToNewCommit(tagName, Status.REJECTED_OTHER_REASON);
allowForcePushOnRefsTags(); allowForcePushOnRefsTags();
forceUpdateTagToExistingCommit(tagType, tagName, Status.OK); forceUpdateTagToExistingCommit(tagName, Status.OK);
forceUpdateTagToNewCommit(tagType, tagName, Status.OK); forceUpdateTagToNewCommit(tagName, Status.OK);
removePushFromRefsTags(); removePushFromRefsTags();
}
} }
@Test @Test
public void delete() throws Exception { public void delete() throws Exception {
for (TagType tagType : TagType.values()) { allowTagCreation();
allowTagCreation(tagType); String tagName = pushTagForExistingCommit(Status.OK);
String tagName = pushTagForExistingCommit(tagType, Status.OK);
pushTagDeletion(tagType, tagName, Status.REJECTED_OTHER_REASON); pushTagDeletion(tagName, Status.REJECTED_OTHER_REASON);
allowPushOnRefsTags(); allowPushOnRefsTags();
pushTagDeletion(tagType, tagName, Status.REJECTED_OTHER_REASON); pushTagDeletion(tagName, Status.REJECTED_OTHER_REASON);
}
allowForcePushOnRefsTags(); allowForcePushOnRefsTags();
for (TagType tagType : TagType.values()) { tagName = pushTagForExistingCommit(Status.OK);
String tagName = pushTagForExistingCommit(tagType, Status.OK); pushTagDeletion(tagName, Status.OK);
pushTagDeletion(tagType, tagName, Status.OK);
}
removePushFromRefsTags(); removePushFromRefsTags();
allowTagDeletion(); allowTagDeletion();
for (TagType tagType : TagType.values()) { tagName = pushTagForExistingCommit(Status.OK);
String tagName = pushTagForExistingCommit(tagType, Status.OK); pushTagDeletion(tagName, Status.OK);
pushTagDeletion(tagType, tagName, Status.OK);
}
} }
private String pushTagForExistingCommit(TagType tagType, Status expectedStatus) throws Exception { private String pushTagForExistingCommit(Status expectedStatus) throws Exception {
return pushTag(tagType, null, false, false, expectedStatus); return pushTag(null, false, false, expectedStatus);
} }
private String pushTagForNewCommit(TagType tagType, Status expectedStatus) throws Exception { private String pushTagForNewCommit(Status expectedStatus) throws Exception {
return pushTag(tagType, null, true, false, expectedStatus); return pushTag(null, true, false, expectedStatus);
} }
private void fastForwardTagToExistingCommit( private void fastForwardTagToExistingCommit(String tagName, Status expectedStatus)
TagType tagType, String tagName, Status expectedStatus) throws Exception {
pushTag(tagType, tagName, false, false, expectedStatus);
}
private void fastForwardTagToNewCommit(TagType tagType, String tagName, Status expectedStatus)
throws Exception { throws Exception {
pushTag(tagType, tagName, true, false, expectedStatus); pushTag(tagName, false, false, expectedStatus);
} }
private void forceUpdateTagToExistingCommit( private void fastForwardTagToNewCommit(String tagName, Status expectedStatus) throws Exception {
TagType tagType, String tagName, Status expectedStatus) throws Exception { pushTag(tagName, true, false, expectedStatus);
pushTag(tagType, tagName, false, true, expectedStatus);
} }
private void forceUpdateTagToNewCommit(TagType tagType, String tagName, Status expectedStatus) private void forceUpdateTagToExistingCommit(String tagName, Status expectedStatus)
throws Exception { throws Exception {
pushTag(tagType, tagName, true, true, expectedStatus); pushTag(tagName, false, true, expectedStatus);
} }
private String pushTag( private void forceUpdateTagToNewCommit(String tagName, Status expectedStatus) throws Exception {
TagType tagType, String tagName, boolean newCommit, boolean force, Status expectedStatus) pushTag(tagName, true, true, expectedStatus);
}
private String pushTag(String tagName, boolean newCommit, boolean force, Status expectedStatus)
throws Exception { throws Exception {
if (force) { if (force) {
testRepo.reset(initialHead); testRepo.reset(initialHead);
@@ -234,15 +221,14 @@ public class PushTagIT extends AbstractDaemonTest {
return tagName; return tagName;
} }
private void pushTagDeletion(TagType tagType, String tagName, Status expectedStatus) private void pushTagDeletion(String tagName, Status expectedStatus) throws Exception {
throws Exception {
String tagRef = tagRef(tagName); String tagRef = tagRef(tagName);
PushResult r = deleteRef(testRepo, tagRef); PushResult r = deleteRef(testRepo, tagRef);
RemoteRefUpdate refUpdate = r.getRemoteUpdate(tagRef); RemoteRefUpdate refUpdate = r.getRemoteUpdate(tagRef);
assertThat(refUpdate.getStatus()).named(tagType.name()).isEqualTo(expectedStatus); assertThat(refUpdate.getStatus()).named(tagType.name()).isEqualTo(expectedStatus);
} }
private void allowTagCreation(TagType tagType) throws Exception { private void allowTagCreation() throws Exception {
grant(project, "refs/tags/*", tagType.createPermission, false, REGISTERED_USERS); grant(project, "refs/tags/*", tagType.createPermission, false, REGISTERED_USERS);
} }

12
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BUILD
View File

@@ -6,6 +6,7 @@ acceptance_tests(
labels = ["rest"], labels = ["rest"],
deps = [ deps = [
":project", ":project",
":push_tag_util",
":refassert", ":refassert",
], ],
) )
@@ -35,3 +36,14 @@ java_library(
"//lib:truth", "//lib:truth",
], ],
) )
java_library(
name = "push_tag_util",
testonly = 1,
srcs = [
"AbstractPushTag.java",
],
deps = [
"//gerrit-acceptance-tests:lib",
],
)

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushAnnotatedTagIT.java Normal file
View File

@@ -0,0 +1,23 @@
// Copyright (C) 2017 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.project;
public class PushAnnotatedTagIT extends AbstractPushTag {
@Override
protected TagType getTagType() {
return TagType.ANNOTATED;
}
}

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushLightweightTagIT.java Normal file
View File

@@ -0,0 +1,23 @@
// Copyright (C) 2017 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.project;
public class PushLightweightTagIT extends AbstractPushTag {
@Override
protected TagType getTagType() {
return TagType.LIGHTWEIGHT;
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -1070,14 +1070,7 @@ public class ReceiveCommits {
} }
RefControl ctl = projectControl.controlForRef(cmd.getRefName()); RefControl ctl = projectControl.controlForRef(cmd.getRefName());
boolean ok; if (ctl.canCreate(rp.getRepository(), obj)) {
try {
permissions.ref(cmd.getRefName()).check(RefPermission.CREATE);
ok = true;
} catch (AuthException err) {
ok = false;
}
if (ok && ctl.canCreate(rp.getRepository(), obj)) {
if (!validRefOperation(cmd)) { if (!validRefOperation(cmd)) {
return; return;
} }