From b17f26e6a8bf0391c3003b7beae2e533e2d31bab Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <sasha.skvortsov109@gmail.com>
Date: Wed, 23 Mar 2022 15:29:55 -0400
Subject: [PATCH] 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.
---
 .../src/Database/InsertsViaSubqueryTrait.php  | 73 +++++++++++++++++++
 .../src/Post/Command/PostReplyHandler.php     |  2 +-
 framework/core/src/Post/Post.php              |  6 ++
 3 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 framework/core/src/Database/InsertsViaSubqueryTrait.php

diff --git a/framework/core/src/Database/InsertsViaSubqueryTrait.php b/framework/core/src/Database/InsertsViaSubqueryTrait.php
new file mode 100644
index 000000000..78d57418c
--- /dev/null
+++ b/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();
+    }
+}
diff --git a/framework/core/src/Post/Command/PostReplyHandler.php b/framework/core/src/Post/Command/PostReplyHandler.php
index 40a52de7f..96de698d0 100644
--- a/framework/core/src/Post/Command/PostReplyHandler.php
+++ b/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);
         }
 
diff --git a/framework/core/src/Post/Post.php b/framework/core/src/Post/Post.php
index f9deff3d2..fae314772 100644
--- a/framework/core/src/Post/Post.php
+++ b/framework/core/src/Post/Post.php
@@ -10,6 +10,7 @@
 namespace Flarum\Post;
 
 use Flarum\Database\AbstractModel;
+use Flarum\Database\InsertsViaSubqueryTrait;
 use Flarum\Database\ScopeVisibilityTrait;
 use Flarum\Discussion\Discussion;
 use Flarum\Foundation\EventGeneratorTrait;
@@ -41,6 +42,7 @@ class Post extends AbstractModel
 {
     use EventGeneratorTrait;
     use ScopeVisibilityTrait;
+    use InsertsViaSubqueryTrait;
 
     protected $table = 'posts';
 
@@ -102,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');
+        };
     }
 
     /**