-
Improvement
-
Resolution: Unresolved
-
Minor
-
None
-
5.0
-
None
Migration of component callback inplace_editable to new a new callback manager.
This came about as a result of discussion between Paul and I.
We are all aware that the current lib.php callbacks are a hot mess. There's simply too much undocumented magic, and one must hunt through a variety of places to find out:
- their existence
- their usage
They're very difficult to maintain, and to understand.
We've already started to move some of the old lib.php callbacks away into hooks, but hooks are intended as a one-to-many approach where one (or theoretically multiple) part of the codebase can fire off a hook implementation and a set of listeners can subscribe to that hook. The system is based upon the PSR-14 Event Listener spec.
However, hooks are one-to-many and component callbacks are one-to-one. One (or theoretically multiple) part of the codebase can fire off a callback to a single implementation, identified by its component name.
At the moment this is implemented in magic territory.
We typically use one of the following methods or combinations:
- component_callback_exists
- plugin_callback
- component_callback
We do also have a class-based variant of this:
- component_class_callback
There are probably other methods too.
Essentially though we load a lib.php, concatenate the component name (or some variation of it), and the method name, and then call that method with a bunch of args. Or we do the same thing with a static method on a class.
The issues here are:
- the function signatures are entirely opaque
- there is no documentation of these methods or classes
- we have to load the entire lib.php just to find out if something is used
- there is no easy discoverability
This set of changes attempts to address these issues. This patch is loosely based upon the PSR-14 and hooks implementation.
Note: The proposed implementation is a proof-of-concept. It is incomplete and may change.
Rough architecture
- a new \core\callback_manager class
- a new \core\callbacks\abstract_callback class which all callbacks must implement
- a new \core\callbacks\replaces_legacy_callback_interface interface which callbacks should implement if they replace a legacy callback
- a new \core\callback\must_exist_interface interface which callbacks must implement if the component must return a value
Instead of defining an arbitrary method name and looking for and calling it, we define:
- a callback object class for the usage which extends the abstract_callback class; and
- an interface for the set of one or more related callbacks
For example instead of this:
if (!$functionname = component_callback_exists($component, 'inplace_editable')) {
|
throw new \moodle_exception('inplaceeditableerror');
|
}
|
$tmpl = component_callback($params['component'], 'inplace_editable', array($params['itemtype'], $params['itemid'], $params['value']));
|
if (!$tmpl || !($tmpl instanceof \core\output\inplace_editable)) {
|
throw new \moodle_exception('inplaceeditableerror');
|
}
|
return $tmpl->export_for_template($PAGE->get_renderer('core'));
|
We have:
A callback object:
namespace core\callbacks\form;
|
|
use core\callbacks\abstract_callback;
|
use core\callbacks\must_exist_interface;
|
use core\callbacks\replaces_legacy_callback_interface;
|
use core\exception\coding_exception;
|
|
class inplace_editable_object extends abstract_callback implements
|
must_exist_interface
|
{
|
public ?\core\output\inplace_editable $renderable = null;
|
|
public function __construct(
|
public readonly string $itemtype,
|
public readonly string $itemid,
|
public readonly string $newvalue,
|
) {
|
}
|
|
#[\Override]
|
public function get_implementing_interface_names(): array {
|
return [inplace_editable_interface::class];
|
}
|
}
|
And an interface:
// \core\callbacks\form\inplace_editable_interface
|
interface inplace_editable_interface {
|
public function execute_inplace_editable(inplace_editable $callback): void;
|
}
|
Then we can call the callback like this:
$callback = new \core\callbacks\form\inplace_editable($itemtype, $itemid, $value);
|
\core\di::get(\core\callback_manager::class)->dispatch($component, $callback);
|
|
return $callback->get_renderable()->export_for_template($PAGE->get_renderer('core'));
|
The callback object contains all data that the callback needs to execute, and the callback manager is responsible for calling the correct callback subscriber.
The callback manager's dispatch method will:
- fetch the list of possible implementing interfaces for the callback object. Multiple are supported to allow for versionng of the callback collection
- determine the correct class name for the subscriber. This is based on the interface name but replacing the interface suffix with _callback
- verifying the callback implementation
- call the relevant method on the callback object (defaults to execute_{{callbackname}} but can be overridden by the callback object)
- if the callback does not exist:
- if the callback_object implmenets the replaces_legacy_callback_interface interface, it will attempt to call the legacy callback
- if the callback object implements the must_exist_interface interface, it will throw an exception if both the callback and the legacy implementation did not exist
The callback object also has a range of getters/setters as required for the implementation.
A replaces_legacy_callback_interface interface is also defined. This is used to indicate that the callback is a replacement for a legacy callback, and requires that the callback object also defines a way to call the legacy callback and set the value in the callback object, emitting debugging if it does so.
Too complex?
Paul and I discused a number of options, including:
- interface only and defining the required method and its arguments
- callback object only and using method naming to determine the method to call
- a combination of the two.
Whilst the interface-only method is simplest, and it provides fantastic discovery, it is inherently fixed and hard to update. There is no way to add or remove additional arguments to/from the callback without breaking backwards compatibility.
The callback object-only approach is also relatively simple and solves this issue with flexibility. However this is as the cost of discoverability.
The combination of the two is the most complex, but it provides the best of both worlds. It is flexible and discoverable. It is also much safer than either option on its own as it does not require magic method naming which could conflict. The creation of the interface is not particularly onerous, and it is a one-time cost. It allows the callback implementation class to declare up-front which features it supports and for collections of inter-related methods to be grouped together.