Problem

Currently, entity types are managed by modules in code, so there is no way to create entity types with API methods or using an admin UI.

Proposed Resolution

  • Add derivative support to EntityManager
  • Add a plugin to support entity type config entities using the entity manager discovery
  • Implement CRUD support for entity type storage

Follow-ups

Files: 
CommentFileSizeAuthor
#11 1838676-11.patch1.18 KBfubhy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1838676-11.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 entity-type-1838676.patch13.19 KBfmizzell
FAILED: [[SimpleTest]]: [MySQL] 48,203 pass(es), 10 fail(s), and 3 exception(s).
[ View ]

Comments

StatusFileSize
new13.19 KB
FAILED: [[SimpleTest]]: [MySQL] 48,203 pass(es), 10 fail(s), and 3 exception(s).
[ View ]

Thanks to mitchell for all the help with documentation

Priority:Normal» Major

I'd really like to get this in, feels like at least a major feature request.

Status:Active» Needs review

Marking needs review.

Status:Needs review» Needs work

The last submitted patch, entity-type-1838676.patch, failed testing.

I think this requires a much more detailed architectural plan.

If we fail to do this right, this will turn into a horrible disaster. I'm skeptical whether it is a good idea to add this feature for D8 in the first place.

I am really not too excited about the idea of dynamic entity types at all to be honest. I never understood why that even makes sense... and I still fail to see the point. I am happy to be convinced otherwise though.

Also, I have to agree with @sun here. IF we choose to do this, then we need a much more detailed plan and architectural proposal behind it that also outlines what this would give us as a feature.

I guess that the immediate goal of this issue is to allow for the easy creation of entity types. I don’t know if any of you have tried to create an entity type in D7 or even D8, but it is a lot harder than this:

$et = new EntityType(array(‘name’=>”my_entity_type”));
$et->save();

What this tiny piece of code gives you is the most strip-down version of an entity type possible: It initialized the storage (with an id db field) so you can store the entities, and it exposes the entity type definition to Drupal.

Then you have an entity type ready that you can start adding fields to, to make it useful.

What is interesting about such a strip down entity type, is that we can make it whatever and exactly what we want. Nodes are great, but starting from a much simpler place will mean greater flexibility, and some very useful code refactoring.

I would love to get to a place where any entity type could be as capable as nodes are, if necessary, or simpler if fitting.

I also believe that making entity types more useful help us simplify things, more specifically, I am talking about eliminating bundles. The only purpose of bundles is to allow some of our core entities to be extensible, more specifically, since everyone uses nodes for everything, we had to make nodes more flexible. If we allow users to create as many entity types as needed, and we can attach fields to each entity type, then what do we need bundles for?

I am very excited about the behavior issues in core, field and entity behaviors, as they will allow us to write functionality independent of any specific entity or field. This idea is the best way to maximize the usefulness of our code across the whole entity system IMHO.

I think what the end game could look like is that Drupal could ship we zero predefined entity types, yet with all the capabilities to build drupal as it is now. In the Lego analogy terms, we would have simpler yet more powerful, independent and useful Legos to play with.

I know that I touched on a lot of different subjects, but I just want to make the point that this is just a first step toward a greater goal of making our data modeling systems as capable as possible.

Yes, I completely understand all of that, and I can get behind it. However, those considerations are too high-level.

We need an architectural plan for the (low-level) consequences of this. Race conditions. Circular dependencies. Non-static $entity_type function parameters. Maintenance. Updates. Major version upgrades. Etc.pp.

@sun, I agree, just wanted to give the motivation to why this directions is important and interesting.. now we can talk low level :)

I agree that it would be nice to have, although I'm not sure the regular site builders needs more than node-like content entities, or can understand the consequences of doing it per entity type or per node type.

If we allow users to create as many entity types as needed, and we can attach fields to each entity type, then what do we need bundles for?

For modelling "sub-type" relationships.

