feat: calculate post numbers via subquery

On heavily volatile communities running on distributed systems, if two replies are posted at approximately the same time, each node will try to use the same post number, which results in a failed integrity constraint.

Potential options for fixing this were discussed in https://github.com/flarum/framework/issues/3350. We settled on calculating the post number via subquery during post instance creation, which should solve this issue since DB transactions are atomic.

To facilicate this, we needed to implement support for DB attributes that are calculated via subquery at insert, but otherwise stored and accessed normally. This PR adds a `InsertViaSubqueryTrait` which adds exactly this feature.

extensions/tags/src/Query/TagFilterGambit.php

@@ -55,23 +55,23 @@ class TagFilterGambit extends AbstractRegexGambit implements FilterInterface
     {
         $slugs = explode(',', trim($rawSlugs, '"'));
 
-        $query
-            ->distinct()
-            ->leftJoin('discussion_tag', 'discussions.id', '=', 'discussion_tag.discussion_id')
-            ->where(function (Builder $query) use ($slugs, $negate) {
-                foreach ($slugs as $slug) {
-                    if ($slug === 'untagged' && ! $negate) {
-                        $query->orWhereNull('discussion_tag.tag_id');
-                    } elseif ($slug === 'untagged' && $negate) {
-                        $query->orWhereNotNull('discussion_tag.tag_id');
-                    } elseif ($id = $this->tags->getIdForSlug($slug)) {
-                        $query->orWhere(
-                            'discussion_tag.tag_id',
-                            $negate ? '!=' : '=',
-                            $id
-                        );
-                    }
+        $query->where(function (Builder $query) use ($slugs, $negate) {
+            foreach ($slugs as $slug) {
+                if ($slug === 'untagged') {
+                    $query->whereIn('discussions.id', function (Builder $query) {
+                        $query->select('discussion_id')
+                            ->from('discussion_tag');
+                    }, 'or', ! $negate);
+                } else {
+                    $id = $this->tags->getIdForSlug($slug);
+
+                    $query->whereIn('discussions.id', function (Builder $query) use ($id) {
+                        $query->select('discussion_id')
+                            ->from('discussion_tag')
+                            ->where('tag_id', $id);
+                    }, 'or', $negate);
                 }
-            });
+            }
+        });
     }
 }

framework/core/js/src/forum/components/Post.tsx

@@ -132,7 +132,7 @@ export default abstract class Post<CustomAttrs extends IPostAttrs = IPostAttrs>
       classes.push('Post--by-actor');
     }
 
