RFC: Refactor plugin API

[unclecheese]

Background

Following on from #319

The plugin API is saturated with nearly 10 different interfaces that complicate extensibility and discourage reusable code. In short, it's too focused on the how a schema component was added rather than what the intended outcome is.

Problems

There are at least five major problems with this pattern as architected.

Problem 1: Obtuse and overly restrictive contracts

Current interfaces include:

• FieldPlugin
• TypePlugin
• QueryPlugin
• MutationPlugin
• ModelTypePlugin
• ModelQueryPlugin
• ModelMutationPlugin
• ModelFieldPlugin

Without knowing a lot about the internals of the schema, it's impossible to make an educated decision about which contract you need to use for the thing you want to do.

Problem 2: Surgical by default

Plugins by design are added surgically to specific fields. While models automate this for you most of the time, it's important to remember that most of this plugin state is distributed and very difficult to untangle. See the recent documentation on disabling the versioned plugin as an example.

If you think through the cases of why plugins are used:

• 80% case:
  • "I just want my API to have the stuff I expect it to have" ex: pagination, filter, sort
  • "I want my dataobject to act like a dataobject" ex: versioned fields, inheritance
• 15% case:
  • "I wrote a custom query and I want to paginate/sort/filter it" ex: a complex getter that returns ArrayList
• 5% case:
  • "I have two specific fields that need a custom argument and a resolver middleware to do something special with it" ex: adding a truncate argument to a string field

If you think about it, we're catering to the 5% case, here, not the 80%. Plugins are surgically injected (albeit implicitly by models most of the time) into the configuration tree, and it's not clear how to back out of it.

Problem 3: Naming things is hard

Plugins have identifiers. This how they're fetched at build time. After all, they're just singletons that execute on a schema component, rather than an instance of a service.

This makes naming collisions a real problem. I can create a nice plugin that does sorting for DataList queries and call it sort, but if I also create a plugin that handles non-dataobject queries, I have to call it something else. This has resulted in very confusing naming choices like paginateList, paginate and simpleSort.

Problem 4: Contracts inhibit reusability

The plugin for the DataObject sort may give me 85% of what I need -- the input type, the argument, and the parsing of fields. I may just need it to handle the execution of the sort slightly differently for non-dataobjects, in which case I'm completely hosed because ModelQueryPlugin and QueryPlugin are separate contracts.

Problem 5: Ordering of dependencies is a nightmare

Currently, this relies on before and after attributes of the plugin configuration. There's no realistic way for a developer to know what should go before what until an exception is thrown. It's an undue responsibility put on the user, and should be knowledge that is internal to the plugin (although it creates loose coupling).

Proposal

There are several ways we can improve this architecture to make it more developer-friendly and flexible, and most of it hinges on more loose typing.

Idea 1: Think Webpack

To use a Webpack build as an analogue, plugins are just system-wide services that jump in when they're needed. This is determined by a simple "test" parameter:

test: /some regex/
use: [ some plugins ]

GraphQL plugins can work much the same way. We can register them with the Schema only, and let them figure it out on their own. The key difference would be that the test directives would live inside the plugin, rather than in the schema config.

schema:
  plugins:
    - filter
    - sort
    - pagination
    - reallySpecificPluginForSomeFields

public function apply($thing)
{
  if (!$thingIsAQuery) { return }
  // do stuff
}

Idea 2: Reduce the number of contracts

Most plugins are used to modify fields. (Remember, queries and nested queries are fields). Modifying types is an edge case and only done by versioned and inheritance. We can probably get away with:

• FieldPlugin
• ModelTypePlugin

public function apply(Field $field)
{
  if ($field instanceof Query) {
     /// Basic functionality
  }

  if ($field instanceof ModelQuery) {
    // DataList functionality
  }
}

Now, the pagination or sort plugin can react differently to a DataList than it does to an ArrayList. And if it doesn't get anything that it can work with, it can just bail out.

Idea 3: Make plugins instance based, and procedural

A side effect of a purely config approach to plugins is a loss of control over their composition. Defining them as arrays created procedurally probably makes more sense here.

$schema->registerPlugins(
  new DataObjectSort(),
  new DataObjectFilter(),
  new CanViewPermissionChecker()
  new VersionedRead()
  new VersionedDataObject()
);

This would afford an opportunity to change the plugin composition based on the schema state at build time.

Plus, you could customise plugins with the benefit of the IDE, rather than guessing about config fields.

