fixes for ruby 1.9.3

* gemfile tweaks
 * removed ruby-debug (since it's broken in 1.9.3)
 * removed colons from case statements
 * turned off whiny nils for tests (they cause a 2X perf hit)
 * added utf-8 encoding markers to files with multibyte chars
 * removed an instance of calling String#map, which no longer works
 * fixed an issue in the assets file where the yaml emitter doesn't output the
   same whitespace as it did in 1.8.7
 * fix call to .map without block
 * fix yaml engine initialization for delayed jobs (was happening too late)
 * fix rspec instafail
 * fix UserProfile#id calls
 * fix ModelCache for instance_methods now returning symbols
 * fix user_spec collection not seeing the new objects
 * fix course specs where POST lines are slightly different in 1.9
 * fix utc_datetime in the time initializer

Change-Id: Ic95dda23cb910579e2828fb448323d4fc18902a2
Reviewed-on: https://gerrit.instructure.com/10705
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Zach Wily <zach@instructure.com>

Gemfile

@@ -20,7 +20,6 @@ if !ONE_NINE
 end
 gem 'hashery',        '1.3.0',  :require => 'hashery/dictionary'
 gem 'highline',       '1.6.1'
-gem 'hpricot',        '0.8.2'
 gem 'i18n',           '0.6.0'
 gem 'icalendar',      '1.1.5'
 gem 'jammit',         '0.6.0'
@@ -40,7 +39,9 @@ gem 'rack',           '1.1.3'
 gem 'rake',           '< 0.10'
 gem 'rdoc',           '3.12'
 gem 'ratom-instructure', '0.6.9', :require => "atom" # custom gem until necessary changes are merged into mainstream
-gem 'rbx-require-relative', '0.0.5'
+if !ONE_NINE
+  gem 'rbx-require-relative', '0.0.5'
+end
 gem 'rdiscount',      '1.6.8'
 gem 'require_relative', '1.0.1'
 gem 'ritex',          '1.0.1'
@@ -96,7 +97,9 @@ group :development do
   gem 'coffee-script-source',  '1.3.1' #pinned so everyone's compiled output matches
   gem 'parallel',     '0.5.16'
   if ONE_NINE
-    gem 'ruby-debug19', '0.11.6'
+    # ruby-debug is broken in 1.9.3:
+    # http://blog.wyeworks.com/2011/11/1/ruby-1-9-3-and-ruby-debug
+    # gem 'ruby-debug19', '0.11.6'
   else
     gem 'ruby-debug',   '0.10.4'
   end

app/models/submission.rb

@@ -163,9 +163,9 @@ class Submission < ActiveRecord::Base
       turnitin_data &&
       (assignment.cached_context_grants_right?(user, session, :manage_grades) ||
         case assignment.turnitin_settings[:originality_report_visibility]
-          when 'immediate': true
-          when 'after_grading': current_submission_graded?
-          when 'after_due_date': assignment.due_at && assignment.due_at < Time.now.utc
+          when 'immediate'; true
+          when 'after_grading'; current_submission_graded?
+          when 'after_due_date'; assignment.due_at && assignment.due_at < Time.now.utc
         end
       )
     }

app/models/user.rb

@@ -803,7 +803,7 @@ class User < ActiveRecord::Base
     end
     updates['submission_comments'] = 'author_id'
     updates['conversation_messages'] = 'author_id'
-    updates = updates.map
+    updates = updates.to_a
     updates << ['enrollments', 'associated_user_id']
     updates.each do |table, column|
       begin

app/models/user_profile.rb

@@ -21,23 +21,9 @@ class UserProfile
   def initialize(user)
     @user = user
   end
-  
-  def short_name
-    @user.short_name
-  end
-  
-  def name
-    @user.name
-  end
-  
-  def asset_string
-    @user.asset_string
-  end
-  
-  def opaque_identifier(*args)
-    @user.opaque_identifier(*args)
-  end
-  
+
+  delegate :id, :short_name, :name, :asset_string, :opaque_identifier, :to => :@user
+
   TAB_PROFILE = 0
   TAB_COMMUNICATION_PREFERENCES = 1
   TAB_FILES = 2

config/assets.yml

@@ -61,7 +61,7 @@ css_compressor_options:
     end
     plugin_assets[plugin] = assets
   end
