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.

Sign up for GitHub

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

Jump to bottom

Support for HTTP GET map parameters? #6058

Open
rickyma opened this issue Jan 8, 2025 · 7 comments
Open

Support for HTTP GET map parameters? #6058

rickyma opened this issue Jan 8, 2025 · 7 comments

Comments

@rickyma
Copy link

rickyma commented Jan 8, 2025 •
edited
Loading

Do we support passing map parameters using HTTP GET in Armeria?

My code:

@Get("")
@ProducesJson
public TablesPagedVO listTables(
		@Param("properties") Optional<Map<String, String>> properties,
		@Param("max_results") Optional<Integer> maxResults,
		@Param("page_number") Optional<Integer> pageNumber) {
	TablesPagedDTO tablesListDTO =
			tableService.listTables(
					properties,
					maxResults,
					pageNumber);
	return VOConverters.toVO(tablesListDTO);

An exception occurred:

[2025-01-08 15:26:04.411] [armeria-common-worker-nio-2-2] [ERROR] GlobalExceptionHandler.handleException - 
java.lang.IllegalArgumentException: Can't convert '{is_online=false}' to type 'Map'.
	at com.linecorp.armeria.internal.server.annotation.AnnotatedServiceTypeUtil.stringToType(AnnotatedServiceTypeUtil.java:189)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.convert(AnnotatedValueResolver.java:1103)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.convert(AnnotatedValueResolver.java:1121)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.lambda$resolver$22(AnnotatedValueResolver.java:785)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.resolve(AnnotatedValueResolver.java:1095)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.lambda$toArguments$1(AnnotatedValueResolver.java:167)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575)
	at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260)
	at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616)
	at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622)
	at com.linecorp.armeria.internal.server.annotation.AnnotatedValueResolver.toArguments(AnnotatedValueResolver.java:167)
	at com.linecorp.armeria.internal.server.annotation.DefaultAnnotatedService.invoke(DefaultAnnotatedService.java:405)
	at com.linecorp.armeria.internal.server.annotation.DefaultAnnotatedService.serve0(DefaultAnnotatedService.java:336)
	at com.linecorp.armeria.internal.server.annotation.DefaultAnnotatedService.serve(DefaultAnnotatedService.java:308)
	at com.linecorp.armeria.internal.server.annotation.DefaultAnnotatedService.serve(DefaultAnnotatedService.java:83)
	at com.linecorp.armeria.internal.server.annotation.DefaultAnnotatedService$ExceptionHandlingHttpService.serve(DefaultAnnotatedService.java:584)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:132)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:79)
	at com.linecorp.armeria.server.metric.MetricCollectingService.serve(MetricCollectingService.java:109)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:132)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:79)
	at com.linecorp.armeria.server.logging.LoggingService.serve(LoggingService.java:86)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:132)
	at com.rickyma.uc.server.decorator.AuthenticationDecorator.serve(AuthenticationDecorator.java:54)
	at com.linecorp.armeria.server.FunctionalDecoratingHttpService.serve(FunctionalDecoratingHttpService.java:45)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:132)
	at com.rickyma.uc.server.decorator.ThreadLocalContextDecorator.serve(ThreadLocalContextDecorator.java:18)
	at com.linecorp.armeria.server.FunctionalDecoratingHttpService.serve(FunctionalDecoratingHttpService.java:45)
	at com.linecorp.armeria.internal.server.RouteDecoratingService.serve(RouteDecoratingService.java:132)
	at com.rickyma.uc.server.exception.ExceptionHandlingDecorator.serve(ExceptionHandlingDecorator.java:33)
	at com.linecorp.armeria.server.FunctionalDecoratingHttpService.serve(FunctionalDecoratingHttpService.java:45)
	at com.linecorp.armeria.internal.server.RouteDecoratingService$InitialDispatcherService.serve(RouteDecoratingService.java:202)
	at com.linecorp.armeria.server.HttpServerHandler.serve0(HttpServerHandler.java:490)
	at com.linecorp.armeria.server.HttpServerHandler.handleRequest(HttpServerHandler.java:402)
	at com.linecorp.armeria.server.HttpServerHandler.channelRead(HttpServerHandler.java:282)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at com.linecorp.armeria.server.Http1RequestDecoder.channelRead(Http1RequestDecoder.java:284)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at com.linecorp.armeria.server.HttpServerUpgradeHandler.channelRead(HttpServerUpgradeHandler.java:227)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.logging.LoggingHandler.channelRead(LoggingHandler.java:280)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)