Status:Needs work» Needs review
StatusFileSize
new1.18 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1838676-11.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Time is really running out for in-depth discussion of this. So why don't we provide a simple single-line patch for now that allows us to do this in contrib... namely: Add DerivativeDecorator to the plugin discovery.

@fago I was not present when the creation of bundles happened, but from my experience it seems that using bundles as subtypes is more of an accident than a proper use of them, otherwise we implemented the crappiest inheritance system ever :) . if we want subtypes, maybe we should have a proper inheritance system where you can have parent-children relationships as deep as necessary.

I'm not sure the regular site builders needs more than node-like content entities

I agree, this is definitely an advanced user feature, but when you do need a simple entity (ex. a transaction in a finance system) and you have to use a node its pretty annoying, specially when we already have the capabilities not to be annoying :)
I have also found, through ECK, that developers coming from other frameworks feel more comfortable with these kind of capabilities, so it could make it easier for many to transition to Drupal.

@fubhy I guess that patch would allow us to work this out in contrib, but if we are leaving InfoHookDecorator permanently in Core, we don't really need the Derivatives decorator, even though it wouldn't hurt to leave it there in case someone else could use its capabilities.

Another thing that would be nice to have in core, specially if the community decides that this is not going in D8, would be for the storage controllers to initialize the storage if needed. That will take that responsibility away from the dynamic entity-type manager were it doesn't belong.

One more thing, if we store entity-type definitions in config instead of through annotation or an info hook, that would allow us to get each definition instead of getting them all and then returning one. I am not sure what the performance consequences of storing stuff in config are, but the config approach is definitely more scalable.

I think #11 is the right way to go for now.

@fago I was not present when the creation of bundles happened, but from my experience it seems that using bundles as subtypes is more of an accident than a proper use of them, otherwise we implemented the crappiest inheritance system ever :) . if we want subtypes, maybe we should have a proper inheritance system where you can have parent-children relationships as deep as necessary.

Yes a proper inheritance system would be nice, see #100925: Bundle inheritance and #1380720: Rename entity "bundle" to "subtype".

I don't think we need a UI for this in core (or at least that's a completely separate discussion), but it'd be very nice to have a sufficiently complete Entity API that something like #7 works.

This means entity types being responsible for their own schema (instead of hook_install()) and similar, which would also make pluggable entity storage more realistic.

If possible it'd be good to move changes like that out into their own issues.

This issue came up on the REST team call this morning. Based on that call, I actually want to push back here.

I strongly believe we should not enable dynamic user-created-through-the-UI entity types. It causes more problems than it solves.

Primarily, there are numerous places where code would be considerably easier if we could rely on a 1:1 relationship between entity type and class. The class doesn't have to do anything fancy, but even just having a unique class name makes life easier.

