Commands: Sunsetting `DatabaseObjectAction`

[dtdesign]

DatabaseObjectAction was envisioned to solve problems we had back in version 1 of the framework. The introduction of more and more AJAX based interactions meant that object creation and state updates were no longer a fixed part of forms but could be triggered from many different places. DatabaseObjectAction solved this problem by centralizing the logic to a single point in the software, including the event access which makes it quite easy to observe these actions.

The Shortcomings of DatabaseObjectAction

• Parameters for actions are very opaque. The generic parameter array provides no guidance on the required or optional parameters of an action.
• Consuming an arbitrary array creates a lot of assumptions and potential type mismatches, effectively being unable to utilize static typing to avoid errors.
• The validateAction event is pretty much pointless because it only allows to observe the raw input values. Neither does it permit any modifications (which is a good thing though!) nor does it necessarily correspond to the values used to execute an action.
• The events are pretty generic and require extra checks to figure out which action is being executed.
• Methods sometimes rely on internal variables that are only set in the validate*() method (which is used for AJAX), making it almost impossible to call this action programmatically.
• All member variables are prone to poisoning because they can be modified anywhere in the lifetime of the object, especially when invoking other methods directly instead of dispatching a new DatabaseObjectAction.
• Helper methods are difficult to maintain because there is no apparent connection to the action it belongs to, creating a lot of problems at scale.
• DatabaseObjectAction does not scale properly, instances like wbb\data\thread\ThreadAction or wbb\data\post\PostAction are at or above 4k lines which makes them unmaintainable to some degree.
• Designed with maximum efficiency in mind those actions can be applied to arbitrary large sets of objections which means that a lot of times extra parameters (like isBulkProcessing) are introduced to suppress certain behavior. This creates a lot of branches in the code, eventually leading to a lot of bugs and inconsistencies, often times those bulk actions require a full data rebuild afterwards.
• DatabaseObjectAction relies on strings to identify which method to call, making it difficult to detect all uses of a given action to verify if a change would break the API compatibility. Although this is mostly an issues for us because we run checks against the entire Plugin-Store to decide how far we can go without breaking compatibility.

The “Command” Pattern

In recent versions we have adopted the “Command” pattern that consists of single-purpose classes with a fixed constructor and an __invoke() method to execute it. Commands SHOULD perform a single task only and MAY invoke other commands when necessary.

Commands MUST NOT accept an array for its expected parameters.

The commands are designed to operate on a single object, called from a PHP context and do not perform any validation on their own. They also should not make any assumptions about the context in which they are created, for example, if a user object is required it must be provided in order to decouple it from the active session.

Phasing out DatabaseObjectAction.

This is a mid- to long-term effort and the migration should take place gradually. New actions are being implemented as commands already and the existing actions in the respective DatabaseObjectAction are kept as-is for the time being.

Commands are allowed to forward calls to DatabaseObjectAction if they are built on top of functionality that has existed before. For new features the guideline is to directly invoke the DatabaseObjectEditor to carry out the task.

At the end of every command SHOULD be a dedicated event that provides the result of the action. For example, the command CreateThread should trigger the event ThreadCreated(Thread $thread) to notify any observers.

Delegating Existing Actions to Commands

Actions in DatabaseObjectAction MAY invoke a command to perform the task, but SHOULD NOT delegate to a command that dispatches an event.

Rationale: Delegating the execution would cause all interactions with DatabaseObjectAction to be observable through its built-in events, but not invocations that directly use the command.

It is advisable to only delegate to such commands if the action is being deprecated and marked for removal with the next version, effectively reducing the likelihood of such situations manifesting itself while still offering a way for event consumers to seamlessly migrate.