Migrate UserPreference to JSON storage

app/models/serializers/indifferent_hash_serializer.rb

@@ -0,0 +1,41 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+module Serializers
+  class IndifferentHashSerializer
+    def self.dump(hash)
+      hash
+    end
+
+    def self.load(value)
+      hash = value.is_a?(Hash) ? value : {}
+      hash.with_indifferent_access
+    end
+  end
+end

app/models/user_preference.rb

@@ -30,29 +30,43 @@
 
 class UserPreference < ApplicationRecord
   belongs_to :user
-  serialize :others
-
   delegate :notification_settings, to: :user
+  serialize :settings, ::Serializers::IndifferentHashSerializer
 
   validates_presence_of :user
   validate :time_zone_correctness, if: -> { time_zone.present? }
 
-  after_initialize :init_other_preferences
+  after_initialize :init_settings
 
-  def [](attr_name)
-    attribute?(attr_name) ? super : others[attr_name]
-  end
+  ##
+  # Retrieve keys from settings, and allow accessing
+  # as boolean with ? suffix
+  def method_missing(method_name, *args)
+    key = method_name.to_s
+    action = key[-1]
 
-  def []=(attr_name, value)
-    attribute?(attr_name) ? super : others[attr_name] = value
+    case action
+    when '?'
+      to_boolean settings[key[..-2]]
+    when '='
+      settings[key[..-2]] = args.first
+    else
+      settings[key]
+    end
   end
 
-  def comments_sorting
-    others.fetch(:comments_sorting, OpenProject::Configuration.default_comment_sort_order)
+  # Allow previous array-style accessing as well
+  # delegate :[], :[]=, to: :settings
+
+  ##
+  # We respond to all methods as we retrieve
+  # the key from settings
+  def respond_to_missing?(*)
+    true
   end
 
-  def comments_sorting=(order)
-    others[:comments_sorting] = order
+  def comments_sorting
+    settings.fetch(:comments_sorting, OpenProject::Configuration.default_comment_sort_order)
   end
 
   def comments_in_reverse_order?
@@ -60,20 +74,20 @@ class UserPreference < ApplicationRecord
   end
 
   def auto_hide_popups=(value)
-    others[:auto_hide_popups] = to_boolean(value)
+    settings[:auto_hide_popups] = to_boolean(value)
   end
 
   def auto_hide_popups?
-    others.fetch(:auto_hide_popups) { Setting.default_auto_hide_popups? }
+    settings.fetch(:auto_hide_popups) { Setting.default_auto_hide_popups? }
   end
 
   def warn_on_leaving_unsaved?
     # Need to cast here as previous values were '0' / '1'
-    to_boolean(others.fetch(:warn_on_leaving_unsaved) { true })
+    to_boolean(settings.fetch(:warn_on_leaving_unsaved, true))
   end
 
   def warn_on_leaving_unsaved=(value)
-    others[:warn_on_leaving_unsaved] = to_boolean(value)
+    settings[:warn_on_leaving_unsaved] = to_boolean(value)
   end
 
   # Provide an alias to form builders
@@ -86,7 +100,7 @@ class UserPreference < ApplicationRecord
   end
 
   def time_zone
-    self[:time_zone].presence || Setting.user_default_timezone.presence
+    super.presence || Setting.user_default_timezone.presence
   end
 
   def canonical_time_zone
@@ -98,17 +112,12 @@ class UserPreference < ApplicationRecord
 
   private
 
-  def attribute?(name)
-    attr = name.to_sym
-    has_attribute?(attr) || attr == :user || attr == :user_id
-  end
-
   def to_boolean(value)
     ActiveRecord::Type::Boolean.new.cast(value)
   end
 
-  def init_other_preferences
-    self.others ||= {}
+  def init_settings
+    self.settings ||= {}
   end
 
   def time_zone_correctness

db/migrate/20210615150558_aggregate_journals.rb

@@ -1,6 +1,9 @@
 require_relative './20200924085508_cleanup_orphaned_journal_data'
+require_relative './migration_utils/utils'
 
 class AggregateJournals < ActiveRecord::Migration[6.1]
+  include ::Migration::Utils
+
   def up
     [Attachment,
      Changeset,
@@ -25,7 +28,7 @@ class AggregateJournals < ActiveRecord::Migration[6.1]
   # The change is irreversible (aggregated journals cannot be broken down) but down will not cause database inconsistencies.
 
   def aggregate_journals(klass)
-    klass.in_batches(of: ENV["OPENPROJECT_MIGRATION_AGGREGATE_JOURNALS_BATCH_SIZE"]&.to_i || 1000) do |instances|
+    in_configurable_batches(klass) do |instances|
       # Instantiating is faster than calculating the aggregated journals multiple times.
       aggregated_journals = aggregated_journals_of(klass, instances).to_a
 

db/migrate/20210825183540_make_user_preferences_json.rb

@@ -0,0 +1,55 @@
+require_relative './migration_utils/utils'
+
+class MakeUserPreferencesJson < ActiveRecord::Migration[6.1]
+  include ::Migration::Utils
+
+  class UserPreferenceWithOthers < ::UserPreference
+    self.table_name = 'user_preferences'
+    serialize :others, Hash
+    serialize :settings, ::Serializers::IndifferentHashSerializer
+  end
+
+  def up
+    add_column :user_preferences, :settings, :jsonb
+
+    UserPreferenceWithOthers.reset_column_information
+    in_configurable_batches(UserPreferenceWithOthers).each_record do |pref|
+      migrate_yaml_to_json(pref)
+      pref.save!(validate: false)
+    end
+
+    change_table :user_preferences, bulk: true do |t|
+      t.remove :others, :hide_mail, :time_zone
+    end
+  end
+
+  def down
+    change_table :user_preferences, bulk: true do |t|
+      t.text :others
+      t.boolean :hide_mail, default: true
+      t.text :time_zone
+    end
+
+    UserPreferenceWithOthers.reset_column_information
+    in_configurable_batches(UserPreferenceWithOthers).each_record do |pref|
+      migrate_json_to_yaml(pref)
+      pref.save!(validate: false)
+    end
+
+    remove_column :user_preferences, :settings, :jsonb
+  end
+
+  private
+
+  def migrate_yaml_to_json(pref)
+    pref.settings = pref.others
+    pref.settings[:hide_mail] = pref.hide_mail
+    pref.settings[:time_zone] = pref.time_zone
+  end
+
+  def migrate_json_to_yaml(pref)
+    pref.others = pref.settings
+    pref.hide_mail = pref.settings[:hide_mail]
+    pref.time_zone = pref.settings[:time_zone]
+  end
+end

db/migrate/migration_utils/utils.rb

@@ -38,6 +38,12 @@ module Migration
       end
     end
 
+    def in_configurable_batches(klass, default_batch_size: 1000)
+      batches = ENV["OPENPROJECT_MIGRATION_AGGREGATE_JOURNALS_BATCH_SIZE"]&.to_i || default_batch_size
+
+      klass.in_batches(of: batches)
+    end
+
     def remove_index_if_exists(table_name, index_name)
       if index_name_exists? table_name, index_name
         remove_index table_name, name: index_name