* Validation ran into this problem.
* The parameter upcasting issue ran into this problem (#1798214: Upcast request arguments/attributes to full objects)
* For REST validation, to ensure that you're not trying to submit a new Taxonomy Term object to a Node for instance, we're running into this problem.
* Probably others I've not seen yet.

In all of these cases there's probably a workaround. Some already have them. But really, the root problem is that we cannot round-trip entity type and class. The lack of reliable 1:1 mapping there is an architectural problem, not a benefit.

I understand why ECK exists in Drupal 7: Creating a new entity type is way harder than it should be, so trying to avoid it is logical. But that is getting a LOT cleaner in Drupal 8, to the point where we should be focusing efforts on making new entity types easier to define in code, not in the UI.

There is a point of diminishing returns, after which making things UI configurable causes more problems than it solves, or results in creating epicycles of abstraction that do nothing but require more and more platform-specific coding (even if it's done in the browser, it reaches a level of complexity where it is development). I firmly believe that dynamic entity type creation is on the other side of that threshold.

So we can simplify a great deal of code by stating that there must be a 1:1 correspondence between an entity type and a PHP class. Even if it's an empty class that just extends another class, that is a benefit. All we loose in doing so is the ability to dynamically create entity types through the UI... which I don't think is a worthwhile feature anyway. Just because we could do something doesn't mean we should. If you're doing something that requires a custom entity type (rather than just a custom bundle), it's complex enough to crack open a code editor and write a few lines.

I guess that the immediate goal of this issue is to allow for the easy creation of entity types.

In D8, this now seems pretty easy. For example, Drupal\file\Plugin\Core\Entity\File has a few annotations, and a few public properties and that's it. I'm all for making it easier still if we have ideas on how to do that, like #16. But making it UI-configuration-driven seems like a totally different issue. Are there any actual use cases for needing that? If not, then I agree with #17 about it being desirable to keep a 1:1 relationship between entity type and PHP class.

Duplicate

the root problem is that we cannot round-trip entity type and class

I would like to understand the root of the problem, but I am not sure I understand what round-trip means here

…stating that there must be a 1:1 correspondence between an entity type and a PHP class.

I think this is an unreasonable expectation, not only does this shut down the possibility of dynamic (programmatically or UI created) entity types, but also some of the other things that I find interesting about the future of Drupal; like behaviors. Behaviors encourage fewer classes. In the best case you can have a single entity class that can be reused and its functionality can be augmented through configuration. It also creates and extra layer of separation between data and logic encouraging code reusability.

Even if it's an empty class that just extends another class, that is a benefit.

Like I said, I don’t understand the root of the problem at the moment, but It is hard to see how this is useful at all, or why we should use inheritance in this way (why create a child class when the parent is enough; is it just to define a name? that’s what the entity type definition is for, mapping a ‘name’ to a class … any class)

@effulgentsia I agree that creating entity types has become cleaner, but big configurations array are not friendly to anybody, whether they are in annotation or any other form. This issue allows us to wrap that array in a proper class, and that to me is a great benefit in itself, and would make the creation of entity types easier from a development and documentation standpoint.

All we loose in doing so is the ability to dynamically create entity types through the UI... which I don't think is a worthwhile feature anyway

This might be true, but I guess is hard to miss what we don’t have. IMHO it would be great to have the capabilities to define the data architecture of a complex site in a similar way to how I create UML class diagrams. This would make Drupal a useful tool for prototyping, and at the end you will have a basic site out of your prototype!!! As part of that general idea, I think bundles should go away and be replaced by a proper entity type inheritance system, as I believe the only reason we have bundles is because we do not have dynamic entity types and real inheritance :) . All of these possible features are built on the capabilities proposed in this issue. A lot of people might think that none of this things are worthwhile, but I know a percentage of people that will disagree (I wish I knew what that percentage is), and I would appreciate if you guys don’t shoot down my dreams altogether :)

fmizzell: I appreciate the coolness factor of what you're describing. However, Drupal is a CMS. It's not a UML editor. I can already do the sort of data modeling you describe just fine with nodes, before even getting to other entity types. The only reason to use another entity type is if you want alternate functionality. Not if you simply want to categorize data differently. And, frankly, I already do my data modeling in a Google spreadsheet before punching it into Drupal, a process that is way faster than anything I could do in Drupal.

We do not currently have behaviors or other such things for entities, and won't in Drupal 8. Nor are there any plans to rewrite all of the entity system a second time to do full-on inheritance, with all of the UX implications (and 1000 hours of work just on the UI) that would entail. Really, I already think Rules is skirting the line I mentioned above about "dude, just damn well write 5 lines of code, not 5000 lines of abstraction". Doing the same to our data model has all sorts of complications and implications that I really don't think we want.

By round-tripping, I mean that 1:1 mapping. There are plenty of places where we already have or could easily have a class name. However, if we cannot map that directly to an entity type then it means several more layers of abstraction, indirection, and dependency for such a mapping system. That's a lot of work for, really, relatively little benefit that there are no plans to actually use in Drupal 8. (See above about there not being any behaviors.)

I'm not particularly attached to "UI-defined entity types" either (basically for the same reasons Crell explained - if you need a new entity type it means you need custom behavior, and at this point this means code anyway).

I'm a little worried however by the idea of addressing entity class names directly instead of the machine name of the entity type. This sounds like something we avoid doing pretty much all over the place right now ?

