Fix some more doctrine deprecations

composer · Oct 24, 2023 · b8353f9 · b8353f9
1 parent cc1639d
commit b8353f9
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 37 deletions.
diff --git a/src/Controller/PackageController.php b/src/Controller/PackageController.php
@@ -574,7 +574,14 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn
 
             // load the default branch version as it is used to display the latest available source.* and homepage info
             $version = reset($versions);
-            $this->getEM()->refresh($version);
+            foreach ($versions as $v) {
+                if ($v->isDefaultBranch()) {
+                    $version = $v;
+                    break;
+                }
+            }
+            $version = $versionRepo->find($version->getId());
+            Assert::notNull($version);
 
             $expandedVersion = $version;
             foreach ($versions as $candidate) {
@@ -586,9 +593,12 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn
 
             // load the expanded version fully to be able to display all info including tags
             if ($expandedVersion->getId() !== $version->getId()) {
-                $this->getEM()->refresh($expandedVersion);
+                $expandedVersion = $versionRepo->find($expandedVersion->getId());
+                Assert::notNull($expandedVersion);
+            } else {
+                // ensure we get the reloaded $version with full data if it was overwritten above by $candidate
+                $expandedVersion = $version;
             }
-            $expandedVersion = $versionRepo->getFullVersion($expandedVersion->getId());
         }
 
         $data = [

diff --git a/src/Entity/PackageRepository.php b/src/Entity/PackageRepository.php
@@ -13,6 +13,7 @@
 namespace App\Entity;
 
 use DateTimeImmutable;
+use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\QueryBuilder;
 use Doctrine\ORM\Query;
 use Doctrine\DBAL\Cache\QueryCacheProfile;
@@ -307,33 +308,43 @@ public function iterateStaleDownloadCountPackageIds(): iterable
 
     public function getPartialPackageByNameWithVersions(string $name): Package
     {
-        // first fetch a partial package including joined versions/maintainers, that way
-        // the join is cheap and heavy data (description, readme) is not duplicated for each joined row
-        //
-        // fetching everything partial here to avoid fetching tons of data,
-        // this helps for packages like https://packagist.org/packages/ccxt/ccxt
-        // with huge amounts of versions
+        // first fetch the package alone to avoid joins with heavy data (description, readme) that would be duplicated for each joined row
         $qb = $this->getEntityManager()->createQueryBuilder();
-        $qb->select('partial p.{id}', 'partial v.{id, version, normalizedVersion, development, releasedAt, extra, isDefaultBranch}')
+        $qb->select('p')
             ->from('App\Entity\Package', 'p')
-            ->leftJoin('p.versions', 'v')
-            ->orderBy('v.development', 'DESC')
-            ->addOrderBy('v.releasedAt', 'DESC')
             ->where('p.name = ?0')
             ->setParameters([$name]);
-
         $pkg = $qb->getQuery()->getSingleResult();
 
-        if ($pkg instanceof Package) {
-            // then refresh the package to complete its data and inject the previously
-            // fetched partial versions to get a complete package
-            $versions = $pkg->getVersions();
-            $this->getEntityManager()->refresh($pkg);
-
-            $prop = new \ReflectionProperty($pkg, 'versions');
-            $prop->setAccessible(true);
-            $prop->setValue($pkg, $versions);
+        // then fetch partial version data here to avoid fetching tons of data,
+        // this helps for packages like https://packagist.org/packages/ccxt/ccxt
+        // with huge amounts of versions
+        $qb = $this->getEntityManager()->createQueryBuilder();
+        $qb->select('v.id, v.version, v.normalizedVersion, v.development, v.releasedAt, v.extra, v.isDefaultBranch')
+            ->from('App\Entity\Version', 'v')
+            ->orderBy('v.development', 'DESC')
+            ->addOrderBy('v.releasedAt', 'DESC')
+            ->where('v.package = ?0')
+            ->setParameters([$pkg]);
+
+        $versions = [];
+        $reflId = new \ReflectionProperty(Version::class, 'id');
+        foreach ($qb->getQuery()->getArrayResult() as $row) {
+            $versions[] = $v = new Version;
+            $reflId->setValue($v, $row['id']);
+            $v->setName($pkg->getName());
+            $v->setPackage($pkg);
+            $v->setVersion($row['version']);
+            $v->setNormalizedVersion($row['normalizedVersion']);
+            $v->setDevelopment($row['development']);
+            $v->setReleasedAt($row['releasedAt']);
+            $v->setExtra($row['extra']);
+            $v->setIsDefaultBranch($row['isDefaultBranch']);
         }
+        $versions = new ArrayCollection($versions);
+
+        $prop = new \ReflectionProperty($pkg, 'versions');
+        $prop->setValue($pkg, $versions);
 
         return $pkg;
     }

diff --git a/src/Service/GitHubUserMigrationWorker.php b/src/Service/GitHubUserMigrationWorker.php
@@ -174,7 +174,8 @@ static function ($hook) {
             if (count($hooks) && !Preg::isMatch('{^https://api\.github\.com/repos/'.$repoKey.'/hooks/}', $hooks[0]['url'])) {
                 if (Preg::isMatch('{https://api\.github\.com/repos/([^/]+/[^/]+)/hooks}', $hooks[0]['url'], $match)) {
                     $package->setRepository('https://github.com/'.$match[1]);
-                    $this->getEM()->flush($package);
+                    $this->getEM()->persist($package);
+                    $this->getEM()->flush();
                 }
             }
         } catch (HttpExceptionInterface $e) {

diff --git a/src/Service/QueueWorker.php b/src/Service/QueueWorker.php
@@ -161,14 +161,20 @@ private function process(string $jobId, SignalHandler $signal): bool
             $this->doctrine->resetManager();
         }
 
-        // refetch objects in case the EM was reset during the job run
+        // reset EM for safety to avoid flushing anything not flushed during the job, and refetch objects
         $em = $this->getEM();
+        $em->clear();
         $repo = $em->getRepository(Job::class);
+        $job = $repo->find($jobId);
+        if (null === $job) {
+            throw new \LogicException('At this point a job should always be found');
+        }
 
         if ($result['status'] === Job::STATUS_RESCHEDULE) {
             Assert::keyExists($result, 'after', message: '$result must have an "after" key when returning a reschedule status.');
             $job->reschedule($result['after']);
-            $em->flush($job);
+            $em->persist($job);
+            $em->flush();
 
             $this->logger->reset();
             $this->logger->popProcessor();
@@ -185,15 +191,12 @@ private function process(string $jobId, SignalHandler $signal): bool
             $result['exceptionClass'] = get_class($result['exception']);
         }
 
-        $job = $repo->find($jobId);
-        if (null === $job) {
-            throw new \LogicException('At this point a job should always be found');
-        }
         $job->complete($result);
+        $em->persist($job);
 
         $this->redis->setex('job-'.$job->getId(), 600, json_encode($result));
 
-        $em->flush($job);
+        $em->flush();
         $em->clear();
 
         if ($result['status'] === Job::STATUS_FAILED) {

diff --git a/src/Service/Scheduler.php b/src/Service/Scheduler.php
@@ -54,10 +54,10 @@ public function scheduleUpdate($packageOrId, bool $updateEqualRefs = false, bool
                 return $pendingJob;
             }
 
-            // pending job will somehow execute after the one we are scheduling so we mark it complete and schedule a new job to run immediately
+            // pending job would execute after the one we are scheduling so we mark it complete and schedule a new job to run immediately
             $pendingJob->start();
             $pendingJob->complete(['status' => Job::STATUS_COMPLETED, 'message' => 'Another job is attempting to schedule immediately for this package, aborting scheduled-for-later update']);
-            $this->getEM()->flush($pendingJob);
+            $this->getEM()->flush();
         }
 
         return $this->createJob('package:updates', ['id' => $packageOrId, 'update_equal_refs' => $updateEqualRefs, 'delete_before' => $deleteBefore, 'force_dump' => $forceDump], $packageOrId, $executeAfter);

diff --git a/src/Service/UpdaterWorker.php b/src/Service/UpdaterWorker.php
@@ -135,7 +135,8 @@ public function process(Job $job, SignalHandler $signal): array
                     // invalid/outdated token, wipe it so we don't try it again
                     if (!$rate && isset($http_response_header[0]) && (strpos($http_response_header[0], '403') || strpos($http_response_header[0], '401'))) {
                         $maintainer->setGithubToken(null);
-                        $em->flush($maintainer);
+                        $em->persist($maintainer);
+                        $em->flush();
                         continue;
                     }
                 }
@@ -188,7 +189,7 @@ public function process(Job $job, SignalHandler $signal): array
             if (!$emptyRefCache) {
                 $emptyRefCache = new EmptyReferenceCache($package);
                 $em->persist($emptyRefCache);
-                $em->flush($emptyRefCache);
+                $em->flush();
             }
 
             if ($useVersionCache) {
@@ -217,7 +218,8 @@ public function process(Job $job, SignalHandler $signal): array
 
             $emptyRefCache = $em->merge($emptyRefCache);
             $emptyRefCache->setEmptyReferences($repository->getEmptyReferences());
-            $em->flush($emptyRefCache);
+            $em->persist($emptyRefCache);
+            $em->flush();
 
             // github update downgraded to a git clone, this should not happen, so check through API whether the package still exists
             $driver = $repository->getDriver();
@@ -315,7 +317,8 @@ public function process(Job $job, SignalHandler $signal): array
             // detected a 404 so mark the package as gone and prevent updates for 1y
             if ($found404) {
                 $package->setCrawledAt($found404 === true ? new \DateTime('+1 year') : $found404);
-                $this->getEM()->flush($package);
+                $this->getEM()->persist($package);
+                $this->getEM()->flush();
 
                 return [
                     'status' => Job::STATUS_PACKAGE_GONE,