(new DataObjectFilterPlugin())->setInputTypeName('MyFilter')

Idea 4: Plugin collections

Some modules like versioned and JWT jump in with multiple plugins that modify your schema. This makes opting out laborious. If these were defined as collections, it would be a one-liner.

$schema->registerPlugins(
  new DataObjectSort(),
  new DataObjectFilter(),
  new Versioning(), // <-- collection
  new JWT(), //  <--- collection
);

$schema->removePlugin(VersionedPlugins::class)

Idea 5: Plugin classes declare their dependencies

There are certain plugins that cannot realistically come before or after some others, and there are serious security risks with something like CanViewPermissionChecker. Things can get seriously broken if the developer doesn't know this when configuring them.

While it's a bit ugly to couple the plugins like this, it's a better containment of the concern, and less for a developer to stuff up.

public function getDependencies(): array
{
  return [ ] // a list of plugin classes that are expected to have run beforehand
}

public function getConflicts(): array
{
  return [] // a list of plugins that cannot follow this one
}

public function isFinal(): bool
{

}

public function isFirst(): bool
{

}

Idea 6: Customisation and/or activation is done with arbitrary config

Let's start by removing plugins as a config field from everything but the schema (and maybe that has to be procedural anyway).

We can still customise the plugins at the field level by feeding them arbitrary config at execution time.

...
  operations:
    read:
      config:
        filter: false
        sortFields: [ ... ]

Now we don't have any conflicts over naming. Two filter plugins can both look for the config field filter to determine if they're active.

Imagine this composition:

$schema->registerPlugins(
  new DataObjectSort(),
  new BasicSort(),
)

You might have something like:

DataObjectSort.php

public function apply(Field $field, array $config)
{
   if (!$field instanceof ModelQuery) { return }
   if ($config['filter'] === false) { return }
  $fields = $config['filterFields'] : getAlltheFields()
}

BasicSort.php

public function apply(Field $field, array $config)
{
   if ($field instanceof ModelQuery) { return }
   if ($config['filter'] === false) { return }
  $fields = $config['filterFields'] : getAlltheFields()
}

Because filter isn't any special identifier, but rather just some metadata you attach to the field that the plugin(s) look for, you get more flexibility with multiple plugins cooperating in your schema.

Impact

This would be a high-impact change, since there are multiple core modules relying on the plugin API, in addition to many early-adopters already writing plugins, but this is a key API to get right, and if developers are stymied by the thing that is supposed to make the API extensible, then we've achieved the exact opposite of our goal.

[unclecheese]

A good example of why this needs to be done is this recent fix for versioned, where we had a race condition in plugins adding plugins. Schema-wide plugins would have made this much simpler.

silverstripe/silverstripe-versioned#326

[unclecheese]

Another example https://github.com/silverstripe/silverstripe-versioned/pull/342/files

• Versioned adds new version field to model types
• Versioned creates <TypeName>Version types that are clones of the original type with new fields
• Scalar DB plugin assigns new resolver to the type fields
• <TypeName>Version types are ignored because they're not model types, so it doesn't have the the plugin

Really makes a good case for role-focused plugins rather than context-focused. Right now the plugin contract binds it to a certain taxonomy, e.g. ModelTypePlugin, ModelQueryPlugin, but plugins often need to work against a variety of things, as in this case, where you have an update that has to be done to the ModelType (Page) and the plain Type (PageVersion).

Binding the plugins to a role would make more sense because the order of execution would be more transparent.

• addTypesToSchema(): Type[]
• addFieldsToType(): Field[]
• updateType(Type $type): void
• updateField(Field $field): void

Might want to add "Query" and "mutation" into the mix as well, but those are technically just fields.

Special care should be given to preventing plugins from being too greedy -- e..g if passed a Type instance, they can pretty much do anything they want to it (beyond, say, just adding fields). We could pass a clone as a readonly reference, so the plugin could have some context, but couldn't actually mutate the actual registered type.

[unclecheese]

Another one: #417

[unclecheese]

Idea: Plugins are applied in generated code to provide more transparency.

[MasonD]

I very much like being able to write a plugin that does things inefficiently but with easily-maintained code and have the result generated by the plugin be cached into the generated code, meaning there's no performance hit in runtime. I've noticed that the current plugins that come with silverstripe-graphql have fallen into this pattern as well, doing things like linear searches across lists instead of maintaining hash maps, etc.

Edit: would it be helpful for me to describe some of the use cases I've found myself using plugins for?