support to options http verb in the controller resources through of Opti... #473

Merged
merged 3 commits into from Oct 30, 2012

Conversation

Projects
None yet
2 participants
@douglasrodrigo
Contributor

douglasrodrigo commented Oct 29, 2012

support to opttions http with Options Annotation

ex.:

@Resource
public class SomeController {
...
    @Path("/options")
    @Options
    public void options() {
        Status statusResult = result.use(Results.status());
        statusResult.header("Access-Control-Allow-Origin", "*");        
        statusResult.noContent();
    }
...
}
@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

Following the other annotations, this one should also accept a path attribute.

Following the other annotations, this one should also accept a path attribute.

@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

I don't think we should remove this if here... We should support OPTIONS the same way it is today... @options is an extension for overriding default behavior.

I don't think we should remove this if here... We should support OPTIONS the same way it is today... @options is an extension for overriding default behavior.

@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

this test doesn't apply if you remove that if.

@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

Member

Thank you very much for this pull request, please consider the comments I've made, and update the pull request =)

Member

lucascs commented Oct 30, 2012

Thank you very much for this pull request, please consider the comments I've made, and update the pull request =)

@douglasrodrigo

This comment has been minimized.

Show comment Hide comment
@douglasrodrigo

douglasrodrigo Oct 30, 2012

Collaborator

Hi @lucascs thanks for the tips, I'm already changing the code following your observations.

Collaborator

douglasrodrigo commented on 8b0abb3 Oct 30, 2012

Hi @lucascs thanks for the tips, I'm already changing the code following your observations.

@douglasrodrigo

This comment has been minimized.

Show comment Hide comment
@douglasrodrigo

douglasrodrigo Oct 30, 2012

Contributor

@lucascs can you take a look now?

Contributor

douglasrodrigo commented Oct 30, 2012

@lucascs can you take a look now?

@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

Member

Have you already tested it in a controller? With a real request?

Member

lucascs commented Oct 30, 2012

Have you already tested it in a controller? With a real request?

@douglasrodrigo

This comment has been minimized.

Show comment Hide comment
@douglasrodrigo

douglasrodrigo Oct 30, 2012

Contributor

yep, I've created a custom project here to see working.
@lucascs are there an integration test that I have to create??

    @Options
    public void options() {
        Status resultStatus = result.use(Results.status());
        resultStatus.header("Access-Control-Allow-Origin", "*");        
        resultStatus.noContent();
    }

request:

curl -i http://localhost:8080/vraptor-options-test -X OPTIONS

HTTP/1.1 204 No Content
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Set-Cookie: JSESSIONID=rj7jxgd78hha;Path=/vraptor-options-test
Access-Control-Allow-Origin: *
Server: Jetty(6.1.10)
Contributor

douglasrodrigo commented Oct 30, 2012

yep, I've created a custom project here to see working.
@lucascs are there an integration test that I have to create??

    @Options
    public void options() {
        Status resultStatus = result.use(Results.status());
        resultStatus.header("Access-Control-Allow-Origin", "*");        
        resultStatus.noContent();
    }

request:

curl -i http://localhost:8080/vraptor-options-test -X OPTIONS

HTTP/1.1 204 No Content
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Set-Cookie: JSESSIONID=rj7jxgd78hha;Path=/vraptor-options-test
Access-Control-Allow-Origin: *
Server: Jetty(6.1.10)
@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

Member

Maybe adding a test on PathAnnotationsRoutesParserTest would be good =)

Member

lucascs commented Oct 30, 2012

Maybe adding a test on PathAnnotationsRoutesParserTest would be good =)

@douglasrodrigo

This comment has been minimized.

Show comment Hide comment
@douglasrodrigo

douglasrodrigo Oct 30, 2012

Contributor

I had already created the shouldAcceptAResultWithOptionsWebMethod on PathAnnotationsRoutesParserTest, I'm missing something?

Contributor

douglasrodrigo commented Oct 30, 2012

I had already created the shouldAcceptAResultWithOptionsWebMethod on PathAnnotationsRoutesParserTest, I'm missing something?

@lucascs

This comment has been minimized.

Show comment Hide comment
@lucascs

lucascs Oct 30, 2012

Member

No, that's fine, thanks!

Member

lucascs commented Oct 30, 2012

No, that's fine, thanks!

lucascs added a commit that referenced this pull request Oct 30, 2012

Merge pull request #473 from aparra/options_http
support to options http verb in the controller resources through of Opti...

@lucascs lucascs merged commit ad1e326 into caelum:master Oct 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment