Skip to content

Commit 1ed4a9b

Browse files
SeldaekSeldaek
authored
Move readme data to a join table to avoid heavy joins on package data (#1568)
1 parent 58b6924 commit 1ed4a9b

17 files changed

+142
-90
lines changed

‎src/Controller/PackageController.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use App\Entity\Job;
1818
use App\Entity\Package;
1919
use App\Entity\PackageFreezeReason;
20+
use App\Entity\PackageReadme;
2021
use App\Entity\PackageRepository;
2122
use App\Entity\PhpStat;
2223
use App\Entity\SecurityAdvisory;
@@ -728,6 +729,8 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn
728729
$data['unfreezeCsrfToken'] = $csrfTokenManager->getToken('unfreeze');
729730
}
730731

732+
$data['readme'] = $this->getEM()->find(PackageReadme::class, $package->getId());
733+
731734
return $this->render('package/view_package.html.twig', $data);
732735
}
733736

‎src/Entity/Package.php‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,6 @@ class Package
113113
#[ORM\Column(type: 'string', nullable: true)]
114114
private ?string $language = null;
115115

116-
#[ORM\Column(type: 'text', nullable: true)]
117-
private ?string $readme = null;
118-
119116
#[ORM\Column(type: 'integer', nullable: true, name: 'github_stars')]
120117
private ?int $gitHubStars = null;
121118

@@ -374,28 +371,6 @@ public function getLanguage(): ?string
374371
return $this->language;
375372
}
376373

