Create a user-context if not propagated by Gerrit
Because of (Bug: Issue 10941), the user context could be
absent and thus all the subsequent operations performed on
the change would fail.
Provide a one-off context using the updater's identity
so that even when Gerrit isn't providing a valid context
the overall OWNERS processing logic would still succeed.
Change-Id: If9f2087519180baddd996f41bdda56942b3f6886
diff --git a/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/GitRefListener.java
index 2cfd54d..eae0cb8 100644
--- a/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/GitRefListener.java
+++ b/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/GitRefListener.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes;
+import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -30,12 +31,17 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import java.io.IOException;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
@@ -57,22 +63,58 @@
private final ReviewerManager reviewerManager;
+ private final OneOffRequestContext oneOffReqCtx;
+
+ private Provider<CurrentUser> currentUserProvider;
+
@Inject
public GitRefListener(
GerritApi api,
PatchListCache patchListCache,
GitRepositoryManager repositoryManager,
Accounts accounts,
- ReviewerManager reviewerManager) {
+ ReviewerManager reviewerManager,
+ OneOffRequestContext oneOffReqCtx,
+ Provider<CurrentUser> currentUserProvider) {
this.api = api;
this.patchListCache = patchListCache;
this.repositoryManager = repositoryManager;
this.accounts = accounts;
this.reviewerManager = reviewerManager;
+ this.oneOffReqCtx = oneOffReqCtx;
+ this.currentUserProvider = currentUserProvider;
}
@Override
public void onGitReferenceUpdated(Event event) {
+ AccountInfo updaterAccountInfo = event.getUpdater();
+ CurrentUser currentUser = currentUserProvider.get();
+ if (currentUser.isIdentifiedUser()) {
+ handleGitReferenceUpdated(event);
+ } else if (updaterAccountInfo != null) {
+ handleGitReferenceUpdatedAsUser(event, new Account.Id(updaterAccountInfo._accountId));
+ } else {
+ handleGitReferenceUpdatedAsServer(event);
+ }
+ }
+
+ private void handleGitReferenceUpdatedAsUser(Event event, Account.Id updaterAccountId) {
+ try (ManualRequestContext ctx = oneOffReqCtx.openAs(updaterAccountId)) {
+ handleGitReferenceUpdated(event);
+ } catch (OrmException e) {
+ logger.warn("Unable to process event {} on project {}", event, event.getProjectName(), e);
+ }
+ }
+
+ private void handleGitReferenceUpdatedAsServer(Event event) {
+ try (ManualRequestContext ctx = oneOffReqCtx.open()) {
+ handleGitReferenceUpdated(event);
+ } catch (OrmException e) {
+ logger.warn("Unable to process event {} on project {}", event, event.getProjectName(), e);
+ }
+ }
+
+ private void handleGitReferenceUpdated(Event event) {
String projectName = event.getProjectName();
Repository repository;
try {
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
index aa6db4f..f009bde 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
@@ -21,7 +21,13 @@
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import org.eclipse.jgit.transport.ReceiveCommand.Type;
import org.junit.Test;
@@ -30,6 +36,14 @@
sysModule = "com.vmware.gerrit.owners.common.GitRefListenerIT$TestModule")
public class GitRefListenerIT extends LightweightPluginDaemonTest {
+ @Inject GitRefListenerTest gitRefListener;
+ @Inject SchemaFactory<ReviewDb> schemaFactory;
+ @Inject ThreadLocalRequestContext requestContext;
+
+ String aRefChange = RefNames.REFS_CHANGES + "01/01/01";
+ String anOldObjectId = "anOldRef";
+ String aNewObjectId = "aNewRef";
+
public static class TestModule extends AbstractModule {
@Override
protected void configure() {
@@ -39,15 +53,14 @@
@Test
public void shouldNotProcessNoteDbOnlyRefs() {
- GitRefListenerTest gitRefListener = getPluginInstance(GitRefListenerTest.class);
-
- String aRefChange = RefNames.REFS_CHANGES + "01/01" + RefNames.META_SUFFIX;
- String anOldObjectId = "anOldRef";
- String aNewObjectId = "aNewRef";
-
ReferenceUpdatedEventTest refUpdatedEvent =
new ReferenceUpdatedEventTest(
- project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE);
+ project,
+ RefNames.REFS_CHANGES + "01/01" + RefNames.META_SUFFIX,
+ anOldObjectId,
+ aNewObjectId,
+ Type.CREATE,
+ admin.id);
gitRefListener.onGitReferenceUpdated(refUpdatedEvent);
assertEquals(0, gitRefListener.getProcessedEvents());
@@ -55,21 +68,33 @@
@Test
public void shouldProcessRefChanges() {
- GitRefListenerTest gitRefListener = getPluginInstance(GitRefListenerTest.class);
-
- String aRefChange = RefNames.REFS_CHANGES + "01/01/01";
- String anOldObjectId = "anOldRef";
- String aNewObjectId = "aNewRef";
-
- ReferenceUpdatedEventTest refUpdatedEvent =
- new ReferenceUpdatedEventTest(
- project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE);
-
- gitRefListener.onGitReferenceUpdated(refUpdatedEvent);
+ gitRefListener.onGitReferenceUpdated(newRefUpdateEvent());
assertEquals(1, gitRefListener.getProcessedEvents());
}
- private <T> T getPluginInstance(Class<T> clazz) {
- return plugin.getSysInjector().getInstance(clazz);
+ @Test
+ public void shouldRetrieveChangeFromAnonymousContext() throws Exception {
+ try (ManualRequestContext ctx =
+ new ManualRequestContext(new AnonymousUser(), schemaFactory, requestContext)) {
+ gitRefListener.onGitReferenceUpdated(newRefUpdateEvent());
+ assertEquals(1, gitRefListener.getProcessedEvents());
+ }
+ }
+
+ @Test
+ public void shouldRetrieveChangeFromAnonymousContextWithoutAccountId() throws Exception {
+ ReferenceUpdatedEventTest refUpdateWithoutAccountId =
+ new ReferenceUpdatedEventTest(
+ project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE, null);
+ try (ManualRequestContext ctx =
+ new ManualRequestContext(new AnonymousUser(), schemaFactory, requestContext)) {
+ gitRefListener.onGitReferenceUpdated(refUpdateWithoutAccountId);
+ assertEquals(1, gitRefListener.getProcessedEvents());
+ }
+ }
+
+ private ReferenceUpdatedEventTest newRefUpdateEvent() {
+ return new ReferenceUpdatedEventTest(
+ project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE, admin.id);
}
}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
index ac6b579..8fb940c 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
@@ -16,9 +16,12 @@
package com.vmware.gerrit.owners.common;
import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import org.eclipse.jgit.lib.Repository;
import org.junit.Ignore;
@@ -32,8 +35,17 @@
PatchListCache patchListCache,
GitRepositoryManager repositoryManager,
Accounts accounts,
- ReviewerManager reviewerManager) {
- super(api, patchListCache, repositoryManager, accounts, reviewerManager);
+ ReviewerManager reviewerManager,
+ OneOffRequestContext oneOffReqCtx,
+ Provider<CurrentUser> currentUserProvider) {
+ super(
+ api,
+ patchListCache,
+ repositoryManager,
+ accounts,
+ reviewerManager,
+ oneOffReqCtx,
+ currentUserProvider);
}
@Override
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java
index 446a8bb..9946502 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java
@@ -18,6 +18,8 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Account.Id;
import com.google.gerrit.reviewdb.client.Project;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.junit.Ignore;
@@ -30,18 +32,21 @@
private final String oldObjectId;
private final String newObjectId;
private final ReceiveCommand.Type type;
+ private final Id eventAccountId;
public ReferenceUpdatedEventTest(
Project.NameKey project,
String ref,
String oldObjectId,
String newObjectId,
- ReceiveCommand.Type type) {
+ ReceiveCommand.Type type,
+ Account.Id eventAccountId) {
this.projectName = project.get();
this.ref = ref;
this.oldObjectId = oldObjectId;
this.newObjectId = newObjectId;
this.type = type;
+ this.eventAccountId = eventAccountId;
}
@Override
@@ -81,7 +86,11 @@
@Override
public AccountInfo getUpdater() {
- return null;
+ if (eventAccountId == null) {
+ return null;
+ }
+
+ return new AccountInfo(eventAccountId.get());
}
@Override
