phoenixnap / springmvc-raml-plugin

Spring MVC - RAML Spec Synchroniser Plugin. A Maven plugin designed to Generate Server & Client code in Spring from a RAML API descriptor and conversely, a RAML API document from the SpringMVC Server implementation.
Apache License 2.0
136 stars 84 forks source link

Problem with optional uriParameters #215

Closed saraawss closed 6 years ago

saraawss commented 6 years ago

I am facing a problem, I have a raml file and I have to convert to spring java controller file.I added com.phoenixnap.oss pluging in pom file like this,

  <plugin>
com.phoenixnap.oss springmvc-raml-plugin 0.10.11 src/main/resources/api.raml target/generated-sources false de.adorsys.ramlspringboot.api /api true false false com.phoenixnap.oss.ramlapisync.generation.rules.Spring4ControllerStubRule generate-springmvc-endpoints compile generate-springmvc-endpoints

when I am generating java controller file generating like this,

@RequestMapping(value = "/{id}", method = RequestMethod.GET)
public Todo getTodoById(
    @PathVariable(required = false)
    Long id) {
    return null; 
}

but @PathVariable(required = false) is getting error spring is not supporting required = false in @PathVariable.

Please advice me what should I do.

aweisser commented 6 years ago

It seems like a bug in one of the code generation strategies. The way to annotate an optional PathVariable in Spring is to use a Java8 Optional type refering to the following article: https://www.n-k.de/2016/05/optional-path-variables-with-spring-boot-rest-mvc.html

Workaround in your case: Since you‘re using the StubRule you can simply repair the optional=false in the generated code.

Please note: It‘s not a good practice to alter generated code at all. That‘s why we introduced other code generation rules like Spring4ControllerDecoratorRule or Spring4ControllerInterfaceRule. But in your case you decided to use the Spring4ControllerStubRule in which you have to inject your controller logic in the body of the generated controller method stub. So fixing the wrong required=false in the PathVariable is just another altering of the generated stub. ;)

But: it‘s still a bug.

aweisser commented 6 years ago

The affected rule is:

https://github.com/phoenixnap/springmvc-raml-plugin/blob/master/springmvc-raml-parser/src/main/java/com/phoenixnap/oss/ramlapisync/generation/rules/spring/SpringMethodParamsRule.java

saraawss commented 6 years ago

I tried to use Spring4ControllerDecoratorRule or Spring4ControllerInterfaceRule, but still it is generating @PathVariable(required = false).

How to repair the generated code?

stojsavljevic commented 6 years ago

@aweisser thnx for pointing out the solution (or let's call it a workaround).

@saraawss there is nothing you can to make your build passing until I implement the workaround. But you can make the issue more clear and provide snippet of your raml :) If I'm not mistaken, you have a raml like this:

/todos:
  /{todoId}:
    uriParameters: 
      todoId: 
        type: number
        required: false
    get:
      responses: 
        200:
          body: 
            application/json:
              type: Todo

which leads to the code you pasted that is not supported by Spring although it compiles.

stojsavljevic commented 6 years ago

I'm going to push the fix soon although I'm not very happy with the solution/workaround. The problematic part is multiple values for @RequestMapping. With 1 optional param - there are 2 values, with 2 params - 4 values etc. Bunch of mapping values might lead to unexpected mapping. I added a warning in case of more than 1 optional uri param but I will still let a user to shoot himself in a leg if he wants to.