-    if (user && user === discussion.user()) {
+    if (user && user?.id() === discussion.attribute('startUserId')) {
       classes.push('Post--by-start-user');
     }
 

framework/core/src/Api/Controller/ShowDiscussionController.php

@@ -48,7 +48,6 @@ class ShowDiscussionController extends AbstractShowController
      * {@inheritdoc}
      */
     public $include = [
-        'user',
         'posts',
         'posts.discussion',
         'posts.user',

framework/core/src/Database/InsertsViaSubqueryTrait.php

@@ -0,0 +1,73 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Database;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\MySqlConnection;
+
+trait InsertsViaSubqueryTrait
+{
+    /**
+     * A list that maps attribute names to callables that produce subqueries
+     * used to calculate the inserted value.
+     *
+     * Each callable should take an instance of the model being saved,
+     * and return an Eloquent query builder that queries for the subquery
+     * generated value. The result of the query should be one row of one column.
+     *
+     * Subquery attributes should be added in the static `boot` method of models
+     * using this trait.
+     *
+     * @var array<string, callable(AbstractModel): Builder>
+     */
+    protected static $subqueryAttributes = [];
+
+    /**
+     * Overriden so that some fields can be inserted via subquery.
+     * The goal here is to construct a subquery that returns primitives
+     * for the provided `attributes`, and uses additional subqueries for
+     * statically-specified subqueryAttributes.
+     */
+    protected function insertAndSetId(Builder $query, $attributes)
+    {
+        $subqueryAttrNames = array_keys(static::$subqueryAttributes);
+
+        $literalAttributes = array_diff_key($attributes, array_flip($subqueryAttrNames));
+
+        /** @var Builder */
+        $insertRowSubquery = static::query()->limit(1);
+
+        foreach ($literalAttributes as $attrName => $value) {
+            $parameter = $query->getGrammar()->parameter($value);
+            $insertRowSubquery->addBinding($value, 'select');
+            $insertRowSubquery->selectRaw("$parameter as $attrName");
+        }
+
+        foreach (static::$subqueryAttributes as $attrName => $callback) {
+            $insertRowSubquery->selectSub($callback($this), $attrName);
+        }
+
+        $attrNames = array_merge(array_keys($literalAttributes), $subqueryAttrNames);
+        $query->insertUsing($attrNames, $insertRowSubquery);
+
+        // This should be accurate, as it's the same mechanism used by Laravel's `insertGetId`.
+        // See https://github.com/laravel/framework/blob/master/src/Illuminate/Database/Query/Processors/Processor.php#L30-L37.
+        /** @var MySqlConnection */
+        $con = $query->getQuery()->getConnection();
+        $idRaw = $con->getPdo()->lastInsertId($keyName = $this->getKeyName());
+        $id = is_numeric($idRaw) ? (int) $idRaw : $idRaw;
+
+        $this->setAttribute($keyName, $id);
+
+        // This is necessary to get the computed value of saved attributes.
+        $this->exists = true;
+        $this->refresh();
+    }
+}

framework/core/src/Post/Command/PostReplyHandler.php

@@ -74,7 +74,7 @@ class PostReplyHandler
 
         // If this is the first post in the discussion, it's technically not a
         // "reply", so we won't check for that permission.
-        if ($discussion->post_number_index > 0) {
+        if ($discussion->first_post_id !== null) {
             $actor->assertCan('reply', $discussion);
         }
 

framework/core/src/Post/Post.php

@@ -10,15 +10,14 @@
 namespace Flarum\Post;
 
 use Flarum\Database\AbstractModel;
+use Flarum\Database\InsertsViaSubqueryTrait;
 use Flarum\Database\ScopeVisibilityTrait;
 use Flarum\Discussion\Discussion;
 use Flarum\Foundation\EventGeneratorTrait;
 use Flarum\Notification\Notification;
 use Flarum\Post\Event\Deleted;
 use Flarum\User\User;
-use Illuminate\Database\ConnectionInterface;
 use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Database\Query\Expression;
 
 /**
  * @property int $id
@@ -43,6 +42,7 @@ class Post extends AbstractModel
 {
     use EventGeneratorTrait;
     use ScopeVisibilityTrait;
+    use InsertsViaSubqueryTrait;
 
     protected $table = 'posts';
 
@@ -93,19 +93,7 @@ class Post extends AbstractModel
         // discussion.
         static::creating(function (self $post) {
             $post->type = $post::$type;
-
-            /** @var ConnectionInterface $db */
-            $db = static::getConnectionResolver();
-            $post->number = new Expression('('.$db
-                    ->table('posts', 'pn')
-                    ->whereRaw($db->getTablePrefix().'pn.discussion_id = '.intval($post->discussion_id))
-                    ->select($db->raw('max('.$db->getTablePrefix().'pn.number) + 1'))
-                    ->toSql()
-                .')');
-        });
-
-        static::created(function (self $post) {
-            $post->refresh();
+            $post->number = ++$post->discussion->post_number_index;
             $post->discussion->save();
         });
 
@@ -116,6 +104,10 @@ class Post extends AbstractModel
         });
 
         static::addGlobalScope(new RegisteredTypesScope);
+
+        static::$subqueryAttributes['number'] = function (Post $post) {
+            return static::query()->where('discussion_id', $post->discussion_id)->selectRaw('max(number)+1');
+        };
     }
 
     /**