Fix Atom & RSS id fields and incorrect use of slug (#10)

* Fix atom id fields and incorrect use of slug * Fix "self" link to be a real canonical URI See <https://www.rfc-editor.org/rfc/rfc4287#page-22> and <https://support.apple.com/en-gb/guide/news-publisher/apdc2c7520ff/icloud#apddf66cc2256854> * Add getPermalink() to the feed's controller This way both getFeedId and getSelfLink use the same function by default, but changing one does not affect the other. This is more in line with the real purpose of each function. * Fix: use entry's 'id' field as guid for RSS feed And rename the 'permalink' entry's field into 'link', because it is not a permalink, as it contains the discussion's slug. Also be explicit about the fact that RSS' entries' guid is a permalink by setting 'isPermaLink' to ture, even if it is the default.
imorland · Feb 1, 2023 · 8d3b3d8 · 8d3b3d8
1 parent 262d392
commit 8d3b3d8
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 10 deletions.
diff --git a/src/Controller/AbstractFeedController.php b/src/Controller/AbstractFeedController.php
@@ -85,6 +85,11 @@ abstract class AbstractFeedController implements RequestHandlerInterface
      */
     protected $settings;
 
+    /**
+     * @var string Must be defined by the subclasses to contain the last bit of the route name
+     */
+    protected $routeName;
+
     /**
      * Content-Types for feeds.
      *
@@ -122,7 +127,8 @@ public function handle(ServerRequestInterface $request): ResponseInterface
         $feed_type = in_array($feed_type, ['rss', 'atom']) ? $feed_type : 'rss';
 
         $feed_content = array_merge($this->getFeedContent($request), [
-            'self_link'  => rtrim($request->getUri(), " \t\n\r\0\v/"),
+            'self_link'  => $this->getFeedSelf($request->getQueryParams(), $feed_type),
+            'id'         => $this->getFeedId($request->getQueryParams(), $feed_type),
             'html'       => $this->getSetting('html'),
         ]);
 
@@ -396,4 +402,46 @@ protected function getFeedType(ServerRequestInterface $request): string
 
         return Str::startsWith($path, '/atom') ? 'atom' : 'rss';
     }
+
+    /**
+     * Get the "self" link of the current feed.
+     * A feed's "self" link is kind of the "canonical" link of a Web page.
+     * By default we use the feed's permalink, as it is a
+     * unique URI for this feed, generated from its route.
+     *
+     * @param array  $queryParams Query parameters of the feed request.
+     * @param string $feedType    Type of the current feed.
+     */
+    protected function getFeedSelf(array $queryParams, string $feedType): string
+    {
+        return $this->getPermalink($queryParams, $feedType);
+    }
+
+    /**
+     * Get the Id of the current feed.
+     * By default we use the feed's permalink, as it is a
+     * unique URI for this feed, generated from its route.
+     *
+     * @param array  $queryParams Query parameters of the feed request.
+     * @param string $feedType    Type of the current feed.
+     */
+    protected function getFeedId(array $queryParams, string $feedType): string
+    {
+        return $this->getPermalink($queryParams, $feedType);
+    }
+
+    /**
+     * Get the permalink of the current feed.
+     * It is a unique URI for this feed, generated from the current route
+     * and its query parameters.
+     * The permalink must not change even if the resource's name changed,
+     * so it must not include the slug.
+     *
+     * @param array  $queryParams Query parameters of the feed request.
+     * @param string $feedType    Type of the current feed.
+     */
+    protected function getPermalink(array $queryParams, string $feedType): string
+    {
+        return $this->url->to('forum')->route("feeds.$feedType.$this->routeName", $queryParams);
+    }
 }
