Skip to content

Commit a410556

Browse files
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]>
1 parent 85b8dbd commit a410556

File tree

3 files changed

+89
-6
lines changed

3 files changed

+89
-6
lines changed

echo-core/src/main/java/com/netflix/spinnaker/echo/build/BuildInfoService.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ private List<Artifact> getArtifactsFromPropertyFile(BuildEvent event, String pro
119119
igorConfigurationProperties.isJobNameAsQueryParameter()
120120
? Retrofit2SyncCall.execute(
121121
igorService.getArtifactsWithJobQueryParameter(
122-
buildNumber, propertyFile, master, job))
122+
buildNumber, master, job, propertyFile))
123123
: Retrofit2SyncCall.execute(
124-
igorService.getArtifacts(buildNumber, propertyFile, master, job)));
124+
igorService.getArtifacts(buildNumber, master, job, propertyFile)));
125125
}
126126
return Collections.emptyList();
127127
}

echo-core/src/main/java/com/netflix/spinnaker/echo/services/IgorService.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,16 @@ Call<Map<String, Object>> getPropertyFileWithJobQueryParameter(
5959
@GET("builds/artifacts/{buildNumber}/{master}/{job}")
6060
Call<List<Artifact>> getArtifacts(
6161
@Path("buildNumber") Integer buildNumber,
62-
@Query("propertyFile") String propertyFile,
6362
@Path("master") String master,
64-
@Path(value = "job", encoded = true) String job);
63+
@Path(value = "job", encoded = true) String job,
64+
@Query("propertyFile") String propertyFile);
6565

6666
@GET("builds/artifacts/{buildNumber}/{master}")
6767
Call<List<Artifact>> getArtifactsWithJobQueryParameter(
6868
@Path("buildNumber") Integer buildNumber,
69-
@Query("propertyFile") String propertyFile,
7069
@Path("master") String master,
71-
@Query(value = "job") String job);
70+
@Query(value = "job") String job,
71+
@Query("propertyFile") String propertyFile);
7272

7373
@GET("artifacts/{provider}/{packageName}")
7474
Call<List<String>> getVersions(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2025 OpsMx, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.netflix.spinnaker.echo.services;
18+
19+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
24+
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
25+
26+
import com.github.tomakehurst.wiremock.WireMockServer;
27+
import com.github.tomakehurst.wiremock.client.WireMock;
28+
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
29+
import com.netflix.spinnaker.echo.util.RetrofitUtils;
30+
import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
31+
import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
32+
import okhttp3.OkHttpClient;
33+
import org.junit.jupiter.api.AfterEach;
34+
import org.junit.jupiter.api.BeforeEach;
35+
import org.junit.jupiter.api.Test;
36+
import retrofit2.Retrofit;
37+
import retrofit2.converter.jackson.JacksonConverterFactory;
38+
39+
public class IgorServiceTest {
40+
WireMockServer wireMockServer;
41+
int port;
42+
IgorService igorService;
43+
String baseUrl = "http://localhost:PORT";
44+
45+
@BeforeEach
46+
void setUp() {
47+
wireMockServer = new WireMockServer(WireMockConfiguration.options().dynamicPort());
48+
wireMockServer.start();
49+
port = wireMockServer.port();
50+
WireMock.configureFor("localhost", port);
51+
52+
baseUrl = baseUrl.replaceFirst("PORT", String.valueOf(port));
53+
54+
igorService =
55+
new Retrofit.Builder()
56+
.baseUrl(RetrofitUtils.getBaseUrl(baseUrl))
57+
.client(new OkHttpClient())
58+
.addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
59+
.addConverterFactory(JacksonConverterFactory.create())
60+
.build()
61+
.create(IgorService.class);
62+
}
63+
64+
@AfterEach
65+
void tearDown() {
66+
wireMockServer.stop();
67+
}
68+
69+
@Test
70+
void testIgorService_getArtifacts() {
71+
stubFor(
72+
get(urlEqualTo("/builds/artifacts/1/master/job?propertyFile=propertyFile"))
73+
.willReturn(
74+
aResponse()
75+
.withStatus(200)
76+
.withBody("[{\"id\": \"gs://this/is/my/id1\", \"type\": \"gcs/object\"}]")));
77+
78+
Retrofit2SyncCall.execute(igorService.getArtifacts(1, "master", "job", "propertyFile"));
79+
80+
verify(
81+
1, getRequestedFor(urlEqualTo("/builds/artifacts/1/master/job?propertyFile=propertyFile")));
82+
}
83+
}

0 commit comments

Comments
 (0)