opendev/gerrit
Code Issues Proposed changes

Convert Change.Key to AutoValue

Browse Source 
See I6982fb24 for context.

Change-Id: I3917b0b2a7ef67ea9dfde36adca741d1cba61bfd
This commit is contained in:
Dave Borowitz
2019-04-19 09:21:46 -07:00
parent 5c9a6500a8
commit 5fefac6756
17 changed files with 51 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
java/com/google/gerrit/reviewdb/client/Change.java
View File

@@ -16,10 +16,11 @@ package com.google.gerrit.reviewdb.client;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
import com.google.auto.value.AutoValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gwtorm.client.IntKey;
import com.google.gwtorm.client.StringKey;
import com.google.gwtorm.client.StandardKeyEncoder;
import java.sql.Timestamp;
import java.util.Arrays;
@@ -251,29 +252,26 @@ public final class Change {
}
}
public static Key key(String key) {
return new AutoValue_Change_Key(key);
}
/**
* Globally unique identification of this change. This generally takes the form of a string
* "Ixxxxxx...", and is stored in the Change-Id footer of a commit.
*/
public static class Key extends StringKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L;
protected String id;
protected Key() {}
public Key(String id) {
this.id = id;
@AutoValue
public abstract static class Key {
// TODO(dborowitz): This hardly seems worth it: why would someone pass a URL-encoded change key?
// Ideally the standard key() factory method would enforce the format and throw IAE.
public static Key parse(String str) {
return Change.key(new StandardKeyEncoder().decode(str));
}
@Override
abstract String key();
public String get() {
return id;
}
@Override
protected void set(String newValue) {
id = newValue;
return key();
}
/** Construct a key that is after all keys prefixed by this key. */
@@ -281,7 +279,7 @@ public final class Change {
final StringBuilder revEnd = new StringBuilder(get().length() + 1);
revEnd.append(get());
revEnd.append('\u9fa5');
return new Key(revEnd.toString());
return Change.key(revEnd.toString());
}
/** Obtain a shorter version of this key string, using a leading prefix. */
@@ -290,11 +288,9 @@ public final class Change {
return s.substring(0, Math.min(s.length(), 9));
}
/** Parse a Change.Key out of a string representation. */
public static Key parse(String str) {
final Key r = new Key();
r.fromString(str);
return r;
@Override
public String toString() {
return get();
}
}

2
java/com/google/gerrit/reviewdb/converter/ChangeKeyProtoConverter.java
View File

@@ -28,7 +28,7 @@ public enum ChangeKeyProtoConverter implements ProtoConverter<Entities.Change_Ke
@Override
public Change.Key fromProto(Entities.Change_Key proto) {
return new Change.Key(proto.getId());
return Change.key(proto.getId());
}
@Override

4
java/com/google/gerrit/server/change/ChangeInserter.java
View File

@@ -201,7 +201,7 @@ public class ChangeInserter implements InsertChangeOp {
rw.parseBody(commit);
List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
if (!idList.isEmpty()) {
return new Change.Key(idList.get(idList.size() - 1).trim());
return Change.key(idList.get(idList.size() - 1).trim());
}
ObjectId changeId =
ChangeIdUtil.computeChangeId(
@@ -212,7 +212,7 @@ public class ChangeInserter implements InsertChangeOp {
commit.getShortMessage());
StringBuilder changeIdStr = new StringBuilder();
changeIdStr.append("I").append(ObjectId.toString(changeId));
return new Change.Key(changeIdStr.toString());
return Change.key(changeIdStr.toString());
}
public PatchSet.Id getPatchSetId() {

2
java/com/google/gerrit/server/change/ChangeTriplet.java
View File

@@ -53,7 +53,7 @@ public abstract class ChangeTriplet {
String changeId = Url.decode(triplet.substring(z + 1));
return Optional.of(
new AutoValue_ChangeTriplet(
Branch.nameKey(Project.nameKey(project), branch), new Change.Key(changeId)));
Branch.nameKey(Project.nameKey(project), branch), Change.key(changeId)));
}
public final Project.NameKey project() {

5
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -2087,8 +2087,7 @@ class ReceiveCommits {
List<String> idList = c.getFooterLines(CHANGE_ID);
if (!idList.isEmpty()) {
pending.put(
c, lookupByChangeKey(c, new Change.Key(idList.get(idList.size() - 1).trim())));
pending.put(c, lookupByChangeKey(c, Change.key(idList.get(idList.size() - 1).trim())));
} else {
pending.put(c, lookupByCommit(c));
}
@@ -3180,7 +3179,7 @@ class ReceiveCommits {
byKey = executeIndexQuery(() -> openChangesByKeyByBranch(branch));
}
ChangeNotes onto = byKey.get(new Change.Key(changeId.trim()));
ChangeNotes onto = byKey.get(Change.key(changeId.trim()));
if (onto != null) {
newPatchSets++;
// Hold onto this until we're done with the walk, as the call to

4
java/com/google/gerrit/server/git/receive/ReplaceOp.java
View File

@@ -477,9 +477,9 @@ public class ReplaceOp implements BatchUpdateOp {
List<String> idList = commit.getFooterLines(CHANGE_ID);
if (idList.isEmpty()) {
change.setKey(new Change.Key("I" + commitId.name()));
change.setKey(Change.key("I" + commitId.name()));
} else {
change.setKey(new Change.Key(idList.get(idList.size() - 1).trim()));
change.setKey(Change.key(idList.get(idList.size() - 1).trim()));
}
}

2
java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -237,7 +237,7 @@ class ChangeNotesParser {
return ChangeNotesState.create(
tip.copy(),
id,
new Change.Key(changeId),
Change.key(changeId),
createdOn,
lastUpdatedOn,
ownerId,

2
java/com/google/gerrit/server/notedb/ChangeNotesState.java
View File

@@ -595,7 +595,7 @@ public abstract class ChangeNotesState {
private static ChangeColumns toChangeColumns(Change.Id changeId, ChangeColumnsProto proto) {
ChangeColumns.Builder b =
ChangeColumns.builder()
.changeKey(new Change.Key(proto.getChangeKey()))
.changeKey(Change.key(proto.getChangeKey()))
.createdOn(new Timestamp(proto.getCreatedOn()))
.lastUpdatedOn(new Timestamp(proto.getLastUpdatedOn()))
.owner(Account.id(proto.getOwner()))

2
java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
View File

@@ -225,7 +225,7 @@ public class ProjectsConsistencyChecker {
// important thing for callers is that auto-closable changes are closed. Which of the
// commits is used to auto-close a change if there are several candidates is of minor
// importance and hence can be non-deterministic.
Change.Key changeKey = new Change.Key(changeId);
Change.Key changeKey = Change.key(changeId);
if (!changeIdToMergedSha1.containsKey(changeKey)) {
changeIdToMergedSha1.put(changeKey, commitId);
}

4
java/com/google/gerrit/server/restapi/change/CherryPickChange.java
View File

@@ -230,9 +230,9 @@ public class CherryPickChange {
final List<String> idList = cherryPickCommit.getFooterLines(FooterConstants.CHANGE_ID);
if (!idList.isEmpty()) {
final String idStr = idList.get(idList.size() - 1).trim();
changeKey = new Change.Key(idStr);
changeKey = Change.key(idStr);
} else {
changeKey = new Change.Key("I" + computedChangeId.name());
changeKey = Change.key("I" + computedChangeId.name());
}
Branch.NameKey newDest = Branch.nameKey(project, destRef.getName());

2
java/com/google/gerrit/testing/TestChanges.java
View File

@@ -55,7 +55,7 @@ public class TestChanges {
Change.Id changeId = new Change.Id(id);
Change c =
new Change(
new Change.Key("Iabcd1234abcd1234abcd1234abcd1234abcd1234"),
Change.key("Iabcd1234abcd1234abcd1234abcd1234abcd1234"),
changeId,
userId,
Branch.nameKey(project, "master"),

6
javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@@ -156,7 +156,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
assertCommit(project, "refs/heads/master");
ChangeData cd =
Iterables.getOnlyElement(queryProvider.get().byKey(new Change.Key(r.getChangeId())));
Iterables.getOnlyElement(queryProvider.get().byKey(Change.key(r.getChangeId())));
RevCommit c = r.getCommit();
PatchSet.Id psId = cd.currentPatchSet().getId();
assertThat(psId.get()).isEqualTo(1);
@@ -183,7 +183,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
pushCommitTo(commit, master);
assertCommit(project, master);
ChangeData cd =
Iterables.getOnlyElement(queryProvider.get().byKey(new Change.Key(r.getChangeId())));
Iterables.getOnlyElement(queryProvider.get().byKey(Change.key(r.getChangeId())));
assertThat(cd.change().isMerged()).isTrue();
RemoteRefUpdate.Status status = pushCommitTo(commit, "refs/for/other");
@@ -192,7 +192,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
pushCommitTo(commit, other);
assertCommit(project, other);
for (ChangeData c : queryProvider.get().byKey(new Change.Key(r.getChangeId()))) {
for (ChangeData c : queryProvider.get().byKey(Change.key(r.getChangeId()))) {
if (c.change().getDest().branch().equals(other)) {
assertThat(c.change().isMerged()).isTrue();
}

2
javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
View File

@@ -195,7 +195,7 @@ public abstract class AbstractReindexTests extends StandaloneSiteTest {
// Updating and searching old schema version works.
Provider<InternalChangeQuery> queryProvider =
ctx.getInjector().getProvider(InternalChangeQuery.class);
assertThat(queryProvider.get().byKey(new Change.Key(changeId))).hasSize(1);
assertThat(queryProvider.get().byKey(Change.key(changeId))).hasSize(1);
assertThat(queryProvider.get().byTopicOpen("topic1")).isEmpty();
GerritApi gApi = ctx.getInjector().getInstance(GerritApi.class);

2
javatests/com/google/gerrit/reviewdb/client/ChangeTest.java
View File

@@ -157,7 +157,7 @@ public class ChangeTest {
public void keyToString() {
String key = "Ideadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
assertThat(ObjectId.isId(key.substring(1))).isTrue();
assertThat(new Change.Key(key).toString()).isEqualTo(key);
assertThat(Change.key(key).toString()).isEqualTo(key);
}
private static void assertRef(int changeId, String refName) {

9
javatests/com/google/gerrit/reviewdb/converter/ChangeKeyProtoConverterTest.java
View File

@@ -30,7 +30,7 @@ public class ChangeKeyProtoConverterTest {
@Test
public void allValuesConvertedToProto() {
Change.Key changeKey = new Change.Key("change-1");
Change.Key changeKey = Change.key("change-1");
Entities.Change_Key proto = changeKeyProtoConverter.toProto(changeKey);
@@ -40,7 +40,7 @@ public class ChangeKeyProtoConverterTest {
@Test
public void allValuesConvertedToProtoAndBackAgain() {
Change.Key changeKey = new Change.Key("change-52");
Change.Key changeKey = Change.key("change-52");
Change.Key convertedChangeKey =
changeKeyProtoConverter.fromProto(changeKeyProtoConverter.toProto(changeKey));
@@ -61,7 +61,8 @@ public class ChangeKeyProtoConverterTest {
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
assertThatSerializedClass(Change.Key.class).hasFields(ImmutableMap.of("id", String.class));
public void methodsExistAsExpected() {
assertThatSerializedClass(Change.Key.class)
.hasAutoValueMethods(ImmutableMap.of("key", String.class));
}
}

12
javatests/com/google/gerrit/reviewdb/converter/ChangeProtoConverterTest.java
View File

@@ -38,7 +38,7 @@ public class ChangeProtoConverterTest {
public void allValuesConvertedToProto() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch 74"),
@@ -88,7 +88,7 @@ public class ChangeProtoConverterTest {
public void mandatoryValuesConvertedToProto() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch-74"),
@@ -124,7 +124,7 @@ public class ChangeProtoConverterTest {
public void currentPatchSetIsAlwaysSetWhenConvertedToProto() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch-74"),
@@ -162,7 +162,7 @@ public class ChangeProtoConverterTest {
public void originalSubjectIsNotAutomaticallySetToSubjectWhenConvertedToProto() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch-74"),
@@ -199,7 +199,7 @@ public class ChangeProtoConverterTest {
public void allValuesConvertedToProtoAndBackAgain() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch-74"),
@@ -224,7 +224,7 @@ public class ChangeProtoConverterTest {
public void mandatoryValuesConvertedToProtoAndBackAgain() {
Change change =
new Change(
new Change.Key("change 1"),
Change.key("change 1"),
new Change.Id(14),
Account.id(35),
Branch.nameKey(Project.nameKey("project 67"), "branch-74"),

4
javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
View File

@@ -75,7 +75,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
public void setUp() throws Exception {
cols =
ChangeColumns.builder()
.changeKey(new Change.Key(CHANGE_KEY))
.changeKey(Change.key(CHANGE_KEY))
.createdOn(new Timestamp(123456L))
.lastUpdatedOn(new Timestamp(234567L))
.owner(Account.id(1000))
@@ -98,7 +98,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
newBuilder()
.columns(
cols.toBuilder()
.changeKey(new Change.Key("Ieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"))
.changeKey(Change.key("Ieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"))
.build())
.build(),
ChangeNotesStateProto.newBuilder()