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.

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

allowing controllers to receive optional parameters #618

Merged
merged 2 commits into from Oct 14, 2014

Conversation

nykolaslima
Copy link
Contributor

Currently we can't create a controller method that receives an optional parameter, when using deserialization.

@Post("/doSomething")
@Consumes("application/json")
public void doSomething(Car car) {
}

If we call /doSomething with empty body, this will throw a NullPointerException because it does not have Content-Type header.

POST /doSomething (works great!)
{
  car: {
    ....
  }
}
POST /doSomething (throws a NullPointerException)
<<no body>>

@@ -70,6 +70,12 @@ public void intercept(InterceptorStack stack, ResourceMethod method, Object reso
Consumes consumesAnnotation = method.getMethod().getAnnotation(Consumes.class);
List<String> supported = Arrays.asList(consumesAnnotation.value());

if(request.getContentType() == null) {
logger.info("Request does not have Content-Type and parameters will be not deserialized");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use warn.

@nykolaslima
Copy link
Contributor Author

Thanks for you comment @garcia-jj. Done!

We need to replicate this to VRaptor4?

@Turini
Copy link
Member

Turini commented Oct 14, 2014

can you reproduce it on vr4? If so, it would be nice to replicate the solution

@garcia-jj
Copy link
Member

I can't understand why this solution allow optional parameters, because we
only check if content type is null.

Why content type is related to parameters? Why you simple bypass
deserialization?

Your pull request sounds good, but I did this questions because I don't
understand the scenario.

@garcia-jj
Copy link
Member

+1 to replicate to vraptor 4.

@nykolaslima
Copy link
Contributor Author

When you make a request without content, then it wont have the Content-Type
header and it causes a NullPointerException. @garcia-jj

On Tuesday, October 14, 2014, Otávio Garcia notifications@github.com
wrote:

+1 to replicate to vraptor 4.


Reply to this email directly or view it on GitHub
#618 (comment).

@garcia-jj
Copy link
Member

Hmm, ok, now I see your point. Sounds like good to merge for me. Can you send a pull request in vr4 linked to this pull request?

Thank you

@nykolaslima
Copy link
Contributor Author

Sure!

I'll do this. But this interceptor does not exists in VRaptor4. I'll find out where this logic has been doing there.

Thank you.

@Turini
Copy link
Member

Turini commented Oct 14, 2014

DeserializingObserver at VRaptor 4

@nykolaslima
Copy link
Contributor Author

Thanks Turini, I'll send a pullrequest right now.

[]s

2014-10-14 11:19 GMT-03:00 Rodrigo Turini notifications@github.com:

DeserializingObserver at VRaptor 4


Reply to this email directly or view it on GitHub
#618 (comment).

@nykolaslima
Copy link
Contributor Author

@Turini we already merged the VRaptor4 implementation. Can we merge this now?

Turini added a commit that referenced this pull request Oct 14, 2014
allowing controllers to receive optional parameters
@Turini Turini merged commit 4051db8 into caelum:master Oct 14, 2014
@Turini
Copy link
Member

Turini commented Oct 14, 2014

done! thank you again, @nykolaslima

@nykolaslima
Copy link
Contributor Author

No problem. Thank you.

2014-10-14 18:00 GMT-03:00 Rodrigo Turini notifications@github.com:

done! thank you again, @nykolaslima https://github.com/nykolaslima


Reply to this email directly or view it on GitHub
#618 (comment).

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 this pull request may close these issues.

None yet

3 participants