BundleClassInheritanceException incorrectly thrown when a bundle class does not exist

I'll try to work on that :)

Here is a patch where I added a conditional for the cases were the bundle class given doesn't exist.

All right, thanks for the orientation. I'll work on that.

Thanks for opening this issue and working on fixing it! Sorry for the misleading exception text. Something else we all missed in #2570593: Allow entities to be subclassed using "bundle classes" 😉 Agreed this is a bug and tagging to be smashed. Happy to review / RTBC when ready.

Thanks again!
-Derek

This new patch adds the class_exists() check before the check for inheritance (on the ContentEntityStorageBase class). I haven't sent a diff here because this new patch applies to a different class.

After talking with @joachim he advised that the best approach would be to create a new exception class to use here instead of the "\Symfony\Component\ErrorHandler\Error\ClassNotFoundError" so I'll work on this.

So here is a patch with the new MissingBundleClassException which is called when the given bundle class doesn't exist.
Still not marking this issue as "needs review" though, because I believe we'll need to add a new test to cover it. So, I'll try to do it.

Yeah, even if the test has to move or be updated, we still can’t commit a new exception here without test coverage of it.

e.g. Should be pretty trivial to extend core/tests/Drupal/KernelTests/Core/Entity/BundleClassTest.php for this.

Okay then, I'll adventure myself on this and try to add the new test. :)

Here is it!
I hope I have done all right (that's the first time I try to create a Drupal test).
:)

Sorry, wrong diff extension... here is the .txt

#18 is a random fail. However, instead of re-queuing #16 (which I agree is RTBC), here's a test-only and full pair so we can watch the new test fail as expected. Full patch is identical to #16.

Thanks!
-Derek

Hah, you'd think after dozens (hundreds?) of times trying to speed up the bot by hacking core/drupalci.yml for a test-only patch, that I could get it right the first time. 🤦‍♂️ Nope! 😆 Let's try that again...

Test only failed as expected. Same random fail for the real patch. Weird. Re-queuing once more.

Not my code, just getting good bot results for commit. Back to RTBC.

+++ b/core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module
@@ -32,6 +32,10 @@ function entity_test_bundle_class_entity_bundle_info_alter(&$bundles) {
+  if (\Drupal::state()->get('entity_test_bundle_class_does_not_exist', FALSE)) {
+    $bundles['entity_test']['bundle_class']['class'] = NonExistentClass::class;
+  }

This fails PHPStan checks on Drupal 10. It would be better if this was not NonExistentClass::class; but something like 'Drupal\Core\NonExistentClass' as using a string rather than ::class has got to be preferrable.

But the patch is for Drupal 9, does it need to be changed?
(Also, the other lines on the file follow this same pattern, should we create a separate issue to fix them?)

Thanks @joachin for the explanation! :)
I'll fix it!

Here is the patch.

Committed and pushed 2b1faaacb1 to 10.0.x and 1d7fd63519 to 9.4.x and cac70bf799 to 9.3.x. Thanks!

diff --git a/core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module b/core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module
index c6c3d2ce5f..47a525634e 100644
--- a/core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module
+++ b/core/modules/system/tests/modules/entity_test_bundle_class/entity_test_bundle_class.module
@@ -34,7 +34,7 @@ function entity_test_bundle_class_entity_bundle_info_alter(&$bundles) {
   }
 
   if (\Drupal::state()->get('entity_test_bundle_class_does_not_exist', FALSE)) {
-    $bundles['entity_test']['bundle_class']['class'] = 'NonExistentClass';
+    $bundles['entity_test']['bundle_class']['class'] = '\Drupal\Core\NonExistentClass';
   }
 }
 

Fixed on commit. I'd rather we were using classes that do not exist from our own namespace and not the global one.

Yay, thanks everyone!