Community
Participate
Working Groups
In org.eclipse.core.commands.operations.DefaultOperationHistory.execute(), IStatus status = getExecuteApproval(operation, info); and the status is returned right away without calling execute, so command result will be null because execute was not called on org.eclipse.gmf.runtime.common.core.command.ICommand, and execute is expected to call setResult. We should have a default result, with a default status of IOperationHistory.OPERATION_INVALID_STATUS. Furthermore, the FileModificationApprover can update the CommandResult of ICommands accordingly. The three attached patches will resolve the issue.
Created attachment 51793 [details] Patch for gmf.runtime.common.core
Created attachment 51794 [details] Patch for gmf.runtime.emf.commands.core
Created attachment 51797 [details] Patch for gmf.tests.runtime.common.core
Linda, can you review these changes?
Created attachment 51800 [details] FileModificationApprover - replaces FileModificationApprover from previous patch
I think that the proposed fix is not suitable for the 1.0.2 release because it changes the expected return value of ICommand#getCommandResult(). This is an API change that may break existing clients who are using the operation history and command APIs correctly. The problem is illustrated by the fact that the unit tests had to be modified to accomodate this change. Clients expecting a null command result when the command has not been executed will now find that there is a command result. Clients are required to check that the command result is not null before using it. Null is the expected return value from ICommand#getCommandResult() when the command has not been executed, which is the case when operation approvers reject the operation.
Created attachment 51881 [details] common.core, emf.commands.core, common.core tests Alternative version. CommandResult is not set initially.
Patch 51881 looks better to me. It narrows the change in command behaviour to the specific bug we need to fix, which is that some clients need a return result (with status) on the command when the command fails file validation. I noticed that some of the imports (used by javadocs) were removed. They probably need to be there, or the javadocs would have to be updated to use fully qualified names.
Created attachment 51903 [details] Patch with imports added back
Committed to R1_0_maintenance
[GMF Restructure] Bug 319140 : product GMF and component Runtime Common was the original product and component for this bug
