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

Fixes auth interceptor on MusicJungle #145

Merged
merged 4 commits into from
Oct 1, 2013

Conversation

rponte
Copy link
Contributor

@rponte rponte commented Sep 30, 2013

During my tests on music-jungle when I access an URL not permitted I get this error:

java.lang.IllegalStateException: Cannot forward after response has been committed

When interceptor uses @BeforeCall it can't stop the flow of request when the user isn't logged in, so it's necessary to use @AroundCall instead.

@BeforeCall
public void intercept() throws InterceptionException {
@AroundCall
public void intercept(SimpleInterceptorStack stack) {
Copy link
Member

Choose a reason for hiding this comment

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

weird indent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to change it because I thought the problem was with my IDE so I left that way... Don't worry, I'll fix it :-)

@rponte
Copy link
Contributor Author

rponte commented Sep 30, 2013

So now, @lucascs?

@lucascs
Copy link
Member

lucascs commented Sep 30, 2013

It's good now =)

I'd only check if that result.add("errors", ...) followed by a redirect could be changed by a validator call, what do you think?

@rponte
Copy link
Contributor Author

rponte commented Sep 30, 2013

I tried that, but it didn't work because there's no other interceptor catching ValidatorException.

@lucascs
Copy link
Member

lucascs commented Sep 30, 2013

Bummer. :shipit:
It would be nice to add a comment explaining why you can't use a validator there.

@rponte
Copy link
Contributor Author

rponte commented Sep 30, 2013

Ok, sir!

@rponte
Copy link
Contributor Author

rponte commented Sep 30, 2013

I just add a simple comment. Do you think it's enough?

@lucascs
Copy link
Member

lucascs commented Sep 30, 2013

Ok. 🚢

@rponte
Copy link
Contributor Author

rponte commented Oct 1, 2013

Can we merge it?

@garcia-jj
Copy link
Member

👍

rponte added a commit that referenced this pull request Oct 1, 2013
@rponte rponte merged commit 56bd0a4 into master Oct 1, 2013
@garcia-jj garcia-jj deleted the fix-auth-interceptor-musicjungle branch October 4, 2013 04:00
@Turini Turini modified the milestone: 4.0.0-beta-1 Mar 6, 2014
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