Nobody drunk of the awesomeness kool aid so lets get to the crux of the problem: are we 100% sure that we have to have a php class per entity type definiton? and is there any obvious problem that people see with allowing entity type definition to be stored in config? If these two things are affordable in core, then we can continue to develop these ideas. Whether they get into core or not, I don't care, I just want to be able to explore them if I can.

Not having a 1:1 correspondence is a problem for things like Symfony's Serializer, which we are building our serialization functionality on top of.

Serializer's deserialize function takes the target class as a parameter. So for PUTing a node, there would be a route like /node. Because the REST module knows the allowed entity type for the route and can figure out the target class from that, it can call deserialize($incomingData, 'Drupal\node\Plugin\Core\Entity\Node', 'jsonld'). Then, the Serializer knows what kind of thing it is supposed to create, and the REST route can be sure that it is getting back a node.

If the class 'Drupal\node\Plugin\Core\Entity\Node' could correspond to the entity type node OR to the entity type foobar, REST can no longer trust that deserialize() has returned the correct type of entity, which results in a security issue.

As Larry said, there are work arounds for most of these cases. It would be possible to simply use entity_create in deserialize() and validate the returned entity against the expected entity type in the REST route. However, I don't think sprinkling our code with a half dozen different workarounds for this issue is a good idea.

Because the REST module knows the allowed entity type for the route and can figure out the target class from that, it can call deserialize($incomingData, 'Drupal\node\Plugin\Core\Entity\Node', 'jsonld')

OK, but then couldn't it do that:

$class = determine_class_from_entity_info('node');
deserialize($incomingData, $class, 'jsonld');

instead, and respect the "entity_type machine name -> class" indirection / avoid hardcoding the class name ?

Yes, something like that is fine.

My response was about why we need there to be a 1:1 correspondence between the entity type and the entity class. Serializer expects to work with a class name, not with a string that is specific to the Typed Data API.

We pass deserialize the kind of string it expects (the name of an instantiable class), and we know that it will give us back an entity of the correct type. As long as we're passing Serializer what it expects, it doesn't matter whether we use TypedData API strings or class names to refer to the entity type in the surrounding code.

If I understand correctly, then the problem with #25 is that $incomingData does not contain 'entityType' => 'node' anywhere within it. Now, the caller of deserialize() does know about 'node': for example, if rest.module is the caller, then it knows that based on the route, so getting $class via entity_info is fine. But, at some point, a denormalizer invoked by deserialize() needs to instantiate $class, and Entity::__construct() requires a $entity_type as the second argument, but the only information that the denormalizer has access to is $class and $incomingData.

You might think, ok, no problem, have rest.module put 'entityType' => 'node' into $incomingData, but the problem is at this point in the pipeline, $incomingData is still a serialized string of JSON, or JSON-LD, or XML, or potentially any other format, so inserting something into it isn't so easy.

However, when I look at the public interface of SerializerInterface::deserialize(), the second parameter is $type, not $class. I don't see where it's required to be a class name. Can't it continue to be a type name ('node'), and then Drupal's EntityDenormalizer could instantiate by calling entity_create($type, $data)? Or, if we want to avoid the procedural code there, then we can inject $entityManager (which already exists in the DIC) into EntityDenormalizer so that the denormalizer could call $this->entityManager->createInstance($type, $data).

This is only peripheral to #27, but I don't think $entityManager->createInstance($type, $data) currently works for creating entities.
EntityManager is not used to create any instance of whatever right now, it's currently only a discovery mechanism. entity_create() is still the only official way to create an entity object.

Yeah, clearly we need to fix EntityManager to have a functioning factory. I'll open that issue at some point unless someone beats me to it.

However, when I look at the public interface of SerializerInterface::deserialize(), the second parameter is $type, not $class.

