Skip to content

Commit

Permalink
Merge pull request #3982 from andreaTP/issue-3965
Browse files Browse the repository at this point in the history
[Java] Self expansion of query parameters instead of using reflection
  • Loading branch information
baywet authored Jan 10, 2024
2 parents 0feecd5 + 09b046b commit 41efcb7
Show file tree
Hide file tree
Showing 15 changed files with 224 additions and 28 deletions.
1 change: 1 addition & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ jobs:
- "./tests/Kiota.Builder.IntegrationTests/InheritingErrors.yaml"
- "./tests/Kiota.Builder.IntegrationTests/NoUnderscoresInModel.yaml"
- "./tests/Kiota.Builder.IntegrationTests/ToDoApi.yaml"
- "./tests/Kiota.Builder.IntegrationTests/GeneratesUritemplateHints.yaml"
- "oas::petstore"
- "apisguru::twitter.com:current"
- "apisguru::notion.com"
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Java - Self-extraction of query parameters instead of using reflection. [#3965](https://github.com/microsoft/kiota/issues/3965)
- Fixed a bug where the discriminator validation rule would report false positives on nullable union types.
- Fixed a bug where constructors and model names where clashing in Go. [#3920](https://github.com/microsoft/kiota/issues/3920)
- Fixed a bug where the order of enum declaration might results in a missing enum type. [#3935](https://github.com/microsoft/kiota/issues/3935)
Expand Down
3 changes: 3 additions & 0 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@
}
]
},
"./tests/Kiota.Builder.IntegrationTests/GeneratesUritemplateHints.yaml": {
"MockServerITFolder": "query-params"
},
"apisguru::notion.com": {
"ExcludePatterns": [
{
Expand Down
20 changes: 11 additions & 9 deletions it/exec-cmd.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,14 @@ if ($null -ne $descriptionValue) {
}
}

$mockServerTest = false
$itTestPath = Join-Path -Path $testPath -ChildPath $mockSeverITFolder
if (Test-Path -Path $itTestPath) {
$mockServerTest = true
}

# Start MockServer if needed
if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {
if ($mockServerTest) {
# Kill any leftover MockServer
Kill-MockServer
Push-Location $mockServerPath
Expand All @@ -80,8 +86,7 @@ if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {

Push-Location $testPath
if ($language -eq "csharp") {
if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {
$itTestPath = Join-Path -Path $testPath -ChildPath $mockSeverITFolder
if ($mockServerTest) {
Push-Location $itTestPath

$itTestPathSources = Join-Path -Path $testPath -ChildPath "client"
Expand All @@ -104,8 +109,7 @@ if ($language -eq "csharp") {
}
}
elseif ($language -eq "java") {
if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {
$itTestPath = Join-Path -Path $testPath -ChildPath $mockSeverITFolder
if ($mockServerTest) {
Push-Location $itTestPath

$itTestPathSources = Join-Path -Path $testPath -ChildPath "src" -AdditionalChildPath "*"
Expand All @@ -128,8 +132,7 @@ elseif ($language -eq "java") {
}
}
elseif ($language -eq "go") {
if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {
$itTestPath = Join-Path -Path $testPath -ChildPath $mockSeverITFolder
if ($mockServerTest) {
Push-Location $itTestPath

$itTestPathSources = Join-Path -Path $testPath -ChildPath "client"
Expand Down Expand Up @@ -182,8 +185,7 @@ elseif ($language -eq "python") {
mypy integration_test
} -ErrorAction Stop

if (!([string]::IsNullOrEmpty($mockSeverITFolder))) {
$itTestPath = Join-Path -Path $testPath -ChildPath $mockSeverITFolder
if ($mockServerTest) {
Push-Location $itTestPath

$itTestPathSources = Join-Path -Path $testPath -ChildPath "integration_test" -AdditionalChildPath "client"
Expand Down
3 changes: 3 additions & 0 deletions it/java/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@
target
/basic/src/main
/basic/src/kiota-lock.json
/query-params/src/main
/query-params/src/kiota-lock.json

2 changes: 1 addition & 1 deletion it/java/basic/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<kiota-java.version>0.11.0</kiota-java.version>
<kiota-java.version>0.12.1</kiota-java.version>
</properties>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion it/java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<kiota-java.version>0.11.0</kiota-java.version>
<kiota-java.version>0.12.1</kiota-java.version>
</properties>

<dependencies>
Expand Down
74 changes: 74 additions & 0 deletions it/java/query-params/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?xml version="1.0"?>
<project
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"
xmlns="http://maven.apache.org/POM/4.0.0">
<modelVersion>4.0.0</modelVersion>
<groupId>io.kiota</groupId>
<artifactId>kiota-basic-test</artifactId>
<version>0.1.0-SNAPSHOT</version>

<properties>
<maven.compiler.release>11</maven.compiler.release>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<kiota-java.version>0.12.1</kiota-java.version>
</properties>

<dependencies>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-abstractions</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-serialization-json</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-serialization-text</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-serialization-form</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-serialization-multipart</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>com.microsoft.kiota</groupId>
<artifactId>microsoft-kiota-http-okHttp</artifactId>
<version>${kiota-java.version}</version>
</dependency>
<dependency>
<groupId>jakarta.annotation</groupId>
<artifactId>jakarta.annotation-api</artifactId>
<version>2.1.1</version>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>5.9.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M9</version>
</plugin>
</plugins>
</build>
</project>
52 changes: 52 additions & 0 deletions it/java/query-params/src/test/java/BasicAPITest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import apisdk.ApiClient;
import com.microsoft.kiota.ApiException;
import com.microsoft.kiota.authentication.AnonymousAuthenticationProvider;
import com.microsoft.kiota.http.OkHttpRequestAdapter;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import java.util.concurrent.TimeUnit;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class BasicAPITest {

@Test
void includesSomeQueryParameter() throws Exception {
var client = new ApiClient(new OkHttpRequestAdapter(new AnonymousAuthenticationProvider()));

var reqInf = client.api().something().v1().toGetRequestInformation(config -> {
config.queryParameters.startDateTime = "START";
});
reqInf.pathParameters.put("baseurl", "http://test");

assertEquals("http://test/api/something/v1?startDateTime=START", reqInf.getUri().toString());
}

@Test
void includesSomeOtherQueryParameter() throws Exception {
var client = new ApiClient(new OkHttpRequestAdapter(new AnonymousAuthenticationProvider()));

var reqInf = client.api().something().v1().toGetRequestInformation(config -> {
config.queryParameters.endDateTime = "END";
});
reqInf.pathParameters.put("baseurl", "http://test");

assertEquals("http://test/api/something/v1?EndDateTime=END", reqInf.getUri().toString());
}

@Test
void includesAllTheQueryParameters() throws Exception {
var client = new ApiClient(new OkHttpRequestAdapter(new AnonymousAuthenticationProvider()));

var reqInf = client.api().something().v1().toGetRequestInformation(config -> {
config.queryParameters.startDateTime = "START";
config.queryParameters.endDateTime = "END";
});
reqInf.pathParameters.put("baseurl", "http://test");

assertEquals("http://test/api/something/v1?startDateTime=START&EndDateTime=END", reqInf.getUri().toString());
}

}
41 changes: 37 additions & 4 deletions src/Kiota.Builder/Refiners/JavaRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Kiota.Builder.Configuration;
using Kiota.Builder.Extensions;
using Kiota.Builder.Writers.Java;
using Microsoft.Kiota.Abstractions;

namespace Kiota.Builder.Refiners;
public class JavaRefiner : CommonLanguageRefiner, ILanguageRefiner
Expand All @@ -18,6 +19,7 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
{
cancellationToken.ThrowIfCancellationRequested();
CorrectCommonNames(generatedCode);
AddQueryParameterExtractorMethod(generatedCode);
MoveRequestBuilderPropertiesToBaseType(generatedCode,
new CodeUsing
{
Expand Down Expand Up @@ -224,11 +226,11 @@ private static void AddEnumSetImport(CodeElement currentElement)
private static readonly AdditionalUsingEvaluator[] defaultUsingEvaluators = {
new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.RequestAdapter),
AbstractionsNamespaceName, "RequestAdapter"),
new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.PathParameters),
new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.PathParameters) || x is CodeMethod method && method.IsOfKind(CodeMethodKind.QueryParametersMapper),
"java.util", "HashMap"),
new (static x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestGenerator),
AbstractionsNamespaceName, "RequestInformation", "RequestOption", "HttpMethod"),
new (static x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestGenerator),
new (static x => x is CodeMethod method && (method.IsOfKind(CodeMethodKind.RequestGenerator) || method.IsOfKind(CodeMethodKind.QueryParametersMapper)),
"java.util", "Collection", "Map"),
new (static x => x is CodeClass @class && @class.IsOfKind(CodeClassKind.Model),
SerializationNamespaceName, "Parsable"),
Expand Down Expand Up @@ -257,8 +259,8 @@ private static void AddEnumSetImport(CodeElement currentElement)
x is CodeMethod method && "decimal".Equals(method.ReturnType.Name, StringComparison.OrdinalIgnoreCase) ||
x is CodeParameter para && "decimal".Equals(para.Type.Name, StringComparison.OrdinalIgnoreCase),
"java.math", "BigDecimal"),
new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.QueryParameter) && !string.IsNullOrEmpty(prop.SerializationName),
AbstractionsNamespaceName, "QueryParameter"),
new (static x => x is CodeClass @class && @class.IsOfKind(CodeClassKind.QueryParameters),
AbstractionsNamespaceName, "QueryParameters"),
new (static x => x is CodeClass @class && @class.OriginalComposedType is CodeIntersectionType intersectionType && intersectionType.Types.Any(static y => !y.IsExternal),
SerializationNamespaceName, "ParseNodeHelper"),
new (static x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor, CodeMethodKind.RequestGenerator) && method.Parameters.Any(static y => y.IsOfKind(CodeParameterKind.RequestBody) && y.Type.Name.Equals(MultipartBodyClassName, StringComparison.OrdinalIgnoreCase)),
Expand Down Expand Up @@ -512,4 +514,35 @@ private static void LowerCaseNamespaceNames(CodeElement currentElement)
CrawlTree(currentElement, LowerCaseNamespaceNames);
}
}

private void AddQueryParameterExtractorMethod(CodeElement currentElement, string methodName = "toQueryParameters")
{
if (currentElement is CodeClass currentClass &&
currentClass.IsOfKind(CodeClassKind.QueryParameters))
{
currentClass.StartBlock.AddImplements(new CodeType
{
IsExternal = true,
Name = "QueryParameters"
});
currentClass.AddMethod(new CodeMethod
{
Name = methodName,
Access = AccessModifier.Public,
ReturnType = new CodeType
{
Name = "Map<String, Object>",
IsNullable = false,
},
IsAsync = false,
IsStatic = false,
Kind = CodeMethodKind.QueryParametersMapper,
Documentation = new()
{
Description = "Extracts the query parameters into a map for the URI template parsing.",
},
});
}
CrawlTree(currentElement, x => AddQueryParameterExtractorMethod(x, methodName));
}
}
17 changes: 17 additions & 0 deletions src/Kiota.Builder/Writers/Java/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ public override void WriteCodeElement(CodeMethod codeElement, LanguageWriter wri
case CodeMethodKind.ErrorMessageOverride:
WriteErrorMethodOverride(parentClass, writer);
break;
case CodeMethodKind.QueryParametersMapper:
WriteQueryParametersExtractorBody(codeElement, writer, parentClass);
break;
case CodeMethodKind.ComposedTypeMarker:
throw new InvalidOperationException("ComposedTypeMarker is not required as interface is explicitly implemented.");
default:
Expand Down Expand Up @@ -402,6 +405,20 @@ private void WriteGetterBody(CodeMethod codeElement, LanguageWriter writer, Code
else
writer.WriteLine($"return this.{backingStore.Name}.get(\"{codeElement.AccessedProperty?.Name}\");");
}
private void WriteQueryParametersExtractorBody(CodeMethod codeElement, LanguageWriter writer, CodeClass parentClass)
{
writer.WriteLine("final Map<String, Object> allQueryParams = new HashMap();");
var allQueryParams = parentClass
.GetPropertiesOfKind(CodePropertyKind.QueryParameter)
.OrderBy(static x => x, CodePropertyTypeForwardComparer)
.ThenBy(static x => x.Name);
foreach (var queryParam in allQueryParams)
{
var keyValue = queryParam.IsNameEscaped ? queryParam.SerializationName : queryParam.Name;
writer.WriteLine($"allQueryParams.put(\"{keyValue}\", {queryParam.Name});");
}
writer.WriteLine("return allQueryParams;");
}
private void WriteIndexerBody(CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer, string returnType)
{
if (parentClass.GetPropertyOfKind(CodePropertyKind.PathParameters) is CodeProperty pathParametersProperty && codeElement.OriginalIndexer != null)
Expand Down
3 changes: 0 additions & 3 deletions src/Kiota.Builder/Writers/Java/CodePropertyWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ public override void WriteCodeElement(CodeProperty codeElement, LanguageWriter w
conventions.AddRequestBuilderBody(parentClass, returnType, writer);
writer.CloseBlock();
break;
case CodePropertyKind.QueryParameter when codeElement.IsNameEscaped:
writer.WriteLine($"@QueryParameter(name = \"{codeElement.SerializationName}\")");
goto default;
case CodePropertyKind.Headers or CodePropertyKind.Options when !string.IsNullOrEmpty(codeElement.DefaultValue):
defaultValue = $" = {codeElement.DefaultValue}";
goto default;
Expand Down
2 changes: 1 addition & 1 deletion tests/Kiota.Builder.IntegrationTests/GenerateSample.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public async Task GeneratesUritemplateHints(GenerationLanguage language)
Assert.Contains("`uriparametername:\"startDateTime\"`", fullText);
break;
case GenerationLanguage.Java:
Assert.Contains("@QueryParameter(name = \"EndDateTime\")", fullText);
Assert.Contains("allQueryParams.put(\"EndDateTime\", endDateTime)", fullText);
break;
case GenerationLanguage.PHP:
Assert.Contains("@QueryParameter(\"EndDateTime\")", fullText);
Expand Down
22 changes: 22 additions & 0 deletions tests/Kiota.Builder.Tests/Writers/Java/CodeMethodWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1964,6 +1964,28 @@ public void WritesApiConstructorWithBackingStore()
Assert.Contains("enableBackingStore", result);
}
[Fact]
public void WritesQueryParametersExtractor()
{
setup();
method.Kind = CodeMethodKind.QueryParametersMapper;
var defaultValue = "\"someVal\"";
var propName = "propWithDefaultValue";
parentClass.Kind = CodeClassKind.QueryParameters;
parentClass.AddProperty(new CodeProperty
{
Name = propName,
DefaultValue = defaultValue,
Kind = CodePropertyKind.QueryParameter,
Type = new CodeType
{
Name = "String"
}
});
writer.Write(method);
var result = tw.ToString();
Assert.Contains("allQueryParams.put(\"propWithDefaultValue\", propWithDefaultValue);", result);
}
[Fact]
public async Task AccessorsTargetingEscapedPropertiesAreNotEscapedThemselves()
{
setup();
Expand Down
Loading

0 comments on commit 41efcb7

Please sign in to comment.