fix(retrofit2): fix A @path parameter must not come after a @Query …

…error (#1478) (#1479) * test(retrofit2): add a test to demonstrate the following error: java.lang.IllegalArgumentException: A @path parameter must not come after a @query. * fix(retrofit2): fix `A @path parameter must not come after a @Query` error (cherry picked from commit f170364) Co-authored-by: Kiran Godishala <[email protected]>
spinnaker · Feb 27, 2025 · a410556 · a410556
1 parent 85b8dbd
commit a410556
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 6 deletions.
diff --git a/echo-core/src/main/java/com/netflix/spinnaker/echo/build/BuildInfoService.java b/echo-core/src/main/java/com/netflix/spinnaker/echo/build/BuildInfoService.java
@@ -119,9 +119,9 @@ private List<Artifact> getArtifactsFromPropertyFile(BuildEvent event, String pro
               igorConfigurationProperties.isJobNameAsQueryParameter()
                   ? Retrofit2SyncCall.execute(
                       igorService.getArtifactsWithJobQueryParameter(
-                          buildNumber, propertyFile, master, job))
+                          buildNumber, master, job, propertyFile))
                   : Retrofit2SyncCall.execute(
-                      igorService.getArtifacts(buildNumber, propertyFile, master, job)));
+                      igorService.getArtifacts(buildNumber, master, job, propertyFile)));
     }
     return Collections.emptyList();
   }

diff --git a/echo-core/src/main/java/com/netflix/spinnaker/echo/services/IgorService.java b/echo-core/src/main/java/com/netflix/spinnaker/echo/services/IgorService.java
@@ -59,16 +59,16 @@ Call<Map<String, Object>> getPropertyFileWithJobQueryParameter(
   @GET("builds/artifacts/{buildNumber}/{master}/{job}")
   Call<List<Artifact>> getArtifacts(
       @Path("buildNumber") Integer buildNumber,
-      @Query("propertyFile") String propertyFile,
       @Path("master") String master,
-      @Path(value = "job", encoded = true) String job);
+      @Path(value = "job", encoded = true) String job,
+      @Query("propertyFile") String propertyFile);
 
   @GET("builds/artifacts/{buildNumber}/{master}")
   Call<List<Artifact>> getArtifactsWithJobQueryParameter(
       @Path("buildNumber") Integer buildNumber,
-      @Query("propertyFile") String propertyFile,
       @Path("master") String master,
-      @Query(value = "job") String job);
+      @Query(value = "job") String job,
+      @Query("propertyFile") String propertyFile);
 
   @GET("artifacts/{provider}/{packageName}")
   Call<List<String>> getVersions(

diff --git a/echo-core/src/test/java/com/netflix/spinnaker/echo/services/IgorServiceTest.java b/echo-core/src/test/java/com/netflix/spinnaker/echo/services/IgorServiceTest.java
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2025 OpsMx, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.echo.services;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import com.netflix.spinnaker.echo.util.RetrofitUtils;
+import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
+import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
+import okhttp3.OkHttpClient;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import retrofit2.Retrofit;
+import retrofit2.converter.jackson.JacksonConverterFactory;
+
+public class IgorServiceTest {
+  WireMockServer wireMockServer;
+  int port;
+  IgorService igorService;
+  String baseUrl = "http://localhost:PORT";
+
+  @BeforeEach
+  void setUp() {
+    wireMockServer = new WireMockServer(WireMockConfiguration.options().dynamicPort());
+    wireMockServer.start();
+    port = wireMockServer.port();
+    WireMock.configureFor("localhost", port);
+
+    baseUrl = baseUrl.replaceFirst("PORT", String.valueOf(port));
+
+    igorService =
+        new Retrofit.Builder()
+            .baseUrl(RetrofitUtils.getBaseUrl(baseUrl))
+            .client(new OkHttpClient())
+            .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
+            .addConverterFactory(JacksonConverterFactory.create())
+            .build()
+            .create(IgorService.class);
+  }
+
+  @AfterEach
+  void tearDown() {
+    wireMockServer.stop();
+  }
+
+  @Test
+  void testIgorService_getArtifacts() {
+    stubFor(
+        get(urlEqualTo("/builds/artifacts/1/master/job?propertyFile=propertyFile"))
+            .willReturn(
+                aResponse()
+                    .withStatus(200)
+                    .withBody("[{\"id\": \"gs://this/is/my/id1\", \"type\": \"gcs/object\"}]")));
+
+    Retrofit2SyncCall.execute(igorService.getArtifacts(1, "master", "job", "propertyFile"));
+
+    verify(
+        1, getRequestedFor(urlEqualTo("/builds/artifacts/1/master/job?propertyFile=propertyFile")));
+  }
+}