Fix FetchRefReplicatedEvent remote URI
When triggered by an applyObject execution, the triggered
FetchRefReplicatedEvent contained the `instanceLabel` that was used in
the request rather than the actual remote the ref was fetched from.
Lookup the remoteName and retrieve the actual URI from the source, so
that consumers of the FetchRefReplicatedEvent have visibility on the
actual source URL rather than the remote name in the configuration.
Bug: Issue 14865
Change-Id: I425b3deabd33d68ccdb4121d0772c04660292e31
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
index 7be4971..53adaaa 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
@@ -27,7 +27,11 @@
import com.googlesource.gerrit.plugins.replication.RemoteConfiguration;
import com.googlesource.gerrit.plugins.replication.ReplicationConfig;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
@@ -35,7 +39,7 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Source.Factory sourceFactory;
- private volatile List<Source> sources;
+ private volatile Map<String, Source> sources;
private boolean shuttingDown;
private final Provider<ReplicationQueue> replicationQueue;
@@ -56,22 +60,26 @@
@Override
public List<Source> getAll() {
- return sources.stream().filter(Objects::nonNull).collect(toList());
+ return sources.values().stream().filter(Objects::nonNull).collect(toList());
}
- private List<Source> allSources(
+ public Optional<Source> getByRemoteName(String remoteName) {
+ return Optional.ofNullable(sources.get(remoteName));
+ }
+
+ private Map<String, Source> allSources(
Source.Factory sourceFactory, List<RemoteConfiguration> sourceConfigurations) {
return sourceConfigurations.stream()
.filter((c) -> c instanceof SourceConfiguration)
.map((c) -> (SourceConfiguration) c)
.map(sourceFactory::create)
- .collect(toList());
+ .collect(Collectors.toMap(Source::getRemoteConfigName, Function.identity()));
}
@Override
public void startup(WorkQueue workQueue) {
shuttingDown = false;
- for (Source cfg : sources) {
+ for (Source cfg : sources.values()) {
cfg.start(workQueue);
}
}
@@ -96,7 +104,7 @@
}
int discarded = 0;
- for (Source cfg : sources) {
+ for (Source cfg : sources.values()) {
discarded += cfg.shutdown();
}
return discarded;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
index cdbd91e..3e6c07b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommand.java
@@ -31,17 +31,17 @@
import com.googlesource.gerrit.plugins.replication.pull.PullReplicationStateLogger;
import com.googlesource.gerrit.plugins.replication.pull.ReplicationState;
import com.googlesource.gerrit.plugins.replication.pull.ReplicationState.RefFetchResult;
+import com.googlesource.gerrit.plugins.replication.pull.Source;
+import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException;
import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject;
import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState;
import java.io.IOException;
-import java.net.URISyntaxException;
import java.util.Set;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.transport.RefSpec;
-import org.eclipse.jgit.transport.URIish;
public class ApplyObjectCommand {
@@ -58,17 +58,20 @@
private final ApplyObject applyObject;
private final ApplyObjectMetrics metrics;
private final DynamicItem<EventDispatcher> eventDispatcher;
+ private final SourcesCollection sourcesCollection;
@Inject
public ApplyObjectCommand(
PullReplicationStateLogger fetchStateLog,
ApplyObject applyObject,
ApplyObjectMetrics metrics,
- DynamicItem<EventDispatcher> eventDispatcher) {
+ DynamicItem<EventDispatcher> eventDispatcher,
+ SourcesCollection sourcesCollection) {
this.fetchStateLog = fetchStateLog;
this.applyObject = applyObject;
this.metrics = metrics;
this.eventDispatcher = eventDispatcher;
+ this.sourcesCollection = sourcesCollection;
}
public void applyObject(
@@ -82,16 +85,23 @@
try {
Context.setLocalEvent(true);
+ Source source =
+ sourcesCollection
+ .getByRemoteName(sourceLabel)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format("Could not find URI for %s remote", sourceLabel)));
eventDispatcher
.get()
.postEvent(
new FetchRefReplicatedEvent(
name.get(),
refName,
- new URIish(sourceLabel),
+ source.getURI(name),
getStatus(refUpdateState),
refUpdateState.getResult()));
- } catch (PermissionBackendException | URISyntaxException e) {
+ } catch (PermissionBackendException | IllegalStateException e) {
logger.atSevere().withCause(e).log(
"Cannot post event for ref '%s', project %s", refName, name);
} finally {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java
index 51051c0..dfd06f8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectCommandTest.java
@@ -31,6 +31,8 @@
import com.googlesource.gerrit.plugins.replication.pull.ApplyObjectMetrics;
import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent;
import com.googlesource.gerrit.plugins.replication.pull.PullReplicationStateLogger;
+import com.googlesource.gerrit.plugins.replication.pull.Source;
+import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData;
import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData;
import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException;
@@ -38,8 +40,11 @@
import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject;
import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState;
import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.URIish;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -54,6 +59,7 @@
private static final String TEST_REF_NAME = "refs/changes/01/1/1";
private static final NameKey TEST_PROJECT_NAME = Project.nameKey("test-project");
private static final String TEST_REMOTE_NAME = "test-remote-name";
+ private static URIish TEST_REMOTE_URI;
@Mock private PullReplicationStateLogger fetchStateLog;
@Mock private ApplyObject applyObject;
@@ -61,20 +67,26 @@
@Mock private DynamicItem<EventDispatcher> eventDispatcherDataItem;
@Mock private EventDispatcher eventDispatcher;
@Mock private Timer1.Context<String> timetContext;
+ @Mock private SourcesCollection sourceCollection;
+ @Mock private Source source;
@Captor ArgumentCaptor<Event> eventCaptor;
private ApplyObjectCommand objectUnderTest;
@Before
- public void setup() throws MissingParentObjectException, IOException {
+ public void setup() throws MissingParentObjectException, IOException, URISyntaxException {
RefUpdateState state = new RefUpdateState(TEST_REMOTE_NAME, RefUpdate.Result.NEW);
+ TEST_REMOTE_URI = new URIish("git://some.remote.uri");
when(eventDispatcherDataItem.get()).thenReturn(eventDispatcher);
when(metrics.start(anyString())).thenReturn(timetContext);
when(timetContext.stop()).thenReturn(100L);
when(applyObject.apply(any(), any(), any())).thenReturn(state);
+ when(sourceCollection.getByRemoteName(TEST_SOURCE_LABEL)).thenReturn(Optional.of(source));
+ when(source.getURI(TEST_PROJECT_NAME)).thenReturn(TEST_REMOTE_URI);
objectUnderTest =
- new ApplyObjectCommand(fetchStateLog, applyObject, metrics, eventDispatcherDataItem);
+ new ApplyObjectCommand(
+ fetchStateLog, applyObject, metrics, eventDispatcherDataItem, sourceCollection);
}
@Test
@@ -90,6 +102,7 @@
FetchRefReplicatedEvent fetchEvent = (FetchRefReplicatedEvent) sentEvent;
assertThat(fetchEvent.getProjectNameKey()).isEqualTo(TEST_PROJECT_NAME);
assertThat(fetchEvent.getRefName()).isEqualTo(TEST_REF_NAME);
+ assertThat(fetchEvent.targetUri).isEqualTo(TEST_REMOTE_URI.toASCIIString());
}
private RevisionData createSampleRevisionData() {
