Revert "Revert prevent race condition in plugin settings #15249 (#15325)" (#15328)

This reverts commit 520ba4881ab9d3e30c3d16da9e32a74633e6653c.

3 files changed, 90 insertions, 56 deletions

diff --git a/core/Settings/Storage/Backend/BaseSettingsTable.php b/core/Settings/Storage/Backend/BaseSettingsTable.php
new file mode 100644
index 0000000000..3b22fefc91
--- /dev/null
+++ b/core/Settings/Storage/Backend/BaseSettingsTable.php
@@ -0,0 +1,41 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Settings\Storage\Backend;
+
+use Piwik\Concurrency\Lock;
+use Piwik\Container\StaticContainer;
+use Piwik\Db;
+
+abstract class BaseSettingsTable implements BackendInterface
+{
+    /**
+     * @var Db\AdapterInterface
+     */
+    protected $db;
+
+    /** @var Lock */
+    protected $lock;
+
+    public function __construct()
+    {
+        $this->lock = StaticContainer::getContainer()->make(
+            Lock::class,
+            array ('lockKeyStart' => 'PluginSettingsTable')
+        );
+    }
+
+    protected function initDbIfNeeded()
+    {
+        if (!isset($this->db)) {
+            // we do not want to create a db connection on backend creation
+            $this->db = Db::get();
+        }
+    }
+
+}
\ No newline at end of file
diff --git a/core/Settings/Storage/Backend/MeasurableSettingsTable.php b/core/Settings/Storage/Backend/MeasurableSettingsTable.php
index e635f331bb..77c56150fc 100644
--- a/core/Settings/Storage/Backend/MeasurableSettingsTable.php
+++ b/core/Settings/Storage/Backend/MeasurableSettingsTable.php
@@ -10,6 +10,8 @@
 namespace Piwik\Settings\Storage\Backend;
 
 use Piwik\Common;
+use Piwik\Concurrency\Lock;
+use Piwik\Container\StaticContainer;
 use Piwik\Db;
 use Exception;
 use Piwik\Version;
@@ -19,7 +21,7 @@ use Piwik\Version;
  *
  * If a value that needs to be stored is an array, will insert a new row for each value of this array.
  */
-class MeasurableSettingsTable implements BackendInterface
+class MeasurableSettingsTable extends BaseSettingsTable
 {
     /**
      * @var int
@@ -31,13 +33,10 @@ class MeasurableSettingsTable implements BackendInterface
      */
     private $pluginName;
 
-    /**
-     * @var Db\AdapterInterface
-     */
-    private $db;
-
     public function __construct($idSite, $pluginName)
     {
+        parent::__construct();
+
         if (empty($pluginName)) {
             throw new Exception('No plugin name given for MeasurableSettingsTable backend');
         }
@@ -50,39 +49,19 @@ class MeasurableSettingsTable implements BackendInterface
         $this->pluginName = $pluginName;
     }
 
-    private function initDbIfNeeded()
-    {
-        if (!isset($this->db)) {
-            // we need to avoid db creation on instance creation, especially important in tracker mode
-            // the db might be never actually used when values are eg fetched from a cache
-            $this->db = Db::get();
-        }
-    }
-
-    /**
-     * @inheritdoc
-     */
-    public function getStorageId()
-    {
-        return 'MeasurableSettings_' . $this->idSite . '_' . $this->pluginName;
-    }
-
     /**
      * Saves (persists) the current setting values in the database.
+     * @param array $values Key/value pairs of setting values to be written
      */
     public function save($values)
     {
         $this->initDbIfNeeded();
 
-        $table = $this->getTableName();
-
-        $this->delete();
-
+        $valuesKeep = array();
         foreach ($values as $name => $value) {
             if (!isset($value)) {
                 continue;
             }
-
             if (is_array($value) || is_object($value)) {
                 $jsonEncoded = 1;
                 $value = json_encode($value);
@@ -94,12 +73,28 @@ class MeasurableSettingsTable implements BackendInterface
                     $value = (int) $value;
                 }
             }
+            $valuesKeep[] = array($this->idSite, $this->pluginName, $name, $value, $jsonEncoded);
+        }
 
-            $sql  = "INSERT INTO $table (`idsite`, `plugin_name`, `setting_name`, `setting_value`, `json_encoded`) VALUES (?, ?, ?, ?, ?)";
-            $bind = array($this->idSite, $this->pluginName, $name, $value, $jsonEncoded);
+        $columns = array('idsite', 'plugin_name', 'setting_name', 'setting_value', 'json_encoded');
 
-            $this->db->query($sql, $bind);
-        }
+        $table = $this->getTableName();
+        $lockKey = $this->getStorageId();
+        $this->lock->execute($lockKey, function() use ($valuesKeep, $table, $columns) {
+            $this->delete();
+            // No values = nothing to save
+            if (!empty($valuesKeep)) {
+                Db\BatchInsert::tableInsertBatchSql($table, $columns, $valuesKeep, true);
+            }
+        });
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function getStorageId()
+    {
+        return 'MeasurableSettings_' . $this->idSite . '_' . $this->pluginName;
     }
 
     private function jsonEncodedMissingError(Exception $e)
@@ -149,7 +144,7 @@ class MeasurableSettingsTable implements BackendInterface
         return $flat;
     }
 
