Skip to content

Commit d482a39

Browse files
samvaitysamvaity
authored
Add support for non-null path param in codegen URLs (Azure#44696)
1 parent 922364f commit d482a39

File tree

13 files changed

+217
-139
lines changed

13 files changed

+217
-139
lines changed

‎sdk/clientcore/annotation-processor-test/src/main/java/io/clientcore/annotation/processor/test/implementation/TestInterfaceClientImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Response<FooListResult> listNextFooListResult(@PathParam(value = "nextLink", enc
9393
RequestOptions requestOptions);
9494

9595
@HttpRequestInformation(method = HttpMethod.GET, path = "foos", expectedStatusCodes = { 200 })
96-
Response<List<Foo>> listFoo(@HostParam("uri") String uri, @QueryParam(value = "tags", multipleQueryParams = true) List<String> tags, RequestOptions requestOptions);
96+
Response<List<Foo>> listFoo(@HostParam("uri") String uri, RequestOptions requestOptions);
9797

9898
@HttpRequestInformation(method = HttpMethod.GET, path = "{nextLink}", expectedStatusCodes = { 200 })
9999
Response<List<Foo>> listNextFoo(@PathParam(value = "nextLink", encoded = true) String nextLink,

‎sdk/clientcore/annotation-processor-test/src/test/java/io/clientcore/annotation/processor/test/PagingOperationTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void testListFoo() {
5757
TestInterfaceClientService testInterface = TestInterfaceClientService.getNewInstance(pipeline);
5858

5959
// Retrieve initial response
60-
Response<List<Foo>> initialResponse = testInterface.listFoo(uri, null, RequestOptions.none());
60+
Response<List<Foo>> initialResponse = testInterface.listFoo(uri, RequestOptions.none());
6161

6262
List<Foo> fooFirstPageResponse = initialResponse.getValue();
6363
assertNotNull(fooFirstPageResponse);

‎sdk/clientcore/annotation-processor/checkstyle-suppressions.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<!-- This file is generated by the /eng/scripts/linting_suppression_generator.py script. -->
44

55
<suppressions>
6-
<suppress files="io.clientcore.annotation.processor.utils.ResponseBodyModeGeneration.java" checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" />
6+
<suppress files="io.clientcore.annotation.processor.utils.ResponseHandler.java" checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" />
77
<suppress files="io.clientcore.annotation.processor.AnnotationProcessor.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
88
<suppress files="io.clientcore.annotation.processor.models.HttpRequestContext.java" checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck" />
99
</suppressions>

‎sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/AnnotationProcessor.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,7 @@ private HttpRequestContext createHttpRequestContext(ExecutableElement requestMet
150150
method.setHttpMethod(httpRequestInfo.method());
151151
method.setExpectedStatusCodes(httpRequestInfo.expectedStatusCodes());
152152

153-
// Add return type as an import
154-
setReturnTypeForMethod(method, requestMethod, templateInput);
153+
method.setMethodReturnType(requestMethod.getReturnType());
155154
boolean isEncoded = false;
156155
// Process parameters
157156
for (VariableElement param : requestMethod.getParameters()) {
@@ -170,6 +169,10 @@ private HttpRequestContext createHttpRequestContext(ExecutableElement requestMet
170169
if (pathParam.encoded()) {
171170
isEncoded = true;
172171
}
172+
if (pathParam.value() == null) {
173+
throw new IllegalArgumentException(
174+
"Path parameter '" + param.getSimpleName().toString() + "' must not be null.");
175+
}
173176
method.addSubstitution(
174177
new Substitution(pathParam.value(), param.getSimpleName().toString(), pathParam.encoded()));
175178
} else if (headerParam != null) {
@@ -194,11 +197,6 @@ private HttpRequestContext createHttpRequestContext(ExecutableElement requestMet
194197
return method;
195198
}
196199

197-
private void setReturnTypeForMethod(HttpRequestContext method, ExecutableElement requestMethod,
198-
TemplateInput templateInput) {
199-
method.setMethodReturnType(requestMethod.getReturnType());
200-
}
201-
202200
private static String getHost(TemplateInput templateInput, HttpRequestContext method, boolean isEncoded) {
203201
String rawHost;
204202
if (isEncoded) {

‎sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/templating/JavaParserTemplateProcessor.java‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
import java.util.Map;
5353
import java.util.stream.Collectors;
5454

55-
import static io.clientcore.annotation.processor.utils.ResponseBodyModeGeneration.generateResponseHandling;
55+
import static io.clientcore.annotation.processor.utils.ResponseHandler.generateResponseHandling;
5656

5757
/**
5858
* This class generates the implementation of the service interface.
@@ -310,7 +310,6 @@ private void configureInternalMethod(MethodDeclaration internalMethod, HttpReque
310310
BlockStmt body = internalMethod.getBody().get();
311311

312312
initializeHttpRequest(body, method);
313-
addHeadersToRequest(body, method);
314313
boolean serializationFormatSet = addRequestBody(body, method);
315314
addRequestOptionsToRequestIfPresent(body, method);
316315
finalizeHttpRequest(body, method.getMethodReturnType(), method, serializationFormatSet);
@@ -336,7 +335,7 @@ private void addRequestOptionsToRequestIfPresent(BlockStmt body, HttpRequestCont
336335
}
337336
}
338337

339-
private void initializeHttpRequest(BlockStmt body, HttpRequestContext method) {
338+
void initializeHttpRequest(BlockStmt body, HttpRequestContext method) {
340339
body.tryAddImportToParentCompilationUnit(HttpRequest.class);
341340
body.tryAddImportToParentCompilationUnit(HttpMethod.class);
342341

@@ -347,17 +346,31 @@ private void initializeHttpRequest(BlockStmt body, HttpRequestContext method) {
347346

348347
if (useProvidedUri) {
349348
body.addStatement(
350-
StaticJavaParser.parseStatement("String host = uri + \"/\" + \"" + method.getPath() + "\";"));
349+
StaticJavaParser.parseStatement("String url = uri + \"/\" + \"" + method.getPath() + "\";"));
351350
} else {
352-
body.addStatement(StaticJavaParser.parseStatement("String host = " + method.getHost() + ";"));
351+
body.addStatement(StaticJavaParser.parseStatement("String url = " + method.getHost() + ";"));
352+
}
353+
354+
// Iterate through the query parameters and append them to the url string if they are not null
355+
if (!method.getQueryParams().isEmpty()) {
356+
// Declare newUrl once
357+
Statement newUrlDeclaration = StaticJavaParser.parseStatement("String newUrl;");
358+
newUrlDeclaration.setComment(new LineComment("\n Append non-null query parameters"));
359+
body.addStatement(newUrlDeclaration);
360+
361+
method.getQueryParams().forEach((key, value) -> {
362+
body.addStatement(String.format("newUrl = CoreUtils.appendQueryParam(url, \"%s\", %s);", key, value));
363+
body.addStatement("if (newUrl != null) { url = newUrl; }");
364+
});
353365
}
354366

355367
Statement statement
356368
= StaticJavaParser.parseStatement("HttpRequest httpRequest = new HttpRequest().setMethod(HttpMethod."
357-
+ method.getHttpMethod() + ").setUri(host);");
369+
+ method.getHttpMethod() + ").setUri(url);");
358370

359371
statement.setLineComment("\n Create the HTTP request");
360372
body.addStatement(statement);
373+
addHeadersToRequest(body, method);
361374
}
362375

363376
private void addHeadersToRequest(BlockStmt body, HttpRequestContext method) {
@@ -366,25 +379,32 @@ private void addHeadersToRequest(BlockStmt body, HttpRequestContext method) {
366379
}
367380

368381
body.tryAddImportToParentCompilationUnit(HttpHeaderName.class);
369-
382+
StringBuilder httpRequestBuilder = new StringBuilder("httpRequest.getHeaders()"); // Start the header chaining
370383
for (Map.Entry<String, String> header : method.getHeaders().entrySet()) {
371384
boolean isStringType = method.getParameters()
372385
.stream()
373386
.anyMatch(parameter -> parameter.getName().equals(header.getValue())
374387
&& "String".equals(parameter.getShortTypeName()));
375388
String value = isStringType ? header.getValue() : "String.valueOf(" + header.getValue() + ")";
376-
377389
String constantName
378390
= LOWERCASE_HEADER_TO_HTTPHEADENAME_CONSTANT.get(header.getKey().toLowerCase(Locale.ROOT));
379391
if (constantName != null) {
380-
body.addStatement(StaticJavaParser.parseStatement(
381-
"httpRequest.getHeaders().add(HttpHeaderName." + constantName + ", " + value + ");"));
392+
httpRequestBuilder.append(".add(HttpHeaderName.")
393+
.append(constantName)
394+
.append(", ")
395+
.append(value)
396+
.append(")");
382397
} else {
383-
body.addStatement(
384-
StaticJavaParser.parseStatement("httpRequest.getHeaders().add(HttpHeaderName.fromString(\""
385-
+ header.getKey() + "\"), " + value + ");"));
398+
httpRequestBuilder.append(".add(HttpHeaderName.fromString(\"")
399+
.append(header.getKey())
400+
.append("\"), ")
401+
.append(value)
402+
.append(")");
403+
386404
}
387405
}
406+
// Finalize the statement
407+
body.addStatement(StaticJavaParser.parseStatement(httpRequestBuilder.toString() + ";"));
388408
}
389409

390410
private boolean addRequestBody(BlockStmt body, HttpRequestContext method) {

‎sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/utils/PathBuilder.java‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ public static String buildPath(String rawHost, HttpRequestContext method) {
3737
throw new NullPointerException("method cannot be null");
3838
}
3939

40-
boolean hasQueryParams = !method.getQueryParams().isEmpty();
41-
4240
// Pattern for substitution placeholders
4341
Pattern pattern = Pattern.compile("\\{(.+?)}");
4442
Matcher matcher = pattern.matcher(rawHost);
@@ -65,32 +63,6 @@ public static String buildPath(String rawHost, HttpRequestContext method) {
6563

6664
matcher.appendTail(buffer);
6765

68-
if (hasQueryParams) {
69-
buffer.append("?");
70-
71-
method.getQueryParams().forEach((key, value) -> {
72-
// Only append if both key and value are non-null and non-empty
73-
74-
if (key == null || key.isEmpty()) {
75-
throw new IllegalArgumentException("Query parameter key must not be null or empty");
76-
}
77-
78-
if (value != null && !value.isEmpty()) {
79-
buffer.append(key).append("=\" + ").append(value).append(" + \"&");
80-
}
81-
});
82-
83-
// Remove the trailing '&' if present
84-
if (buffer.length() > 0 && buffer.charAt(buffer.length() - 1) == '&') {
85-
buffer.setLength(buffer.length() - 1);
86-
}
87-
88-
// Remove the trailing '?' if no valid query parameters were appended
89-
if (buffer.length() > 0 && buffer.charAt(buffer.length() - 1) == '?') {
90-
buffer.setLength(buffer.length() - 1);
91-
}
92-
}
93-
9466
// Ensure the output is properly quoted
9567
if (buffer.charAt(0) != '"' && !rawHost.startsWith("{")) {
9668
buffer.insert(0, '"');

‎sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/utils/ResponseBodyModeGeneration.java‎ renamed to ‎sdk/clientcore/annotation-processor/src/main/java/io/clientcore/annotation/processor/utils/ResponseHandler.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
/**
2929
* Utility class to generate response body mode assignment and response handling based on the response body mode.
3030
*/
31-
public final class ResponseBodyModeGeneration {
31+
public final class ResponseHandler {
3232

3333
/**
3434
* Handles the generation of the complete response processing flow based on the return type.
@@ -279,6 +279,6 @@ private static void createResponseIfNecessary(BlockStmt body) {
279279
body.addStatement(StaticJavaParser.parseStatement("return response;"));
280280
}
281281

282-
private ResponseBodyModeGeneration() {
282+
private ResponseHandler() {
283283
}
284284
}

‎sdk/clientcore/annotation-processor/src/test/java/io/clientcore/annotation/processor/mocks/MockPathParam.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package io.clientcore.annotation.processor.mocks;
5+
6+
import io.clientcore.core.http.annotations.PathParam;
7+
8+
import java.lang.annotation.Annotation;
9+
10+
/**
11+
* Mock implementation of {@link PathParam} for testing purposes.
12+
*/
13+
public class MockPathParam implements PathParam {
14+
private final boolean encoded;
15+
private final String value;
16+
17+
/**
18+
* Creates an instance of {@link MockPathParam}.
19+
* @param value the path param value to be used for creating the mock.
20+
* @param encoded if the path param is encoded.
21+
*/
22+
public MockPathParam(String value, boolean encoded) {
23+
this.value = value;
24+
this.encoded = encoded;
25+
}
26+
27+
@Override
28+
public boolean encoded() {
29+
return encoded;
30+
}
31+
32+
@Override
33+
public String value() {
34+
return value;
35+
}
36+
37+
@Override
38+
public Class<? extends Annotation> annotationType() {
39+
return PathParam.class;
40+
}
41+
}

‎sdk/clientcore/annotation-processor/src/test/java/io/clientcore/annotation/processor/models/TemplateInputTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
package io.clientcore.annotation.processor.models;
55

66
import io.clientcore.annotation.processor.mocks.MockDeclaredType;
7+
import io.clientcore.annotation.processor.mocks.MockPathParam;
78
import io.clientcore.annotation.processor.mocks.MockTypeMirror;
9+
import io.clientcore.core.http.annotations.PathParam;
810
import io.clientcore.core.http.annotations.UnexpectedResponseExceptionDetail;
911
import org.junit.jupiter.api.Test;
1012

@@ -18,6 +20,7 @@
1820
import static org.junit.jupiter.api.Assertions.assertEquals;
1921
import static org.junit.jupiter.api.Assertions.assertFalse;
2022
import static org.junit.jupiter.api.Assertions.assertNull;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
2124
import static org.junit.jupiter.api.Assertions.assertTrue;
2225

2326
/**
@@ -97,6 +100,22 @@ void setAndGetUnexpectedResponseExceptionDetails() {
97100
assertEquals(details, templateInput.getUnexpectedResponseExceptionDetails());
98101
}
99102

103+
@Test
104+
void shouldThrowExceptionWhenPathParamValueIsNull() {
105+
String paramName = "testParam";
106+
PathParam pathParam = new MockPathParam(null, false); // Mock with null value
107+
HttpRequestContext method = new HttpRequestContext();
108+
109+
IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> {
110+
if (pathParam.value() == null) {
111+
throw new IllegalArgumentException("Path parameter '" + paramName + "' must not be null.");
112+
}
113+
method.addSubstitution(new Substitution(pathParam.value(), paramName, pathParam.encoded()));
114+
});
115+
116+
assertEquals("Path parameter 'testParam' must not be null.", exception.getMessage());
117+
}
118+
100119
private static final class MockUnexpectedResponseExceptionDetail implements UnexpectedResponseExceptionDetail {
101120
@Override
102121
public int[] statusCode() {

‎sdk/clientcore/annotation-processor/src/test/java/io/clientcore/annotation/processor/templating/HttpRequestInitializerTest.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package io.clientcore.annotation.processor.templating;
5+
6+
import io.clientcore.annotation.processor.models.HttpRequestContext;
7+
import io.clientcore.core.http.models.HttpMethod;
8+
import org.junit.jupiter.params.ParameterizedTest;
9+
import org.junit.jupiter.params.provider.CsvSource;
10+
11+
import static org.junit.jupiter.api.Assertions.assertTrue;
12+
13+
/**
14+
* Verify the formation of the HTTP request URL in the code generation.
15+
*/
16+
public class HttpRequestInitializerTest {
17+
18+
@ParameterizedTest
19+
@CsvSource({
20+
"GET, \"/my/uri/path\", key1, value1, key2, value2",
21+
"POST, \"/my/uri/path2\", key3, value3, key4, value4" })
22+
public void testInitializeHttpRequestWithParameterizedQueryParams(String httpMethod, String url, String queryKey1,
23+
String queryValue1, String queryKey2, String queryValue2) {
24+
25+
com.github.javaparser.ast.stmt.BlockStmt body = new com.github.javaparser.ast.stmt.BlockStmt();
26+
HttpRequestContext method = new HttpRequestContext();
27+
JavaParserTemplateProcessor processor = new JavaParserTemplateProcessor();
28+
29+
// Arrange: Set up method with query params
30+
method.setHost(url);
31+
method.setHttpMethod(HttpMethod.valueOf(httpMethod));
32+
method.addQueryParam(queryKey1, queryValue1);
33+
method.addQueryParam(queryKey2, queryValue2);
34+
method.addHeader("Content-Type", "application/json");
35+
method.addHeader("Content-Length", String.valueOf(0));
36+
37+
// Act: Call the method
38+
processor.initializeHttpRequest(body, method);
39+
40+
// Assert: Check if the generated code matches expectations
41+
String normalizedBody = body.toString().replaceAll("\\s+", " ").trim();
42+
43+
// Ensure URL initialization is present
44+
String expectedUrlStatement = "String url = " + url + ";";
45+
assertTrue(normalizedBody.contains(expectedUrlStatement));
46+
47+
// Ensure newUrl is declared only once
48+
assertTrue(normalizedBody.contains("String newUrl;"));
49+
50+
// Ensure each query parameter is appended correctly
51+
String expectedQueryStatement1 = "newUrl = CoreUtils.appendQueryParam(url, \"" + queryKey1 + "\", "
52+
+ queryValue1 + "); if (newUrl != null) { url = newUrl; }";
53+
String expectedQueryStatement2 = "newUrl = CoreUtils.appendQueryParam(url, \"" + queryKey2 + "\", "
54+
+ queryValue2 + "); if (newUrl != null) { url = newUrl; }";
55+
56+
assertTrue(normalizedBody.contains(expectedQueryStatement1));
57+
assertTrue(normalizedBody.contains(expectedQueryStatement2));
58+
59+
// Ensure the final HttpRequest construction is correct
60+
String expectedHttpRequestStatement
61+
= "HttpRequest httpRequest = new HttpRequest().setMethod(HttpMethod." + httpMethod + ").setUri(url);";
62+
assertTrue(normalizedBody.contains(expectedHttpRequestStatement));
63+
64+
String expectedHttpRequestHeaderStatement
65+
= "httpRequest.getHeaders().add(HttpHeaderName.CONTENT_LENGTH, String.valueOf(0)).add(HttpHeaderName.CONTENT_TYPE, String.valueOf(application / json));";
66+
assertTrue(normalizedBody.contains(expectedHttpRequestHeaderStatement));
67+
}
68+
69+
}

0 commit comments

Comments
 (0)