From 052c5cbc38210476ffce7d98dc052fa09b4e5e5f Mon Sep 17 00:00:00 2001
From: Ingo Schommer <me@chillu.com>
Date: Wed, 8 Apr 2020 13:58:02 +1200
Subject: [PATCH 1/2] BUG Infinite loops in TempDatabase (fixes #8902)

Ugly, but so is the original implementation that this works around (swallowing an exception to trigger functionality)
---
 src/ORM/Connect/TempDatabase.php | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php
index a47c50078..d88185ccf 100644
--- a/src/ORM/Connect/TempDatabase.php
+++ b/src/ORM/Connect/TempDatabase.php
@@ -24,6 +24,13 @@ class TempDatabase
      */
     protected $name = null;
 
+    /**
+     * Workaround to avoid infinite loops.
+     *
+     * @var Exception
+     */
+    private $skippedException = null;
+
     /**
      * Optionally remove the test DB when the PHP process exits
      *
@@ -293,6 +300,13 @@ class TempDatabase
         try {
             $this->rebuildTables($extraDataObjects);
         } catch (DatabaseException $ex) {
+            // Avoid infinite loops
+            if ($this->skippedException && $this->skippedException->getMessage() == $ex->getMessage()) {
+                throw $ex;
+            }
+
+            $this->skippedException = $ex;
+
             // In case of error during build force a hard reset
             // e.g. pgsql doesn't allow schema updates inside transactions
             $this->kill();

From a50e15e5eec7406e6034875ec9c3d8da6788daee Mon Sep 17 00:00:00 2001
From: Ingo Schommer <me@chillu.com>
Date: Thu, 9 Apr 2020 14:17:43 +1200
Subject: [PATCH 2/2] FIX Avoid VACUUM on test dbs in Postgres

The Postgres implementation was always faulty,
but the database exception was swallowed until
See https://github.com/silverstripe/silverstripe-framework/pull/9456.

Now that the the exception is only swallowed the first time,
the second recurrence will cause failing test execution.

This is a bit of an awkward fix, but the indirection "through" DataObject doesn't allow for anything else without changing public API surface.
The logic goes from TempDatabase to DBSchemaManager, then through the closure into DataObject->requireTable(),
then back into DBSchemaManager->requireTable(). And updateschema() is subclassed in SQLite3, making it difficult to add more arguments.

VACUUM is described as:

> VACUUM reclaims storage occupied by dead tuples. In normal PostgreSQL operation, tuples that are deleted or obsoleted by an update are not physically removed from their table; they remain present until a VACUUM is done. Therefore it's necessary to do VACUUM periodically, especially on frequently-updated tables.

https://www.postgresql.org/docs/9.1/sql-vacuum.html

Since test databases are short-lived, there's no reason to delete dead tuples, they'll be garbage collected when either the transaction is rolled back, or the database is destroyed after the test run.
---
 src/ORM/Connect/TempDatabase.php | 37 +++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/ORM/Connect/TempDatabase.php b/src/ORM/Connect/TempDatabase.php
index d88185ccf..208662250 100644
--- a/src/ORM/Connect/TempDatabase.php
+++ b/src/ORM/Connect/TempDatabase.php
@@ -239,29 +239,36 @@ class TempDatabase
         $dataClasses = ClassInfo::subclassesFor(DataObject::class);
         array_shift($dataClasses);
 
+        $oldCheckAndRepairOnBuild = Config::inst()->get(DBSchemaManager::class, 'check_and_repair_on_build');
+        Config::modify()->set(DBSchemaManager::class, 'check_and_repair_on_build', false);
+
         $schema = $this->getConn()->getSchemaManager();
         $schema->quiet();
-        $schema->schemaUpdate(function () use ($dataClasses, $extraDataObjects) {
-            foreach ($dataClasses as $dataClass) {
-                // Check if class exists before trying to instantiate - this sidesteps any manifest weirdness
-                if (class_exists($dataClass)) {
-                    $SNG = singleton($dataClass);
-                    if (!($SNG instanceof TestOnly)) {
-                        $SNG->requireTable();
+        $schema->schemaUpdate(
+            function () use ($dataClasses, $extraDataObjects) {
+                foreach ($dataClasses as $dataClass) {
+                    // Check if class exists before trying to instantiate - this sidesteps any manifest weirdness
+                    if (class_exists($dataClass)) {
+                        $SNG = singleton($dataClass);
+                        if (!($SNG instanceof TestOnly)) {
+                            $SNG->requireTable();
+                        }
                     }
                 }
-            }
 
-            // If we have additional dataobjects which need schema, do so here:
-            if ($extraDataObjects) {
-                foreach ($extraDataObjects as $dataClass) {
-                    $SNG = singleton($dataClass);
-                    if (singleton($dataClass) instanceof DataObject) {
-                        $SNG->requireTable();
+                // If we have additional dataobjects which need schema, do so here:
+                if ($extraDataObjects) {
+                    foreach ($extraDataObjects as $dataClass) {
+                        $SNG = singleton($dataClass);
+                        if (singleton($dataClass) instanceof DataObject) {
+                            $SNG->requireTable();
+                        }
                     }
                 }
             }
-        });
+        );
+
+        Config::modify()->set(DBSchemaManager::class, 'check_and_repair_on_build', $oldCheckAndRepairOnBuild);
 
         ClassInfo::reset_db_cache();
         DataObject::singleton()->flushCache();