Closed (fixed)
Project:
Drupal core
Version:
9.0.x-dev
Component:
entity system
Priority:
Major
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
12 Nov 2012 at 19:11 UTC
Updated:
19 Oct 2016 at 11:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
fmizzell commentedThanks to mitchell for all the help with documentation
Comment #2
catchI'd really like to get this in, feels like at least a major feature request.
Comment #3
webchickMarking needs review.
Comment #5
sunI 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.
Comment #6
fubhy commentedI 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.
Comment #7
fmizzell commentedI 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:
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.
Comment #8
sunYes, 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.
Comment #9
fmizzell commented@sun, I agree, just wanted to give the motivation to why this directions is important and interesting.. now we can talk low level :)
Comment #10
fagoI 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.
For modelling "sub-type" relationships.
Comment #11
fubhy commentedTime 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.
Comment #12
fmizzell commented@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 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.
Comment #13
fmizzell commentedOne 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.
Comment #14
tim.plunkettI think #11 is the right way to go for now.
Comment #15
fagoYes a proper inheritance system would be nice, see #100925: Add "OO" bundle inheritance features and #1380720: Rename entity "bundle" to "subtype" in the UI text and help.
Comment #16
catchI 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.
Comment #17
Crell commentedThis 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.
Comment #18
effulgentsia commentedIn 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.
Comment #19
fmizzell commentedDuplicate
Comment #20
fmizzell commentedI would like to understand the root of the problem, but I am not sure I understand what round-trip means here
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.
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.
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 :)
Comment #21
Crell commentedfmizzell: 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.)
Comment #22
yched commentedI'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 ?
Comment #23
fmizzell commentedNobody 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.
Comment #24
Anonymous (not verified) commentedNot 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 calldeserialize($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.
Comment #25
yched commentedOK, but then couldn't it do that:
instead, and respect the "entity_type machine name -> class" indirection / avoid hardcoding the class name ?
Comment #26
Anonymous (not verified) commentedYes, 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.
Comment #27
effulgentsia commentedIf 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).Comment #28
yched commentedThis 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.
Comment #29
effulgentsia commentedYeah, 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.
Comment #30
Anonymous (not verified) commentedThis 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.
Comment #31
Crell commentedAnother 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.
Comment #32
catchThat'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:
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.
Comment #33
rbayliss commentedCan 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.
Comment #34
effulgentsia commentedQuickly responding to #23:
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.
With this, I have 3 concerns, though all are potentially addressable:
Comment #35
sunAll 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.
Comment #36
Crell commentedsun: 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.
Comment #37
effulgentsia commented#1867228: Make EntityTypeManager provide an entity factory.
Comment #38
fagoI'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).
Comment #39
kerasai commented#11: 1838676-11.patch queued for re-testing.
Comment #41
jhedstromQuoting #11
Seems reasonable if not doing so would block contrib from pursuing this.
Comment #42
berdirThis is an old issue ;)
EntityManager is using the default plugin manager now, which includes the DerivativeDecorator. That however doesn't help because that enforces : in the plugin/entity type ID, which is not valid for entity type IDs. However, we have a build and alter hook, there's nothing you can't do in there.
Comment #43
joachim commentedSo is core now in a state where contrib modules such as ECK and Data can define entity types dynamically?
Comment #44
berdirTo the best of my knowledge, yes. We won't know until we try :)
Comment #45
mustanggb commentedSeems like the sort of changes required would be too problematic for D8.
Comment #46
philsward commentedSeeing how old this issue is, it might not be the best place to post, however I agree that D9 should entertain the idea of a GUI based entity building system.
I've used Drupal since the last part of D5. Entities were great and all for D7, but I honestly expected more out of D8. We're still locked into the same use case of content types, blocks (which I never use), tags, users, comments and now the new entity type: forms. Core is still painting people into a corner when it should give a base set of tools to work from, then allow the site builder take off from there. @Crell mentioned that small site builders won't ever need anything outside of a content type... How does he know? Drupal doesn't give the tools for small site builders to create content for anything except through content types. I'm a small site builder (not a site developer) and I would love to create custom entities, throw some fields at it and then display it through a view. What I'm not going to do, is install an extra module (ECK) just to add a few pieces of content in a way that makes logical sense for me because blocks and content types don't really meet the criteria for what I want to accomplish. In the end, I usually end up with a content type because it's the only entity available to me. D8 does change that a bit with blocks being entities, but I still don't think I'll use them because there's no easy way to logically group content.
While I do think core entities should be de-coupled so they are less custom and use more common code, I also think it could be just as easy to take the "content type" module, clone it, pull off the URL, call it something else and now you have your "custom entity" with it's own class or 1:1 or whatever. It's done with blocks, tags, forms, users, comments, why not for a custom entity?
Comment #47
catchhttps://www.drupal.org/project/eck has an 8.x release now, so this can be marked fixed afaict.
Adding ECK to core should be proposed in the new ideas queue if anyone's thinking about taking that on. We have a lot of 8.x issues trying to make the boilerplate for defining a new entity less which should happen before that though.