Adding option to allow repositories to be fetched from the service container #727

weaverryan · 2017-10-29T01:21:53Z

Hi guys!

Sorry again for the noise :). The goal is to allow repositories to be added to the container easily (i.e. in an autowired way). There are 2 possible approaches:

~~1) In symfony/symfony#24693, we add a new trait to make creating services that are similar to repositories easily.~~

In this PR, we allow for the $em->getRepository() to return a service, instead of it being created by Doctrine.

I'm currently leaning towards the approach in this PR, as it keeps only one idea of a "repository" and migrating to the new system would be super easy. This idea was inspired by @beberlei in #725. ~~I have a few pending questions:~~

Answered: #727 (comment)
~~A) Can an entity only be managed by exactly one EntityManager (as you seem to suggest)? This PR (which uses your code) suggests this.~~

~~B) Are there any issues with Registry::getManagerForClass()? It loops over all of the EM's to find the correct one. Is there a possible issue with instantiating (thanks to this loop) other EM's?~~

Example

The feature is automatically enabled. But existing repositories will still be created the "default" way.

The users will configure their repositoryClass like normal. But now, repositoryClass is allowed to be a service id (which usually == the class name these days, but that's not important).

/**
 * @ORM\Entity(repositoryClass="App\Repository\CoolStuffRepository")
 */
class CoolStuff
{

}

Your repository class will (with default config) auto-registered as a service. So, without touching any config, you create the class and it just works:

class CoolStuffRepository extends ServiceEntityRepository /* this class extends EntityRepository, implements ServiceEntityRepositoryInterface */
{
    public function __construct(RegistryInterface $registry)
    {
        parent::__construct($registry, CoolStuff::class);
    }

    // your custom methods
}

That's it! The new ContainerRepositoryFactory is capable of fetching repositories from the container or creating them like normal. How does it know which to do?

A) If your class still extends EntityRepository, then it's created like normal. This is a great BC/upgrade path.
B) If your class implements a new ServiceEntityRepositoryInterface, this is a signal to fetch from the container.

The entire system is functionally-tested, including strict errors if you mess up (e.g. implement the new interface, but forget to register your service).

Your new services are required to extend EntityRepository, because outside systems could be type-hinting against this. If you don't like that, you could ignore this feature and create your own service classes outside of the repository system.

Cheers!

alcaeus

Of all the approaches I’ve seen, I like this one the best. Only thing that feels a bit kludgy is the constructor for the repository, see below.

alcaeus · 2017-10-29T04:39:56Z

Resources/config/dbal.xml

@@ -64,6 +64,7 @@
            <argument>%doctrine.default_entity_manager%</argument>
        </service>
        <service id="Doctrine\Common\Persistence\ManagerRegistry" alias="doctrine" public="false" />
+        <service id="Symfony\Bridge\Doctrine\RegistryInterface" alias="doctrine" public="false" />


The alias looks wrong here.

Yea... this is a bit unrelated. The issue is that the existing alias is a bit difficult, because there is an abstract class in the Bridge by the same name. It’s one of those situations where the user needs to guess the correct use statement, and only one will work. I’d like to add this ORM-specific alias and tell people to target it (but also mention the other interface, for those who care).

alcaeus · 2017-10-29T04:43:43Z

DependencyInjection/DoctrineExtension.php

@@ -375,6 +378,9 @@ protected function ormLoad(array $config, ContainerBuilder $container)
                $def->addTag('doctrine.event_subscriber');
            }
        }
+
+        $container->registerForAutoconfiguration(EntityRepository::class)


Shouldn’t the tag only be applied to the new ServiceEntityRepository?

doing so wuld allow removing the config option altogether, which would be nice, isn't it?

Interesting... to remove the option entirely, it would mean that, when an entity has a custom repository, we would sometimes instantiate that class (like current behavior) and sometimes fetch it from the container. Effectively, if their repo extends the new ServiceEntityRepository, then we fetch from container (or error if not exists). Otherwise, we instantiate.

