From 212ee1580452b00d882bc0f031fc3d5436bd5588 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 23 Aug 2018 12:51:22 +0800
Subject: [PATCH] FIX: Create `BaseDropper` functions in a different schema.

https://meta.discourse.org/t/error-when-restore-db-backup/93145/25?u=tgxworld
---
 lib/backup_restore/restorer.rb                 | 18 ------------------
 lib/migration/base_dropper.rb                  | 15 ++++++++++++++-
 .../migration/column_dropper_spec.rb           | 16 ++++++++++------
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/lib/backup_restore/restorer.rb b/lib/backup_restore/restorer.rb
index 56d37480fe3..a546a3072c2 100644
--- a/lib/backup_restore/restorer.rb
+++ b/lib/backup_restore/restorer.rb
@@ -65,24 +65,6 @@ module BackupRestore
 
         BackupRestore.move_tables_between_schemas("public", "backup")
 
-        # This is a temp fix to allow restores to work again.
-        # @tgxworld is currently working on a fix that namespaces functions
-        # created by Discourse so that we can alter the schema of those
-        # functions before restoring.
-        %w{
-          raise_email_logs_reply_key_readonly
-          raise_email_logs_skipped_reason_readonly
-        }.each do |function|
-          begin
-            DB.exec(<<~SQL)
-              DROP FUNCTION IF EXISTS backup.#{function};
-              ALTER FUNCTION public.#{function} SET SCHEMA "backup";
-            SQL
-          rescue PG::UndefinedFunction
-            # the function does not exist, no need to worry about this
-          end
-        end
-
         @db_was_changed = true
         restore_dump
         migrate_database
diff --git a/lib/migration/base_dropper.rb b/lib/migration/base_dropper.rb
index 39fa11ac095..eff5fcdb1b5 100644
--- a/lib/migration/base_dropper.rb
+++ b/lib/migration/base_dropper.rb
@@ -1,5 +1,7 @@
 module Migration
   class BaseDropper
+    FUNCTION_SCHEMA_NAME = "discourse_functions".freeze
+
     def initialize(after_migration, delay, on_drop, after_drop)
       @after_migration = after_migration
       @on_drop = on_drop
@@ -46,6 +48,10 @@ module Migration
     end
 
     def self.create_readonly_function(table_name, column_name = nil)
+      DB.exec <<~SQL
+        CREATE SCHEMA IF NOT EXISTS #{FUNCTION_SCHEMA_NAME};
+      SQL
+
       message = column_name ?
                   "Discourse: #{column_name} in #{table_name} is readonly" :
                   "Discourse: #{table_name} is read only"
@@ -69,7 +75,14 @@ module Migration
     end
 
     def self.readonly_function_name(table_name, column_name = nil)
-      ["raise", table_name, column_name, "readonly()"].compact.join("_")
+      function_name = [
+        "raise",
+        table_name,
+        column_name,
+        "readonly()"
+      ].compact.join("_")
+
+      "#{FUNCTION_SCHEMA_NAME}.#{function_name}"
     end
 
     def self.readonly_trigger_name(table_name, column_name = nil)
diff --git a/spec/components/migration/column_dropper_spec.rb b/spec/components/migration/column_dropper_spec.rb
index 6dad4a4f7a6..4dc80556f87 100644
--- a/spec/components/migration/column_dropper_spec.rb
+++ b/spec/components/migration/column_dropper_spec.rb
@@ -152,19 +152,23 @@ RSpec.describe Migration::ColumnDropper do
       )
 
       expect(dropped_proc_called).to eq(true)
-
     end
+
     it 'should prevent updates to the readonly column' do
-      expect do
+      begin
         DB.exec <<~SQL
         UPDATE #{table_name}
         SET email = 'testing@email.com'
         WHERE topic_id = 1;
         SQL
-      end.to raise_error(
-        PG::RaiseException,
-        /Discourse: email in #{table_name} is readonly/
-      )
+      rescue PG::RaiseException => e
+        [
+          "Discourse: email in #{table_name} is readonly",
+          'discourse_functions.raise_table_with_readonly_column_email_readonly()'
+        ].each do |message|
+          expect(e.message).to include(message)
+        end
+      end
     end
 
     it 'should allow updates to the other columns' do