Call upload URL using Retrofit's @url annotation (#391)

* call upload URL using Retrofit's @url annotation

* fix formatting

Co-authored-by: David Menear <dmenear@users.noreply.github.com>

Author: dmenear

analytics-core/src/main/java/com/segment/analytics/http/SegmentService.java

@@ -1,12 +1,14 @@
 package com.segment.analytics.http;
 
 import com.segment.analytics.messages.Batch;
+import okhttp3.HttpUrl;
 import retrofit2.Call;
 import retrofit2.http.Body;
 import retrofit2.http.POST;
+import retrofit2.http.Url;
 
 /** REST interface for the Segment API. */
 public interface SegmentService {
-  @POST(".")
-  Call<UploadResponse> upload(@Body Batch batch);
+  @POST
+  Call<UploadResponse> upload(@Url HttpUrl uploadUrl, @Body Batch batch);
 }

analytics/src/main/java/com/segment/analytics/Analytics.java

@@ -431,14 +431,15 @@ public void log(String message) {
       Retrofit restAdapter =
           new Retrofit.Builder()
               .addConverterFactory(GsonConverterFactory.create(gson))
-              .baseUrl(endpoint)
+              .baseUrl(DEFAULT_ENDPOINT)
               .client(client)
               .build();
 
       SegmentService segmentService = restAdapter.create(SegmentService.class);
 
       AnalyticsClient analyticsClient =
           AnalyticsClient.create(
+              endpoint,
               segmentService,
               queueCapacity,
               flushQueueSize,

analytics/src/main/java/com/segment/analytics/internal/AnalyticsClient.java

@@ -30,6 +30,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import okhttp3.HttpUrl;
 import retrofit2.Call;
 import retrofit2.Response;
 
@@ -50,6 +51,7 @@ public class AnalyticsClient {
   }
 
   private final BlockingQueue<Message> messageQueue;
+  private final HttpUrl uploadUrl;
   private final SegmentService service;
   private final int size;
   private final int maximumRetries;
@@ -63,6 +65,7 @@ public class AnalyticsClient {
   private final AtomicBoolean isShutDown;
 
   public static AnalyticsClient create(
+      HttpUrl uploadUrl,
       SegmentService segmentService,
       int queueCapacity,
       int flushQueueSize,
@@ -75,6 +78,7 @@ public static AnalyticsClient create(
       List<Callback> callbacks) {
     return new AnalyticsClient(
         new LinkedBlockingQueue<Message>(queueCapacity),
+        uploadUrl,
         segmentService,
         flushQueueSize,
         flushIntervalInMillis,
@@ -89,6 +93,7 @@ public static AnalyticsClient create(
 
   AnalyticsClient(
       BlockingQueue<Message> messageQueue,
+      HttpUrl uploadUrl,
       SegmentService service,
       int maxQueueSize,
       long flushIntervalInMillis,
@@ -100,6 +105,7 @@ public static AnalyticsClient create(
       List<Callback> callbacks,
       AtomicBoolean isShutDown) {
     this.messageQueue = messageQueue;
+    this.uploadUrl = uploadUrl;
     this.service = service;
     this.size = maxQueueSize;
     this.maximumRetries = maximumRetries;
@@ -355,7 +361,7 @@ boolean upload() {
       client.log.print(VERBOSE, "Uploading batch %s.", batch.sequence());
 
       try {
-        Call<UploadResponse> call = client.service.upload(batch);
+        Call<UploadResponse> call = client.service.upload(client.uploadUrl, batch);
         Response<UploadResponse> response = call.execute();
 
         if (response.isSuccessful()) {

analytics/src/test/java/com/segment/analytics/internal/AnalyticsClientTest.java

@@ -93,6 +93,7 @@ public void setUp() {
   AnalyticsClient newClient() {
     return new AnalyticsClient(
         messageQueue,
+        null,
         segmentService,
         50,
         TimeUnit.HOURS.toMillis(1),
@@ -271,6 +272,7 @@ public void flushHowManyTimesNecessaryToStayWithinLimit() throws InterruptedExce
     AnalyticsClient client =
         new AnalyticsClient(
             messageQueue,
+            null,
             segmentService,
             50,
             TimeUnit.HOURS.toMillis(1),
@@ -360,7 +362,7 @@ public void batchRetriesForNetworkErrors() {
     Response<UploadResponse> failureResponse = Response.error(429, ResponseBody.create(null, ""));
 
     // Throw a network error 3 times.
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenReturn(Calls.response(failureResponse))
         .thenReturn(Calls.response(failureResponse))
         .thenReturn(Calls.response(failureResponse))
@@ -370,7 +372,7 @@ public void batchRetriesForNetworkErrors() {
     batchUploadTask.run();
 
     // Verify that we tried to upload 4 times, 3 failed and 1 succeeded.
-    verify(segmentService, times(4)).upload(batch);
+    verify(segmentService, times(4)).upload(null, batch);
     verify(callback).success(trackMessage);
   }
 
@@ -385,7 +387,7 @@ public void batchRetriesForHTTP5xxErrors() {
     Response<UploadResponse> successResponse = Response.success(200, response);
     Response<UploadResponse> failResponse =
         Response.error(500, ResponseBody.create(null, "Server Error"));
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenReturn(Calls.response(failResponse))
         .thenReturn(Calls.response(failResponse))
         .thenReturn(Calls.response(failResponse))
@@ -395,7 +397,7 @@ public void batchRetriesForHTTP5xxErrors() {
     batchUploadTask.run();
 
     // Verify that we tried to upload 4 times, 3 failed and 1 succeeded.
-    verify(segmentService, times(4)).upload(batch);
+    verify(segmentService, times(4)).upload(null, batch);
     verify(callback).success(trackMessage);
   }
 
@@ -409,7 +411,7 @@ public void batchRetriesForHTTP429Errors() {
     Response<UploadResponse> successResponse = Response.success(200, response);
     Response<UploadResponse> failResponse =
         Response.error(429, ResponseBody.create(null, "Rate Limited"));
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenReturn(Calls.response(failResponse))
         .thenReturn(Calls.response(failResponse))
         .thenReturn(Calls.response(failResponse))
@@ -419,7 +421,7 @@ public void batchRetriesForHTTP429Errors() {
     batchUploadTask.run();
 
     // Verify that we tried to upload 4 times, 3 failed and 1 succeeded.
-    verify(segmentService, times(4)).upload(batch);
+    verify(segmentService, times(4)).upload(null, batch);
     verify(callback).success(trackMessage);
   }
 
@@ -432,13 +434,13 @@ public void batchDoesNotRetryForNon5xxAndNon429HTTPErrors() {
     // Throw a HTTP error that should not be retried.
     Response<UploadResponse> failResponse =
         Response.error(404, ResponseBody.create(null, "Not Found"));
-    when(segmentService.upload(batch)).thenReturn(Calls.response(failResponse));
+    when(segmentService.upload(null, batch)).thenReturn(Calls.response(failResponse));
 
     BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES);
     batchUploadTask.run();
 
     // Verify we only tried to upload once.
-    verify(segmentService).upload(batch);
+    verify(segmentService).upload(null, batch);
     verify(callback).failure(eq(trackMessage), any(IOException.class));
   }
 
@@ -449,13 +451,13 @@ public void batchDoesNotRetryForNonNetworkErrors() {
     Batch batch = batchFor(trackMessage);
 
     Call<UploadResponse> networkFailure = Calls.failure(new RuntimeException());
-    when(segmentService.upload(batch)).thenReturn(networkFailure);
+    when(segmentService.upload(null, batch)).thenReturn(networkFailure);
 
     BatchUploadTask batchUploadTask = new BatchUploadTask(client, BACKO, batch, DEFAULT_RETRIES);
     batchUploadTask.run();
 
     // Verify we only tried to upload once.
-    verify(segmentService).upload(batch);
+    verify(segmentService).upload(null, batch);
     verify(callback).failure(eq(trackMessage), any(RuntimeException.class));
   }
 
@@ -465,7 +467,7 @@ public void givesUpAfterMaxRetries() {
     TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build();
     Batch batch = batchFor(trackMessage);
 
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenAnswer(
             new Answer<Call<UploadResponse>>() {
               public Call<UploadResponse> answer(InvocationOnMock invocation) {
@@ -480,7 +482,7 @@ public Call<UploadResponse> answer(InvocationOnMock invocation) {
 
     // DEFAULT_RETRIES == maxRetries
     // tries 11(one normal run + 10 retries) even though default is 50 in AnalyticsClient.java
-    verify(segmentService, times(11)).upload(batch);
+    verify(segmentService, times(11)).upload(null, batch);
     verify(callback)
         .failure(
             eq(trackMessage),
@@ -499,7 +501,7 @@ public void hasDefaultRetriesSetTo3() {
     TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build();
     Batch batch = batchFor(trackMessage);
 
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenAnswer(
             new Answer<Call<UploadResponse>>() {
               public Call<UploadResponse> answer(InvocationOnMock invocation) {
@@ -514,7 +516,7 @@ public Call<UploadResponse> answer(InvocationOnMock invocation) {
 
     // DEFAULT_RETRIES == maxRetries
     // tries 11(one normal run + 10 retries)
-    verify(segmentService, times(4)).upload(batch);
+    verify(segmentService, times(4)).upload(null, batch);
     verify(callback)
         .failure(
             eq(trackMessage),
@@ -619,7 +621,7 @@ public void neverRetries() {
     TrackMessage trackMessage = TrackMessage.builder("foo").userId("bar").build();
     Batch batch = batchFor(trackMessage);
 
-    when(segmentService.upload(batch))
+    when(segmentService.upload(null, batch))
         .thenAnswer(
             new Answer<Call<UploadResponse>>() {
               public Call<UploadResponse> answer(InvocationOnMock invocation) {
@@ -633,7 +635,7 @@ public Call<UploadResponse> answer(InvocationOnMock invocation) {
     batchUploadTask.run();
 
     // runs once but never retries
-    verify(segmentService, times(1)).upload(batch);
+    verify(segmentService, times(1)).upload(null, batch);
     verify(callback)
         .failure(
             eq(trackMessage),
@@ -830,6 +832,7 @@ public void submitBatchBelowThreshold() throws InterruptedException {
     AnalyticsClient client =
         new AnalyticsClient(
             messageQueue,
+            null,
             segmentService,
             50,
             TimeUnit.HOURS.toMillis(1),
@@ -869,6 +872,7 @@ public void submitBatchAboveThreshold() throws InterruptedException {
     AnalyticsClient client =
         new AnalyticsClient(
             messageQueue,
+            null,
             segmentService,
             50,
             TimeUnit.HOURS.toMillis(1),
@@ -907,6 +911,7 @@ public void submitManySmallMessagesBatchAboveThreshold() throws InterruptedExcep
     AnalyticsClient client =
         new AnalyticsClient(
             messageQueue,
+            null,
             segmentService,
             50,
             TimeUnit.HOURS.toMillis(1),