Spring Redirect Enpoint : produces attribute not empty

[drapej]

Setup information

hikaku version: 3.0.0
specification converter: OpenApiConverter
implementation converter: SpringConverter
build tool and version: gradle 5.6.4
test framework: junit5

Describe the bug

Hi,
we have a problem with Spring endpoints which must return a redirect URL (status 302 or 301).
When we run the Hikaku test, Hikaku generate a "produces" attribute for this endpoint, and so the test fail because our OpenApi Endpoint doesn't have a content.

This generation of produces attribute is in the ProducesSpringExtension, and for now, to have a empty produces, the endpoint have to respect one of these condition :

• no return and no HttpServletResponseParam and no error in path
• no @responsebody and no @RestController and no error in path
  In other case, it used the produces (if it's present) or put a default produces

Expected behavior

When endpoint :

• return a "RedirectView" or
• method has a param HttpServletResponse and no return
  Then ProducesSpringExtension return the produces attribute define in the endpoint.

N.B. : when we do a redirect with the HttpServletResponse, we don't have to specify a @ResponseStatus but this is the only way I think to differentiate in the ProducesSpringExtension a Redirect from another status

Code samples

Sample of the current bug

@GetMapping("/github")
    fun redirectToGithub(): RedirectView {
        return RedirectView("https://github.com/codecentric/hikaku")
    }

//or with the HttpServletReponse
@GetMapping("/github")
fun redirectToGithub(response: HttpServletResponse) {
        response.sendRedirect("https://github.com/codecentric/hikaku")
    }

I can if you want suggest a PR.

[cc-jhr]

Hey @drapej,

thank you for reporting this issue.

I created a test with the sample that you provided for the return type RedirectView. You are correct. The spring internal for produces is empty for that case. So I fixed that.

The other case is tricky. Because the following works fine:

@SpringBootApplication
open class DummyApp

fun main(args: Array<String>) {
    runApplication<DummyApp>(*args)
}

@Controller
open class ProducesServletResponseTestController {

    @GetMapping("/todos")
    @ResponseBody
    fun getTest(response: HttpServletResponse) {
        response.outputStream.println("Hello, world!")
    }
}

Although I have no return type on the function I can call the endpoint and receive the Hello world. I don't see any possibility to distinguish the normal case from a redirect for that setup.
I would have to document that for this particular case it won't return anything for produces, but it does for others and so on. It would also create a weird behaviour for the code above.
So instead of creating a special case, I'd rather keep it simple and leave the HttpServletResponse case as is. This is where hikaku has it's limits.

[drapej]

Hi @cc-jhr,
Thank you for your answer.

In your second case, if your put that, that's not work because Hikaku produces endpoint with "application/json".

 @GetMapping("/todos", produces = ["text/plain"])
    @ResponseBody
    fun getTest(response: HttpServletResponse) {
        response.outputStream.println("Hello, world!")
    }

That's what I don't understand, why Hikaku generates a produce for endpoints that don't have a return and not uses only the produces specified.

[cc-jhr]

Hi @drapej,

sorry it took me some time to respond.
You are correct. If there is no return type the list of produces entries should be empty. That case was currently missing.
In a case like the one we discussed above (HttpServletResponse, no return type), I'd expect the developer to set the value produces explicitly. The library shouldn't make any assumptions there.

I created a second test for that. If that looks fine to you, we could release that.

[drapej]

Hi @cc-jhr,
No problem for the delay ;)

"In a case like the one we discussed above (HttpServletResponse, no return type), I'd expect the developer to set the value produces explicitly. The library shouldn't make any assumptions there." => totally agree with you 👍

Thank you for the new test with no return type, it's perfect.