Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entity & Field plugins #155

Open
jonathanjfshaw opened this issue Dec 28, 2017 · 4 comments
Open

Entity & Field plugins #155

jonathanjfshaw opened this issue Dec 28, 2017 · 4 comments
Milestone

Comments

@jonathanjfshaw
Copy link
Contributor

I'm most of the way there with my Christmas present: an initial rough implementation of

  1. Field plugins (real D8 plugins that developer can supplement for their own project); these replace the home-baked Field handlers.
  2. Entity plugins (rationalising up all the node, term, user, etc. creation & deletion logic and allowing developers to override/supplement it.
  3. Extensive driver-side kernel tests to make sure everything works the same after the refactoring.

I'm thinking this should be for version 2.0; do you have a timeline for that or other things on the roadmap for it?

@jonathanjfshaw
Copy link
Contributor Author

Initial discussion about this happened a long time ago on the extension-side at jhedstrom/drupalextension#337

@jhedstrom jhedstrom added this to the 2.0 milestone Dec 29, 2017
@jhedstrom
Copy link
Owner

This sounds great. I'm not sure if it would be the same plugin system or not, but I'd like to split the individual drivers up into collections of plugins so they aren't these singular monolithic classes (I'm not sure of how that would work in practice).

@jonathanjfshaw
Copy link
Contributor Author

jonathanjfshaw commented Dec 29, 2017 via email

@jonathanjfshaw
Copy link
Contributor Author

I've completed initial development of an entity and field plugin/wrapper system, see PR #157.

Benefits

Customisation: users can add special handling for new field or entity types, or override handling of standard types.

Simplicity: we can get rid of all entity-type specific methods in the driver and extension. We can even throw away the whole hook system in the extension.

Labels: identifying entity types, bundles, fields and referenced entities by labels rather than machine names is (more) possible and customisable.

Cleaning up: we can specify additional tear down instructions for particular entity types.

Overview

In a nutshell if you want to create a new Drupal entity using the driver what now happens is:

  • you ask the driver for a new DriverEntity wrapper object and tell it what entity type.
  • you tell the DriverEntity wrapper object additional information about the entity (like bundle and fields)
  • When it needs to, the DriverEntity wrapper uses the DriverEntityPluginManager to discover and use the most-targeted DriverEntity plugin
  • the DriverEntity plugin abstracts in a customisable way a real Drupal entity.
  • whenever a user-supplied field value needs to be interpreted the DriverEntity wrapper or plugin calls up the DriverFieldPluginManager and uses that to discover and process the field value through all matching DriverField plugins.

From the point of view of the end-consumer, it's dead easy: you ask the driver for a new entity wrapper and then interact with that wrapper as if it were a native D8 entity.

In the future the Drupal extension only has to know about that one method getNewEntity, we deprecate all the other entity-type-specific create & delete methods.

Behind the scenes it's necessarily complex, and there are many possible implementation questions. Here I'll raise some general matters:

Status

This definitely needs testing on existing test suites as it's changing fundamental APIs and it would be easy to have missed a hidden consequence somewhere.

There's a lot that's rough and debatable in the implementation, but it's fully backwards compatible and has extensive test coverage. Therefore I believe it's OK to commit as it, warts and all, and then tackle the specific problems in more manageable follow-ups.

However, I can't spend mountains more time on this, and have hit some challenges that need other people's skills, so it might not be me completing those follow-ups though I'll be reviewing for sure.

What belongs in Driver vs Extension

Currently the EntityReference field handler expands labels into id's, a feature that is enormously helpful for BDD. I've expanded that approach further, allowing labels instead of ids as widely as possible.

In #116 @pfrenssen suggested that the current behavior of the EntityReferenceHandler is an abuse of the handler for a Behat use case, and that the role of the Driver is only to provide a version-agnostic Drupal API. If this is the case, then potentially we might want to keep all natural language parsing out of the Driver.

In this case the Extension could have its own wrappers extending the driver's wrappers a little, and its own plugins adding to the driver's plugins and doing more aggressive processing of field inputs. There is some added complication, but not much.

Discovery of project plugins

Ideally you could put plugins in your project Behat code folder, the one autoloaded based on what's specified in behat.yml: something like features/bootstrap/plugin/DriverEntity/

I've opened up an argument called $projectPluginRoot which is passed around amongst the driver, wrapper, pluginmanagers and plugins in order to facilitate this. But my small attempt to tickle the plugin managers into instantiating a plugin from this folder failed, probably because I don't understand the relationship between namespaces, plugin discovery, and composer autoloading.

D6, D7, and DefaultPluginManager

Drupal 8's plugin component is available as a drupal-independent composer dependency, so we can use it with D6 & D7. However, I cut a corner and extended the DriverPluginMangerBase from Drupal 8's DefaultPluginManager. To get things working for D7 this need to be reworked to not have this dependency.

I have created plugins for D8 (which is easy) but not D7 and D6. Creating the needed plugins is really pretty easy if you understand D7, which I do not. It's largely a matter of copying code from a current driver method into a new plugin class.

Travis & test running

Now we have lots of nice kernel tests, it would be great if travis could run them. run-tests.sh won't discover tests using the @group annotation outside of drupal core and modules. Maybe php unit will help. Tickling Travis is not something I'm very good at so I haven't attempted this. There's currently a hardcoded path in DriverKernelTestTrait that will need addressing.

validateDrupal in userCreate

There was a call to validateDrupal() in the driver's userCreate().
I've commented it out as it crashes kernel tests. What was the point of it? I wonder if it's something to do with using user_cancel (which invokes batch processes) during userDelete().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants