Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: request.getUri should respond include path and query values #1853

Merged
merged 6 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import io.micronaut.http.MutableHttpRequest;
import io.micronaut.http.cookie.Cookie;
import io.micronaut.http.cookie.Cookies;
import io.micronaut.http.uri.UriBuilder;
import io.micronaut.servlet.http.MutableServletHttpRequest;
import io.micronaut.servlet.http.BodyBuilder;
import io.micronaut.servlet.http.ServletExchange;
Expand Down Expand Up @@ -108,6 +109,30 @@ protected ApiGatewayServletRequest(

public abstract byte[] getBodyBytes() throws IOException;

/**
* Given a path and the query params from the event, build a URI.
*
* @param path the request path
* @param queryParameters the query parameters from the event
* @param multiQueryParameters the multi-value query parameters from the event
* @return the URI
* @since 4.0.3
*/
protected static URI buildUri(
timyates marked this conversation as resolved.
Show resolved Hide resolved
String path,
@Nullable Map<String, String> queryParameters,
@Nullable Map<String, List<String>> multiQueryParameters
) {
UriBuilder uriBuilder = UriBuilder.of(path);
if (queryParameters != null) {
queryParameters.forEach((key, value) -> splitCommaSeparatedValue(value).forEach(token -> uriBuilder.queryParam(key, token)));
}
if (multiQueryParameters != null) {
multiQueryParameters.forEach((key, values) -> values.forEach(value -> uriBuilder.queryParam(key, value)));
}
return uriBuilder.build();
}

protected static HttpMethod parseMethod(Supplier<String> httpMethodConsumer) {
try {
return HttpMethod.valueOf(httpMethodConsumer.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.URI;

/**
* Implementation of {@link ServletHttpRequest} for Application Load Balancer events.
Expand All @@ -54,7 +53,11 @@ public ApplicationLoadBalancerServletRequest(
super(
conversionService,
requestEvent,
URI.create(requestEvent.getPath()),
ApiGatewayServletRequest.buildUri(
requestEvent.getPath(),
requestEvent.getQueryStringParameters(),
requestEvent.getMultiValueQueryStringParameters()
),
parseMethod(requestEvent::getHttpMethod),
LOG,
bodyBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.URI;
import java.util.Base64;

/**
Expand All @@ -56,7 +55,11 @@ public ApiGatewayProxyServletRequest(
super(
conversionService,
requestEvent,
URI.create(requestEvent.getPath()),
ApiGatewayServletRequest.buildUri(
requestEvent.getPath(),
requestEvent.getQueryStringParameters(),
requestEvent.getMultiValueQueryStringParameters()
),
parseMethod(requestEvent::getHttpMethod),
LOG,
bodyBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.URI;
import java.util.Collections;

/**
Expand All @@ -55,7 +54,11 @@ public APIGatewayV2HTTPEventServletRequest(
super(
conversionService,
requestEvent,
URI.create(requestEvent.getRequestContext().getHttp().getPath()),
ApiGatewayServletRequest.buildUri(
requestEvent.getRequestContext().getHttp().getPath(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use rawPath and rawQuery ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the advantage of using rawPath and rawQuery? 🤔

Copy link
Contributor

@driverpt driverpt Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to parse and treat it

https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html

You can just join it, e.g.: ${path}?${query}

requestEvent.getQueryStringParameters(),
Collections.emptyMap()
),
parseMethod(() -> requestEvent.getRequestContext().getHttp().getMethod()),
LOG,
bodyBuilder
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2017-2023 original authors
*
* 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
*
* https://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 io.micronaut.http.server.tck.lambda.tests;

import io.micronaut.context.annotation.Requires;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.tck.AssertionUtils;
import io.micronaut.http.tck.HttpResponseAssertion;
import io.micronaut.http.uri.UriBuilder;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static io.micronaut.http.tck.TestScenario.asserts;

//TODO Delete when https://github.com/micronaut-projects/micronaut-core/pull/9791
@SuppressWarnings({
"java:S5960", // We're allowed assertions, as these are used in tests only
"checkstyle:MissingJavadocType",
"checkstyle:DesignForExtension"
})
public class RequestUriContainsQueryValueTest {

public static final String SPEC_NAME = "RequestUriContainsQueryValueTest";

@Test
void testRequestUriContainsQueryValue() throws IOException {
asserts(SPEC_NAME,
HttpRequest.GET(UriBuilder.of("/requesturi").queryParam("foo", "bar").build()),
(server, request) -> AssertionUtils.assertDoesNotThrow(server, request,
HttpResponseAssertion.builder()
.status(HttpStatus.OK)
.body("/requesturi?foo=bar")
.build()));
}

@Requires(property = "spec.name", value = SPEC_NAME)
@Controller("/requesturi")
static class TestController {
@Get
String index(HttpRequest<?> request) {
return request.getUri().toASCIIString();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2017-2023 original authors
*
* 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
*
* https://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 io.micronaut.http.server.tck.lambda.tests;

import io.micronaut.context.annotation.Requires;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.tck.AssertionUtils;
import io.micronaut.http.tck.HttpResponseAssertion;
import io.micronaut.http.uri.UriBuilder;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static io.micronaut.http.tck.TestScenario.asserts;

//TODO Delete when https://github.com/micronaut-projects/micronaut-core/pull/9791
@SuppressWarnings({
"java:S5960", // We're allowed assertions, as these are used in tests only
"checkstyle:MissingJavadocType",
"checkstyle:DesignForExtension"
})
public class RequestUriContainsQueryValueTest {

public static final String SPEC_NAME = "RequestUriContainsQueryValueTest";

@Test
void testRequestUriContainsQueryValue() throws IOException {
asserts(SPEC_NAME,
HttpRequest.GET(UriBuilder.of("/requesturi").queryParam("foo", "bar").build()),
(server, request) -> AssertionUtils.assertDoesNotThrow(server, request,
HttpResponseAssertion.builder()
.status(HttpStatus.OK)
.body("/requesturi?foo=bar")
.build()));
}

@Requires(property = "spec.name", value = SPEC_NAME)
@Controller("/requesturi")
static class TestController {
@Get
String index(HttpRequest<?> request) {
return request.getUri().toASCIIString();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2017-2023 original authors
*
* 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
*
* https://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 io.micronaut.http.server.tck.lambda.tests;

import io.micronaut.context.annotation.Requires;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.tck.AssertionUtils;
import io.micronaut.http.tck.HttpResponseAssertion;
import io.micronaut.http.uri.UriBuilder;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static io.micronaut.http.tck.TestScenario.asserts;

//TODO Delete when https://github.com/micronaut-projects/micronaut-core/pull/9791
@SuppressWarnings({
"java:S5960", // We're allowed assertions, as these are used in tests only
"checkstyle:MissingJavadocType",
"checkstyle:DesignForExtension"
})
public class RequestUriContainsQueryValueTest {

public static final String SPEC_NAME = "RequestUriContainsQueryValueTest";

@Test
void testRequestUriContainsQueryValue() throws IOException {
asserts(SPEC_NAME,
HttpRequest.GET(UriBuilder.of("/requesturi").queryParam("foo", "bar").build()),
(server, request) -> AssertionUtils.assertDoesNotThrow(server, request,
HttpResponseAssertion.builder()
.status(HttpStatus.OK)
.body("/requesturi?foo=bar")
.build()));
}

@Requires(property = "spec.name", value = SPEC_NAME)
@Controller("/requesturi")
static class TestController {
@Get
String index(HttpRequest<?> request) {
return request.getUri().toASCIIString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
import org.junit.platform.suite.api.SuiteDisplayName;

@Suite
@SelectPackages("io.micronaut.http.server.tck.tests")
@SelectPackages({
"io.micronaut.http.server.tck.tests",
"io.micronaut.http.server.tck.lambda.tests"
})
@ExcludeClassNamePatterns({
"io.micronaut.http.server.tck.tests.LocalErrorReadingBodyTest", // Binding body different type (e.g. a String in error handler)
"io.micronaut.http.server.tck.tests.FilterProxyTest" // Immmutable request
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2017-2023 original authors
*
* 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
*
* https://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 io.micronaut.http.server.tck.lambda.tests;

import io.micronaut.context.annotation.Requires;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.tck.AssertionUtils;
import io.micronaut.http.tck.HttpResponseAssertion;
import io.micronaut.http.uri.UriBuilder;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static io.micronaut.http.tck.TestScenario.asserts;

//TODO Delete when https://github.com/micronaut-projects/micronaut-core/pull/9791
@SuppressWarnings({
"java:S5960", // We're allowed assertions, as these are used in tests only
"checkstyle:MissingJavadocType",
"checkstyle:DesignForExtension"
})
public class RequestUriContainsQueryValueTest {

public static final String SPEC_NAME = "RequestUriContainsQueryValueTest";

@Test
void testRequestUriContainsQueryValue() throws IOException {
asserts(SPEC_NAME,
HttpRequest.GET(UriBuilder.of("/requesturi").queryParam("foo", "bar").build()),
(server, request) -> AssertionUtils.assertDoesNotThrow(server, request,
HttpResponseAssertion.builder()
.status(HttpStatus.OK)
.body("/requesturi?foo=bar")
.build()));
}

@Requires(property = "spec.name", value = SPEC_NAME)
@Controller("/requesturi")
static class TestController {
@Get
String index(HttpRequest<?> request) {
return request.getUri().toASCIIString();
}
}
}