diff --git a/src/Controller/DiscussionFeedController.php b/src/Controller/DiscussionFeedController.php
@@ -54,6 +54,8 @@
  */
 class DiscussionFeedController extends AbstractFeedController
 {
+    protected $routeName = 'discussion';
+
     public function __construct(Factory $view, ApiClient $api, TranslatorInterface $translator, SettingsRepositoryInterface $settings, UrlGenerator $url)
     {
         parent::__construct($view, $api, $translator, $settings, $url);
@@ -100,7 +102,8 @@ protected function getFeedContent(Request $request)
             $entries[] = [
                 'title'       => $discussion->attributes->title,
                 'content'     => $this->summarize($this->stripHTML($post->attributes->contentHtml)),
-                'permalink'   => $this->url->to('forum')->route('discussion', ['id' => $discussion->attributes->slug, 'near' => $post->attributes->number]),
+                'link'        => $this->url->to('forum')->route('discussion', ['id' => $discussion->attributes->slug, 'near' => $post->attributes->number]),
+                'id'          => $this->url->to('forum')->route('discussion', ['id' => $discussion->id, 'near' => $post->attributes->number]),
                 'pubdate'     => $this->parseDate($post->attributes->createdAt),
                 'author'      => isset($post->relationships->user) ? $this->getRelationship($posts, $post->relationships->user)->username : '[deleted]',
             ];
@@ -115,7 +118,7 @@ protected function getFeedContent(Request $request)
         return [
             'title'        => $this->translator->trans('ianm-syndication.forum.feeds.titles.discussion_title', ['{discussion_name}' => $discussion->attributes->title]),
             'description'  => $this->translator->trans('ianm-syndication.forum.feeds.titles.discussion_subtitle', ['{discussion_name}' => $discussion->attributes->title]),
-            'link'         => $this->url->to('forum')->route('discussion', ['id' => $discussion->id.'-'.$discussion->attributes->slug]),
+            'link'         => $this->url->to('forum')->route('discussion', ['id' => $discussion->attributes->slug]),
             'pubDate'      => new \DateTime(),
             'lastModified' => $lastModified,
             'entries'      => $entries,
@@ -153,4 +156,16 @@ protected function getPostsDocument(Request $request, User $actor, array $params
     {
         return $this->getAPIDocument($request, '/posts', $actor, $params);
     }
+
+    protected function getPermalink(array $queryParams, string $feedType): string
+    {
+        // Remove the slug from the Id if it is present.
+        // The Id of the feed should not change if the discussion is renamed.
+        $id = strstr($queryParams['id'], '-', true);
+        if ($id != false) {
+            $queryParams['id'] = $id;
+        }
+
+        return parent::getPermalink($queryParams, $feedType);
+    }
 }
diff --git a/src/Controller/DiscussionsActivityFeedController.php b/src/Controller/DiscussionsActivityFeedController.php
@@ -60,6 +60,8 @@ class DiscussionsActivityFeedController extends AbstractFeedController
      */
     private $lastTopics;
 
+    protected $routeName = 'global';
+
     /**
      * @param Factory             $view
      * @param ApiClient           $api
@@ -133,8 +135,8 @@ protected function getFeedContent(Request $request)
             $entries[] = [
                 'title'       => $discussion->attributes->title,
                 'content'     => $this->summarize($this->stripHTML($content->contentHtml)),
-                'id'          => $this->url->to('forum')->route('discussion', ['id' => $discussion->id.'-'.$discussion->attributes->slug]),
-                'permalink'   => $this->url->to('forum')->route('discussion', ['id' => $discussion->attributes->slug, 'near' => $content->number]),
+                'id'          => $this->url->to('forum')->route('discussion', ['id' => $discussion->id, 'near' => $content->number]),
+                'link'        => $this->url->to('forum')->route('discussion', ['id' => $discussion->attributes->slug, 'near' => $content->number]),
                 'pubdate'     => $this->parseDate($this->lastTopics ? $discussion->attributes->createdAt : $discussion->attributes->lastPostedAt),
                 'author'      => $author,
             ];

diff --git a/src/Controller/LastDiscussionsByTagFeedController.php b/src/Controller/LastDiscussionsByTagFeedController.php
@@ -55,6 +55,8 @@
  */
 class LastDiscussionsByTagFeedController extends TagsFeedController
 {
+    protected $routeName = 'tag_discussions';
+
     public function __construct(Factory $view, ApiClient $api, TranslatorInterface $translator, SettingsRepositoryInterface $settings, UrlGenerator $url, ExtensionManager $extensions, TagRepository $tagRepository)
     {
         parent::__construct($view, $api, $translator, $settings, $url, $extensions, $tagRepository, true);

diff --git a/src/Controller/LastDiscussionsFeedController.php b/src/Controller/LastDiscussionsFeedController.php
@@ -50,6 +50,8 @@
  */
 class LastDiscussionsFeedController extends DiscussionsActivityFeedController
 {
+    protected $routeName = 'discussions';
+
     public function __construct(Factory $view, ApiClient $api, TranslatorInterface $translator, SettingsRepositoryInterface $settings, UrlGenerator $url)
     {
         parent::__construct($view, $api, $translator, $settings, $url, true);

diff --git a/src/Controller/TagsFeedController.php b/src/Controller/TagsFeedController.php
@@ -55,6 +55,8 @@
  */
 class TagsFeedController extends DiscussionsActivityFeedController
 {
+    protected $routeName = 'tag';
+
     /**
      * @var TagRepository
      */

diff --git a/views/atom.blade.php b/views/atom.blade.php
@@ -6,14 +6,14 @@
     <subtitle><![CDATA[{!! $description !!}]]></subtitle>
     <link href="{{ $self_link }}" rel="self" />
     <link href="{{ $link }}/" />
-    <id><![CDATA[{!! $link !!}/]]></id>
+    <id>{{ $id }}</id>
     <updated>{{ $pubDate->format(DateTime::ATOM) }}</updated>
 
     @foreach ($entries as $entry)
     <entry>
         <title><![CDATA[{!! $entry['title'] !!}]]></title>
-        <link rel="alternate" type="text/html" href="{{ $entry['permalink'] }}"/>
-        <id>{{ $entry['permalink'] }}</id>
+        <link rel="alternate" type="text/html" href="{{ $entry['link'] }}"/>
+        <id>{{ $entry['id'] }}</id>
         <updated>{{ $entry['pubdate']->format(DateTime::ATOM) }}</updated>
         <content
         @if ($html)

diff --git a/views/rss.blade.php b/views/rss.blade.php
@@ -12,9 +12,9 @@
         @foreach ($entries as $entry)
         <item>
             <title><![CDATA[{!! $entry['title'] !!}]]></title>
-            <link>{{ $entry['permalink'] }}</link>
+            <link>{{ $entry['link'] }}</link>
             <description><![CDATA[{!! $entry['content'] !!}]]></description>
-            <guid>{{ $entry['permalink'] }}</guid>
+            <guid isPermaLink="true">{{ $entry['id'] }}</guid>
             <pubDate>{{ $entry['pubdate']->format(DateTime::RSS) }}</pubDate>
         </item>
         @endforeach