Add a field type for plugins

This is an outline of what I had in mind. I'll have to dive into typed data a bit more to find out how to return computed values for particular properties (the plugin instances). We probably want to reset the instance and the plugin configuration when the plugin ID is changed.

Oh yeah. I have no idea what data type to use for an arbitrary object instance and if there's any chance we don't have to force developers to make their plugins be typed data as well, I want to take it.

We probably shouldn't use an actual PluginBag anymore, but make the field item behave in a similar way.

What I'm trying to do is make the plugin_configuration property behave like a map while the plugin has not been instantiated. After the plugin has been instantiated, the plugin_configuration property must proxy getting and setting its value to the actual plugin. I haven't figured out how to do this. Tips are welcome.

I have plenty of time to work on this. If you don't have to work on this issue, please provide feedback instead of writing patches, as this issue to me is a way to understand the typed data API a bit better.

This is still untested, but it's pretty much what I was going for. I do need to find a solution for getTypedDataPluginId() as we obviously cannot have an abstract static method (its caller is static, so that's why it can't be non-static).

I guess now is the time for uncensored feedback to see if this is getting close to a good solution or just a complete mess. Thanks!

This patch contains a test that works 25%. Setting and getting the plugin ID on the field does not work yet and I'm not sure why.

Did you considered to add a plugin type feature to plugin managers or discoveries or something else? This would allow to write a generic implementation here, without relying on service IDs directly.

That would be cool. Is there an issue about that we can link to?

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/PluginBagItemBase.php
   @@ -0,0 +1,122 @@
   +    if ($property_name == 'plugin_id' || $property_name == 'plugin_configuration') {
   +      $this->set('plugin_instance', NULL, FALSE);
   +    }
   

   The instance should only be rest if the plugin ID changes. If the configuration is changed, it should be proxied to the instance if it is configurable. Similarly, when the plugin configuration property is read, its value should come from the instance, if it exists and it is configurable.

2. +++ /dev/null
   @@ -1,18 +0,0 @@
   -Place downloaded and custom modules that extend your site functionality beyond
   -Drupal core in this directory to ensure clean separation from core modules and
   -to facilitate safe, self-contained code updates. Contributed modules from the
   -Drupal community may be downloaded at http://drupal.org/project/modules.
   -
   -It is safe to organize modules into subdirectories, such as "contrib" for
   -contributed modules, and "custom" for custom modules. Note that if you move a
   -module to a subdirectory after it has been enabled, you may need to clear the
   -Drupal cache so that it can be found.
   -
   -In multisite configuration, modules found in this directory are available to
   -all sites. In addition to this directory, shared common modules may also be kept
   -in the sites/all/modules directory and will take precedence over modules in this
   -directory. Alternatively, the sites/your_site_name/modules directory pattern may
   -be used to restrict modules to a specific site instance.
   -
   -Refer to the "Developing for Drupal" section of the README.txt in the Drupal
   -root directory for further information on extending Drupal with custom modules.
   

   Oops.

Use case: load a content entity that has plugin data in this field. Some code instantiates the plugin, works with it, and changes its configuration. Entity API then saves the entity, but if the call to the value of the plugin configuration property is not proxied to the plugin instance, the configuration that is retrieved from the entity is the old one that was originally loaded from storage, instead of the up-to-date configuration from the plugin instance.

Proxying the plugin ID is not necessary, because once that changes, the instance must be unset from the field anyway. The instance may continue to exist, but no longer as a value of the field.

Surely it will work for base fields, but it's not really pretty. The patch from #12 shows, I believe, that proxying is not crazy hard to do. Once someone decides to turn this in a configurable field, the preSave() solution will no longer work.

Unless there is a good reason not to try and proxy calls, I think we should implement it.

Because when a user adds a configurable plugin bag field to a content entity, the preSave() method will not automatically be added/update with code to store the field's instance's values.

It's actually not just saving an entity. If you set the field's configuration, then get the instance, then set new configuration, it will not be in sync with the instance that may be used elsewhere.

The tests should pass. I will need to re-evaluate whether we should still expose the instance as typed data if the contained plugin implements TypedDataInterface.

will require protected $pluginManager on class

It won't, because 1) dependency injection is still impossible for typed data and 2) even when it will we, the base field doesn't care about that. It only needs the method.

So maybe better use field setting to define plugin manager like ER fields needs entity_type setting configured

How? We cannot assume all plugin managers can be retrieved from the container, for instance.

So no way to pass plugin specific defaults?

Good point!

this should chain settings clear!

What do you mean?

plugin_id maybe?

I thought of that, but the plugin ID itself does not say much about the contained plugin if it is configurable. I concluded not having a main property name would be the safest way to let people ask for a specific property instead of giving them something that might be useless.

So no way to pass plugin specific defaults?

I implemented this.

@andypost: could you please reply to #33? I would love for this to get into 8.0.0, otherwise I will have to postpone some contrib release to 8.1.0 and that would be a terrible shame.

For this issue to be useful for base fields, #2248977: Complete support for multi-value base fields in ContentEntitySchemaHandler and use it for the user.roles field needs to be fixed first.

I re-opened #2134761: Convert plugins on payment entities to fields where I continued using the patch from #35. I figured a practical use case would allow for better manual testing. I found many more bugs that I fixed and tested. Once that issue has been fixed, I will try and use the improved code to finish the patch here.

This issue is pretty hard to solve because Field API magically creates empty items, ignores field item's default values (applyDefaultValue() and instead sets NULL everywhere), and sets $notify to FALSE in many places for no documented reasons. These behaviors pretty much prevent developers from re-using existing data type plugins if they want any sort of synchronization between properties in a parent field/property.

I know there are issues to remove magic empty field item creation. Am I wrong about the other things and is there actually any documentation for those behaviors?

I've been working on this feature in #2134761: Convert plugins on payment entities to fields for the past few weeks. Developing it for a practical use case helped me discover several flaws in the different approaches I tried to use. I now have a working and re-usable solution that will be committed to Payment. I will use this experience to port to code to Drupal core, which, because the actual plugin code is not Payment-specific, should be fairly straightforward.

There will of course be tests for this functionality, but because some aspects need integration testing, I am wondering if there is a part of core that I can convert as part of the patch, both to provide a concrete use case people can learn from and to have integration testing.

For those who missed it: this idea eventually ended up in the Plugin module, along with some other plugin system improvements.

Unassigning myself.