Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
#2570593: Allow entities to be subclassed using "bundle classes" introduced a BC layer in core/lib/Drupal/Core/Entity/EntityStorageBase.php
for legacy code that directly accesses the now removed entityClass
protected property.
We can remove all this complication once 10.0.x branch is open.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#20 | 3244802-20.patch | 23.84 KB | andypost |
| |||
#20 | interdiff.txt | 1.51 KB | andypost |
Comments
Comment #2
larowlanComment #3
SpokjeComment #4
SpokjeComment #5
longwaveOnly removals, no additions - looks good to me.
Comment #6
dwwThanks for working on this!
Back to NR for at least 1, perhaps 2 as well.
Comment #7
dwwRe 2, see #3251854: [META] Requirements for tagging Drupal 10.0.0-alpha1
I also just asked about this at #d10readiness Slack meeting: https://drupal.slack.com/archives/C014CT1CN1M/p1638817559258800
Comment #8
catchYes IMO we should make 10.0.0-alpha1 as much as possible 9.4 + dependency updates, so that contrib can get a deprecation report for as much as possible without having to update anything yet. Then once it's out start committing deprecation removals.
Going to mark as postponed on the alpha1 issue for now.
Comment #9
SpokjeThanks @dww and @catch, I got carried away in my sheer enthusiasm to get rid of deprecations.
Will stop working on those until 10.0.0-alpha1 is out.
Comment #10
fgmReturning
$this->entityType->getClass()
may cause a problem because it can return aNULL
but the signature does not allow it. This causes issues like #3259941: getEntityClass() must be of the type string, null returned.--- copied from closed issue #3246150: Bundle class changes mean the entity class is now determined by the active entity-type definition -----
This change introduces appears to be causing a compatibility issue described at #3259941: getEntityClass() must be of the type string, null returned where the
EntityStorageBase::baseEntityClass
is not set, and returns a badly typed result:That function returns a
NULL
when thebaseEntityClass
is not set and$this->entityType->getClass()
returnsNULL
too (the signature of that function allows an empty return), but the function signature does not allow forNULL
results.Rewriting it like this fixes the problem, but it may well be a symptom of a deeper problem
Comment #11
SpokjeUnpostponing
Comment #12
andypostComment #13
andypostRemoved remains from the Entity namespace
Comment #15
andypostremove legacy test
Comment #16
andypostprobably could use better wording
Comment #17
daffie CreditAttribution: daffie commentedI shall leave it up to the committer to decide if the query exception message should be improved and if so to what exactly. For me it is good enough.
All code changes look good to me.
For me it is RTBC.
Comment #18
catchOpened some phpstan-drupal issues for the deprecation removals.
https://github.com/mglaman/phpstan-drupal/issues/336
https://github.com/mglaman/phpstan-drupal/issues/335
https://github.com/mglaman/phpstan-drupal/issues/334
On the exception message:
Maybe this?
Entity queries must explicitly set whether the query should be access checked or not. See Drupal\Core\Entity\Query\QueryInterface::accessCheck()
Comment #19
catchWidening the title to match the patch a bit better.
Comment #20
andypostFixed the exception message
Comment #21
catchLooks better in the patch :) Ready to go once the bot is green.
Comment #23
catchCommitted d47cd98 and pushed to 10.0.x. Thanks!