Browse Source

bug-fixes

imwald
Silberengel 21 hours ago
parent
commit
8f4e123c89
  1. 5
      Dockerfile
  2. 16
      src/Controller/ArticleController.php
  3. 31
      src/Repository/ArticleHighlightRepository.php
  4. 19
      src/Repository/ArticleRepository.php
  5. 22
      src/Service/ArticleBodyHighlightInjector.php
  6. 8
      tests/Service/ArticleBodyHighlightInjectorTest.php
  7. 155
      tests/Service/ArticleHighlightCommonMarkPipelineTest.php

5
Dockerfile

@ -28,9 +28,12 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
cron \ cron \
&& rm -rf /var/lib/apt/lists/* && 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; \ RUN set -eux; \
install-php-extensions \ install-php-extensions \
@composer \
apcu \ apcu \
intl \ intl \
opcache \ opcache \

16
src/Controller/ArticleController.php

@ -4,6 +4,7 @@ namespace App\Controller;
use App\Entity\Article; use App\Entity\Article;
use App\Repository\ArticleHighlightRepository; use App\Repository\ArticleHighlightRepository;
use App\Repository\ArticleRepository;
use App\Service\ArticleBodyHighlightInjector; use App\Service\ArticleBodyHighlightInjector;
use App\Enum\KindsEnum; use App\Enum\KindsEnum;
use App\Nostr\Nip22CommentTags; use App\Nostr\Nip22CommentTags;
@ -346,21 +347,10 @@ class ArticleController extends AbstractController
private function loadLatestArticleBySlug(EntityManagerInterface $entityManager, string $slug): ?Article private function loadLatestArticleBySlug(EntityManagerInterface $entityManager, string $slug): ?Article
{ {
/** @var ArticleRepository $repository */
$repository = $entityManager->getRepository(Article::class); $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( private function renderArticle(

31
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<ArticleHighlight> * @return list<ArticleHighlight>
*/ */
public function findByArticle(Article $article): array public function findByArticle(Article $article): array
@ -55,13 +58,29 @@ class ArticleHighlightRepository extends ServiceEntityRepository
return []; 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<ArticleHighlight> $out */ /** @var list<ArticleHighlight> $out */
$out = $this->createQueryBuilder('h') $out = $qb->getQuery()->getResult();
->where('h.article = :art')
->setParameter('art', $article)
->orderBy('h.eventCreatedAt', 'DESC')
->getQuery()
->getResult();
return $out; return $out;
} }

19
src/Repository/ArticleRepository.php

@ -193,6 +193,25 @@ class ArticleRepository extends ServiceEntityRepository
return $this->findOneBy(['eventId' => $eventId]); 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 * Find articles by author's public key
*/ */

22
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 * 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 * in NIP-84 order: event `content` (highlighted span) first, then the `context` tag when set, then
* non-empty, then `textquoteselector` passage. The first string that matches the body wins. * 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 * 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 * 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). * 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 * Same priority as the card: event `content` (NIP-84 sub-span) first, then the `context` tag when
* still empty, `textquoteselector` passage. Article injection tries each in order until one * set, then {@see HighlightEventTags::fullPassageForHighlightDisplay} (so missing/empty `context`
* matches the rendered body (so a highlight with only `textquoteselector` still inlines a mark). * is treated as “passage = `content`” before `textquoteselector`). Tries each in order until one
* matches the rendered body.
*/ */
private function primaryNeedleForGrouping(ArticleHighlight $h): string private function primaryNeedleForGrouping(ArticleHighlight $h): string
{ {
@ -384,11 +386,15 @@ final class ArticleBodyHighlightInjector
private function injectionNeedleBasesInPriority(ArticleHighlight $h): array private function injectionNeedleBasesInPriority(ArticleHighlight $h): array
{ {
$c = \trim($h->getContent()); $c = \trim($h->getContent());
$ctx = \trim(HighlightEventTags::contextFromTags($h->getTags())); $tags = $h->getTags();
$tq = \trim(HighlightEventTags::textquoteselectorPassageFromTags($h->getTags())); $ctx = \trim(HighlightEventTags::contextFromTags($tags));
$fullPassage = \trim(HighlightEventTags::fullPassageForHighlightDisplay($c, $tags));
$tq = \trim(HighlightEventTags::textquoteselectorPassageFromTags($tags));
$out = []; $out = [];
$seen = []; $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])) { if ($s === '' || isset($seen[$s])) {
continue; continue;
} }

8
tests/Service/ArticleBodyHighlightInjectorTest.php

@ -149,12 +149,18 @@ final class ArticleBodyHighlightInjectorTest extends TestCase
*/ */
private function assertHighlightFragmentsPresent(string $html, array $eventIds): void private function assertHighlightFragmentsPresent(string $html, array $eventIds): void
{ {
$this->assertStringContainsString(
'<mark',
$html,
'In-article highlights must include at least one <mark> (see ArticleBodyHighlightInjector).'
);
$this->assertStringContainsString('user-highlight__marker', $html);
foreach ($eventIds as $eid) { foreach ($eventIds as $eid) {
$eid = strtolower($eid); $eid = strtolower($eid);
$this->assertMatchesRegularExpression( $this->assertMatchesRegularExpression(
'/\bid="highlight-'.preg_quote($eid, '/').'"/', '/\bid="highlight-'.preg_quote($eid, '/').'"/',
$html, $html,
'Expected in-article fragment id highlight-'.$eid 'Each event id must have a #highlight-'.$eid.' anchor (on the <mark> or a zero-width fragment <span>).'
); );
} }
} }

155
tests/Service/ArticleHighlightCommonMarkPipelineTest.php

@ -0,0 +1,155 @@
<?php
declare(strict_types=1);
namespace App\Tests\Service;
use App\Entity\ArticleHighlight;
use App\Service\ArticleBodyHighlightInjector;
use App\Service\HighlightAuthorMetadataProvider;
use App\Util\CommonMark\Converter;
use League\CommonMark\Exception\CommonMarkException;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
/**
* Exercises the same two steps as {@see \App\Controller\ArticleController::renderArticle}:
* {@see Converter::convertToHTML} then {@see ArticleBodyHighlightInjector::inject}. Unit tests
* that only hand-build HTML can pass while production (real CommonMark output) still produces
* no &lt;mark&gt; if the pipeline or matching rules diverge; these tests assert the literal
* &lt;mark class="user-highlight__marker"&gt; in the final string.
*/
final class ArticleHighlightCommonMarkPipelineTest extends KernelTestCase
{
private const AUTHOR_HEX = 'd475ce4b3977507130f42c7f8634fef936800f3ae74d5ecf8089280cdc1923e9';
private function getConverter(): Converter
{
$container = static::getContainer();
if (!$container->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('<mark', $out, 'Injected body must include a <mark> 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 <sup>.');
$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('<mark', $out);
$this->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('<mark', $out);
$this->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(
'/<mark\\b[^>]*\\bclass="user-highlight__marker"[^>]*\\bid="highlight-'.preg_quote($eid, '/').'"/',
$html
)) {
return;
}
if (1 === preg_match(
'/<mark\\b[^>]*\\bid="highlight-'.preg_quote($eid, '/').'"[^>]*\\bclass="user-highlight__marker"/',
$html
)) {
return;
}
$this->fail('Expected <mark> 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).'…';
}
}
Loading…
Cancel
Save