-    private function getTableName()
+    protected function getTableName()
     {
         return Common::prefixTable('site_setting');
     }
diff --git a/core/Settings/Storage/Backend/PluginSettingsTable.php b/core/Settings/Storage/Backend/PluginSettingsTable.php
index d12b15cf2f..fddd60fc36 100644
--- a/core/Settings/Storage/Backend/PluginSettingsTable.php
+++ b/core/Settings/Storage/Backend/PluginSettingsTable.php
@@ -10,6 +10,8 @@
 namespace Piwik\Settings\Storage\Backend;
 
 use Piwik\Common;
+use Piwik\Concurrency\Lock;
+use Piwik\Container\StaticContainer;
 use Piwik\Db;
 use Exception;
 use Piwik\Version;
@@ -19,7 +21,7 @@ use Piwik\Version;
  *
  * If a value that needs to be stored is an array, will insert a new row for each value of this array.
  */
-class PluginSettingsTable implements BackendInterface
+class PluginSettingsTable extends BaseSettingsTable
 {
     /**
      * @var string
@@ -31,13 +33,10 @@ class PluginSettingsTable implements BackendInterface
      */
     private $userLogin;
 
-    /**
-     * @var Db\AdapterInterface
-     */
-    private $db;
-
     public function __construct($pluginName, $userLogin)
     {
+        parent::__construct();
+
         if (empty($pluginName)) {
             throw new Exception('No plugin name given for PluginSettingsTable backend');
         }
@@ -50,14 +49,6 @@ class PluginSettingsTable implements BackendInterface
         $this->userLogin = $userLogin;
     }
 
-    private function initDbIfNeeded()
-    {
-        if (!isset($this->db)) {
-            // we do not want to create a db connection on backend creation
-            $this->db = Db::get();
-        }
-    }
-
     /**
      * @inheritdoc
      */
@@ -68,20 +59,18 @@ class PluginSettingsTable implements BackendInterface
 
     /**
      * Saves (persists) the current setting values in the database.
+     * @param array $values Key/value pairs of setting values to be written
      */
     public function save($values)
     {
         $this->initDbIfNeeded();
 
-        $table = $this->getTableName();
-
-        $this->delete();
+        $valuesKeep = array();
 
         foreach ($values as $name => $value) {
             if (!isset($value)) {
                 continue;
             }
-
             if (is_array($value) || is_object($value)) {
                 $jsonEncoded = 1;
                 $value = json_encode($value);
@@ -94,11 +83,20 @@ class PluginSettingsTable implements BackendInterface
                 }
             }
 
-            $sql  = "INSERT INTO $table (`plugin_name`, `user_login`, `setting_name`, `setting_value`, `json_encoded`) VALUES (?, ?, ?, ?, ?)";
-            $bind = array($this->pluginName, $this->userLogin, $name, $value, $jsonEncoded);
-
-            $this->db->query($sql, $bind);
+            $valuesKeep[] = array($this->pluginName, $this->userLogin, $name, $value, $jsonEncoded);
         }
+
+        $columns = array('plugin_name', 'user_login', 'setting_name', 'setting_value', 'json_encoded');
+
+        $table = $this->getTableName();
+        $lockKey = $this->getStorageId();
+        $this->lock->execute($lockKey, function() use ($valuesKeep, $table, $columns) {
+            $this->delete();
+            // No values = nothing to save
+            if (!empty($valuesKeep)) {
+                Db\BatchInsert::tableInsertBatchSql($table, $columns, $valuesKeep);
+            }
+        });
     }
 
     private function jsonEncodedMissingError(Exception $e)
@@ -145,7 +143,7 @@ class PluginSettingsTable implements BackendInterface
         return $flat;
     }
 
-    private function getTableName()
+    protected function getTableName()
     {
         return Common::prefixTable('plugin_setting');
     }