diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2024-02-27 00:07:36 +0000 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2024-03-20 00:25:13 +0000 |
commit | 6fdefee3915550177f820d26cef9c258486e31cc (patch) | |
tree | b2f9ec90dd740f0b06bfc7b9ac70e76daed83bea | |
parent | 1d9663378f6b990f19d54b60491f456583c09102 (diff) |
Expose change's serverid in Change class
The Change class is created based on index lookup and subsequent
load from NoteDb. Before this change, even though the serverId information
was potentially available, it was not copied to the in-memory object
making more difficult for the other parts of the Gerrit code to understand
if the change was created locally or imported from a different server.
Copy the serverId to the Change fields so that it become easier and
clearer to understand where the change was initially coming from.
Release-Notes: skip
Forward-Compatible: checked
Change-Id: Ic0e2cf4d8e77a6c3f9a64e2572698b48e3dd90e1
3 files changed, 34 insertions, 1 deletions
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java index 66e1a9644d..567d82d1cd 100644 --- a/java/com/google/gerrit/entities/Change.java +++ b/java/com/google/gerrit/entities/Change.java @@ -427,6 +427,9 @@ public final class Change { /** Locally assigned unique identifier of the change */ private Id changeId; + /** ServerId of the Gerrit instance that has created the change */ + private String serverId; + /** Globally assigned unique identifier of the change */ private Key changeKey; @@ -528,6 +531,22 @@ public final class Change { return changeId; } + /** + * Set the serverId of the Gerrit instance that created the change. It can be set to null for + * testing purposes in the protobuf converter tests. + */ + public void setServerId(@Nullable String serverId) { + this.serverId = serverId; + } + + /** + * ServerId of the Gerrit instance that created the change. It could be null when the change is + * not fetched from NoteDb but obtained through protobuf deserialisation. + */ + public @Nullable String getServerId() { + return serverId; + } + /** 32 bit integer identity for a change. */ public int getChangeId() { return changeId.get(); diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java index b0079d7002..dd50966750 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java @@ -358,6 +358,7 @@ public abstract class ChangeNotesState { change.setOwner(c.owner()); change.setDest(BranchNameKey.create(change.getProject(), c.branch())); change.setCreatedOn(c.createdOn()); + change.setServerId(serverId()); copyNonConstructorColumnsTo(change); } diff --git a/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java index bd4b2b1562..5ed6400589 100644 --- a/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java +++ b/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java @@ -32,6 +32,7 @@ import org.junit.Test; public class ChangeProtoConverterTest { private final ChangeProtoConverter changeProtoConverter = ChangeProtoConverter.INSTANCE; + private static final String TEST_SERVER_ID = "test-server-id"; @Test public void allValuesConvertedToProto() { @@ -42,6 +43,7 @@ public class ChangeProtoConverterTest { Account.id(35), BranchNameKey.create(Project.nameKey("project 67"), "branch 74"), Instant.ofEpochMilli(987654L)); + change.setServerId(TEST_SERVER_ID); change.setLastUpdatedOn(Instant.ofEpochMilli(1234567L)); change.setStatus(Change.Status.MERGED); change.setCurrentPatchSet( @@ -91,6 +93,7 @@ public class ChangeProtoConverterTest { Account.id(35), BranchNameKey.create(Project.nameKey("project 67"), "branch-74"), Instant.ofEpochMilli(987654L)); + change.setServerId(TEST_SERVER_ID); Entities.Change proto = changeProtoConverter.toProto(change); @@ -126,6 +129,7 @@ public class ChangeProtoConverterTest { Account.id(35), BranchNameKey.create(Project.nameKey("project 67"), "branch-74"), Instant.ofEpochMilli(987654L)); + change.setServerId(TEST_SERVER_ID); // O as ID actually means that no current patch set is present. change.setCurrentPatchSet(PatchSet.id(Change.id(14), 0), null, null); @@ -163,6 +167,7 @@ public class ChangeProtoConverterTest { Account.id(35), BranchNameKey.create(Project.nameKey("project 67"), "branch-74"), Instant.ofEpochMilli(987654L)); + change.setServerId(TEST_SERVER_ID); change.setCurrentPatchSet(PatchSet.id(Change.id(14), 23), "subject ABC", null); Entities.Change proto = changeProtoConverter.toProto(change); @@ -191,7 +196,7 @@ public class ChangeProtoConverterTest { } @Test - public void allValuesConvertedToProtoAndBackAgain() { + public void allValuesConvertedToProtoAndBackAgainExceptServerId() { Change change = new Change( Change.key("change 1"), @@ -199,6 +204,7 @@ public class ChangeProtoConverterTest { Account.id(35), BranchNameKey.create(Project.nameKey("project 67"), "branch-74"), Instant.ofEpochMilli(987654L)); + change.setServerId(TEST_SERVER_ID); change.setLastUpdatedOn(Instant.ofEpochMilli(1234567L)); change.setStatus(Change.Status.MERGED); change.setCurrentPatchSet( @@ -212,6 +218,11 @@ public class ChangeProtoConverterTest { change.setRevertOf(Change.id(180)); Change convertedChange = changeProtoConverter.fromProto(changeProtoConverter.toProto(change)); + + // Change serverId is not one of the protobuf definitions, hence is not supposed to be converted + // from proto + assertThat(convertedChange.getServerId()).isNull(); + change.setServerId(null); assertEqualChange(convertedChange, change); } @@ -278,6 +289,7 @@ public class ChangeProtoConverterTest { .hasFields( ImmutableMap.<String, Type>builder() .put("changeId", Change.Id.class) + .put("serverId", String.class) .put("changeKey", Change.Key.class) .put("createdOn", Instant.class) .put("lastUpdatedOn", Instant.class) @@ -302,6 +314,7 @@ public class ChangeProtoConverterTest { // an AutoValue. private static void assertEqualChange(Change change, Change expectedChange) { assertThat(change.getChangeId()).isEqualTo(expectedChange.getChangeId()); + assertThat(change.getServerId()).isEqualTo(expectedChange.getServerId()); assertThat(change.getKey()).isEqualTo(expectedChange.getKey()); assertThat(change.getCreatedOn()).isEqualTo(expectedChange.getCreatedOn()); assertThat(change.getLastUpdatedOn()).isEqualTo(expectedChange.getLastUpdatedOn()); |