I like this because it makes migrating to the new system even easier and removes this option. But is it too magic? The choice of your base class will determine how your repo is fetched/created.

That's what autoconfig is already, adding behavior based on the base classes :)

alcaeus · 2017-10-29T04:51:01Z

...cyInjection/Fixtures/Bundles/RepositoryServiceBundle/Repository/TestCustomRepoRepository.php

+
+class TestCustomRepoRepository extends ServiceEntityRepository
+{
+    public function __construct(RegistryInterface $registry)


Having to override the constructor is going to feel weird. People are likely to forget to override it or to change the signature. One option to avoid this is to loop through the metadata array from the entity manager and look for an entity that has our class as custom repository. It would be nice if we could even move that to the service itself to avoid looking up that information on every instantiation, but i believe changing this will improve DX.

The need to override the constructor is indeed the weirdest part :/ (but you only need to do it if you need to add more constructor args, otherwise the default constructor is fine). So, I’m interested in your idea, but I don’t understand yet. Where would we loop over the class metadata? And how would that help? Doesn’t the user need to (somehow) do some work/chores in their service to set the protected properties that EntityRepository needs internally?

I thought that the constructor override was necessary to get the new repositories to play nice with service auto-registration. Since there will most likely be a repository for a single entity (unlike the default EntityRepository), it would make sense trying to deduct the entity class name from the entity manager in ServiceEntityRepository if it isn't given:

If entity class name is missing, fetch a list of all metadata from the document managers (yes, plural unfortunately).

Loop over metadata, looking for an entity with a customRepositoryClass of get_class($this).

If a single entity is found, use it. If more/less than one entity is foun 8000 d, throw an exception.

container

beberlei · 2017-10-31T12:28:00Z

@weaverryan to answer your questions:

a.) Theoretically, an entity can be in multiple managers, but I haven't seen someone using it this way and would consider it a 1% special case where its ok that repository as service + autowiring doesn't work. Other code in the DoctrineBundle will not work (Forms, Validators) if you are using one entity with multiple managers.

b.) You are right, this might cause additional overhead with multiple EMs, and you need to probably look out for issues with automatically connecting to databases, although afaik this problem has been fixed recently? Again, this method is used in form and validation component already, so any potential problems should have surfaced there already.

In general I think this approach works for the 80-90% use case and as a workaround users can just register the service manually to provide the correct entity manager.

beberlei

Am I correct in assuming that this requires users to create a service for each repository themselves?

Is this desired or would an even better result be that repository services are automagically registered?

I don't have a good solution on how to implement this, but would be willing to think about it if you want this as a feature.

beberlei · 2017-10-31T12:31:36Z

Repository/ContainerRepositoryFactory.php

+
+        if (null !== $repositoryServiceId) {
+            if (!$this->container->has($metadata->customRepositoryClassName)) {
+                throw new \RuntimeException(sprintf('Could not find the repository service for the "%s" entity. Make sure the "%s" service exists and is tagged with "%s".', $entityName, $repositoryServiceId, ServiceRepositoryCompilerPass::REPOSITORY_SERVICE_TAG));


This is a bit harsh, wouldn't it be better to default to the original use by checking $customRepositoryClassName does not implement the ServiceEntityRepository and instantiate it with the regular approach?

The way to do this would be remove the if (null !== $repositoryServiceId) check to get back into the default code starting in line 64+.

weaverryan · 2017-10-31T12:45:22Z

weaverryan commented

Oct 31, 2017

Thank you for the answers!

Am I correct in assuming that this requires users to create a service for each repository themselves?
Is this desired or would an even better result be that repository services are automagically registered?

Yes, you're correct! But the registration of the services can already be automated with the "service auto-registration" feature in 3.3. And for repositories that don't have a custom class, auto-registering a service with an EntityRepository class doesn't help the user much anyways, as you wouldn't be able to autowire those services as arguments to other services. So, I think it's not a problem :).

I'm working on a few changes related to your comments and others right now...

beberlei · 2017-10-31T13:13:21Z

@weaverryan can you point me to details of the "service auto-registration" feature of 3.3? I am still living in my small symfony 2 world ;-)

weaverryan · 2017-10-31T14:35:34Z

@beberlei LOL, of course :).