I tried to use a converter, but it seemed only worked when using POST methods:

package com.rickyma.uc.server.config;

import com.linecorp.armeria.common.AggregatedHttpRequest;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.annotation.RequestConverterFunction;
import java.lang.reflect.ParameterizedType;
import java.util.HashMap;
import java.util.Map;

public class MapRequestConverter implements RequestConverterFunction {

    @Override
    public @Nullable Object convertRequest(
            ServiceRequestContext ctx,
            AggregatedHttpRequest request,
            Class<?> expectedResultType,
            @Nullable ParameterizedType expectedParameterizedResultType)
            throws Exception {
        if (Map.class.isAssignableFrom(expectedResultType)) {
            // Parse the query parameters into a Map
            Map<String, String> result = new HashMap<>();
            String query = request.uri().getRawQuery();
            if (query != null) {
                for (String param : query.split("&")) {
                    String[] keyValue = param.split("=");
                    if (keyValue.length == 2) {
                        result.put(keyValue[0], keyValue[1]);
                    }
                }
            }
            return result;
        }
        return RequestConverterFunction.fallthrough();
    }
}
@minwoox
Copy link
Contributor

minwoox commented Jan 8, 2025

Could you check if it works when using @RequestObject?

armeria/core/src/test/java/com/linecorp/armeria/server/AnnotatedServiceBindingBuilderTest.java

Line 213 in a8220b5

public Map<String, Object> bar(@RequestObject Map<String, Object> map) {

@rickyma
Copy link
Author

rickyma commented Jan 9, 2025 •
edited
Loading

It does not work for HTTP GET:

@Get("")
@ProducesJson
public TablesPagedVO listTables(
		@Param("properties") @RequestObject Optional<Map<String, String>> properties,
		@Param("max_results") Optional<Integer> maxResults,
		@Param("page_number") Optional<Integer> pageNumber) {
	TablesPagedDTO tablesListDTO =
			tableService.listTables(
					properties,
					maxResults,
					pageNumber);
	return VOConverters.toVO(tablesListDTO);

Error msg:

{"error":"Internal Server Error","message":"Can't convert '{is_online=false}' to type 'Map'.","path":"/api/1.0/unity-catalog/tables?properties=%7Bis_online%3Dfalse%7D","status":500,"timestamp":"2025-01-09T07:23:44.997689200Z"}

@minwoox
Copy link
Contributor

minwoox commented Jan 13, 2025

Hi, @rickyma
I discovered that only the POST JSON request is converted into a Map with the @RequestObject.
Yeah, we definitely need to fix it to support the mapping with queries as well.
Are you interested in fixing it?

@rickyma
Copy link
Author

rickyma commented Jan 13, 2025

we definitely need to fix it to support the mapping with queries as well.

I implemented this in another way. The client passes a custom KV string, and the server performs the corresponding parsing.

Are you interested in fixing it?

Sorry, but I haven't had much time recently. It would be great if someone else could fix it.

@kwondh5217
Copy link

Hi @minwoox ! I'm interested in contributing to this issue as my first contribution.

Would it be okay if I worked on this? If there are any specific guidelines or expectations, I'd appreciate your advice. Thank you!

@minwoox
Copy link
Contributor

minwoox commented Jan 16, 2025

Would it be okay if I worked on this?

Sure, it's all yours. 👍

If there are any specific guidelines or expectations,

We should support both Map and multi map:

@Get("/foo")
public HttpResponse foo(@Param Map<String, Object> params) {...}

// Used when a key have multiple values.
@Get("/bar")
public HttpResponse bar(@Param SomeMultiMap<String, Object> params) {...}
// We cannot use Guava's Multipmap because we shade the dependency.

But, let's support the Map first and then move to the next one. 😉

@kwondh5217
Copy link

@minwoox Thanks, I'll try it !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rickyma @minwoox @kwondh5217