quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.61k stars 2.64k forks source link

Funqy parameter passing #14019

Open steve-hb opened 3 years ago

steve-hb commented 3 years ago

Disclaimer: I think it's not a "bug" - just not implemented (yet). If it is a bug, pls relabel it. Maybe I and a colleague misunderstood the docs - then we should improve them :) //EDIT: https://quarkus.io/guides/funqy-http#get-query-parameter-mapping Documentation also states only beans and a map are supported (for http)

Description Currently I have to create DTOs in case I want to call a function with only one/two parameters. Coming from Spring/Quarkus, this behaviour is a little bit surprising tbh. Here is my current scenario: We are writing functions using native funqy and currently call it via REST either using Funqy HTTP in our tests or in AWS using AWS "native" deployments for "production" use. I know there are annotations/modules directly addressing REST, but these would limit us to REST.

Implementation ideas The current (working) approach:

@Funqy
public UUID getUserIdByEmail(GetByEmailDTO query) { // dto just holding the email as a parameter we can pass via query params
...
}

Requested approach:

@Funqy
public UUID getUserIdByEmail(String email) { // email could be in query, body or an event - we don't care where it comes from
...
}
ghost commented 3 years ago

/cc @matejvasek, @patriot1burke

matejvasek commented 3 years ago
@Funqy
public UUID getUserIdByEmail(String email) { // email could be in query, body or an event - we don't care where it comes from
...
}

Sounds interesting but I not sure how we could implement it. How would Funqy know which query parameter to use if there were multiple parameters? Would it work only if there is exactly one query parameter? By match on parameter name?

steve-hb commented 3 years ago

My approach were to examine what Jetty etc are currently doing, because this could be a security concern very fast. (http parameter pollution) In general I would just do some parameter matching like Spring does with @RequestParams. A general idea was to also prioritize different kinds of input like the http body over query params for REST etc.

According to OWASP, we should always use the first parameter matching the expected key of the query string since Jetty and Tomcat are doing it this way.

So the acceptance criteria for me would be something like this: query > body & param[x] > param[x + 1] > param[x + n] Next concern could the naming strategies like camel case, pascal case and so on - usually only exact matches are allowed, but some companies use a common strategy across multiple stacks which differ from the code guidelines for e.g. Java. We could provide a parameter like Spring does (if there isn't already one for Quarkus): spring.jackson.property-naming-strategy=SNAKE_CASE (maybe also an annotation for exceptions from the rule)

What are your thoughts on this?

patriot1burke commented 3 years ago

We should have something has meaning across cloud function environments as Funqy is supposed to be portable. This leaves out any type of @Param annotation as it doesn't translate to Lambda. If you want REST rest is portable across cloud function environments and we have an extension that works with Lambda, GCF, and Azure functions. This would allow you to use JAX-RS, Spring MVC (the annotations we support), servlet, and Vertx Web.

EzxD commented 3 years ago

Let me try to say it in other words. What he means is that instead of passing a dto (in Json) into the function the json which gets passed for example through the body automatically into the working fields. So you can create a method with public void test(String name, String lastName) instead of a method which looks like public void test(PersonDto person)

steve-hb commented 3 years ago

If you want REST rest is portable across cloud function environments and we have an extension that works with Lambda, GCF, and Azure functions.

I think you misunterstood, we only use REST locally/for (integration) tests and I continued to use it as an example for an extension providing arbitary input for Funcs. The input should be, at least as an option, mapped to method parameters instead of DTOs/POJOs in order to improve readability and maintainability of the code.

Here's a little example I've in mind:

Function input (currently represented by a bean/DTO in the code itself):

{
  "s1": "Hello",
  "s2": "World",
}

REST input: /say?s1=Hello&s2World (could also be in the body encoded in some format like JSON or XML) My comment above just described how we could handle REST -> Funcy translation.

Method:

@Func
public void say(String s1, String s2)
patriot1burke commented 3 years ago

I understand what he wants, but you would need to compile with parameter names (which would add a ton to metaspace) so funqy could reflect to find out what the parameter name was.

patriot1burke commented 3 years ago

I guess we could write an annotation processor to get parameter names, but not sure how that works with quarkus dev mode.