New Symfony projects ship with this optional config: https://github.com/symfony/symfony-standard/blob/3.3/app/config/services.yml#L19-L23

This automatically registers every service in the container. The service id === the class name. Those services are private (important because it means that unused services are removed before dumping the compiled container... so no extra overhead if some classes in src/ are not actually services). Thanks to _defaults (https://github.com/symfony/symfony-standard/blob/3.3/app/config/services.yml#L8-L15), these services are also autowired (+ private and "autoconfigured" - fancy word that means we will try to add tags automatically based on the interface of services).

The Repository directory is excluded from the auto-registration (https://github.com/symfony/symfony-standard/blob/3.3/app/config/services.yml#L23). That's actually not needed, just a minor performance boost in dev. If this PR is merged, users would uncomment that and their repositories would instantly become services (well, autowiring will fail until they change their base class to the new, autowiring friendly ServiceEntityRepository from this PR... and stolen from you).

So, the user won't care much if their default "EntityRepository" repositories are in the container or not... using $em->getRepository() is even easier than manually fetching/wiring up some service id.

But as soon as they do have a custom repo class, they'll want that to be auto-registered so that it is available for autowiring. That's actually pretty simple - only the ServiceEntityRepository is necessary for that. Most of this PR is really centered around making $em->getRepository() also smart enough to now get things from the container so that there's consistency.

weaverryan · 2017-10-31T15:22:21Z

Update! The config option is gone. The feature is now always on, but you need to activate it by making your repository implement a new interface. So, all existing repositories will work exactly like before.

The new functionality is still Symfony 3.3+ only, and clear exceptions are thrown if you're on a lower version. Actually, most of the ugliness in this PR is keeping 3.2 and lower compat. If we bumped the next version of this bundle to 3.3, we could remove all of that.

There are no current blockers. This is ready again for review.

fabpot · 2017-10-31T15:24:13Z

DependencyInjection/Compiler/ServiceRepositoryCompilerPass.php

+        // Symfony 3.2 and lower sanity check
+        if (!class_exists(ServiceLocatorTagPass::class)) {
+            if (!empty($repoServiceIds)) {
+                throw new RuntimeException(sprintf('The "%s" tag can only be used with Symfony 3.3 or higher. Remove the tag from the following services (%s) or upgrade to Symfony 3.3.', self::REPOSITORY_SERVICE_TAG, implode(', ', $repoServiceIds)));


I would say "upgrade to Symfony 3.3 or higher".

alcaeus · 2017-11-02T07:40:19Z

DependencyInjection/DoctrineExtension.php

+         */
+        if (class_exists(ServiceLocator::class)) {
+            $container->getDefinition('doctrine.orm.container_repository_factory')
+                ->replaceArgument(0, (new Definition(ServiceLocator::class))->setArgument(0, array()));


Nitpick: use short array syntax.

This adds a new "marker" interface that is used to determine whether or not the user determined for their repository to be fetched as a service or created manually by the entity manager. A lot of attention is given to throwing clear errors.

weaverryan · 2017-11-02T18:49:26Z

Nit-pick handled :)

fabpot · 2017-11-05T19:31:56Z

FYI, I've bumped the version of the bundle to 1.8 before merging this PR, which will be part of 1.8.0 stable.

Taluu · 2017-11-07T10:48:08Z

Repository/ContainerRepositoryFactory.php

+    public function getRepository(EntityManagerInterface $entityManager, $entityName)
+    {
+        $metadata = $entityManager->getClassMetadata($entityName);
+        $repositoryServiceId = $metadata->customRepositoryClassName;


unless I read something wrong, this variable doesn't seem to be used anywhere within the body of this method

https://github.com/doctrine/DoctrineBundle/pull/727/files/4efcb135344e55403a0fd193c23a8c7249cfd7d0#diff-f36aa436e9aae3ea54bbaca0caf377e3R65 ;)

10 years later 😆I agree with @Taluu , the variable doesn't seem to be used, as it used only used for the message of the RuntimeException that is thrown a few lines below.

But the $customRepositoryName class contains exactly the same value, and could be used instead. Maybe @weaverryan can tell more about it :)

weaverryan · 2017-11-11T15:19:42Z

Hmm. Did you change your entity’s base class to the new ServiceEntityRepository? That should fix autowiring :)

yceruto · 2017-11-12T01:57:54Z

yep, I did, that's why the autowiring asks me by the $entityClass argument :/

ogizanagi · 2017-11-12T09:19:08Z

@yceruto : Overwritting the ServiceEntityRepository::__construct to pass the $entityClass argument yourself in own PostRepository is a required step. I don't think we can/want to skip it, as that'll mean accessing the doctrine metadata during compile time to get the entity class associated to the repository service and also because the service constructor does belong to you, so you're free to not expose the $entityClass argument the same way or at all.

yceruto · 2017-11-13T03:41:41Z

@ogizanagi I thought extending from ServiceEntityRepository was enough (https://gist.github.com/weaverryan/eec33e4a6d3f98efd3b56939bc08697d), that means I need to do this also for each repository, right?

public function __construct(RegistryInterface $registry)
{
    parent::__construct($registry, CoolStuff::class);
}

…anywhere (yceruto) This PR was merged into the master branch. Discussion ---------- Make Doctrine repository as service and autowire/use it anywhere Living in the edge of the [new awesome features like this](doctrine/DoctrineBundle#727), I'm proposing before doctrine-bundle 1.8 release, show/test this great feature that simplify our lives and to see how it works! and because soon, best practices should adopt this approach by default. (In fact [maker-bundle will do it](https://github.com/symfony/maker-bundle/blob/master/src/Resources/skeleton/doctrine/Repository.php.txt) <3) What do you think? Commits ------- ceac1df Make repository as service and autowire/use it anywhere

ostrolucky · 2017-11-25T20:54:39Z

Yes. Not a fan myself of this constructor overriding requirement. Not sure how is this

+use App\Entity\Company;
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\Common\Persistence\ManagerRegistry;
 
-class CompanyRepository extends Repository
+class CompanyRepository extends ServiceEntityRepository
 {
+    public function __construct(ManagerRegistry $registry, $entityClass)
+    {
+        parent::__construct($registry, Company::class);
+    }
+
 ////////////////
+use App\Entity\Project\Project;
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\Common\Persistence\ManagerRegistry;
 
-class ProjectRepository extends Repository
+class ProjectRepository extends ServiceEntityRepository
 {
+    public function __construct(ManagerRegistry $registry, $entityClass)
+    {
+        parent::__construct($registry, Project::class);
+    }
+
 ////////////////
+use App\Entity\User\UserAdmin;
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\Common\Persistence\ManagerRegistry;
 
-class UserAdminRepository extends Repository
+class UserAdminRepository extends ServiceEntityRepository
 {
+    public function __construct(ManagerRegistry $registry, $entityClass)
+    {
+        parent::__construct($registry, UserAdmin::class);
+    }
+
 ////////////////
+use App\Entity\User\UserClient;
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\Common\Persistence\ManagerRegistry;
 
-class UserClientRepository extends Repository
+class UserClientRepository extends ServiceEntityRepository
 {
+    public function __construct(ManagerRegistry $registry, $entityClass)
+    {
+        parent::__construct($registry, UserClient::class);
+    }
+
 ////////////////
+use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
+use Doctrine\Common\Persistence\ManagerRegistry;
 
-class UserPermissionRepository extends Repository
+class UserPermissionRepository extends ServiceEntityRepository
 {
+    public function __construct(ManagerRegistry $registry, $entityClass)
+    {
+        parent::__construct($registry, UserPermission::class);
+    }
+

Better than this:

services:
    repository_factory:
        public: false
        abstract: true
        factory: ['@doctrine', getRepository]

    App\Repository\CompanyRepository:
        parent: repository_factory
        arguments: [App\Entity\Company]

    App\Repository\UserAdminRepository:
        parent: repository_factory
        arguments: [App\Entity\User\UserAdmin]

    App\Repository\UserClientRepository:
        parent: repository_factory
        arguments: [App\Entity\User\UserClient]

    App\Repository\UserPermissionRepository:
        parent: repository_factory
        arguments: [App\Entity\Project\UserPermission]

    App\Repository\ProjectRepository:
        parent: repository_factory
        arguments: [App\Entity\Project\Project]

Can I have a go with different approach, where I get rid of this overriding and access metadata during compilation? I will do the benchmark as part of that.

Koc · 2017-12-13T15:11:26Z

@ostrolucky accessing metadata during compilation is not good idea.

Koc · 2017-12-13T15:23:34Z

@weaverryan why not create ServiceEntityRepository abstract with method getEntityClass? We could do not import ManagerRegistry in this case and do not override and call parent constructor.

alcaeus · 2017-12-15T13:52:28Z

accessing metadata during compilation is not good idea.

Please elaborate as to why it's a bad idea.

why not create ServiceEntityRepository abstract with method getEntityClass

IIRC, @weaverryan didn't want users to implement additional methods - it should "just work". On top of that, entities and repositories are tied together using the mapping (namely the repositoryClass option in the Entity mapping). There shouldn't be the need for the repository class to be tied to an entity in a second place. This is also a great antipattern, as from a Doctrine perspective an entity has one repository class, but a repository class may be used for multiple entities.

Koc · 2017-12-15T14:42:37Z

@stof explained why symfony/symfony#18673 (comment) and some time ago was some attempts to deprecate this behavior symfony/symfony#18728

yceruto · 2017-12-15T14:43:59Z

@weaverryan didn't want users to implement additional methods - it should "just work".

Users need override the __constructor method though to make it work.

There shouldn't be the need for the repository class to be tied to an entity in a second place.

It mean we've already a antipattern now parent::__construct($registry, CoolStuff::class);? We can reuse this repository for another entity?

alcaeus · 2017-12-15T14:46:56Z

@yceruto

It mean we've already a antipattern now parent::__construct($registry, CoolStuff::class);? We can reuse this repository for another entity?

Correct - I've also mentioned that this approach feels wrong in one of the pull requests involved with this feature. I still think that this shouldn't have to be handled by the user.

alcaeus · 2017-12-15T14:51:19Z

@Koc the issues you linked deal with fetching a service from an unbuilt container builder. Since this bundle is the place where the services in questions are generated, there are ways to work around this if you wanted to.

bobvandevijver · 2018-03-12T19:09:05Z

I wonder why this is merged without any documentation, as it is actually I feature which is pretty awesome!

xabbuh · 2018-03-13T09:27:57Z

@bobvandevijver see http://symfony.com/doc/current/doctrine.html#querying-for-objects-the-repository

…anywhere (yceruto) This PR was merged into the master branch. Discussion ---------- Make Doctrine repository as service and autowire/use it anywhere Living in the edge of the [new awesome features like this](doctrine/DoctrineBundle#727), I'm proposing before doctrine-bundle 1.8 release, show/test this great feature that simplify our lives and to see how it works! and because soon, best practices should adopt this approach by default. (In fact [maker-bundle will do it](https://github.com/symfony/maker-bundle/blob/master/src/Resources/skeleton/doctrine/Repository.php.txt) <3) What do you think? Commits ------- ceac1df Make repository as service and autowire/use it anywhere

See doctrine/DoctrineBundle#727 But also, this directory was always safe to auto-register. If they cannot be autowired, unless you try to *use* them as services, they will be discarded without showing the autowiring failure.

…anywhere (yceruto) This PR was merged into the master branch. Discussion ---------- Make Doctrine repository as service and autowire/use it anywhere Living in the edge of the [new awesome features like this](doctrine/DoctrineBundle#727), I'm proposing before doctrine-bundle 1.8 release, show/test this great feature that simplify our lives and to see how it works! and because soon, best practices should adopt this approach by default. (In fact [maker-bundle will do it](https://github.com/symfony/maker-bundle/blob/master/src/Resources/skeleton/doctrine/Repository.php.txt) <3) What do you think? Commits ------- ceac1df Make repository as service and autowire/use it anywhere

See doctrine/DoctrineBundle#727 But also, this directory was always safe to auto-register. If they cannot be autowired, unless you try to *use* them as services, they will be discarded without showing the autowiring failure.

weaverryan mentioned this pull request Oct 29, 2017

New Trait to make service repositories very easy #725

Closed

weaverryan force-pushed the service_repositories_try2 branch 2 times, most recently from f4632fb to 09fe708 Compare October 29, 2017 02:55

alcaeus reviewed Oct 29, 2017

View reviewed changes

weaverryan force-pushed the service_repositories_try2 branch from 09fe708 to ca926d5 Compare October 29, 2017 16:56

Adding a new option to allow repositories to be fetched from the service

3af691b

container

weaverryan force-pushed the service_repositories_try2 branch from ca926d5 to 3af691b Compare October 30, 2017 18:20

beberlei reviewed Oct 31, 2017

View reviewed changes

weaverryan force-pushed the service_repositories_try2 branch 4 times, most recently from 105454d to f7e14a5 Compare October 31, 2017 15:21

fabpot reviewed Oct 31, 2017

View reviewed changes

weaverryan force-pushed the service_repositories_try2 branch from f7e14a5 to bec9174 Compare October 31, 2017 16:11

alcaeus reviewed Nov 2, 2017

View reviewed changes

weaverryan force-pushed the service_repositories_try2 branch from bec9174 to 4efcb13 Compare November 2, 2017 18:49

weaverryan mentioned this pull request Nov 4, 2017

Reverts the bump from PHP 5 to PHP 7.1: re-adds php 5.5 support #726

Merged

fabpot mentioned this pull request Nov 4, 2017

Adding a new RepositoryTrait to make service repos easy symfony/symfony#24693

Closed

fabpot merged commit b8eb928 into doctrine:master Nov 5, 2017

weaverryan deleted the service_repositories_try2 branch November 6, 2017 15:24

weaverryan mentioned this pull request Nov 6, 2017

Remove "Repository" directory from being ignored in service auto-registration symfony/recipes#238

Merged

Taluu reviewed Nov 7, 2017

View reviewed changes

yceruto mentioned this pull request Nov 20, 2017

Make Doctrine repository as service and autowire/use it anywhere symfony/demo#701

Merged

alcaeus mentioned this pull request Nov 27, 2017

Add aliases for autowiring doctrine/DoctrineMongoDBBundle#441

Merged

xabbuh mentioned this pull request Jan 9, 2018

Validating form with UniqueEntityValidator throws exception v => 4.0 symfony/symfony#25730

Closed

michaelperrin mentioned this pull request Feb 2, 2018

[WIP] Autowiring for repository classes doctrine/DoctrineMongoDBBundle#449

Closed

bobvandevijver mentioned this pull request Mar 12, 2018

Doctrine getRepository() PhanUndeclaredMethod error phan/phan#1248

Closed

stof mentioned this pull request Apr 27, 2018

[DI][FrameworkBundle] (Idea) PHP-DSL base class for simple shortcuts for DI symfony/symfony#27073

Closed

alcaeus mentioned this pull request Sep 17, 2018

Add support for service repositories doctrine/DoctrineMongoDBBundle#473

Merged

This was referenced Dec 27, 2019

Weird behaviour due to EM resets after update to 2.0.6 #1114

Closed

Entity not managed since version 1.12.6 when using refresh #1112

Closed

Uh oh!

Adding option to allow repositories to be fetched from the service container #727

Adding option to allow repositories to be fetched from the service container #727

Uh oh!

Conversation

Uh oh!

Example

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!