Closes #778. Checking result.used() before forward #867
Conversation
|
what do you think @lucascs @garcia-jj? |
|
I'm unable to test in my application right now. But the code sounds like good. |
| @@ -111,7 +115,8 @@ public Object intercept(T proxy, Method method, Object[] args, SuperMethod super | |||
| if (!(returnType == void.class)) { | |||
| request.setAttribute(extractor.nameFor(returnType), methodResult); | |||
| } | |||
| if (response.isCommitted()) { | |||
|
|
|||
| if (response.isCommitted() || result.used()) { | |||
There was a problem hiding this comment.
result is always used here.
because you do:
result.forwardTo(...)...
//which calls
result.use(logic()).forwardTo(...)💣
There was a problem hiding this comment.
@lucascs it won't, take a closer look at the other change.
Result#forwardTo isn't setting used as true anymore.
I couldn't find a place that requires it (am I missing something?)
There was a problem hiding this comment.
yes you are...
if you forward and the request ends it will try to forward to default view.
There was a problem hiding this comment.
if you forward to a method that only does result.nothing(), for example, it will forward to the original method's jsp, or something like it.
There was a problem hiding this comment.
No @lucascs, it doesn't break the cases you've mentioned... are you looking
for the entire PR? Please, take a look at DefaultResult#forwardTo method.
There was a problem hiding this comment.
Ok... but it will break if the person used result.use(logic()).forwardTo(...)
But ok, 🚢
There was a problem hiding this comment.
@lucascs, you're wrong... result.use(logic()).forwardTo(...)still working too.
I didn't change the use behavior. Only default forwardTo that doesn't set as used.
Closes #778. Checking result.used() before forward
I'm not sure if its the best implementation, but it fixes #778 without broke forward.