Skip to content

@Get with a bean generates code to deserialize as json from request body (but there is no request body) #580

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

Closed
rob-bygrave opened this issue Mar 20, 2025 · 3 comments · Fixed by #581
Assignees
Milestone

Comments

@rob-bygrave
Copy link
Contributor

Given a GET like this:

    @Get("/search/criteria")
    public SearchResult findByCriteria(SearchCriteria criteria) {

... the generated code looks to read critera as json content from the request body - but its a GET and so there is no request body so we get a strange json deserialization error.

Suggested Fix:

The proposed fix, is that the generated code should see this is a GET and so it must look to populate the criteria bean using path parameters instead.

Note that currently for POST and @Form, criteria would be populated from form parameters so this is pretty similar to what should be generated [but use path parameters rather than form parameters]. Example:

    @Post("/search/criteria")
    public SearchResult findByCriteria(@Form SearchCriteria criteria) {
@SentryMan
Copy link
Collaborator

Given a GET like this:

is this a thing that people do?

@SentryMan
Copy link
Collaborator

I'm more inclined to make a compile error saying to use @BeanParam.

@rbygrave
Copy link
Contributor

compile error saying to use @BeanParam.

Yes, I like that idea better than my original suggestion. I'll have a go at that.

is this a thing that people do?

It's a mistake someone makes ... that I didn't "see" for a while because it became an interesting runtime error. So yeah, its basically unintentional. Making it a compile error sounds exactly the right way to go imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants