Remove latest patchset assumption when creating SCC events.
We assumed that patchsets we created SCC events for always was the
latest patchset. This is not true. This new code use the patchset-data
for the correct commit instead of the latest patchset.
Solves: Jira RNDTS-4243
Change-Id: I33570906fb55b7a1cf5d0788c8e72bc1090316af
diff --git a/src/main/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EiffelEventMapper.java b/src/main/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EiffelEventMapper.java
index a57cf6a..11b4af2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EiffelEventMapper.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EiffelEventMapper.java
@@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -99,11 +100,17 @@
Optional<UUID> id)
throws ConfigInvalidException, IOException {
Optional<ChangeData> change = findChange(commit, repoName, branch);
+ Integer changeNbr = null;
+ Long epochMillisCreated = null;
/* Don't trust the ChangeData if there are inconsistencies between the index and what's
* actually merged. */
- if (change.isPresent() && isCurrentPatchsetOf(commit, change.get())) {
- return toScc(change.get(), parentEventIds);
+ if (change.isPresent()) {
+ Optional<PatchSet> patchSet = getPatchSetOf(commit, change.get());
+ if (patchSet.isPresent()) {
+ changeNbr = change.get().change().getId().get();
+ epochMillisCreated = patchSet.get().createdOn().toEpochMilli();
+ }
}
PersonInfo author = toPersonInfo(commit.getAuthorIdent());
@@ -116,30 +123,12 @@
repoName,
branch,
commit.getName(),
- null,
- commitTimeInEpochMillis(commit),
+ changeNbr,
+ epochMillisCreated != null ? epochMillisCreated : commitTimeInEpochMillis(commit),
parentEventIds,
id);
}
- @VisibleForTesting
- EiffelSourceChangeCreatedEventInfo toScc(ChangeData cd, List<UUID> parentEventIds)
- throws ConfigInvalidException, IOException {
- PersonInfo author = toPersonInfo(cd.getAuthor());
- return eventFactoryProvider
- .get()
- .createScc(
- author.name,
- author.email,
- author.username,
- cd.project().get(),
- cd.change().getDest().branch(),
- cd.notes().getCurrentPatchSet().commitId().getName(),
- cd.change().getId().get(),
- cd.notes().getCurrentPatchSet().createdOn().toEpochMilli(),
- parentEventIds);
- }
-
public EiffelSourceChangeSubmittedEventInfo toScs(
RevCommit commit,
String repoName,
@@ -213,14 +202,11 @@
return eventFactoryProvider.get().tagPURL(projectName, tagName);
}
- private boolean isCurrentPatchsetOf(RevCommit commit, ChangeData change) {
- return change
- .notes()
- .getCurrentPatchSet()
- .commitId()
- .getName()
- .toLowerCase()
- .equals(commit.getName().toLowerCase());
+ private Optional<PatchSet> getPatchSetOf(RevCommit commit, ChangeData change) {
+ String commitSha = commit.getName().toLowerCase();
+ return change.notes().getPatchSets().values().stream()
+ .filter(patchSet -> patchSet.commitId().getName().toLowerCase().equals(commitSha))
+ .findAny();
}
private PersonInfo toPersonInfo(IdentifiedUser user) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EventMappingIT.java b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EventMappingIT.java
index 47dbec9..344646f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EventMappingIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/eventseiffel/mapping/EventMappingIT.java
@@ -32,7 +32,6 @@
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.AbstractModule;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.eventseiffel.EiffelEventsTest;
@@ -125,47 +124,32 @@
}
@Test
- public void sccFromChangeData() throws Exception {
+ public void sccFromChangeRevCommit() throws Exception {
PushOneCommit.Result res = createChange();
- ChangeData cd =
- queryProvider
- .get()
- .byBranchCommitOpen(
- project.get(),
- res.getChange().change().getDest().branch(),
- res.getCommit().getName())
- .get(0);
- EiffelSourceChangeCreatedEventInfo event = mapper.toScc(cd, PARENT_UUIDS);
-
- assertPersonInfo(event.data.author, admin.fullName(), admin.email(), admin.username());
- assertGitIdentifier(
- event.data.gitIdentifier, project.get(), "master", res.getCommit().getName());
- assertChangeInfo(
- event.data.change,
- String.valueOf(res.getPatchSetId().changeId().get()),
- project.get(),
- res.getCommit().getName());
- assertSccMeta(
- event.meta, Long.valueOf(res.getChange().currentPatchSet().createdOn().toEpochMilli()));
- assertSccLinks(event.links);
+ sccFromChangeRevCommit(res);
}
@Test
- public void sccFromChangeRevCommit() throws Exception {
+ public void sccFromChangeRevCommitThatIsNotCurrent() throws Exception {
PushOneCommit.Result res = createChange();
+ amendChange(res.getChangeId());
+ sccFromChangeRevCommit(res);
+ }
+
+ private void sccFromChangeRevCommit(PushOneCommit.Result result) throws Exception {
EiffelSourceChangeCreatedEventInfo event =
- mapper.toScc(res.getCommit(), project.get(), "master", PARENT_UUIDS);
+ mapper.toScc(result.getCommit(), project.get(), "master", PARENT_UUIDS);
assertPersonInfo(event.data.author, admin.fullName(), admin.email(), admin.username());
assertGitIdentifier(
- event.data.gitIdentifier, project.get(), "master", res.getCommit().getName());
+ event.data.gitIdentifier, project.get(), "master", result.getCommit().getName());
assertChangeInfo(
event.data.change,
- String.valueOf(res.getPatchSetId().changeId().get()),
+ String.valueOf(result.getPatchSetId().changeId().get()),
project.get(),
- res.getCommit().getName());
+ result.getCommit().getName());
assertSccMeta(
- event.meta, Long.valueOf(res.getChange().currentPatchSet().createdOn().toEpochMilli()));
+ event.meta, Long.valueOf(result.getChange().currentPatchSet().createdOn().toEpochMilli()));
assertSccLinks(event.links);
}
