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

HttpInterceptor #8

Closed
wants to merge 11 commits into from
Closed

HttpInterceptor #8

wants to merge 11 commits into from

Conversation

lukaszlenart
Copy link
Member

New annotation and interceptor to allow limit access to action based on used http request type, ie:

@GetOnly
public class MyAction extends ActionSupport {

     @PostOnly
     public String execute() {

     }
}

@jogep
Copy link
Contributor

jogep commented Apr 25, 2014

Nice, but why not extending the current @action annotation? May like this:

@action(value="my-action", httpMethod=GET)

@action(value="my-action", httpMethod=ALL) <- default

@lukaszlenart
Copy link
Member Author

You don't have to use @Action annotation to convert class/method into action, the Convention plugin can do it for you. Another thing is that you can use wildcard mapping in struts.xml and you won't have @Action annotations as well.

That's why using dedicated set of annotations is the less intrusive way to have such functionality (I though about extending <action .. http-method="get|post|all"/> but thus requires DTD change)

@jogep
Copy link
Contributor

jogep commented Apr 25, 2014

The DTD change should not be the problem because it is a compatible change to previous releases.
The convention plugin applies the default values and all action related configurations should configured with the @Action annotation.

I think the simplest possible way to configure it should be like this:

@Action(httpMethod=GET) instead of @GetOnly

@lukaszlenart
Copy link
Member Author

But then you limit usage of it only to the Convention plugin based applications. Even with extending DTD and adding httpMethod to <action/> you will have limitations of using this to whole set of actions, eg. <action name="empoyee-*" httpMethod="get"> - all actions can be called via GET only.

Another problem is the REST plugin and its default behaviour (/orders/1/edit -> edit()) - you will have to annotated each method with @Action - instead, with current approach, you can annotate the whole Controller class with @GetOnly and add few exceptions on methods (check OrdersController in rest-showcase)

This approach is far more flexible then extending @Action annotation - you can precisely set limitations where you want without changing existing behaviour (@Action can override far more settings than just allowed http method)

@jogep
Copy link
Contributor

jogep commented Apr 28, 2014

Good point. :-)

+1 for merge this.

@lukaszlenart
Copy link
Member Author

Let's treat this as a Proof-of-Concept and if will work we can think about further extensions (like extending @Action annotation)

@davelnewton
Copy link

I'm not excited about @GetOnly, either; if we're tying it to the HTTP protocol then let's make it explicit we're dealing with requests, e.g., @GetRequests or @HttpGet etc.

@lukaszlenart
Copy link
Member Author

Ok, I wasn't sure about naming as well, I will rename the annotations to @HttpGet, @HttpPost, etc

@jogep
Copy link
Contributor

jogep commented Apr 29, 2014

Is it possible to apply Get and Post together to one method in an action and Delete to an other?

public class MyAction extends ActionSupport {

     @HttpGet @HttpPost
     public String execute() {

     }
     @HttpDelete
     public String execute() {

     }
}

@lukaszlenart
Copy link
Member Author

Good questions, will check that. I have added @AllowedMethod({GET, POST, PUT}) to cover that case - but I need advice on naming as I'm not convenient is a good name though ;-)

@asfgit asfgit closed this Sep 21, 2014
@asfgit asfgit deleted the feature/http-interceptor branch September 21, 2014 19:42
@lukaszlenart
Copy link
Member Author

I've moved this branch into my fork - will work on that there

@lukaszlenart lukaszlenart mentioned this pull request Sep 21, 2014
asfgit pushed a commit that referenced this pull request Jun 12, 2016
kusalk added a commit that referenced this pull request Oct 4, 2023
…er provider

Merge in BAM/struts2-atlassian from issue/CONFSERVER-79847-remove-container-provider-ability to STRUTS_2_5_30-atlassian

* commit 'd8abbcd35a5bc4d3047b84d1a3a23dc2a81aa9ce':
  CONFSERVER-79847 Add unit test
  CONFSERVER-79847 Implement ability to remove container provider
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.

4 participants