This was covered in the other issue. The documentation of Serializer isn't great for this parameter. But it gets passed directly from deserialize into denormalize without any manipulation, and denormalize expects a class.

    /**
     * Denormalizes data back into an object of the given class
     *
     * @param mixed  $data   data to restore
     * @param string $class  the expected class to instantiate
     * @param string $format format the given data was extracted from
     * @return object
     */
    public function denormalize($data, $class, $format = null);

Another issue made more complicated by dynamic entities: #1798214: Upcast request arguments/attributes to full objects

If we don't know at code time what entities we have, registering converters for them in the DIC becomes way harder.

That's a problem registering anything in the DIC that's defined by configuration. This issue would expose that one, but it doesn't cause the problem which is inherent to the fact that most systems don't have a dynamic configuration-driven DIC (which we do, except it's currently only bound by configuration at the level of which modules are enabled, not within modules yet).

Agree with this though:

to the point where we should be focusing efforts on making new entity types easier to define in code, not in the UI.

There's still some boilerplate code you need to write outside of the Foo extends *Entity class, like hook_schema(). That would need to be removed for configuration-driven entity types, and it's something that the entity system should handle on behalf of individual entity types regardless - so we can get a lot closer to this, then if we decide it's a bad idea at least it's easier to define things in code.

Can anyone lay out an actual use case for this? Not how you'd do it, but why you'd want to? I'm not trying to be snide - I just can't really picture what we'd gain.

Quickly responding to #23:

are we 100% sure that we have to have a php class per entity type definiton?

At this point, I'm not sure. I'm not yet convinced by #30, but I'll leave that discussion for the other issue. And I don't yet know enough about the other use cases mentioned in #17.

is there any obvious problem that people see with allowing entity type definition to be stored in config?

With this, I have 3 concerns, though all are potentially addressable:

  • Per #31 and #32, I'd be very concerned about us introducing any definitions into the DIC that depend on any configuration other than the list of enabled modules. We're having a hard enough time working through the issues that just the dependency on the module list is causing, and I think allowing it to depend on any other configuration would make the problems much harder. However, I'm also not yet convinced that we want per-entity-type entries in the DIC, so whether or not this concern is relevant depends on how we settle that question.
  • We currently have ConfigEntity types and other Entity types both treated as the same plugin type. I'd be concerned about hard to manage dependency chains and infinite loop possibilities caused by ConfigEntity definitions depending on the content of other ConfigEntity objects. This can be alleviated by either making ConfigEntity a different plugin type from Entity and only allowing config-based derivatives of the latter, or else by making sure that configuration that drives entity definitions is raw configuration, and not config entities.
  • My final concern is that it just doesn't make any conceptual sense to me. Even if we do manage to implement behaviors that could be runtime attached to entities, I don't understand why it would ever make sense to create configuration-driven entity types to attach them to. I see bundles as serving that purpose. The distinction between entity types as things that have code meaning and bundles as things that have configuration meaning feels clean to me, and muddying those waters feels needlessly complex. But, I'm open to being convinced otherwise or overruled.

All of the additional arguments that have been brought up so far actually present reasons for this functionality in my book, since they are centered around the extensibility and swappability/pluggability of the overall system.

Also, the specific argument about the need for something to be in code is moot — Drupal 8 is able to write code, register, and execute it. (Not invented here.)

However, I still don't think that this should be material for D8 core (see #8), but I know that there are many use-cases for ECK, which do not necessarily need a line of custom code to make sense, so I do think that D8 should support the idea and concept of ECK in contrib.

Essentially, I agree with @catch in #32 and others.

sun: I have no idea how "it makes everything else more complicated and some problems it causes we're not even sure how to solve" turns into "reasons for this functionality". That's doublethink.

clearly we need to fix EntityManager to have a functioning factory. I'll open that issue at some point unless someone beats me to it.

#1867228: Make EntityManager provide an entity factory.

I've opened #1867880: Bring data type plugins closer to their PHP classes for bringing the thinking of #17 to the TypedData API (while not enforcing a 1:1 relationship yet).

Issue tags:-Entity system

#11: 1838676-11.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 1838676-11.patch, failed testing.