dmaicher / doctrine-test-bundle

Symfony bundle to isolate your app's doctrine database tests and improve the test performance
MIT License
1.08k stars 60 forks source link

Using both the EntityManager and QueryBuilder gives unexpected results #215

Closed mariusjp closed 2 years ago

mariusjp commented 2 years ago

I'm having a very weird issue:

In one of my tests I'm revoking a user related token, fetching all the tokens after the revoke and comparing the result to my expectations. This works fine, but my guess is this works fine because the update (revoke) is done with the querybuilder AND the "findAllActiveTokens" is done with the querybuilder.

Now onto my problem:

Same sort of test, revoke a (different) user related token, this is again done with the querybuilder to update. BUT retrieving the result is done with a "findOneBy" (this directly uses the entityManager in stead of the QueryBuilder). And my result is that the "revoke" has not been executed (but no error has been thrown).

I'm only mentioning the "difference" in the "findOneBy" and QueryBuilder approach because, as you can see, that is the only actual difference between the two scenarios.

A bit of code to make it more clear:

The first case, which works (the Repository):

    // ...
    public function findAllActiveForUser(User $user): OAuthAuthorizationCodeCollection
    {
        $queryBuilder = $this->entityManager->createQueryBuilder();
        $queryBuilder
            ->select('oauth_auth_code')
            ->from(OAuthAuthorizationCode::class, 'oauth_auth_code')
            ->where('oauth_auth_code.user = :user')
            ->andWhere('oauth_auth_code.revokedAt IS NULL')
            ->andWhere('oauth_auth_code.expiresAt > :now')
            ->setParameter('user', $user)
            ->setParameter('now', new \DateTimeImmutable());

        /** @var array<OAuthAuthorizationCode> $authorizationCodes */
        $authorizationCodes = $queryBuilder->getQuery()->getResult();

        return OAuthAuthorizationCodeCollection::create($authorizationCodes);
    }

    public function revoke(OAuthAuthorizationCode $authorizationCode): void
    {
        $queryBuilder = $this->entityManager->createQueryBuilder();
        $query = $queryBuilder
            ->update(OAuthAuthorizationCode::class, 'oauth_auth_code')
            ->set('oauth_auth_code.revokedAt', ':revokedAt')
            ->where('oauth_auth_code.id = :id')
            ->setParameter('revokedAt', new \DateTimeImmutable())
            ->setParameter('id', $authorizationCode->getId())
            ->getQuery();

        $query->execute();
    }

And the test:

    // ...
    public function revoke(): void
    {
        /** @noinspection PhpUnhandledExceptionInspection */
        $activeAuthorizationCode = $this->repository->getOneByExternalId('authorization_code_external_id');

        $this->repository->revoke($activeAuthorizationCode);

        /** @var UserDbalRepository $userRepository */
        $userRepository = self::$kernel->getContainer()
            ->get(UserDbalRepository::class);

        /** @noinspection PhpUnhandledExceptionInspection */
        $user = $userRepository->getOneById(1);

        $allActiveAuthorizationCodes = $this->repository->findAllActiveForUser($user);
        $allAuthorizationCodes = $this->repository->findAll();

        self::assertCount(3, $allAuthorizationCodes);
        self::assertCount(0, $allActiveAuthorizationCodes->all());
    }

The second case, which doesn't work (the Repository):

    // ...
    public function getOneByExternalId(string $externalId): OAuthRefreshToken
    {
        /** @var OAuthRefreshToken|null $refreshToken */
        $refreshToken = $this->findOneBy(['externalId' => $externalId]);

        if (!$refreshToken instanceof OAuthRefreshToken) {
            throw OAuthEntityNotFoundException::fromClassNameAndExternalId(
                OAuthRefreshToken::class,
                $externalId,
            );
        }

        return $refreshToken;
    }

    public function revoke(OAuthRefreshToken $refreshToken): void
    {
        $queryBuilder = $this->entityManager->createQueryBuilder();
        $query = $queryBuilder
            ->update(OAuthRefreshToken::class, 'oauth_refresh_token')
            ->set('oauth_refresh_token.revokedAt', ':revokedAt')
            ->where('oauth_refresh_token.id = :refreshTokenId')
            ->setParameter('revokedAt', new \DateTimeImmutable())
            ->setParameter('refreshTokenId', $refreshToken->getId())
            ->getQuery();

        $query->execute();
    }

And the test:

    // ...
    public function revoke(): void
    {
        $refreshToken = $this->repository->getOneByExternalId(OAuthRefreshToken::EXTERNAL_ID->value);
        self::assertNull($refreshToken->getRevokedAt());

        $this->repository->revoke($refreshToken);

        $refreshToken = $this->repository->getOneByExternalId(OAuthRefreshToken::EXTERNAL_ID->value);
        // This throws an error because "getRevokedAt()" is still NULL
        self::assertInstanceOf(\DateTimeInterface::class, $refreshToken->getRevokedAt());
    }
dmaicher commented 2 years ago

Hm sorry I fail to see how this could be related to this bundle. Does the test pass without using this bundle?

mariusjp commented 2 years ago

Well, after losing a few hours of work yesterday trying to wrap my head around what was happening and starting up the next day (today)...

I did as you asked, ran the tests without your extension in the configuration file. Tests passed, re-added your extension... tests passed.

I have now re-run above statement multiple times and I don't have the error anymore. Sorry to have bothered you, but it really seemed related (as I couldn't think of anything else that influenced my tests)