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
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index 66e1a96..567d82d 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -427,6 +427,9 @@
/** 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 @@
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 b0079d7..dd50966 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -358,6 +358,7 @@
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 bd4b2b1..5ed6400 100644
--- a/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/ChangeProtoConverterTest.java
@@ -32,6 +32,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
@Test
- public void allValuesConvertedToProtoAndBackAgain() {
+ public void allValuesConvertedToProtoAndBackAgainExceptServerId() {
Change change =
new Change(
Change.key("change 1"),
@@ -199,6 +204,7 @@
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 @@
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 @@
.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 @@
// 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());
