Repo JSON Performance Improvements

src/Controller/RepoController.php

@@ -34,10 +34,14 @@ public function __construct(PackageQuery $packageQuery, PackageManager $packageM
      * @Route("/packages.json", host="{organization}.repo.{domain}", name="repo_packages", methods={"GET"}, defaults={"domain":"%domain%"}, requirements={"domain"="%domain%"})
      * @Cache(public=false)
      */
-    public function packages(Organization $organization): JsonResponse
+    public function packages(Request $request, Organization $organization): JsonResponse
     {
-        return new JsonResponse([
-            'packages' => $this->packageManager->findProviders($organization->alias(), $this->packageQuery->getAllNames($organization->id())),
+        $packageNames = $this->packageQuery->getAllNames($organization->id());
+        [$lastModified, $packages] = $this->packageManager->findProviders($organization->alias(), $packageNames);
+
+        $response = (new JsonResponse([
+            'packages' => $packages,
+            'available-packages' => array_map(static fn (PackageName $packageName) => $packageName->name(), $packageNames),
             'metadata-url' => '/p2/%package%.json',
             'notify-batch' => $this->generateUrl('repo_package_downloads', [
                 'organization' => $organization->alias(),
@@ -53,7 +57,13 @@ public function packages(Organization $organization): JsonResponse
                     'preferred' => true,
                 ],
             ],
-        ]);
+        ]))
+        ->setPrivate()
+        ->setLastModified($lastModified);
+
+        $response->isNotModified($request);
+
+        return $response;
     }
 
     /**
@@ -120,14 +130,24 @@ public function downloads(Request $request, Organization $organization): JsonRes
      *      requirements={"domain"="%domain%","package"="%package_name_pattern%"})
      * @Cache(public=false)
      */
-    public function providerV2(Organization $organization, string $package): JsonResponse
+    public function providerV2(Request $request, Organization $organization, string $package): JsonResponse
     {
-        $providerData = $this->packageManager->findProviders(
+        [$lastModified, $providerData] = $this->packageManager->findProviders(
             $organization->alias(),
             [new PackageName('', $package)]
         );
 
-        return new JsonResponse($providerData === [] ? new \stdClass() : $providerData);
+        if ($providerData === []) {
+            throw new NotFoundHttpException();
+        }
+
+        $response = (new JsonResponse($providerData))
+            ->setLastModified($lastModified)
+            ->setPrivate();
+
+        $response->isNotModified($request);
+
+        return $response;
     }
 
     /**

src/Service/Organization/PackageManager.php

@@ -30,22 +30,33 @@ public function __construct(Storage $distStorage, string $baseDir, Filesystem $f
     /**
      * @param PackageName[] $packages
      *
-     * @return mixed[]
+     * @return array{\DateTimeImmutable|null, mixed[]}
      */
     public function findProviders(string $organizationAlias, array $packages): array
     {
         $data = [];
+        $lastModified = null;
+
         foreach ($packages as $package) {
             $filepath = $this->filepath($organizationAlias, $package->name());
             if (!is_readable($filepath)) {
                 continue;
             }
 
-            $json = unserialize((string) file_get_contents($filepath));
-            $data = array_merge($data, $json['packages'] ?? []);
+            $fileModifyDate = (new \DateTimeImmutable())->setTimestamp((int) filemtime($filepath));
+
+            if ($fileModifyDate > $lastModified) {
+                $lastModified = $fileModifyDate;
+            }
+
+            $json = unserialize((string) file_get_contents($filepath), ['allowed_classes' => false]);
+            $data[] = $json['packages'] ?? [];
         }
 
-        return $data;
+        return [
+            $lastModified,
+            array_merge(...$data),
+        ];
     }
 
     /**

src/Service/Security/PackageScanner/SensioLabsPackageScanner.php

@@ -102,7 +102,7 @@ private function findDistribution(Package $package): string
         $normalizedVersion = $latestReleasedVersion === 'no stable release' ?
             '9999999-dev' : $this->versionParser->normalize((string) $latestReleasedVersion);
 
-        $providerData = $this->packageManager->findProviders(
+        [,$providerData] = $this->packageManager->findProviders(
             $package->organizationAlias(),
             [new PackageName($package->id()->toString(), (string) $package->name())]
         );

tests/Functional/Controller/RepoControllerTest.php

@@ -5,6 +5,7 @@
 namespace Buddy\Repman\Tests\Functional\Controller;
 
 use Buddy\Repman\Tests\Functional\FunctionalTestCase;
+use Ramsey\Uuid\Uuid;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -63,7 +64,11 @@ public function testOrganizationPackagesAction(): void
     {
         // create and login admin to check token last used date is changed
         $adminId = $this->createAndLoginAdmin('[email protected]', 'secret');
-        $this->fixtures->createToken($this->fixtures->createOrganization('buddy', $adminId), 'secret-org-token');
+        $organizationId = $this->fixtures->createOrganization('buddy', $adminId);
+        $this->fixtures->createToken($organizationId, 'secret-org-token');
+        $packageId = Uuid::uuid4()->toString();
+        $this->fixtures->createPackage($packageId, 'buddy', $organizationId);
+        $this->fixtures->syncPackageWithData($packageId, 'buddy-works/repman', 'Test', '1.1.1', new \DateTimeImmutable());
 
         // check token was never used
         $this->client->request('GET', $this->urlTo('organization_tokens', ['organization' => 'buddy']));
@@ -75,9 +80,24 @@ public function testOrganizationPackagesAction(): void
             'PHP_AUTH_PW' => 'secret-org-token',
         ]);
 
-        self::assertMatchesPattern('
+        self::assertJsonStringEqualsJsonString('
         {
-            "packages": [],
+            "packages": {
+               "buddy-works\/repman": {
+                    "1.2.3": {
+                        "version": "1.2.3",
+                        "version_normalized": "1.2.3.0",
+                        "dist": {
+                            "type": "zip",
+                            "url": "\/path\/to\/reference.zip",
+                            "reference": "ac7dcaf888af2324cd14200769362129c8dd8550"
+                        }
+                    }
+                }
+            },
+            "available-packages": [
+                "buddy-works/repman"
+            ],
             "metadata-url": "/p2/%package%.json",
             "notify-batch": "http://buddy.repo.repman.wip/downloads",
             "search": "https://packagist.org/search.json?q=%query%&type=%type%",
@@ -191,4 +211,37 @@ public function testProviderV2Action(): void
         }
         ', $this->client->getResponse()->getContent());
     }
+
+    public function testProviderV2ActionWithCache(): void
+    {
+        $adminId = $this->createAndLoginAdmin('[email protected]', 'secret');
+        $this->fixtures->createToken($this->fixtures->createOrganization('buddy', $adminId), 'secret-org-token');
+
+        $fileModifiedTime = (new \DateTimeImmutable())
+            ->setTimestamp((int) filemtime(__DIR__.'/../../Resources/p2/buddy-works/repman.json'));
+
+        $this->client->request('GET', '/p2/buddy-works/repman.json', [], [], [
+            'HTTP_HOST' => 'buddy.repo.repman.wip',
+            'PHP_AUTH_USER' => 'token',
+            'PHP_AUTH_PW' => 'secret-org-token',
+            'HTTP_IF_MODIFIED_SINCE' => $fileModifiedTime->format('D, d M Y H:i:s \G\M\T'),
+        ]);
+
+        self::assertEquals(304, $this->client->getResponse()->getStatusCode());
+        self::assertEmpty($this->client->getResponse()->getContent());
+    }
+
+    public function testProviderV2ForMissingPackage(): void
+    {
+        $adminId = $this->createAndLoginAdmin('[email protected]', 'secret');
+        $this->fixtures->createToken($this->fixtures->createOrganization('buddy', $adminId), 'secret-org-token');
+
+        $this->client->request('GET', '/p2/buddy-works/fake.json', [], [], [
+            'HTTP_HOST' => 'buddy.repo.repman.wip',
+            'PHP_AUTH_USER' => 'token',
+            'PHP_AUTH_PW' => 'secret-org-token',
+        ]);
+
+        self::assertTrue($this->client->getResponse()->isNotFound());
+    }
 }

tests/Unit/Service/Organization/PackageManagerTest.php

@@ -33,7 +33,7 @@ protected function setUp(): void
 
     public function testFindProvidersForPackage(): void
     {
-        $providers = $this->manager->findProviders('buddy', [
+        [,$providers] = $this->manager->findProviders('buddy', [
             new PackageName('id', 'buddy-works/repman'),
             new PackageName('id', 'not-exist/missing'),
         ]);

tests/Unit/Service/Security/SensioLabsPackageScannerTest.php

@@ -4,6 +4,7 @@
 
 namespace Buddy\Repman\Tests\Unit\Service\Security;
 
+use Buddy\Repman\Entity\Organization\Package;
 use Buddy\Repman\Message\Security\SendScanResult;
 use Buddy\Repman\Repository\ScanResultRepository;
 use Buddy\Repman\Service\Organization\PackageManager;
@@ -52,6 +53,8 @@ public function testScanVersionNotFound(): void
         $package = PackageMother::synchronized('buddy-works/repman', '1.0.0', '');
 
         $this->scanner->scan($package);
+
+        $this->assertPackageSecurity('error', $package);
     }
 
     public function testSuccessfulScanNoAdvisories(): void
@@ -63,6 +66,8 @@ public function testSuccessfulScanNoAdvisories(): void
         $package = PackageMother::synchronized('buddy-works/repman', self::VERSION);
 
         $this->scanner->scan($package);
+
+        $this->assertPackageSecurity('ok', $package);
     }
 
     public function testSuccessfulScanWithAdvisories(): void
@@ -85,6 +90,8 @@ public function testSuccessfulScanWithAdvisories(): void
         $this->checkerMock = $this->createMock(SecurityChecker::class);
         $this->checkerMock->method('check')->willReturn($result);
         $this->prepareScanner()->scan($package);
+
+        $this->assertPackageSecurity('warning', $package);
     }
 
     public function testLockFileNotFound(): void
@@ -96,6 +103,8 @@ public function testLockFileNotFound(): void
         $package = PackageMother::synchronized('buddy-works/repman', self::VERSION);
 
         $this->prepareScanner('repman-no-lock')->scan($package);
+
+        $this->assertPackageSecurity('n/a', $package);
     }
 
     public function testBrokenZip(): void
@@ -107,21 +116,32 @@ public function testBrokenZip(): void
         $package = PackageMother::synchronized('buddy-works/repman', self::VERSION);
 
         $this->prepareScanner('repman-invalid-archive')->scan($package);
+
+        $this->assertPackageSecurity('error', $package);
     }
 
     private function prepareScanner(string $fixtureType = 'repman'): SensioLabsPackageScanner
     {
         $distFile = realpath(__DIR__.'/../../../Resources/fixtures/buddy/dist/buddy-works/'.$fixtureType.'/1.2.3.0_ac7dcaf888af2324cd14200769362129c8dd8550.zip');
         $packageManager = $this->createMock(PackageManager::class);
-        $packageManager->method('findProviders')->willReturn(['buddy-works/repman' => [self::VERSION => [
-            'version' => self::VERSION,
-            'dist' => [
-                'type' => 'zip',
-                'url' => $distFile,
-                'reference' => 'ac7dcaf888af2324cd14200769362129c8dd8550',
-            ],
-            'version_normalized' => '1.2.3.0',
-        ]]]);
+        $packageManager->method('findProviders')->willReturn(
+            [
+                new \DateTimeImmutable(),
+                [
+                    'buddy-works/repman' => [
+                        self::VERSION => [
+                            'version' => self::VERSION,
+                            'dist' => [
+                                'type' => 'zip',
+                                'url' => $distFile,
+                                'reference' => 'ac7dcaf888af2324cd14200769362129c8dd8550',
+                            ],
+                            'version_normalized' => '1.2.3.0',
+                        ],
+                    ],
+                ],
+            ]
+        );
 
         $packageManager->method('distFilename')->willReturn(Option::some($distFile));
 
@@ -137,4 +157,13 @@ private function prepareScanner(string $fixtureType = 'repman'): SensioLabsPacka
             $messageBusMock
         );
     }
+
+    private function assertPackageSecurity(string $expected, Package $package): void
+    {
+        $reflection = new \ReflectionObject($package);
+        $property = $reflection->getProperty('lastScanStatus');
+        $property->setAccessible(true);
+
+        self::assertEquals($expected, $property->getValue($package));
+    }
 }