opendev/gerrit
Code Issues Proposed changes

RefNames: Add a method to check whether a ref is used to store group data

Browse Source 
All refs that are used to store group data in NoteDb are protected
(group branches and refs/meta/group-names). Instead of checking for
group refs in different places have a method for this so that the check
needs to be implemented only once.

Change-Id: I7bcc1421f698bdefb5ef05a9a666fac88e58e9d2
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-11-20 11:13:30 +01:00
parent d7848539ca
commit 2369ea7b6e
5 changed files with 37 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
java/com/google/gerrit/reviewdb/client/RefNames.java
View File

@@ -224,10 +224,22 @@ public class RefNames {
return ref.startsWith(REFS_USERS);
}
/**
* Whether the ref is a group branch that stores NoteDb data of a group. Returns {@code true} for
* all refs that start with {@code refs/groups/}.
*/
public static boolean isRefsGroups(String ref) {
return ref.startsWith(REFS_GROUPS);
}
/**
* Whether the ref is used for storing group data in NoteDb. Returns {@code true} for all group
* branches and refs/meta/group-names.
*/
public static boolean isGroupRef(String ref) {
return isRefsGroups(ref) || REFS_GROUPNAMES.equals(ref);
}
static Integer parseShardedRefPart(String name) {
if (name == null) {
return null;

8
java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@@ -778,7 +778,7 @@ public class CommitValidators {
}
}
/** Rejects updates to group branches (refs/groups/* and refs/meta/group-names). */
/** Rejects updates to group branches. */
public static class GroupCommitValidator implements CommitValidationListener {
private final AllUsersName allUsers;
@@ -789,8 +789,7 @@ public class CommitValidators {
@Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
throws CommitValidationException {
// Groups are stored inside 'refs/groups/' refs inside the 'All-Users' repository.
// Group names are stored inside 'refs/meta/group-names' refs inside the 'All-Users' repository.
// Groups are stored inside the 'All-Users' repository.
if (!allUsers.equals(receiveEvent.project.getNameKey())) {
return Collections.emptyList();
}
@@ -801,8 +800,7 @@ public class CommitValidators {
return Collections.emptyList();
}
if (receiveEvent.command.getRefName().startsWith(RefNames.REFS_GROUPS)
|| receiveEvent.command.getRefName().equals(RefNames.REFS_GROUPNAMES)) {
if (RefNames.isGroupRef(receiveEvent.command.getRefName())) {
throw new CommitValidationException("group update not allowed");
}
return Collections.emptyList();

6
java/com/google/gerrit/server/git/validators/MergeValidators.java
View File

@@ -318,11 +318,9 @@ public class MergeValidators {
PatchSet.Id patchSetId,
IdentifiedUser caller)
throws MergeValidationException {
// Groups are stored inside 'refs/groups/' refs inside the 'All-Users' repository.
// Group names are stored inside 'refs/meta/group-names' inside the 'All-Users' repository.
// Groups are stored inside the 'All-Users' repository.
if (!allUsersName.equals(destProject.getNameKey())
|| (!destBranch.get().startsWith(RefNames.REFS_GROUPS)
&& !destBranch.get().equals(RefNames.REFS_GROUPNAMES))) {
|| !RefNames.isGroupRef(destBranch.get())) {
return;
}

3
java/com/google/gerrit/server/git/validators/RefOperationValidators.java
View File

@@ -149,8 +149,7 @@ public class RefOperationValidators {
}
}
if (refEvent.command.getRefName().startsWith(RefNames.REFS_GROUPS)
|| refEvent.command.getRefName().equals(RefNames.REFS_GROUPNAMES)) {
if (RefNames.isGroupRef(refEvent.command.getRefName())) {
if (refEvent.command.getType().equals(ReceiveCommand.Type.CREATE)) {
throw new ValidationException("Not allowed to create group branch.");
} else if (refEvent.command.getType().equals(ReceiveCommand.Type.DELETE)) {

19
javatests/com/google/gerrit/reviewdb/client/RefNamesTest.java
View File

@@ -123,6 +123,25 @@ public class RefNamesTest {
assertThat(RefNames.isRefsUsers("refs/users/23/1011123/edit-67473/42")).isTrue();
assertThat(RefNames.isRefsUsers("refs/heads/master")).isFalse();
assertThat(RefNames.isRefsUsers("refs/groups/" + TEST_SHARDED_GROUP_UUID)).isFalse();
}
@Test
public void isRefsGroups() throws Exception {
assertThat(RefNames.isRefsGroups("refs/groups/" + TEST_SHARDED_GROUP_UUID)).isTrue();
assertThat(RefNames.isRefsGroups("refs/heads/master")).isFalse();
assertThat(RefNames.isRefsGroups("refs/users/23/1011123")).isFalse();
assertThat(RefNames.isRefsGroups(RefNames.REFS_GROUPNAMES)).isFalse();
}
@Test
public void isGroupRef() throws Exception {
assertThat(RefNames.isGroupRef("refs/groups/" + TEST_SHARDED_GROUP_UUID)).isTrue();
assertThat(RefNames.isGroupRef(RefNames.REFS_GROUPNAMES)).isTrue();
assertThat(RefNames.isGroupRef("refs/heads/master")).isFalse();
assertThat(RefNames.isGroupRef("refs/users/23/1011123")).isFalse();
}
@Test