Merge pull request #3431 from oliverguenther/fix/syck_migration

Remove hard syck dependency, make it optional instead

.travis.yml

@@ -58,6 +58,8 @@ env:
     - "TEST_SUITE=spec:legacy DB=postgres"
 
 before_install:
+  # TODO: Remove when bundler 1.10 is available on travis per default
+  - "gem install bundler"
   - "echo `firefox -v`"
   - "export DISPLAY=:99.0"
   - "/sbin/start-stop-daemon --start -v --pidfile ./tmp/custom_xvfb_99.pid --make-pidfile --background --exec /usr/bin/Xvfb -- :99 -ac -screen 0 1920x1080x16"

Gemfile

@@ -88,8 +88,6 @@ gem 'rack-protection', :git => "https://github.com/finnlabs/rack-protection.git"
 # https://github.com/kickstarter/rack-attack
 gem 'rack-attack'
 
-gem 'syck', :platforms => [:mri, :mingw, :x64_mingw], :require => false
-
 gem 'gon', '~> 4.0'
 
 # catch exceptions and send them to any airbrake compatible backend
@@ -183,6 +181,10 @@ group :ldap do
   gem "net-ldap", '~> 0.8.0'
 end
 
+group :syck, optional: true do
+  gem "syck", require: false
+end
+
 group :development do
   gem 'letter_opener', '~> 1.3.0'
   gem 'rails-dev-tweaks', '~> 0.6.1'

Gemfile.lock

@@ -452,7 +452,7 @@ GEM
       railties (~> 3.0)
     structured_warnings (0.1.4)
     svg-graph (1.0.5)
-    syck (1.0.1)
+    syck (1.0.5)
     thin (1.5.1)
       daemons (>= 1.0.9)
       eventmachine (>= 0.12.6)

db/migrate/20130612120042_migrate_serialized_yaml_from_syck_to_psych.rb

@@ -27,28 +27,50 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
-require_relative 'migration_utils/yaml_migrator'
+require_relative 'migration_utils/legacy_yamler'
 
 class MigrateSerializedYamlFromSyckToPsych < ActiveRecord::Migration
-  include Migration::YamlMigrator
+  include Migration::LegacyYamler
 
   def up
-    migrate_yaml_columns('syck', 'psych')
+    %w(filters column_names sort_criteria).each do |column|
+      migrate_to_psych('queries', column)
+    end
+
+    migrate_to_psych('custom_field_translations', 'possible_values')
+    migrate_to_psych('roles', 'permissions')
+    migrate_to_psych('settings', 'value')
+    migrate_to_psych('timelines', 'options')
+    migrate_to_psych('user_preferences', 'others')
+    migrate_to_psych('wiki_menu_items', 'options')
   end
 
   def down
-    migrate_yaml_columns('psych', 'syck')
+    puts 'YAML data serialized with Psych is still compatible with Syck. Skipping migration.'
   end
 
-  def migrate_yaml_columns(source_yamler, target_yamler)
-    ['filters', 'column_names', 'sort_criteria'].each do |column|
-      migrate_yaml('queries', column, source_yamler, target_yamler)
+  private
+
+  def migrate_to_psych(table, column)
+    table_name = ActiveRecord::Base.connection.quote_table_name(table)
+    column_name = ActiveRecord::Base.connection.quote_column_name(column)
+
+    fetch_data(table_name, column_name).each do |row|
+      transformed = ::Psych.dump(load_with_sych(row[column]))
+
+      ActiveRecord::Base.connection.execute <<-SQL
+        UPDATE #{table_name}
+        SET #{column_name} = #{ActiveRecord::Base.connection.quote(transformed)}
+        WHERE id = #{row['id']};
+      SQL
     end
