DEV: Move array type custom fields to JSON type in automation (#26939)

The automation plugin has 4 custom field types that are array typed. However, array typed custom fields are deprecated and should be migrated to JSON type.

This commit does a couple of things:

1. Migrate all four custom fields to JSON
2. Fix a couple of small bugs that have been discovered while migrating the custom fields to JSON (see the comments on this commit's PR for details #26939)

app/models/post_custom_field.rb

@@ -19,11 +19,10 @@ class PostCustomField < ActiveRecord::Base
 #
 # Indexes
 #
-#  idx_post_custom_fields_discourse_automation_unique_id_partial  (post_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text)
-#  index_post_custom_fields_on_name_and_value                     (name, "left"(value, 200))
-#  index_post_custom_fields_on_notice                             (post_id) UNIQUE WHERE ((name)::text = 'notice'::text)
-#  index_post_custom_fields_on_post_id                            (post_id) UNIQUE WHERE ((name)::text = 'missing uploads'::text)
-#  index_post_custom_fields_on_post_id_and_name                   (post_id,name)
-#  index_post_custom_fields_on_stalled_wiki_triggered_at          (post_id) UNIQUE WHERE ((name)::text = 'stalled_wiki_triggered_at'::text)
-#  index_post_id_where_missing_uploads_ignored                    (post_id) UNIQUE WHERE ((name)::text = 'missing uploads ignored'::text)
+#  index_post_custom_fields_on_name_and_value             (name, "left"(value, 200))
+#  index_post_custom_fields_on_notice                     (post_id) UNIQUE WHERE ((name)::text = 'notice'::text)
+#  index_post_custom_fields_on_post_id                    (post_id) UNIQUE WHERE ((name)::text = 'missing uploads'::text)
+#  index_post_custom_fields_on_post_id_and_name           (post_id,name)
+#  index_post_custom_fields_on_stalled_wiki_triggered_at  (post_id) UNIQUE WHERE ((name)::text = 'stalled_wiki_triggered_at'::text)
+#  index_post_id_where_missing_uploads_ignored            (post_id) UNIQUE WHERE ((name)::text = 'missing uploads ignored'::text)
 #

app/models/topic_custom_field.rb

@@ -19,8 +19,7 @@ class TopicCustomField < ActiveRecord::Base
 #
 # Indexes
 #
-#  idx_topic_custom_fields_auto_responder_triggered_ids_partial    (topic_id,value) UNIQUE WHERE ((name)::text = 'auto_responder_triggered_ids'::text)
-#  idx_topic_custom_fields_discourse_automation_unique_id_partial  (topic_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text)
-#  index_topic_custom_fields_on_topic_id_and_name                  (topic_id,name)
-#  topic_custom_fields_value_key_idx                               (value,name) WHERE ((value IS NOT NULL) AND (char_length(value) < 400))
+#  idx_topic_custom_fields_auto_responder_triggered_ids_partial  (topic_id,value) UNIQUE WHERE ((name)::text = 'auto_responder_triggered_ids'::text)
+#  index_topic_custom_fields_on_topic_id_and_name                (topic_id,name)
+#  topic_custom_fields_value_key_idx                             (value,name) WHERE ((value IS NOT NULL) AND (char_length(value) < 400))
 #

app/models/user_custom_field.rb

@@ -26,6 +26,5 @@ class UserCustomField < ActiveRecord::Base
 #
 # Indexes
 #
-#  idx_user_custom_fields_discourse_automation_unique_id_partial  (user_id,value) UNIQUE WHERE ((name)::text = 'discourse_automation_ids'::text)
-#  index_user_custom_fields_on_user_id_and_name                   (user_id,name)
+#  index_user_custom_fields_on_user_id_and_name  (user_id,name)
 #

plugins/automation/app/models/discourse_automation/automation.rb

@@ -34,41 +34,38 @@ def running_in_background!
     MAX_NAME_LENGTH = 30
     validates :name, length: { in: MIN_NAME_LENGTH..MAX_NAME_LENGTH }
 
-    def attach_custom_field(target)
+    def add_id_to_custom_field(target, custom_field_key)
       if ![Topic, Post, User].any? { |m| target.is_a?(m) }
         raise "Expected an instance of Topic/Post/User."
       end
 
-      now = Time.now
-      fk = target.custom_fields_fk
-      row = {
-        fk => target.id,
-        :name => DiscourseAutomation::CUSTOM_FIELD,
-        :value => id,
-        :created_at => now,
-        :updated_at => now,
-      }
-
-      relation = "#{target.class.name}CustomField".constantize
-      relation.upsert(
-        row,
-        unique_by:
-          "idx_#{target.class.name.downcase}_custom_fields_discourse_automation_unique_id_partial",
-      )
+      change_automation_ids_custom_field_in_mutex(target, custom_field_key) do
+        target.reload
+        ids = Array(target.custom_fields[custom_field_key])
+        if !ids.include?(self.id)
+          ids << self.id
+          ids = ids.compact.uniq
+          target.custom_fields[custom_field_key] = ids
+          target.save_custom_fields
+        end
+      end
     end
 
-    def detach_custom_field(target)
+    def remove_id_from_custom_field(target, custom_field_key)
       if ![Topic, Post, User].any? { |m| target.is_a?(m) }
         raise "Expected an instance of Topic/Post/User."
       end
 
-      fk = target.custom_fields_fk
-      relation = "#{target.class.name}CustomField".constantize
-      relation.where(
-        fk => target.id,
-        :name => DiscourseAutomation::CUSTOM_FIELD,
-        :value => id,
-      ).delete_all
+      change_automation_ids_custom_field_in_mutex(target, custom_field_key) do
+        target.reload
+        ids = Array(target.custom_fields[custom_field_key])
+        if ids.include?(self.id)
+          ids = ids.compact.uniq
+          ids.delete(self.id)
+          target.custom_fields[custom_field_key] = ids
+          target.save_custom_fields
+        end
+      end
     end
 
     def trigger_field(name)
@@ -187,5 +184,12 @@ def new_user_custom_field_name
     def validate_trigger_fields
       !triggerable || triggerable.valid?(self)
     end
+
+    def change_automation_ids_custom_field_in_mutex(target, key)
+      DistributedMutex.synchronize(
+        "automation_custom_field_#{key}_#{target.class.table_name}_#{target.id}",
+        validity: 5.seconds,
+      ) { yield }
+    end
   end
 end

plugins/automation/db/post_migrate/20240507112651_switch_topic_automation_custom_fields_to_json.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class SwitchTopicAutomationCustomFieldsToJson < ActiveRecord::Migration[7.0]
+  def up
+    results = DB.query(<<~SQL)
+      SELECT topic_id, ARRAY_AGG(value) AS values
+      FROM topic_custom_fields
+      WHERE name = 'discourse_automation_ids'
+      GROUP BY topic_id
+    SQL
+
+    execute(<<~SQL)
+      DELETE FROM topic_custom_fields
+      WHERE name = 'discourse_automation_ids'
+    SQL
+
+    results.each do |row|
+      parsed = row.values.map(&:to_i).uniq
+
+      DB.exec(<<~SQL, topic_id: row.topic_id, value: parsed.to_json)
+        INSERT INTO topic_custom_fields
+        (topic_id, name, value, created_at, updated_at)
+        VALUES
+        (:topic_id, 'discourse_automation_ids_json', :value, NOW(), NOW())
+      SQL
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugins/automation/db/post_migrate/20240507112751_switch_user_automation_custom_fields_to_json.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class SwitchUserAutomationCustomFieldsToJson < ActiveRecord::Migration[7.0]
+  def up
+    results = DB.query(<<~SQL)
+      SELECT user_id, ARRAY_AGG(value) AS values
+      FROM user_custom_fields
+      WHERE name = 'discourse_automation_ids'
+      GROUP BY user_id
+    SQL
+
+    execute(<<~SQL)
+      DELETE FROM user_custom_fields
+      WHERE name = 'discourse_automation_ids'
+    SQL
+
+    results.each do |row|
+      parsed = row.values.map(&:to_i).uniq
+
+      DB.exec(<<~SQL, user_id: row.user_id, value: parsed.to_json)
+        INSERT INTO user_custom_fields
+        (user_id, name, value, created_at, updated_at)
+        VALUES
+        (:user_id, 'discourse_automation_ids_json', :value, NOW(), NOW())
+      SQL
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugins/automation/db/post_migrate/20240507112851_switch_topic_automation_triggered_ids_custom_fields_to_json.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class SwitchTopicAutomationTriggeredIdsCustomFieldsToJson < ActiveRecord::Migration[7.0]
+  def up
+    results = DB.query(<<~SQL)
+      SELECT topic_id, ARRAY_AGG(value) AS values
+      FROM topic_custom_fields
+      WHERE name = 'auto_responder_triggered_ids'
+      GROUP BY topic_id
+    SQL
+
+    execute(<<~SQL)
+      DELETE FROM topic_custom_fields
+      WHERE name = 'auto_responder_triggered_ids'
+    SQL
+
+    results.each do |row|
+      parsed = row.values.map(&:to_i).uniq
+
+      DB.exec(<<~SQL, topic_id: row.topic_id, value: parsed.to_json)
+        INSERT INTO topic_custom_fields
+        (topic_id, name, value, created_at, updated_at)
+        VALUES
+        (:topic_id, 'auto_responder_triggered_ids_json', :value, NOW(), NOW())
+      SQL
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugins/automation/db/post_migrate/20240507112951_drop_automation_ids_custom_field_indexes.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class DropAutomationIdsCustomFieldIndexes < ActiveRecord::Migration[7.0]
+  def change
+    remove_index :topic_custom_fields,
+                 name: :idx_topic_custom_fields_discourse_automation_unique_id_partial,
+                 if_exists: true
+    remove_index :user_custom_fields,
+                 name: :idx_user_custom_fields_discourse_automation_unique_id_partial,
+                 if_exists: true
+    remove_index :post_custom_fields,
+                 name: :idx_post_custom_fields_discourse_automation_unique_id_partial,
+                 if_exists: true
+  end
+end

plugins/automation/lib/discourse_automation/event_handlers.rb

@@ -75,7 +75,9 @@ def self.handle_user_updated(user, new_user: false)
         .find_each do |automation|
           once_per_user = automation.trigger_field("once_per_user")["value"]
           if once_per_user &&
-               UserCustomField.exists?(name: DiscourseAutomation::CUSTOM_FIELD, user_id: user.id)
+               user.custom_fields[
+                 DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD
+               ].presence&.include?(automation.id)
             next
           end
 
@@ -135,7 +137,8 @@ def self.handle_user_updated(user, new_user: false)
             user.save_custom_fields
           end
 
-          automation.attach_custom_field(user)
+          automation.add_id_to_custom_field(user, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD)
+
           automation.trigger!("kind" => name, "user" => user, "user_data" => user_data)
         end
     end

plugins/automation/lib/discourse_automation/post_extension.rb

@@ -10,9 +10,9 @@ def discourse_automation_topic_required_words
       return if !SiteSetting.discourse_automation_enabled
       return if self.post_type == Post.types[:small_action]
       return if !topic
-      return if topic.custom_fields[DiscourseAutomation::CUSTOM_FIELD].blank?
+      return if topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD].blank?
 
-      topic.custom_fields[DiscourseAutomation::CUSTOM_FIELD].each do |automation_id|
+      topic.custom_fields[DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD].each do |automation_id|
         automation = DiscourseAutomation::Automation.find_by(id: automation_id)
         next if automation&.script != DiscourseAutomation::Scripts::TOPIC_REQUIRED_WORDS
 

plugins/automation/lib/discourse_automation/scripts/auto_responder.rb

@@ -70,9 +70,7 @@
         end
         .join("\n\n")
 
-    value = (Array(post.topic.custom_fields[key]) << automation.id).compact.uniq
-    post.topic.custom_fields[key] = value
-    post.topic.save_custom_fields
+    automation.add_id_to_custom_field(post.topic, key)
 
     PostCreator.create!(
       answering_user,

plugins/automation/lib/discourse_automation/triggers/topic.rb

@@ -12,17 +12,19 @@
         previous_topic = Topic.find_by(id: previous_topic_id)
 
         if previous_topic
-          TopicCustomField.where(
-            topic_id: previous_topic_id,
-            name: DiscourseAutomation::CUSTOM_FIELD,
-            value: automation.id,
-          ).delete_all
+          automation.remove_id_from_custom_field(
+            previous_topic,
+            DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD,
+          )
         end
       end
 
       if topic_id
         topic = Topic.find_by(id: topic_id)
-        topic&.upsert_custom_fields(DiscourseAutomation::CUSTOM_FIELD => automation.id)
+
+        next if !topic
+
+        automation.add_id_to_custom_field(topic, DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD)
       end
     end
   end

plugins/automation/plugin.rb

@@ -14,7 +14,7 @@
 module ::DiscourseAutomation
   PLUGIN_NAME = "automation"
 
-  CUSTOM_FIELD = "discourse_automation_ids"
+  AUTOMATION_IDS_CUSTOM_FIELD = "discourse_automation_ids_json"
   TOPIC_LAST_CHECKED_BY = "discourse_automation_last_checked_by"
   TOPIC_LAST_CHECKED_AT = "discourse_automation_last_checked_at"
 
@@ -26,7 +26,7 @@ module ::DiscourseAutomation
     { id: "TL34", name: "discourse_automation.triggerables.user_promoted.trust_levels.TL34" },
   ]
 
-  AUTO_RESPONDER_TRIGGERED_IDS = "auto_responder_triggered_ids"
+  AUTO_RESPONDER_TRIGGERED_IDS = "auto_responder_triggered_ids_json"
   USER_GROUP_MEMBERSHIP_THROUGH_BADGE_BULK_MODIFY_START_COUNT = 1000
 
   def self.set_active_automation(id)
@@ -203,15 +203,15 @@ def self.get_active_automation
 
   on(:post_created) { |post| DiscourseAutomation::EventHandlers.handle_stalled_topic(post) }
 
-  register_topic_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer])
-  register_topic_custom_field_type(DiscourseAutomation::AUTO_RESPONDER_TRIGGERED_IDS, [:integer])
+  register_topic_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json)
+  register_topic_custom_field_type(DiscourseAutomation::AUTO_RESPONDER_TRIGGERED_IDS, :json)
 
   on(:user_updated) { |user| DiscourseAutomation::EventHandlers.handle_user_updated(user) }
   on(:user_created) do |user|
     DiscourseAutomation::EventHandlers.handle_user_updated(user, new_user: true)
   end
 
-  register_user_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer])
-  register_post_custom_field_type(DiscourseAutomation::CUSTOM_FIELD, [:integer])
+  register_user_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json)
+  register_post_custom_field_type(DiscourseAutomation::AUTOMATION_IDS_CUSTOM_FIELD, :json)
   register_post_custom_field_type("stalled_wiki_triggered_at", :string)
 end