google-code-export / gwt-dispatch

Automatically exported from code.google.com/p/gwt-dispatch
1 stars 0 forks source link

execute() method of AbstractDispatch is never invoked for batch commands #47

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

BatchActionHandler calls context.execute( a ) which in turn calls 
AbstractDispatch private method doExecute(). Public method AbstractDispatch 
execute() of Dispatch interface is never called for commands that included into 
a batch like BatchActionHandler or custom one. It does not allow, for example, 
authorize execution of each command in a batch before execution. 

Making AbstractDispatch's method doExecute() to be a protected method could 
help to solve this issue.

Original issue reported on code.google.com by ora...@gmail.com on 19 Jan 2011 at 11:44

GoogleCodeExporter commented 9 years ago
I need to understand more about your use case here. Do you override 
AbstractDispatch.execute() to perform security checks ?

If so , would a 

protected void beforeExecute(A action, ExecutionContext ctx) {

}

method which would be invoked first this in doExecute() help you?

Original comment by robert.munteanu on 21 Apr 2011 at 7:46

GoogleCodeExporter commented 9 years ago
I would like to be able to check if the user is allowed to execute the command 
and do not call handler's execute method he/she is is not allowed. And the 
dispatcher  Dispatch execute() method is a very good method to  override to do 
it. Unfortunately, it does not work for batch commands because of the 
implementation described above. beforeExecute() would allow to throw an 
exception if the user is not authorized but afterExecute() is needed as well to 
filter out (or process) results. Making doExecute() protected allow to do both. 
I understand that is not the best approach but it is a quick one.  

Original comment by ora...@gmail.com on 21 Apr 2011 at 9:07

GoogleCodeExporter commented 9 years ago
So you would need

protected void afterExecute(A action, R result, ExecutionContext ctx) {

}

as well, right?

Original comment by robert.munteanu on 26 Apr 2011 at 9:22

GoogleCodeExporter commented 9 years ago
In this case I would prefer to add ActionHandler as a parameter to both methods 
to get information from it.
protected void beforeExecute(A action, ActionHandler handler, ExecutionContext 
ctx) {
} 
and
protected void afterExecute(A action, ActionHandler handler,  R result, 
ExecutionContext ctx) {
}

Original comment by ora...@gmail.com on 27 Apr 2011 at 4:28

GoogleCodeExporter commented 9 years ago

Original comment by robert.munteanu on 27 Apr 2011 at 9:17

GoogleCodeExporter commented 9 years ago
One clarification: do you need to throw DispatchExceptions from those methods?

Original comment by robert.munteanu on 27 Apr 2011 at 9:56

GoogleCodeExporter commented 9 years ago
Yes I need to be able to throw DispatchExceptions from the methods at least 
from beforeExecute().

Original comment by ora...@gmail.com on 28 Apr 2011 at 12:11

GoogleCodeExporter commented 9 years ago
The fixes are now in hg. Three new methods have been added: `executing`, 
`executed` and `failed` .

Original comment by robert.munteanu on 28 Apr 2011 at 7:32