NateWr
commented
5 years ago This is a great idea, thanks @ViRuSTriNiTy!
I'm trying to think through the implications here, and figure out what is the best approach. One concern I have with putting it into Services::get() is that it's called over and over again. I understand why it can't be in the init(), but I also worry that if a plugin can inject a service at any time, we'll end up with a lot of headaches down the line where a developer won't be able to know when a service will be registered. They'll have to find it out on a plugin-by-plugin basis.
In WordPress, there are a few low-level hooks like init, admin_init, after_plugins_loaded and after_themes_loaded. These are good for when a plugin or theme wants to register some kind of asset that is available generally.
I'd like to try a similar approach here. On the one hand, we'll have a bit more certainty about when a custom Service is registered. On the other hand, we'll be able to use the same hook for lots of things like this, which will help keep our documentation smaller (when we get to writing it out properly!).
Here's what I'm thinking.
Every request to the application is kicked off by Dispatcher::dispatch(). Here, every generic plugin is loaded and the context schema is reinitialized:
https://github.com/pkp/pkp-lib/blob/master/classes/core/Dispatcher.inc.php#L131-L138
AppLocale::initialize($request);
PluginRegistry::loadCategory('generic', true);
// Reload the context after generic plugins have loaded so that changes to
// the context schema can take place
import('classes.core.Services');
$contextSchema = \Services::get('schema')->get(SCHEMA_CONTEXT, true);
$request->getRouter()->getContext($request, 1, true);I'd like to add a new hook here:
AppLocale::initialize($request);
PluginRegistry::loadCategory('generic', true);
HookRegistry::call('Dispatcher::dispatch', $request);
// Reload the context after generic plugins have loaded so that changes to
// the context schema can take place
import('classes.core.Services');
$contextSchema = \Services::get('schema')->get(SCHEMA_CONTEXT, true);
$request->getRouter()->getContext($request, 1, true);Then, in PKPServices, we should add a static method for registering a new ServiceContainer:
public static function register($service) {
self::_instance()->container->register($service);
}If a plugin wants to register a new Service, it should then create a new ServiceProvider class, just like the OJSServiceProvider:
namespace PluginName;
use \Pimple\Container;
class PluginNameServiceProvider implements \Pimple\ServiceProviderInterface {
public function register(Container $pimple) {
$pimple['serviceName'] = function() {
return new \PluginName\ServiceNameService();
};
}
}Finally, it should hook to Dispatcher::dispatch and register the new container:
public function registerServiceContainer($hookName, $request) {
Services::register(new \PluginName\PluginNameServiceProvider);
}Cons:
- There is a little more boilerplate here. But I think only a big complex plugin should bother with a Service class, so I think that's ok.
- Only
genericplugins can use this hook. I think that's the right call. Other plugin categories should provide all the methods needed to handle those use-cases. If plugin developers find them lacking, we should extend that category's base class.
Pros:
- Plugins can register multiple services using the same Provider architecture as the apps.
- The base hook is highly reusable for other low-level plugin needs.
- If a plugin has a service class, any third-party developer can know with certainty when that service class will be available in the app.
Notes:
- If a plugin really wants to, it can register a new service class anywhere. This will remain undocumented and not be considered a recommended practice.
@asmecher do you have any thoughts about this?
@ViRuSTriNiTy I haven't tested this at all, but would you be willing to work up a Pull Request along these lines, once we have input from Alec?
ViRuSTriNiTy
asmecher
Hi there,
as described here ...
https://forum.pkp.sfu.ca/t/hook-for-service-registration-in-a-plugin/50799
... there should be a hook for providing services from a plugin.
I will provide a PR.
So lonG