opendev/gerrit
Code Issues Proposed changes

Improve signature of GroupNameNotes#loadOneGroupReference

Browse Source 
The other methods of GroupNameNotes use AccountGroup.NameKey instead of
String as parameter type for the name. For consistency reasons, we
adjust GroupNameNotes#loadOneGroupReference accordingly.

In addition, use a shorter but still precise name for the method.

Change-Id: I10f11f921c2f2407a8bcd62b93cb1d188c22e2f0
This commit is contained in:
Alice Kober-Sotzek
2018-01-11 18:07:41 +01:00
parent 9e3b51f584
commit f62506f3fc
4 changed files with 20 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -94,8 +94,9 @@ public class GroupNameNotes extends VersionedMetaData {
return groupNameNotes;
}
public static Optional<GroupReference> loadOneGroupReference(
Repository allUsersRepo, String groupName) throws IOException, ConfigInvalidException {
public static Optional<GroupReference> loadGroup(
Repository allUsersRepo, AccountGroup.NameKey groupName)
throws IOException, ConfigInvalidException {
Ref ref = allUsersRepo.exactRef(RefNames.REFS_GROUPNAMES);
if (ref == null) {
return Optional.empty();
@@ -105,7 +106,7 @@ public class GroupNameNotes extends VersionedMetaData {
ObjectReader reader = revWalk.getObjectReader()) {
RevCommit notesCommit = revWalk.parseCommit(ref.getObjectId());
NoteMap noteMap = NoteMap.read(reader, notesCommit);
ObjectId noteDataBlobId = noteMap.get(getNoteKey(new AccountGroup.NameKey(groupName)));
ObjectId noteDataBlobId = noteMap.get(getNoteKey(groupName));
if (noteDataBlobId == null) {
return Optional.empty();
}

15
java/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyChecker.java
View File

@@ -218,7 +218,7 @@ public class GroupsNoteDbConsistencyChecker {
Repository allUsersRepo, InternalGroup group) throws IOException {
List<ConsistencyCheckInfo.ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, group.getName(), group.getGroupUUID());
allUsersRepo, group.getNameKey(), group.getGroupUUID());
problems.forEach(GroupsNoteDbConsistencyChecker::logConsistencyProblem);
}
@@ -232,10 +232,10 @@ public class GroupsNoteDbConsistencyChecker {
*/
@VisibleForTesting
static List<ConsistencyProblemInfo> checkWithGroupNameNotes(
Repository allUsersRepo, String groupName, AccountGroup.UUID groupUUID) throws IOException {
Repository allUsersRepo, AccountGroup.NameKey groupName, AccountGroup.UUID groupUUID)
throws IOException {
try {
Optional<GroupReference> groupRef =
GroupNameNotes.loadOneGroupReference(allUsersRepo, groupName);
Optional<GroupReference> groupRef = GroupNameNotes.loadGroup(allUsersRepo, groupName);
if (!groupRef.isPresent()) {
return ImmutableList.of(
@@ -243,7 +243,6 @@ public class GroupsNoteDbConsistencyChecker {
}
AccountGroup.UUID uuid = groupRef.get().getUUID();
String name = groupRef.get().getName();
List<ConsistencyProblemInfo> problems = new ArrayList<>();
if (!Objects.equals(groupUUID, uuid)) {
@@ -253,9 +252,11 @@ public class GroupsNoteDbConsistencyChecker {
groupName, groupUUID, uuid));
}
if (!Objects.equals(groupName, name)) {
String name = groupName.get();
String actualName = groupRef.get().getName();
if (!Objects.equals(name, actualName)) {
problems.add(
warning("group note of name '%s' claims to represent name of '%s'", groupName, name));
warning("group note of name '%s' claims to represent name of '%s'", name, actualName));
}
return problems;
} catch (ConfigInvalidException e) {

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

@@ -517,7 +517,7 @@ public class GroupNameNotesTest {
}
private Optional<GroupReference> loadGroup(AccountGroup.NameKey groupName) throws Exception {
return GroupNameNotes.loadOneGroupReference(repo, groupName.get());
return GroupNameNotes.loadGroup(repo, groupName);
}
private void commit(GroupNameNotes groupNameNotes) throws IOException {

14
javatests/com/google/gerrit/server/group/db/GroupsNoteDbConsistencyCheckerTest.java
View File

@@ -30,7 +30,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
public void groupNamesRefIsMissing() throws Exception {
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(warning("Group with name 'g-1' doesn't exist in the list of all names"));
}
@@ -40,7 +40,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-2", "[group]\n\tuuid = uuid-2\n\tname = g-2\n");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(warning("Group with name 'g-1' doesn't exist in the list of all names"));
}
@@ -50,7 +50,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-1\n\tname = g-1\n");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems).isEmpty();
}
@@ -59,7 +59,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-2\n\tname = g-1\n");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(
warning(
@@ -72,7 +72,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-1\n\tname = g-2\n");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(warning("group note of name 'g-1' claims to represent name of 'g-2'"));
}
@@ -82,7 +82,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-1", "[group]\n\tuuid = uuid-2\n\tname = g-2\n");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(
warning(
@@ -97,7 +97,7 @@ public class GroupsNoteDbConsistencyCheckerTest extends AbstractGroupTest {
updateGroupNamesRef("g-1", "[invalid");
List<ConsistencyProblemInfo> problems =
GroupsNoteDbConsistencyChecker.checkWithGroupNameNotes(
allUsersRepo, "g-1", new AccountGroup.UUID("uuid-1"));
allUsersRepo, new AccountGroup.NameKey("g-1"), new AccountGroup.UUID("uuid-1"));
assertThat(problems)
.containsExactly(
warning(