From 8f4e123c89826802116caed1dec9d74b0bdc5292 Mon Sep 17 00:00:00 2001 From: Silberengel Date: Mon, 27 Apr 2026 18:05:32 +0200 Subject: [PATCH] bug-fixes --- Dockerfile | 5 +- src/Controller/ArticleController.php | 16 +- src/Repository/ArticleHighlightRepository.php | 31 +++- src/Repository/ArticleRepository.php | 19 +++ src/Service/ArticleBodyHighlightInjector.php | 22 ++- .../ArticleBodyHighlightInjectorTest.php | 8 +- ...ArticleHighlightCommonMarkPipelineTest.php | 155 ++++++++++++++++++ 7 files changed, 227 insertions(+), 29 deletions(-) create mode 100644 tests/Service/ArticleHighlightCommonMarkPipelineTest.php diff --git a/Dockerfile b/Dockerfile index 119d258..d4bb6ea 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,9 +28,12 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ cron \ && rm -rf /var/lib/apt/lists/* +# Composer: copy from the official image instead of @composer on install-php-extensions, which +# curl's getcomposer.org and fails when build DNS is broken (e.g. curl: (6) Could not resolve host). +COPY --from=composer:2 /usr/bin/composer /usr/bin/composer + RUN set -eux; \ install-php-extensions \ - @composer \ apcu \ intl \ opcache \ diff --git a/src/Controller/ArticleController.php b/src/Controller/ArticleController.php index b3b0fca..82391c7 100644 --- a/src/Controller/ArticleController.php +++ b/src/Controller/ArticleController.php @@ -4,6 +4,7 @@ namespace App\Controller; use App\Entity\Article; use App\Repository\ArticleHighlightRepository; +use App\Repository\ArticleRepository; use App\Service\ArticleBodyHighlightInjector; use App\Enum\KindsEnum; use App\Nostr\Nip22CommentTags; @@ -346,21 +347,10 @@ class ArticleController extends AbstractController private function loadLatestArticleBySlug(EntityManagerInterface $entityManager, string $slug): ?Article { + /** @var ArticleRepository $repository */ $repository = $entityManager->getRepository(Article::class); - $articles = $repository->findBy(['slug' => $slug]); - $revisions = \count($articles); - if ($revisions === 0) { - return null; - } - if ($revisions > 1) { - usort($articles, function ($a, $b) { - return $b->getCreatedAt() <=> $a->getCreatedAt(); - }); - - return end($articles); - } - return $articles[0]; + return $repository->findLatestBySlug($slug); } private function renderArticle( diff --git a/src/Repository/ArticleHighlightRepository.php b/src/Repository/ArticleHighlightRepository.php index 060aa9b..59565cd 100644 --- a/src/Repository/ArticleHighlightRepository.php +++ b/src/Repository/ArticleHighlightRepository.php @@ -46,6 +46,9 @@ class ArticleHighlightRepository extends ServiceEntityRepository } /** + * Returns highlights for this NIP-33 address (kind + pubkey + slug), not only the current + * {@see Article} row id — `article` can have multiple DB rows for the same slug (revisions). + * * @return list */ public function findByArticle(Article $article): array @@ -55,13 +58,29 @@ class ArticleHighlightRepository extends ServiceEntityRepository return []; } + $pubkey = (string) $article->getPubkey(); + if ('' === $pubkey) { + return []; + } + $slug = trim((string) $article->getSlug()); + if ('' === $slug) { + return []; + } + + $qb = $this->createQueryBuilder('h') + ->innerJoin('h.article', 'a') + ->where('a.pubkey = :pubkey') + ->andWhere('a.slug = :slug') + ->setParameter('pubkey', $pubkey) + ->setParameter('slug', $slug) + ->orderBy('h.eventCreatedAt', 'DESC'); + + // Do not filter on `a.kind`: replaceable long-form can leave several `article` rows per slug + // with different kind (or NULL vs 30023). Highlights are still tied to the same NIP-33 + // address; filtering by the *current* row's kind dropped rows synced to an older revision. + /** @var list $out */ - $out = $this->createQueryBuilder('h') - ->where('h.article = :art') - ->setParameter('art', $article) - ->orderBy('h.eventCreatedAt', 'DESC') - ->getQuery() - ->getResult(); + $out = $qb->getQuery()->getResult(); return $out; } diff --git a/src/Repository/ArticleRepository.php b/src/Repository/ArticleRepository.php index 1cbce15..4b40839 100644 --- a/src/Repository/ArticleRepository.php +++ b/src/Repository/ArticleRepository.php @@ -193,6 +193,25 @@ class ArticleRepository extends ServiceEntityRepository return $this->findOneBy(['eventId' => $eventId]); } + /** + * Newest row for a NIP-23/24 `d` value (replaceable long-form can leave multiple `article` rows per slug). + */ + public function findLatestBySlug(string $slug): ?Article + { + $slug = trim($slug); + if ($slug === '') { + return null; + } + + return $this->createQueryBuilder('a') + ->where('a.slug = :slug') + ->setParameter('slug', $slug) + ->orderBy('a.createdAt', 'DESC') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + } + /** * Find articles by author's public key */ diff --git a/src/Service/ArticleBodyHighlightInjector.php b/src/Service/ArticleBodyHighlightInjector.php index 415814a..1e51283 100644 --- a/src/Service/ArticleBodyHighlightInjector.php +++ b/src/Service/ArticleBodyHighlightInjector.php @@ -14,8 +14,9 @@ use swentel\nostr\Key\Key; /** * Injects kind-9802 highlight marks into the rendered article body by searching the visible text - * in NIP-84 order: event `content` (highlighted span) first, then the `context` tag when present and - * non-empty, then `textquoteselector` passage. The first string that matches the body wins. + * in NIP-84 order: event `content` (highlighted span) first, then the `context` tag when set, then + * the full passage ({@see HighlightEventTags::fullPassageForHighlightDisplay}, same as `content` + * when `context` is missing), then `textquoteselector`. The first string that matches the body wins. * Matches across inline elements (e.g. em, strong) by concatenating text in document order. Text * inside a prior `mark.user-highlight__marker` is still considered so a narrower 9802 can * be nested and receive its own fragment id (deep link from the landing aside). @@ -367,9 +368,10 @@ final class ArticleBodyHighlightInjector } /** - * Same priority as the card: event `content` (NIP-84 sub-span) first; if empty, `context` tag; if - * still empty, `textquoteselector` passage. Article injection tries each in order until one - * matches the rendered body (so a highlight with only `textquoteselector` still inlines a mark). + * Same priority as the card: event `content` (NIP-84 sub-span) first, then the `context` tag when + * set, then {@see HighlightEventTags::fullPassageForHighlightDisplay} (so missing/empty `context` + * is treated as “passage = `content`” before `textquoteselector`). Tries each in order until one + * matches the rendered body. */ private function primaryNeedleForGrouping(ArticleHighlight $h): string { @@ -384,11 +386,15 @@ final class ArticleBodyHighlightInjector private function injectionNeedleBasesInPriority(ArticleHighlight $h): array { $c = \trim($h->getContent()); - $ctx = \trim(HighlightEventTags::contextFromTags($h->getTags())); - $tq = \trim(HighlightEventTags::textquoteselectorPassageFromTags($h->getTags())); + $tags = $h->getTags(); + $ctx = \trim(HighlightEventTags::contextFromTags($tags)); + $fullPassage = \trim(HighlightEventTags::fullPassageForHighlightDisplay($c, $tags)); + $tq = \trim(HighlightEventTags::textquoteselectorPassageFromTags($tags)); $out = []; $seen = []; - foreach ([$c, $ctx, $tq] as $s) { + // NIP-84: `context` = full quote; `content` = highlighted span. Missing/empty `context` is + // the same as “full passage = `content`” (entirely highlighted) — see fullPassageForHighlightDisplay. + foreach ([$c, $ctx, $fullPassage, $tq] as $s) { if ($s === '' || isset($seen[$s])) { continue; } diff --git a/tests/Service/ArticleBodyHighlightInjectorTest.php b/tests/Service/ArticleBodyHighlightInjectorTest.php index d8165cb..410a28b 100644 --- a/tests/Service/ArticleBodyHighlightInjectorTest.php +++ b/tests/Service/ArticleBodyHighlightInjectorTest.php @@ -149,12 +149,18 @@ final class ArticleBodyHighlightInjectorTest extends TestCase */ private function assertHighlightFragmentsPresent(string $html, array $eventIds): void { + $this->assertStringContainsString( + ' (see ArticleBodyHighlightInjector).' + ); + $this->assertStringContainsString('user-highlight__marker', $html); foreach ($eventIds as $eid) { $eid = strtolower($eid); $this->assertMatchesRegularExpression( '/\bid="highlight-'.preg_quote($eid, '/').'"/', $html, - 'Expected in-article fragment id highlight-'.$eid + 'Each event id must have a #highlight-'.$eid.' anchor (on the or a zero-width fragment ).' ); } } diff --git a/tests/Service/ArticleHighlightCommonMarkPipelineTest.php b/tests/Service/ArticleHighlightCommonMarkPipelineTest.php new file mode 100644 index 0000000..2baf943 --- /dev/null +++ b/tests/Service/ArticleHighlightCommonMarkPipelineTest.php @@ -0,0 +1,155 @@ +has(Converter::class)) { + self::fail('Converter service must be registered in the test kernel.'); + } + /** @var Converter $c */ + $c = $container->get(Converter::class); + + return $c; + } + + /** + * @throws CommonMarkException + */ + public function testCommonMarkParagraphYieldsMarkTagsInFinalHtml(): void + { + self::bootKernel(); + $markdown = "Here is a simple paragraph for the test.\n"; + $html = $this->getConverter()->convertToHTML($markdown); + $eid = '0000000000000000000000000000000000000000000000000000000000000cc1'; + $highlights = [ + $this->makeHighlight($eid, 'Here is a simple paragraph for the test.', [], 1), + ]; + $out = $this->createInjector()->inject($html, $highlights)['html']; + + $this->assertStringContainsString(' element.'); + $this->assertStringContainsString('user-highlight__marker', $out); + $this->assertMarkWithFragmentId($out, $eid); + } + + /** + * @throws CommonMarkException + */ + public function testCommonMarkWithFootnoteRendersToHtmlThatStillInjectsMarkTags(): void + { + self::bootKernel(); + $markdown = 'keeping track of things in the informational realm[^a] always implies keeping track of time'."\n\n" + ."[^a]: A footnote for test.\n"; + $html = $this->getConverter()->convertToHTML($markdown); + $this->assertStringContainsString('fnref', $html, 'Sanity: League footnote extension should emit a fnref .'); + + $content = 'keeping track of things in the infor'."\xC2\xAD".'ma'."\xC2\xAD".'tional realm always implies keeping track of time'; + $eid = 'f56a6221e8575b051cd6df34e9b61654e08a241b4c5ced3b48c0b769b24ada7d'; + $highlights = [ + $this->makeHighlight( + $eid, + $content, + [['a', '30023:6e468422dfb74a5738702a8823b9b28168abab8655faacb6853cd0ee15deee93:bitcoin-is-time']], + 1762697677 + ), + ]; + $out = $this->createInjector()->inject($html, $highlights)['html']; + + $this->assertStringContainsString('assertStringContainsString('user-highlight__marker', $out); + $this->assertMarkWithFragmentId($out, $eid); + } + + /** + * @throws CommonMarkException + */ + public function testCommonMarkWithSmartPunctRendersToHtmlThatMatchesAsciiHighlightContentWithMarks(): void + { + self::bootKernel(); + $markdown = "Here's the point...\n"; + $html = $this->getConverter()->convertToHTML($markdown); + $eid = '0000000000000000000000000000000000000000000000000000000000000dd1'; + $highlights = [ + $this->makeHighlight($eid, "Here's the point...", [], 1), + ]; + $out = $this->createInjector()->inject($html, $highlights)['html']; + + $this->assertNotEquals($html, $out, 'Body should change when a mark is injected.'); + $this->assertStringContainsString('assertMarkWithFragmentId($out, $eid); + } + + private function createInjector(): ArticleBodyHighlightInjector + { + $meta = $this->createMock(HighlightAuthorMetadataProvider::class); + $meta->method('getMetadata')->willReturn( + (object) ['display_name' => 'Test', 'name' => 'Test', 'picture' => ''], + ); + + return new ArticleBodyHighlightInjector($meta); + } + + private function makeHighlight( + string $eventId64, + string $content, + array $tags, + int $createdAt, + ): ArticleHighlight { + $h = new ArticleHighlight(); + $h->setEventId($eventId64); + $h->setContent($content); + $h->setTags($tags); + $h->setEventCreatedAt($createdAt); + $h->setAuthorPubkey(self::AUTHOR_HEX); + + return $h; + } + + private function assertMarkWithFragmentId(string $html, string $eid): void + { + $eid = strtolower($eid); + if (1 === preg_match( + '/]*\\bclass="user-highlight__marker"[^>]*\\bid="highlight-'.preg_quote($eid, '/').'"/', + $html + )) { + return; + } + if (1 === preg_match( + '/]*\\bid="highlight-'.preg_quote($eid, '/').'"[^>]*\\bclass="user-highlight__marker"/', + $html + )) { + return; + } + $this->fail('Expected with class user-highlight__marker and id highlight-'.$eid.' in: '.$this->excerpt($html)); + } + + private function excerpt(string $html, int $max = 2000): string + { + if (\strlen($html) <= $max) { + return $html; + } + + return \substr($html, 0, $max).'…'; + } +}