Illya: The source of the event varies.

Could someone suggest a good API/inplementation?

What do you think about this API enhancement and implementation?

I would like to add this method to class ActionManager:

/**
 * Executes the specified action in the thread of the caller (must be the Swing Event Dispatch thread, EDT).
 * If the action is backed by an org.jdesktop.application.Task, then it will be executed asynchronously
 * on the EDT, this method will return immediately. If there's a org.jdesktop.application.Task.BlockingScope
 * specified for the Task, then the UI will be blocked appropriately. 
 *      
 * @param action the Action to be executed
 * @param source the UI component which acts as the source for the execution 
 */
public void executeAction(javax.swing.Action action, java.awt.Component source) {
    action.actionPerformed(new ActionEvent 
        (source, ActionEvent.ACTION_PERFORMED, action.toString()));
}

The implementation is the workaround I'm using right now, but it works fine. Maybe someone has a better idea...

Usage example for loading the application data (asynchronous Taks), the UI will be blocked meanwhile:

appContext.getActionManager().executeAction(
    actionMap.get(LOAD_ACTION), frame);

I really don't think this variant does make much sense. You still need a lot of code to retrieve/create action, to get ActionManager, etc. It will work only for a few cases.
By the way, I don't understand why executeAction method is not implemented as a static method.
I'm sure we need to consider more options in respect of this issue.

The main reason that it's not static is simple: so it's easily mockable in unit tests, also without complex bytecode-manipulation frameworks. The ApplicationContext instance is available everywhere, at least in the Controller class of the application which will probably use this method (the same for the ActionMap to get the action to be started).

Sorry, I don't understand why this is too much code. OK, the static variant would be shorter, but I've described the disadvantages. And the action to be started needs to be passed.

We could also add such a method to the Action subclass in the (B)SAF, but that's not the best place. And we often need to cast, because the ActionMap returns javax.swing.Action instances.

Do you have an alternative solution?

Bye, Stefan

maybe we should consider using this method as a shortcut

SwingUtilities.notifyAction(Action action, KeyStroke ks, KeyEvent event, Object sender, int modifiers)

The method is static by default, which we can keep that way, and it will generate the ActionCommand for the created ActionEvent in a proper way.

What would be the benefit of SwingUtilities.notifyAction(...) compared to my workaround proposal above?
The JavaDoc for SwingUtilities is unfortunately not complete, it doesn't contain the description for the event, sender and modifiers. Do you know the details?

Stefan

Well I think it's quite similar, except than that this one is static and in the SwingUtilities. Maybe it's better to reuse existing swing functionality instead of creating new things. It's actually a shorthand for

action.actionPerformed(new ActionEvent(sender,
ActionEvent.ACTION_PERFORMED, command, event.getWhen(),
modifiers));

where
modifiers the modifier keys held down during this action, which will be 0
when the time the event occurred
sender the object source

we could create a convencience static method in ActionManager that calls this one with default argument. Actions don't necessarily need to be retrieved from an ActionMap. They can eg also be injected by a framework or so. I use aspectj eg to weave in some common application functionality. So declaring it static is ok for me.

Yes, such a convencience method in ActionManager is what I'm looking for, see my proposal above. It doesn't matter for me which way the implementation works, maybe the SwingUtilities method is better. But I can't see any advantages...
I would not like to create a static method, this is much harder to replace, especially in unit tests...

Stefan

I created a patch with two static methods in ActionManager which can be used to invoke Actions.
They use the SwingUtilities#notifyAction method to accomplish this.

I've checked this patch, looks quite complicated. What's the real advantage compared to my proposal above.
One more time: it's not a good idea to make this methods static, it's much harder to replace by mocks in unit tests!

Stefan

I don't really see why one would want to mock this method? Isn't it just a general purpose utility method whose behavior just remains the same? Or am I missing something here?

Example: If you want to unit test the controller logic then you don't want any side effects. You just want to mock the ActionManager to make sure that the executeAction(...) method was called or not.

OK I agree, my patch has a bug, it doesn't work when a Task is returned.

Can etf please review this and put this in the codebase?

thanks

I think it would also be useful to give this 'utility' method the ability to set selected state for the action. Best way to implement this is to provide an overloaded version that allows you to pass a boolean that sets the selected property on the action. Also, we should not forget to check for enabled state before executing the action itself.
This way, the ApplicationAction specific properties stay in sync with the Action itself.