Use refName instead of objectId in the fetch REST Api
When requesting a remote fetch, specify the ref-name to be fetched
instead of the SHA1 of the ref.
Fetching by SHA1 has two issues:
- The remote Git server may disallow exposing the fetch by SHA1
- Fetching the SHA1 does not update the local ref
Change-Id: I52fba1166b0d5df3a498af2702328cc7723b9a41
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
index ce02d56..736fbf0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java
@@ -51,7 +51,7 @@
public static class Input {
public String label;
- public String objectId;
+ public String refName;
public boolean async;
}
@@ -62,8 +62,8 @@
throw new BadRequestException("Source label cannot be null or empty");
}
- if (Strings.isNullOrEmpty(input.objectId)) {
- throw new BadRequestException("Ref-update objectId cannot be null or empty");
+ if (Strings.isNullOrEmpty(input.refName)) {
+ throw new BadRequestException("Ref-update refname cannot be null or empty");
}
if (input.async) {
@@ -83,7 +83,7 @@
private Response<?> applySync(Project.NameKey project, Input input)
throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
TimeoutException {
- command.fetch(project, input.label, input.objectId);
+ command.fetch(project, input.label, input.refName);
return Response.created(input);
}
@@ -117,16 +117,16 @@
@Override
public void run() {
try {
- command.fetch(project, input.label, input.objectId);
+ command.fetch(project, input.label, input.refName);
} catch (InterruptedException
| ExecutionException
| RemoteConfigurationMissingException
| TimeoutException e) {
log.atSevere().withCause(e).log(
- "Exception during the async fetch call for project {}, label {} and object id {}",
+ "Exception during the async fetch call for project {}, label {} and ref name {}",
project.get(),
input.label,
- input.objectId);
+ input.refName);
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
index 749adc2..b3ab83d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java
@@ -45,7 +45,7 @@
this.sources = sources;
}
- public void fetch(Project.NameKey name, String label, String objectId)
+ public void fetch(Project.NameKey name, String label, String refName)
throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
TimeoutException {
ReplicationState state =
@@ -59,7 +59,7 @@
}
try {
- Future<?> future = source.get().schedule(name, objectId, state, true);
+ Future<?> future = source.get().schedule(name, refName, state, true);
state.markAllFetchTasksScheduled();
future.get(source.get().getTimeout(), TimeUnit.SECONDS);
} catch (ExecutionException
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
index 045b882..39c6717 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java
@@ -49,7 +49,7 @@
FetchAction fetchAction;
String label = "instance-2-label";
String url = "file:///gerrit-host/instance-1/git/${name}.git";
- String objectId = "c90989ed7a8ab01f1bdd022872428f020b866358";
+ String refName = "refs/heads/master";
String location = "http://gerrit-host/a/config/server/tasks/08d173e9";
int taskId = 1234;
@@ -83,7 +83,7 @@
public void shouldReturnCreatedResponseCode() throws RestApiException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
Response<?> response = fetchAction.apply(projectResource, inputParams);
@@ -92,10 +92,10 @@
@SuppressWarnings("cast")
@Test
- public void shouldReturnSourceUrlAndObjectIdAsAResponseBody() throws Exception {
+ public void shouldReturnSourceUrlAndrefNameAsAResponseBody() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
Response<?> response = fetchAction.apply(projectResource, inputParams);
@@ -105,7 +105,7 @@
@Test(expected = BadRequestException.class)
public void shouldThrowBadRequestExceptionWhenMissingLabel() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
fetchAction.apply(projectResource, inputParams);
}
@@ -114,13 +114,13 @@
public void shouldThrowBadRequestExceptionWhenEmptyLabel() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = "";
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
fetchAction.apply(projectResource, inputParams);
}
@Test(expected = BadRequestException.class)
- public void shouldThrowBadRequestExceptionWhenMissingObjectId() throws Exception {
+ public void shouldThrowBadRequestExceptionWhenMissingrefName() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
@@ -128,10 +128,10 @@
}
@Test(expected = BadRequestException.class)
- public void shouldThrowBadRequestExceptionWhenEmptyObjectId() throws Exception {
+ public void shouldThrowBadRequestExceptionWhenEmptyrefName() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = "";
+ inputParams.refName = "";
fetchAction.apply(projectResource, inputParams);
}
@@ -142,7 +142,7 @@
RemoteConfigurationMissingException, TimeoutException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
doThrow(new InterruptedException()).when(fetchCommand).fetch(any(), any(), any());
@@ -155,7 +155,7 @@
RemoteConfigurationMissingException, TimeoutException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = "non-existing-label";
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
doThrow(new RemoteConfigurationMissingException(""))
.when(fetchCommand)
@@ -170,7 +170,7 @@
RemoteConfigurationMissingException, TimeoutException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
doThrow(new ExecutionException(new RuntimeException()))
.when(fetchCommand)
@@ -185,7 +185,7 @@
RemoteConfigurationMissingException, TimeoutException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
doThrow(new IllegalStateException()).when(fetchCommand).fetch(any(), any(), any());
@@ -198,7 +198,7 @@
RemoteConfigurationMissingException, TimeoutException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
doThrow(new TimeoutException()).when(fetchCommand).fetch(any(), any(), any());
@@ -209,7 +209,7 @@
public void shouldReturnScheduledTaskForAsyncCall() throws RestApiException {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
inputParams.async = true;
Response<?> response = fetchAction.apply(projectResource, inputParams);
@@ -220,7 +220,7 @@
public void shouldLocationHeaderForAsyncCall() throws Exception {
FetchAction.Input inputParams = new FetchAction.Input();
inputParams.label = label;
- inputParams.objectId = objectId;
+ inputParams.refName = refName;
inputParams.async = true;
Response<?> response = fetchAction.apply(projectResource, inputParams);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
index 7536caf..e625e0f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java
@@ -45,7 +45,7 @@
@RunWith(MockitoJUnitRunner.class)
public class FetchCommandTest {
- private static final String OBJECT_ID_TO_FETCH = "8f22c45da5ce298291b7329552568aae1bb62c10";
+ private static final String REF_NAME_TO_FETCH = "refs/heads/master";
@Mock ReplicationState state;
@Mock ReplicationState.Factory fetchReplicationStateFactory;
@Mock PullReplicationStateLogger fetchStateLog;
@@ -71,27 +71,27 @@
when(fetchReplicationStateFactory.create(any())).thenReturn(state);
when(source.getRemoteConfigName()).thenReturn(label);
when(sources.getAll()).thenReturn(Lists.newArrayList(source));
- when(source.schedule(projectName, OBJECT_ID_TO_FETCH, state, true))
+ when(source.schedule(projectName, REF_NAME_TO_FETCH, state, true))
.thenReturn(CompletableFuture.completedFuture(null));
objectUnderTest = new FetchCommand(fetchReplicationStateFactory, fetchStateLog, sources);
}
@Test
- public void shouldFetchRefUpdate()
+ public void shouldScheduleRefFetch()
throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
TimeoutException {
- objectUnderTest.fetch(projectName, label, OBJECT_ID_TO_FETCH);
+ objectUnderTest.fetch(projectName, label, REF_NAME_TO_FETCH);
- verify(source, times(1)).schedule(projectName, OBJECT_ID_TO_FETCH, state, true);
+ verify(source, times(1)).schedule(projectName, REF_NAME_TO_FETCH, state, true);
}
@Test
public void shouldMarkAllFetchTasksScheduled()
throws InterruptedException, ExecutionException, RemoteConfigurationMissingException,
TimeoutException {
- objectUnderTest.fetch(projectName, label, OBJECT_ID_TO_FETCH);
+ objectUnderTest.fetch(projectName, label, REF_NAME_TO_FETCH);
- verify(source, times(1)).schedule(projectName, OBJECT_ID_TO_FETCH, state, true);
+ verify(source, times(1)).schedule(projectName, REF_NAME_TO_FETCH, state, true);
verify(state, times(1)).markAllFetchTasksScheduled();
}
@@ -99,7 +99,7 @@
public void shouldUpdateStateWhenRemoteConfigNameIsMissing() {
assertThrows(
RemoteConfigurationMissingException.class,
- () -> objectUnderTest.fetch(projectName, "unknownLabel", OBJECT_ID_TO_FETCH));
+ () -> objectUnderTest.fetch(projectName, "unknownLabel", REF_NAME_TO_FETCH));
verify(fetchStateLog, times(1)).error(anyString(), eq(state));
}
@@ -108,12 +108,12 @@
public void shouldUpdateStateWhenInterruptedException()
throws InterruptedException, ExecutionException, TimeoutException {
when(future.get(anyLong(), eq(TimeUnit.SECONDS))).thenThrow(new InterruptedException());
- when(source.schedule(projectName, OBJECT_ID_TO_FETCH, state, true)).thenReturn(future);
+ when(source.schedule(projectName, REF_NAME_TO_FETCH, state, true)).thenReturn(future);
InterruptedException e =
assertThrows(
InterruptedException.class,
- () -> objectUnderTest.fetch(projectName, label, OBJECT_ID_TO_FETCH));
+ () -> objectUnderTest.fetch(projectName, label, REF_NAME_TO_FETCH));
verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
}
@@ -123,12 +123,12 @@
throws InterruptedException, ExecutionException, TimeoutException {
when(future.get(anyLong(), eq(TimeUnit.SECONDS)))
.thenThrow(new ExecutionException(new Exception()));
- when(source.schedule(projectName, OBJECT_ID_TO_FETCH, state, true)).thenReturn(future);
+ when(source.schedule(projectName, REF_NAME_TO_FETCH, state, true)).thenReturn(future);
ExecutionException e =
assertThrows(
ExecutionException.class,
- () -> objectUnderTest.fetch(projectName, label, OBJECT_ID_TO_FETCH));
+ () -> objectUnderTest.fetch(projectName, label, REF_NAME_TO_FETCH));
verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
}
@@ -137,12 +137,12 @@
public void shouldUpdateStateWhenTimeoutException()
throws InterruptedException, ExecutionException, TimeoutException {
when(future.get(anyLong(), eq(TimeUnit.SECONDS))).thenThrow(new TimeoutException());
- when(source.schedule(projectName, OBJECT_ID_TO_FETCH, state, true)).thenReturn(future);
+ when(source.schedule(projectName, REF_NAME_TO_FETCH, state, true)).thenReturn(future);
TimeoutException e =
assertThrows(
TimeoutException.class,
- () -> objectUnderTest.fetch(projectName, label, OBJECT_ID_TO_FETCH));
+ () -> objectUnderTest.fetch(projectName, label, REF_NAME_TO_FETCH));
verify(fetchStateLog, times(1)).error(anyString(), eq(e), eq(state));
}
}