377-
public function setReadme(string $readme): void
378-
{
379-
$this->readme = $readme;
380-
}
381-
382-
public function getReadme(): string
383-
{
384-
return (string) $this->readme;
385-
}
386-
387-
/**
388-
* Get readme with transformations that should not be done in the stored readme as they might not be valid in the long run
389-
*/
390-
public function getOptimizedReadme(): string
391-
{
392-
if ($this->readme === null) {
393-
return '';
394-
}
395-
396-
return str_replace(['<img src="https://raw.github.com/', '<img src="https://raw.githubusercontent.com/'], '<img src="https://rawcdn.githack.com/', $this->readme);
397-
}
398-
399374
public function setGitHubStars(?int $val): void
400375
{
401376
$this->gitHubStars = $val;

‎src/Entity/PackageReadme.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php declare(strict_types=1);
2+
3+
/*
4+
* This file is part of Packagist.
5+
*
6+
* (c) Jordi Boggiano <[email protected]>
7+
* Nils Adermann <[email protected]>
8+
*
9+
* For the full copyright and license information, please view the LICENSE
10+
* file that was distributed with this source code.
11+
*/
12+
13+
namespace App\Entity;
14+
15+
use Doctrine\ORM\Mapping as ORM;
16+
17+
/**
18+
* @author Jordi Boggiano <[email protected]>
19+
*/
20+
#[ORM\Entity]
21+
#[ORM\Table(name: 'package_readme')]
22+
class PackageReadme
23+
{
24+
public function __construct(
25+
#[ORM\Id]
26+
#[ORM\OneToOne(targetEntity: Package::class)]
27+
#[ORM\JoinColumn(referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
28+
public Package $package,
29+
30+
#[ORM\Column(type: 'text', nullable: false)]
31+
public string $contents,
32+
) {
33+
}
34+
35+
/**
36+
* Get contents with transformations that should not be done in the stored contents as they might not be valid in the long run
37+
*/
38+
public string $optimizedContents {
39+
get {
40+
return str_replace(['<img src="https://raw.github.com/', '<img src="https://raw.githubusercontent.com/'], '<img src="https://rawcdn.githack.com/', $this->contents);
41+
}
42+
}
43+
}

‎src/Package/Updater.php‎

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use App\Entity\DevRequireLink;
1818
use App\Entity\Package;
1919
use App\Entity\PackageFreezeReason;
20+
use App\Entity\PackageReadme;
2021
use App\Entity\ProvideLink;
2122
use App\Entity\ReplaceLink;
2223
use App\Entity\RequireLink;
@@ -623,7 +624,9 @@ private function updateReadme(IOInterface $io, Package $package, VcsDriverInterf
623624
case '.txt':
624625
$source = $driver->getFileContent($readmeFile, $driver->getRootIdentifier());
625626
if (!empty($source)) {
626-
$package->setReadme('<pre>'.htmlspecialchars($source).'</pre>');
627+
$this->updatePackageReadme($package, '<pre>'.htmlspecialchars($source).'</pre>');
628+
} else {
629+
$this->updatePackageReadme($package, null);
627630
}
628631
break;
629632

@@ -635,9 +638,9 @@ private function updateReadme(IOInterface $io, Package $package, VcsDriverInterf
635638

636639
if (!empty($readme)) {
637640
if (Preg::isMatch('{^(?:git://|git@|https?://)(gitlab.com|bitbucket.org)[:/]([^/]+)/(.+?)(?:\.git|/)?$}i', $package->getRepository(), $match)) {
638-
$package->setReadme($this->prepareReadme($readme, $match[1], $match[2], $match[3]));
641+
$this->updatePackageReadme($package, $this->prepareReadme($readme, $match[1], $match[2], $match[3]));
639642
} else {
640-
$package->setReadme($this->prepareReadme($readme));
643+
$this->updatePackageReadme($package, $this->prepareReadme($readme));
641644
}
642645
}
643646
}
@@ -653,6 +656,26 @@ private function updateReadme(IOInterface $io, Package $package, VcsDriverInterf
653656
}
654657
}
655658

659+
private function updatePackageReadme(Package $package, ?string $contents): void
660+
{
661+
$readme = $this->getEM()->find(PackageReadme::class, $package->getId());
662+
663+
if ($contents === '' || $contents === null) {
664+
if ($readme) {
665+
$this->getEM()->remove($readme);
666+
}
667+
return;
668+
}
669+
670+
if (!$readme) {
671+
$readme = new PackageReadme($package, $contents);
672+
} else {
673+
$readme->contents = $contents;
674+
}
675+
676+
$this->getEM()->persist($readme);
677+
}
678+
656679
private function updateGitHubInfo(HttpDownloader $httpDownloader, Package $package, string $owner, string $repo, VcsDriverInterface $driver): void
657680
{
658681
if (!$driver instanceof GitHubDriver) {
@@ -663,22 +686,6 @@ private function updateGitHubInfo(HttpDownloader $httpDownloader, Package $packa
663686

664687
$repoData = $driver->getRepoData();
665688

666-
try {
667-
$opts = ['http' => ['header' => ['Accept: application/vnd.github.html+json', 'X-GitHub-Api-Version: 2022-11-28']]];
668-
$readme = $httpDownloader->get($baseApiUrl.'/readme', $opts)->getBody();
669-
} catch (\Exception $e) {
670-
if (!$e instanceof \Composer\Downloader\TransportException || $e->getCode() !== 404) {
671-
return;
672-
}
673-
// 404s just mean no readme present so we proceed with the rest
674-
}
675-
676-
if (!empty($readme)) {
677-
// The content of all readmes, regardless of file type,
678-
// is returned as HTML by GitHub API
679-
$package->setReadme($this->prepareReadme($readme, 'github.com', $owner, $repo));
680-
}
681-
682689
if (!empty($repoData['language']) && \is_string($repoData['language'])) {
683690
$package->setLanguage($repoData['language']);
684691
}
@@ -694,6 +701,21 @@ private function updateGitHubInfo(HttpDownloader $httpDownloader, Package $packa
694701
if (isset($repoData['open_issues_count']) && is_numeric($repoData['open_issues_count'])) {
695702
$package->setGitHubOpenIssues((int) $repoData['open_issues_count']);
696703
}
704+
705+
try {
706+
$opts = ['http' => ['header' => ['Accept: application/vnd.github.html+json', 'X-GitHub-Api-Version: 2022-11-28']]];
707+
$readme = $httpDownloader->get($baseApiUrl.'/readme', $opts)->getBody();
708+
} catch (\Exception $e) {
709+
if (!$e instanceof \Composer\Downloader\TransportException || $e->getCode() !== 404) {
710+
return;
711+
}
712+
// 404s just mean no readme present so we proceed with the rest
713+
$readme = null;
714+
}
715+
716+
// The content of all readmes, regardless of file type,
717+
// is returned as HTML by GitHub API
718+
$this->updatePackageReadme($package, $this->prepareReadme($readme, 'github.com', $owner, $repo));
697719
}
698720

699721
private function updateGitLabInfo(HttpDownloader $httpDownloader, IOInterface $io, Package $package, string $owner, string $repo, VcsDriverInterface $driver): void
@@ -727,8 +749,12 @@ private function updateGitLabInfo(HttpDownloader $httpDownloader, IOInterface $i
727749
/**
728750
* Prepare the readme by stripping elements and attributes that are not supported .
729751
*/
730-
private function prepareReadme(string $readme, ?string $host = null, ?string $owner = null, ?string $repo = null): string
752+
private function prepareReadme(?string $readme, ?string $host = null, ?string $owner = null, ?string $repo = null): ?string
731753
{
754+
if ($readme === null) {
755+
return null;
756+
}
757+
732758
// detect base path for github readme if file is located in a subfolder like docs/README.md
733759
$basePath = '';
734760
if ($host === 'github.com' && Preg::isMatchStrictGroups('{^<div id="readme" [^>]+?data-path="([^"]+)"}', $readme, $match) && str_contains($match[1], '/')) {

‎templates/package/view_package.html.twig‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,11 +342,11 @@
342342
<p class="col-xs-12">This package has no released version yet, and little information is available.</p>
343343
{% endif %}
344344

345-
{% if package.readme != null and (not package.isSuspect() or hasActions) %}
345+
{% if readme is not null and (not package.isSuspect() or hasActions) %}
346346
<hr class="clearfix">
347347
<div class="readme markdown-body">
348348
<h1>README</h1>
349-
{{ package.optimizedReadme|raw }}
349+
{{ readme.optimizedContents|raw }}
350350
</div>
351351
{% endif %}
352352
</div>

‎tests/Controller/AboutControllerTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212

1313
namespace App\Tests\Controller;
1414

15-
class AboutControllerTest extends ControllerTestCase
15+
use App\Tests\IntegrationTestCase;
16+
17+
class AboutControllerTest extends IntegrationTestCase
1618
{
1719
public function testPackagist(): void
1820
{

‎tests/Controller/ApiControllerTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616
use App\SecurityAdvisory\GitHubSecurityAdvisoriesSource;
1717
use App\SecurityAdvisory\RemoteSecurityAdvisory;
1818
use App\SecurityAdvisory\Severity;
19+
use App\Tests\IntegrationTestCase;
1920
use PHPUnit\Framework\Attributes\DataProvider;
2021
use PHPUnit\Framework\Attributes\Depends;
2122

22-
class ApiControllerTest extends ControllerTestCase
23+
class ApiControllerTest extends IntegrationTestCase
2324
{
2425
public function testGithubFailsOnGet(): void
2526
{

‎tests/Controller/ChangePasswordControllerTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
namespace App\Tests\Controller;
1414

1515
use App\Entity\User;
16+
use App\Tests\IntegrationTestCase;
1617
use App\Validator\NotProhibitedPassword;
1718
use PHPUnit\Framework\Attributes\TestWith;
1819
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
1920

20-
class ChangePasswordControllerTest extends ControllerTestCase
21+
class ChangePasswordControllerTest extends IntegrationTestCase
2122
{
2223
#[TestWith(['SuperSecret123', 'ok'])]
2324
#[TestWith(['[email protected]', 'prohibited-password-error'])]

‎tests/Controller/FeedControllerTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212

1313
namespace App\Tests\Controller;
1414

15+
use App\Tests\IntegrationTestCase;
1516
use PHPUnit\Framework\Attributes\DataProvider;
1617
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1718

18-
class FeedControllerTest extends ControllerTestCase
19+
class FeedControllerTest extends IntegrationTestCase
1920
{
2021
#[DataProvider('provideForFeed')]
2122
public function testFeedAction(string $feed, string $format, ?string $vendor = null): void

‎tests/Controller/PackageControllerTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212

1313
namespace App\Tests\Controller;
1414

15-
class PackageControllerTest extends ControllerTestCase
15+
use App\Tests\IntegrationTestCase;
16+
17+
class PackageControllerTest extends IntegrationTestCase
1618
{
1719
public function testView(): void
1820
{

0 commit comments

Comments
 (0)