opendev/gerrit
Code Issues Proposed changes

Improve naming within GroupNameNotes

Browse Source 
This change tries to use more consistent, helpful, and concise names for
methods and parameters.

Change-Id: I0c634a6e37c7f51b1af18de378080e3bcd6e0c8d
This commit is contained in:
Alice Kober-Sotzek
2018-01-12 10:30:45 +01:00
parent cbae08edf8
commit 2b08a28bb5
5 changed files with 34 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
java/com/google/gerrit/server/group/db/GroupNameNotes.java
View File

@@ -68,7 +68,7 @@ public class GroupNameNotes extends VersionedMetaData {
@VisibleForTesting @VisibleForTesting
static final String UNIQUE_REF_ERROR = "GroupReference collection must contain unique references"; static final String UNIQUE_REF_ERROR = "GroupReference collection must contain unique references";
public static GroupNameNotes loadForRename( public static GroupNameNotes forRename(
Repository repository, Repository repository,
AccountGroup.UUID groupUuid, AccountGroup.UUID groupUuid,
AccountGroup.NameKey oldName, AccountGroup.NameKey oldName,
@@ -83,7 +83,7 @@ public class GroupNameNotes extends VersionedMetaData {
return groupNameNotes; return groupNameNotes;
} }
public static GroupNameNotes loadForNewGroup( public static GroupNameNotes forNewGroup(
Repository repository, AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName) Repository repository, AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName)
throws IOException, ConfigInvalidException, OrmDuplicateKeyException { throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
checkNotNull(groupName); checkNotNull(groupName);
@@ -95,14 +95,14 @@ public class GroupNameNotes extends VersionedMetaData {
} }
public static Optional<GroupReference> loadGroup( public static Optional<GroupReference> loadGroup(
Repository allUsersRepo, AccountGroup.NameKey groupName) Repository repository, AccountGroup.NameKey groupName)
throws IOException, ConfigInvalidException { throws IOException, ConfigInvalidException {
Ref ref = allUsersRepo.exactRef(RefNames.REFS_GROUPNAMES); Ref ref = repository.exactRef(RefNames.REFS_GROUPNAMES);
if (ref == null) { if (ref == null) {
return Optional.empty(); return Optional.empty();
} }
try (RevWalk revWalk = new RevWalk(allUsersRepo); try (RevWalk revWalk = new RevWalk(repository);
ObjectReader reader = revWalk.getObjectReader()) { ObjectReader reader = revWalk.getObjectReader()) {
RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId()); RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId());
NoteMap noteMap = NoteMap.read(reader, notesCommit); NoteMap noteMap = NoteMap.read(reader, notesCommit);
@@ -140,8 +140,8 @@ public class GroupNameNotes extends VersionedMetaData {
} }
} }
public static void updateGroupNames( public static void updateAllGroups(
Repository allUsersRepo, Repository repository,
ObjectInserter inserter, ObjectInserter inserter,
BatchRefUpdate bru, BatchRefUpdate bru,
Collection<GroupReference> groupReferences, Collection<GroupReference> groupReferences,
@@ -154,7 +154,7 @@ public class GroupNameNotes extends VersionedMetaData {
RevWalk rw = new RevWalk(reader)) { RevWalk rw = new RevWalk(reader)) {
// Always start from an empty map, discarding old notes. // Always start from an empty map, discarding old notes.
NoteMap noteMap = NoteMap.newEmptyMap(); NoteMap noteMap = NoteMap.newEmptyMap();
Ref ref = allUsersRepo.exactRef(RefNames.REFS_GROUPNAMES); Ref ref = repository.exactRef(RefNames.REFS_GROUPNAMES);
RevCommit oldCommit = ref != null ? rw.parseCommit(ref.getObjectId()) : null; RevCommit oldCommit = ref != null ? rw.parseCommit(ref.getObjectId()) : null;
for (Map.Entry<AccountGroup.UUID, String> e : biMap.entrySet()) { for (Map.Entry<AccountGroup.UUID, String> e : biMap.entrySet()) {

4
java/com/google/gerrit/server/group/db/GroupsUpdate.java
View File

@@ -482,7 +482,7 @@ public class GroupsUpdate {
try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey); AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
GroupNameNotes groupNameNotes = GroupNameNotes groupNameNotes =
GroupNameNotes.loadForNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName); GroupNameNotes.forNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
GroupConfig groupConfig = GroupConfig.createForNewGroup(allUsersRepo, groupCreation); GroupConfig groupConfig = GroupConfig.createForNewGroup(allUsersRepo, groupCreation);
groupConfig.setGroupUpdate(groupUpdate, this::getAccountNameEmail, this::getGroupName); groupConfig.setGroupUpdate(groupUpdate, this::getAccountNameEmail, this::getGroupName);
@@ -515,7 +515,7 @@ public class GroupsUpdate {
if (groupUpdate.getName().isPresent()) { if (groupUpdate.getName().isPresent()) {
AccountGroup.NameKey oldName = originalGroup.getNameKey(); AccountGroup.NameKey oldName = originalGroup.getNameKey();
AccountGroup.NameKey newName = groupUpdate.getName().get(); AccountGroup.NameKey newName = groupUpdate.getName().get();
groupNameNotes = GroupNameNotes.loadForRename(allUsersRepo, groupUuid, oldName, newName); groupNameNotes = GroupNameNotes.forRename(allUsersRepo, groupUuid, oldName, newName);
} }
commit(allUsersRepo, groupConfig, groupNameNotes); commit(allUsersRepo, groupConfig, groupNameNotes);

2
java/com/google/gerrit/server/schema/SchemaCreator.java
View File

@@ -240,7 +240,7 @@ public class SchemaCreator {
AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey); AccountGroup.NameKey groupName = groupUpdate.getName().orElseGet(groupCreation::getNameKey);
GroupNameNotes groupNameNotes = GroupNameNotes groupNameNotes =
GroupNameNotes.loadForNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName); GroupNameNotes.forNewGroup(allUsersRepo, groupCreation.getGroupUUID(), groupName);
commit(allUsersRepo, groupConfig, groupNameNotes); commit(allUsersRepo, groupConfig, groupNameNotes);

44
javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java
View File

@@ -110,13 +110,13 @@ public class GroupNameNotesTest {
@Test @Test
public void uuidOfNewGroupMustNotBeNull() throws Exception { public void uuidOfNewGroupMustNotBeNull() throws Exception {
expectedException.expect(NullPointerException.class); expectedException.expect(NullPointerException.class);
GroupNameNotes.loadForNewGroup(repo, null, groupName); GroupNameNotes.forNewGroup(repo, null, groupName);
} }
@Test @Test
public void nameOfNewGroupMustNotBeNull() throws Exception { public void nameOfNewGroupMustNotBeNull() throws Exception {
expectedException.expect(NullPointerException.class); expectedException.expect(NullPointerException.class);
GroupNameNotes.loadForNewGroup(repo, groupUuid, null); GroupNameNotes.forNewGroup(repo, groupUuid, null);
} }
@Test @Test
@@ -135,7 +135,7 @@ public class GroupNameNotesTest {
AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("AnotherGroup"); AccountGroup.UUID anotherGroupUuid = new AccountGroup.UUID("AnotherGroup");
expectedException.expect(OrmDuplicateKeyException.class); expectedException.expect(OrmDuplicateKeyException.class);
expectedException.expectMessage(groupName.get()); expectedException.expectMessage(groupName.get());
GroupNameNotes.loadForNewGroup(repo, anotherGroupUuid, groupName); GroupNameNotes.forNewGroup(repo, anotherGroupUuid, groupName);
} }
@Test @Test
@@ -179,7 +179,7 @@ public class GroupNameNotesTest {
createGroup(groupUuid, groupName); createGroup(groupUuid, groupName);
expectedException.expect(NullPointerException.class); expectedException.expect(NullPointerException.class);
GroupNameNotes.loadForRename(repo, groupUuid, groupName, null); GroupNameNotes.forRename(repo, groupUuid, groupName, null);
} }
@Test @Test
@@ -188,7 +188,7 @@ public class GroupNameNotesTest {
AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins"); AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
expectedException.expect(NullPointerException.class); expectedException.expect(NullPointerException.class);
GroupNameNotes.loadForRename(repo, groupUuid, null, anotherName); GroupNameNotes.forRename(repo, groupUuid, null, anotherName);
} }
@Test @Test
@@ -199,7 +199,7 @@ public class GroupNameNotesTest {
AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins"); AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
expectedException.expect(ConfigInvalidException.class); expectedException.expect(ConfigInvalidException.class);
expectedException.expectMessage(anotherOldName.get()); expectedException.expectMessage(anotherOldName.get());
GroupNameNotes.loadForRename(repo, groupUuid, anotherOldName, anotherName); GroupNameNotes.forRename(repo, groupUuid, anotherOldName, anotherName);
} }
@Test @Test
@@ -211,7 +211,7 @@ public class GroupNameNotesTest {
expectedException.expect(OrmDuplicateKeyException.class); expectedException.expect(OrmDuplicateKeyException.class);
expectedException.expectMessage(anotherGroupName.get()); expectedException.expectMessage(anotherGroupName.get());
GroupNameNotes.loadForRename(repo, groupUuid, groupName, anotherGroupName); GroupNameNotes.forRename(repo, groupUuid, groupName, anotherGroupName);
} }
@Test @Test
@@ -220,7 +220,7 @@ public class GroupNameNotesTest {
AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins"); AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
expectedException.expect(NullPointerException.class); expectedException.expect(NullPointerException.class);
GroupNameNotes.loadForRename(repo, null, groupName, anotherName); GroupNameNotes.forRename(repo, null, groupName, anotherName);
} }
@Test @Test
@@ -231,7 +231,7 @@ public class GroupNameNotesTest {
AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins"); AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
expectedException.expect(ConfigInvalidException.class); expectedException.expect(ConfigInvalidException.class);
expectedException.expectMessage(groupUuid.get()); expectedException.expectMessage(groupUuid.get());
GroupNameNotes.loadForRename(repo, anotherGroupUuid, groupName, anotherName); GroupNameNotes.forRename(repo, anotherGroupUuid, groupName, anotherName);
} }
@Test @Test
@@ -287,7 +287,7 @@ public class GroupNameNotesTest {
@Test @Test
public void newCommitIsNotCreatedWhenCommittingGroupCreationTwice() throws Exception { public void newCommitIsNotCreatedWhenCommittingGroupCreationTwice() throws Exception {
GroupNameNotes groupNameNotes = GroupNameNotes.loadForNewGroup(repo, groupUuid, groupName); GroupNameNotes groupNameNotes = GroupNameNotes.forNewGroup(repo, groupUuid, groupName);
commit(groupNameNotes); commit(groupNameNotes);
ImmutableList<CommitInfo> commitsAfterFirstCommit = log(); ImmutableList<CommitInfo> commitsAfterFirstCommit = log();
@@ -303,7 +303,7 @@ public class GroupNameNotesTest {
AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins"); AccountGroup.NameKey anotherName = new AccountGroup.NameKey("admins");
GroupNameNotes groupNameNotes = GroupNameNotes groupNameNotes =
GroupNameNotes.loadForRename(repo, groupUuid, groupName, anotherName); GroupNameNotes.forRename(repo, groupUuid, groupName, anotherName);
commit(groupNameNotes); commit(groupNameNotes);
ImmutableList<CommitInfo> commitsAfterFirstCommit = log(); ImmutableList<CommitInfo> commitsAfterFirstCommit = log();
@@ -403,7 +403,7 @@ public class GroupNameNotesTest {
GroupReference g2 = newGroup("b"); GroupReference g2 = newGroup("b");
PersonIdent ident = newPersonIdent(); PersonIdent ident = newPersonIdent();
updateGroupNames(ident, g1, g2); updateAllGroups(ident, g1, g2);
ImmutableList<CommitInfo> log = log(); ImmutableList<CommitInfo> log = log();
assertThat(log).hasSize(1); assertThat(log).hasSize(1);
@@ -416,7 +416,7 @@ public class GroupNameNotesTest {
// Updating the same set of names is a no-op. // Updating the same set of names is a no-op.
String commit = log.get(0).commit; String commit = log.get(0).commit;
updateGroupNames(newPersonIdent(), g1, g2); updateAllGroups(newPersonIdent(), g1, g2);
log = log(); log = log();
assertThat(log).hasSize(1); assertThat(log).hasSize(1);
assertThat(log.get(0)).commit().isEqualTo(commit); assertThat(log.get(0)).commit().isEqualTo(commit);
@@ -445,7 +445,7 @@ public class GroupNameNotesTest {
.copy(); .copy();
ident = newPersonIdent(); ident = newPersonIdent();
updateGroupNames(ident, g1, g2); updateAllGroups(ident, g1, g2);
assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2); assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
@@ -467,11 +467,11 @@ public class GroupNameNotesTest {
GroupReference g2 = newGroup("b"); GroupReference g2 = newGroup("b");
PersonIdent ident = newPersonIdent(); PersonIdent ident = newPersonIdent();
updateGroupNames(ident, g1, g2); updateAllGroups(ident, g1, g2);
assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2); assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g1, g2);
updateGroupNames(ident); updateAllGroups(ident);
assertThat(GroupNameNotes.loadAllGroups(repo)).isEmpty(); assertThat(GroupNameNotes.loadAllGroups(repo)).isEmpty();
@@ -496,7 +496,7 @@ public class GroupNameNotesTest {
@Test @Test
public void emptyGroupName() throws Exception { public void emptyGroupName() throws Exception {
GroupReference g = newGroup(""); GroupReference g = newGroup("");
updateGroupNames(newPersonIdent(), g); updateAllGroups(newPersonIdent(), g);
assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g); assertThat(GroupNameNotes.loadAllGroups(repo)).containsExactly(g);
assertThat(readNameNote(g)).isEqualTo("[group]\n\tuuid = -1\n\tname = \n"); assertThat(readNameNote(g)).isEqualTo("[group]\n\tuuid = -1\n\tname = \n");
@@ -504,14 +504,14 @@ public class GroupNameNotesTest {
private void createGroup(AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName) private void createGroup(AccountGroup.UUID groupUuid, AccountGroup.NameKey groupName)
throws Exception { throws Exception {
GroupNameNotes groupNameNotes = GroupNameNotes.loadForNewGroup(repo, groupUuid, groupName); GroupNameNotes groupNameNotes = GroupNameNotes.forNewGroup(repo, groupUuid, groupName);
commit(groupNameNotes); commit(groupNameNotes);
} }
private void renameGroup( private void renameGroup(
AccountGroup.UUID groupUuid, AccountGroup.NameKey oldName, AccountGroup.NameKey newName) AccountGroup.UUID groupUuid, AccountGroup.NameKey oldName, AccountGroup.NameKey newName)
throws Exception { throws Exception {
GroupNameNotes groupNameNotes = GroupNameNotes.loadForRename(repo, groupUuid, oldName, newName); GroupNameNotes groupNameNotes = GroupNameNotes.forRename(repo, groupUuid, oldName, newName);
commit(groupNameNotes); commit(groupNameNotes);
} }
@@ -549,10 +549,10 @@ public class GroupNameNotesTest {
return GroupNameNotes.getNoteKey(new AccountGroup.NameKey(g.getName())); return GroupNameNotes.getNoteKey(new AccountGroup.NameKey(g.getName()));
} }
private void updateGroupNames(PersonIdent ident, GroupReference... groupRefs) throws Exception { private void updateAllGroups(PersonIdent ident, GroupReference... groupRefs) throws Exception {
try (ObjectInserter inserter = repo.newObjectInserter()) { try (ObjectInserter inserter = repo.newObjectInserter()) {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
GroupNameNotes.updateGroupNames(repo, inserter, bru, Arrays.asList(groupRefs), ident); GroupNameNotes.updateAllGroups(repo, inserter, bru, Arrays.asList(groupRefs), ident);
inserter.flush(); inserter.flush();
RefUpdateUtil.executeChecked(bru, repo); RefUpdateUtil.executeChecked(bru, repo);
} }
@@ -563,7 +563,7 @@ public class GroupNameNotesTest {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate(); BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
PersonIdent ident = newPersonIdent(); PersonIdent ident = newPersonIdent();
try { try {
GroupNameNotes.updateGroupNames(repo, inserter, bru, Arrays.asList(groupRefs), ident); GroupNameNotes.updateAllGroups(repo, inserter, bru, Arrays.asList(groupRefs), ident);
assert_().fail("Expected IllegalArgumentException"); assert_().fail("Expected IllegalArgumentException");
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo(GroupNameNotes.UNIQUE_REF_ERROR); assertThat(e).hasMessageThat().isEqualTo(GroupNameNotes.UNIQUE_REF_ERROR);

2
javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java
View File

@@ -536,7 +536,7 @@ public class GroupRebuilderTest extends AbstractGroupTest {
try (ObjectInserter inserter = repo.newObjectInserter()) { try (ObjectInserter inserter = repo.newObjectInserter()) {
ImmutableList<GroupReference> refs = ImmutableList<GroupReference> refs =
ImmutableList.of(GroupReference.forGroup(g1), GroupReference.forGroup(g2)); ImmutableList.of(GroupReference.forGroup(g1), GroupReference.forGroup(g2));
GroupNameNotes.updateGroupNames(repo, inserter, bru, refs, newPersonIdent()); GroupNameNotes.updateAllGroups(repo, inserter, bru, refs, newPersonIdent());
inserter.flush(); inserter.flush();
} }