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

src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchAction.java

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);
       }
     }
   }

src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommand.java

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

src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchActionTest.java

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);

src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/FetchCommandTest.java

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));
   }
 }