-  subdoc = YAML.dump('plugins' => plugin_assets).gsub(/^--- \n/, '')
+  subdoc = YAML.dump('plugins' => plugin_assets).gsub(/^---\s?\n/, '')
 
   # add anchors to the various bundles in the imported plugin asset definitions.
   # these bundles will be included in the known bundle types below with a

config/environments/development.rb

@@ -40,4 +40,7 @@ config.cache_store = Canvas.cache_store_config
 Dir[File.dirname(__FILE__) + "/" + File.basename(__FILE__, ".rb") + "-*.rb"].each { |localfile| eval(File.new(localfile).read) }
 
 # allow debugging only in development environment by default
-require "ruby-debug"
+# ruby-debug is currently broken in 1.9.3
+if RUBY_VERSION < "1.9.3"
+  require "ruby-debug"
+end

config/environments/test.rb

@@ -7,7 +7,12 @@
 config.cache_classes = true
 
 # Log error messages when you accidentally call methods on nil.
-config.whiny_nils = true
+if RUBY_VERSION >= "1.9."
+  # in 1.9, whiny_nils causes a huge performance penalty on tests for some reason
+  config.whiny_nils = false
+else
+  config.whiny_nils = true
+end
 
 # Show full error reports and disable caching
 config.action_controller.consider_all_requests_local = true

config/initializers/active_record.rb

@@ -320,7 +320,7 @@ class ActiveRecord::Base
     native = (connection.adapter_name == 'PostgreSQL')
     options[:select] = "DISTINCT ON (#{Array(columns).join(', ')}) " + (options[:select] || '*') if native
     raise "can't use limit with distinct on" if options[:limit] # while it's possible, it would be gross for non-native, so we don't allow it
-    raise "distinct on columns must match the leftmost part of the order-by clause" unless options[:order] && options[:order] =~ /\A#{columns.map{ |c| Regexp.escape(c) }.join(' (asc|desc)?,')}/i
+    raise "distinct on columns must match the leftmost part of the order-by clause" unless options[:order] && options[:order] =~ /\A#{Array(columns).map{ |c| Regexp.escape(c) }.join(' (asc|desc)?,')}/i
 
     result = find(:all, options)
 

config/initializers/ruby193-helpers-fix.rb

@@ -0,0 +1,4 @@
+# See https://groups.google.com/forum/#!msg/rubyonrails-core/gb5woRkmDlk/iQ2G7jjNWKkJ
+if RUBY_VERSION >= "1.9.3"
+  MissingSourceFile::REGEXPS << [/^cannot load such file -- (.+)$/i, 1]
+end

config/initializers/ruby_version_compat.rb

@@ -3,9 +3,6 @@ if RUBY_VERSION > "1.9."
   # make an alias for CSV, which has replaced FasterCSV
   require 'csv'
   FasterCSV = CSV
-  # we still depend on the syck YAML for delayed jobs serialization
-  require 'yaml'
-  YAML::ENGINE.yamler = 'syck'
 else
   require 'fastercsv'
 end

config/initializers/time.rb

@@ -1,5 +1,5 @@
-if RUBY_VERSION < "1.9."
-  Time.class_eval do
+Time.class_eval do
+  if RUBY_VERSION < "1.9."
     def strftime_with_1_9_parity(string)
       string = string.gsub(/%[369]?N|%./) do |match|
         case match
@@ -14,15 +14,15 @@ if RUBY_VERSION < "1.9."
       strftime_without_1_9_parity(string)
     end
     alias_method_chain :strftime, :'1_9_parity'
+  end
 
-    def utc_datetime
-      timestamp = self.getutc
-      DateTime.civil(timestamp.strftime("%Y").to_i, 
-                     timestamp.strftime("%m").to_i,
-                     timestamp.strftime("%d").to_i,
-                     timestamp.strftime("%H").to_i, 
-                     timestamp.strftime("%M").to_i)
-    end
+  def utc_datetime
+    timestamp = self.getutc
+    DateTime.civil(timestamp.strftime("%Y").to_i, 
+                   timestamp.strftime("%m").to_i,
+                   timestamp.strftime("%d").to_i,
+                   timestamp.strftime("%H").to_i, 
+                   timestamp.strftime("%M").to_i)
   end
 end
 