-    migrate_yaml('custom_field_translations', 'possible_values', source_yamler, target_yamler)
-    migrate_yaml('roles', 'permissions', source_yamler, target_yamler)
-    migrate_yaml('settings', 'value', source_yamler, target_yamler)
-    migrate_yaml('timelines', 'options', source_yamler, target_yamler)
-    migrate_yaml('user_preferences', 'others', source_yamler, target_yamler)
-    migrate_yaml('wiki_menu_items', 'options', source_yamler, target_yamler)
+  end
+
+  def fetch_data(table_name, column_name)
+    ActiveRecord::Base.connection.select_all <<-SQL
+      SELECT id, #{column_name}
+      FROM #{table_name}
+      WHERE #{column_name} LIKE '---%'
+    SQL
   end
 end

db/migrate/migration_utils/legacy_journal_migrator.rb

@@ -29,7 +29,7 @@
 
 require_relative 'db_worker'
 require_relative 'legacy_table_checker'
-require 'syck'
+require_relative 'legacy_yamler'
 
 module Migration
   class IncompleteJournalsError < ::StandardError
@@ -41,6 +41,7 @@ module Migration
   class LegacyJournalMigrator
     include DbWorker
     include LegacyTableChecker
+    include LegacyYamler
 
     attr_accessor :table_name,
                   :type,
@@ -274,17 +275,7 @@ module Migration
     def deserialize_changed_data(journal)
       changed_data = journal['changed_data']
       return Hash.new if changed_data.nil?
-
-      current_yamler = YAML::ENGINE.yamler || 'psych'
-      begin
-        # The change to 'syck' ensures that legacy data is correctly read from
-        # the 'legacy_journals' table. Otherwise, we would end up with false
-        # encoded data in the new journal.
-        YAML::ENGINE.yamler = 'syck'
-        YAML.load(changed_data)
-      ensure
-        YAML::ENGINE.yamler = current_yamler
-      end
+      load_with_sych(changed_data)
     end
 
     def deserialize_journal(journal)

db/migrate/migration_utils/legacy_yamler.rb

@@ -28,40 +28,37 @@
 #++
 
 require_relative 'db_worker'
-require 'syck'
 
 module Migration
-  module YamlMigrator
-    include DbWorker
-
-    def migrate_yaml(table, column, source_yamler, target_yamler)
-      current_yamler = YAML::ENGINE.yamler
-      fetch_data(table, column).each do | data |
-        db_execute <<-SQL
-          UPDATE #{quoted_table_name(table)}
-          SET #{db_column(column)} = #{quote_value(yaml_to_yaml(data[column], source_yamler, target_yamler))}
-          WHERE id = #{data['id']};
-        SQL
-      end
-    ensure
-      # psych is the default starting at ruby 1.9.3, so we explicitely set it here
-      # in case no yamler was set to return to a sensible default
-      YAML::ENGINE.yamler = current_yamler.present? ? current_yamler : 'psych'
+  module LegacyYamler
+    ##
+    # Tries to load syck and fails with an error
+    # if it was not installed.
+    # To continue with the affected migrations, install syck with `bundle install --with syck`
+    def load_with_syck(yaml)
+      @@syck ||= load_syck
+      @@syck.load(yaml)
     end
 
-    def fetch_data(table, column)
-      ActiveRecord::Base.connection.select_all <<-SQL
-        SELECT #{db_column('id')}, #{db_column(column)}
-        FROM #{quoted_table_name(table)}
-        WHERE #{db_column(column)} LIKE #{quote_value('---%')}
-      SQL
-    end
+    private
+
+    def load_syck
+      require 'syck'
+      ::Syck
+    rescue LoadError => e
+      abort = -> (str) { abort("\e[31m#{str}\e[0m") }
+      abort.call <<-WARN
+      It appears you have existing serialized YAML in your database.
+
+      This YAML may have been serialized with Syck, which allowed to parse YAML
+      that is now considered invalid given the default Ruby YAML parser (Psych),
+      we need to convert that YAML to be Psych-compatible.
+
+      Use `bundle install --with syck` to install the syck YAML parser
+      and re-run the migrations.
+      WARN
 
-    def yaml_to_yaml(data, source_yamler, target_yamler)
-      YAML::ENGINE.yamler = source_yamler
-      original = YAML.load(data)
-      YAML::ENGINE.yamler = target_yamler
-      YAML.dump original
+      raise e
     end
   end
 end