config/routes.rb

@@ -478,7 +478,7 @@ ActionController::Routing::Routes.draw do |map|
 
   map.menu_courses 'menu_courses', :controller => 'users', :action => 'menu_courses'
   map.all_menu_courses 'all_menu_courses', :controller => 'users', :action => 'all_menu_courses'
-  map.resources :favorites, :only => [:create, :destroy], :collection => 'reset'
+  map.resources :favorites, :only => [:create, :destroy]
 
   map.grades "grades", :controller => "users", :action => "grades"
 

lib/api/v1/user.rb

@@ -56,7 +56,7 @@ module Api::V1::User
   # if a site admin is making the request or they can manage_students
   def user_json_is_admin?(context = @context, current_user = @current_user)
     @user_json_is_admin ||= {}
-    @user_json_is_admin[[context.id, current_user.id]] ||= (
+    @user_json_is_admin[[context.class.name, context.id, current_user.id]] ||= (
       if context.is_a?(UserProfile)
         permissions_context = permissions_account = @domain_root_account
       else

lib/model_cache.rb

@@ -118,7 +118,7 @@ module ModelCache
     orig_method = "super"
     alias_method = nil
 
-    if (options[:type] == :instance ? klass.instance_methods : klass.methods).include?(method.to_s)
+    if (options[:type] == :instance ? klass.instance_methods : klass.methods).map(&:to_s).include?(method.to_s)
       orig_method = "#{method}_without_cache(*args)"
       alias_method = "alias_method_chain #{method.inspect}, :cache"
       method = "#{method}_with_cache"
@@ -130,7 +130,7 @@ module ModelCache
     expected_args = options[:key_method] ? 0 : 1
     maybe_reset = "cache[#{key_value}] = #{orig_method} if args.size > #{expected_args}"
 
-    klass.send(options[:type] == :instance ? :class_eval : :instance_eval, <<-CODE)
+    klass.send(options[:type] == :instance ? :class_eval : :instance_eval, <<-CODE, __FILE__, __LINE__+1)
       def #{method}(*args)
         if cache = ModelCache[#{options[:cache_name].inspect}] and cache = cache[#{options[:key_lookup].inspect}]
           #{maybe_reset}
@@ -149,4 +149,4 @@ module ModelCache
     klass.after_create :add_to_caches
     klass.after_update :update_in_caches
   end
-end
+end

lib/turnitin.rb

@@ -126,9 +126,9 @@ module Turnitin
         exclude_value = settings[:exclude_value].to_i
         settings[:exclude_type] = '0' unless ['0', '1', '2'].include?(settings[:exclude_type])
         settings[:exclude_value] = case settings[:exclude_type]
-          when '0': ''
-          when '1': [exclude_value, 1].max.to_s
-          when '2': (0..100).include?(exclude_value) ? exclude_value.to_s : '0'
+          when '0'; ''
+          when '1'; [exclude_value, 1].max.to_s
+          when '2'; (0..100).include?(exclude_value) ? exclude_value.to_s : '0'
         end
       end
       settings

spec/apis/v1/enrollments_api_spec.rb

@@ -1,3 +1,4 @@
+# coding: utf-8
 #
 # Copyright (C) 2011 Instructure, Inc.
 #

spec/factories/quiz_factory.rb

@@ -1,3 +1,4 @@
+# coding: utf-8
 #
 # Copyright (C) 2011 Instructure, Inc.
 #

spec/models/attachment_spec.rb

@@ -1,3 +1,4 @@
+# coding: utf-8
 #
 # Copyright (C) 2011 Instructure, Inc.
 #

spec/models/course_spec.rb

@@ -1786,6 +1786,23 @@ describe Course, 'grade_publishing' do
   end
 
   context 'integration suite' do
+    def verify_post_matches(post_lines, expected_post_lines)
+      # first lines should match
+      post_lines[0].should == expected_post_lines[0]
+
+      # now extract the headers
+      post_headers = post_lines[1..post_lines.index("")]
+      expected_post_headers = expected_post_lines[1..expected_post_lines.index("")]
+      if RUBY_VERSION >= "1.9."
+        expected_post_headers << "User-Agent: Ruby"
+      end
+      post_headers.sort.should == expected_post_headers.sort
+
+      # now check payload
+      post_lines[post_lines.index(""),-1].should ==
+        expected_post_lines[expected_post_lines.index(""),-1]
+    end
+    
     def quick_sanity_check(user)
       Course.valid_grade_export_types["test_export"] = {
           :name => "test export",
@@ -1811,12 +1828,12 @@ describe Course, 'grade_publishing' do
       @course.grading_standard_id = 0
       @course.publish_final_grades(user)
       server_thread.join
-      post_lines.should == [
+      verify_post_matches(post_lines, [
           "POST /endpoint HTTP/1.1",
           "Accept: */*",
           "Content-Type: application/jtmimetype",
           "",
-          "test-jt-data"]
+          "test-jt-data"])
     end
 
     it 'should pass a quick sanity check' do
@@ -1868,13 +1885,13 @@ describe Course, 'grade_publishing' do
       @course.grading_standard_id = 0
       @course.publish_final_grades(@user)
       server_thread.join
-      post_lines.should == [
+      verify_post_matches(post_lines, [
           "POST /endpoint HTTP/1.1",
           "Accept: */*",
           "Content-Type: text/csv",
           "",
           "publisher_id,publisher_sis_id,course_id,course_sis_id,section_id,section_sis_id,student_id," +
-          "student_sis_id,enrollment_id,enrollment_status,score,grade\n"]
+          "student_sis_id,enrollment_id,enrollment_status,score,grade\n"])
     end
 
     it 'should publish grades' do
@@ -1983,7 +2000,7 @@ describe Course, 'grade_publishing' do
 
       @course.publish_final_grades(teacher.user)
       server_thread.join
-      post_lines.should == [
+      verify_post_matches(post_lines, [
           "POST /endpoint HTTP/1.1",
           "Accept: */*",
           "Content-Type: text/csv",
@@ -1995,7 +2012,8 @@ describe Course, 'grade_publishing' do
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S2").id},S2,#{getpseudonym("S3").user.id},S3,#{getenroll("S3", "S2").id},active,80\n" +
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S1").id},S1,#{getpseudonym("S4").user.id},S4,#{getenroll("S4", "S1").id},active,0\n" +
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S3").id},S3,#{stud5.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud5.user.id, getsection("S3").id).id},active,85\n" +
-          "#{teacher.user.id},T1,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90\n"]
+          "#{teacher.user.id},T1,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90\n"])
+
       @course.grading_standard_id = 0
       @course.save
       server, server_thread, post_lines = start_test_http_server
@@ -2005,7 +2023,7 @@ describe Course, 'grade_publishing' do
       @ps.save!
       @course.publish_final_grades(teacher.user)
       server_thread.join
-      post_lines.should == [
+      verify_post_matches(post_lines, [
           "POST /endpoint HTTP/1.1",
           "Accept: */*",
           "Content-Type: text/csv",
@@ -2017,7 +2035,7 @@ describe Course, 'grade_publishing' do
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S2").id},S2,#{getpseudonym("S3").user.id},S3,#{getenroll("S3", "S2").id},active,80,B-\n" +
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S1").id},S1,#{getpseudonym("S4").user.id},S4,#{getenroll("S4", "S1").id},active,0,F\n" +
           "#{teacher.user.id},T1,#{@course.id},C1,#{getsection("S3").id},S3,#{stud5.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud5.user.id, getsection("S3").id).id},active,85,B\n" +
-          "#{teacher.user.id},T1,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90,A-\n"]
+          "#{teacher.user.id},T1,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90,A-\n"])
       admin = user_model
       server, server_thread, post_lines = start_test_http_server
       @ps.posted_settings = @plugin.default_settings.merge({
@@ -2026,7 +2044,7 @@ describe Course, 'grade_publishing' do
       @ps.save!
       @course.publish_final_grades(admin)
       server_thread.join
-      post_lines.should == [
+      verify_post_matches(post_lines, [
           "POST /endpoint HTTP/1.1",
           "Accept: */*",
           "Content-Type: text/csv",
@@ -2038,7 +2056,7 @@ describe Course, 'grade_publishing' do
           "#{admin.id},,#{@course.id},C1,#{getsection("S2").id},S2,#{getpseudonym("S3").user.id},S3,#{getenroll("S3", "S2").id},active,80,B-\n" +
           "#{admin.id},,#{@course.id},C1,#{getsection("S1").id},S1,#{getpseudonym("S4").user.id},S4,#{getenroll("S4", "S1").id},active,0,F\n" +
           "#{admin.id},,#{@course.id},C1,#{getsection("S3").id},S3,#{stud5.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud5.user.id, getsection("S3").id).id},active,85,B\n" +
-          "#{admin.id},,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90,A-\n"]
+          "#{admin.id},,#{@course.id},C1,#{sec4.id},S4,#{stud6.user.id},,#{Enrollment.find_by_user_id_and_course_section_id(stud6.user.id, sec4.id).id},active,90,A-\n"])
     end
 
   end

spec/models/user_spec.rb

@@ -1323,9 +1323,9 @@ describe User do
     it "should return active pseudonyms only" do
       course :active_all => true, :account => Account.default
       u = User.create!
-      u.pseudonyms.new(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'deleted'; x.sis_user_id = "user2"; x.save!}
+      u.pseudonyms.create!(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'deleted'; x.sis_user_id = "user2" }
       u.sis_pseudonym_for(@course).should be_nil
-      @p = u.pseudonyms.new(:account => Account.default, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'active'; x.sis_user_id = "user1"; x.save!}
+      @p = u.pseudonyms.create!(:account => Account.default, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'active'; x.sis_user_id = "user1" }
       u.sis_pseudonym_for(@course).should == @p
     end
 
@@ -1333,18 +1333,18 @@ describe User do
       course :active_all => true, :account => Account.default
       other_account = account_model
       u = User.create!
-      u.pseudonyms.new(:account => other_account, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'active'; x.sis_user_id = "user1"; x.save!}
+      u.pseudonyms.create!(:account => other_account, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'active'; x.sis_user_id = "user1" }
       u.sis_pseudonym_for(@course).should be_nil
-      @p = u.pseudonyms.new(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'active'; x.sis_user_id = "user2"; x.save!}
+      @p = u.pseudonyms.create!(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'active'; x.sis_user_id = "user2" }
       u.sis_pseudonym_for(@course).should == @p
     end
 
     it "should return pseudonyms with a sis id only" do
       course :active_all => true, :account => Account.default
       u = User.create!
-      u.pseudonyms.new(:account => Account.default, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'active'; x.save!}
+      u.pseudonyms.create!(:account => Account.default, :unique_id => "user1@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'active' }
       u.sis_pseudonym_for(@course).should be_nil
-      @p = u.pseudonyms.new(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf").tap{|x| x.workflow_state = 'active'; x.sis_user_id = "user2"; x.save!}
+      @p = u.pseudonyms.create!(:account => Account.default, :unique_id => "user2@example.com", :password => "asdfasdf", :password_confirmation => "asdfasdf") {|x| x.workflow_state = 'active'; x.sis_user_id = "user2" }
       u.sis_pseudonym_for(@course).should == @p
     end
 

spec/selenium/profile_spec.rb

@@ -1,3 +1,5 @@
+# coding: utf-8
+
 require File.expand_path(File.dirname(__FILE__) + '/common')
 
 describe "profile" do

spec/selenium/sections_spec.rb

@@ -1,3 +1,5 @@
+# coding: utf-8
+
 require File.expand_path(File.dirname(__FILE__) + '/common')
 
 describe "sections" do

spec/spec.opts

@@ -1,5 +1,4 @@
 --colour
---require spec/nested_instafail_formatter
+--require ./spec/nested_instafail_formatter.rb
 --format RSpec::NestedInstafailFormatter
 --format html:tmp/spec_html/index.html
-

vendor/plugins/delayed_job/lib/delayed/yaml_extensions.rb

@@ -1,5 +1,10 @@
 # New definitions for YAML to aid in serialization and deserialization of delayed jobs.
 
+require 'yaml'
+# this code needs to be updated to work with the new Psych YAML engine in ruby 1.9.x
+# for now we force Syck
+YAML::ENGINE.yamler = 'syck' if defined?(YAML::ENGINE)
+
 # First, tell YAML how to load a Module. This depends on Rails .constantize and autoloading.
 YAML.add_ruby_type("object:Module") do |type, val|
   val.constantize