don't drop sis rows if transaction timeout is reached

Change-Id: Ib6744eb6410727afbd8b86f55d379d1c126a96db
Reviewed-on: https://gerrit.instructure.com/5830
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: JT Olds <jt@instructure.com>

lib/sis/enrollment_importer.rb

@@ -98,7 +98,8 @@ module SIS
         Enrollment.transaction do
           tx_end_time = Time.now + transaction_timeout
           enrollment = nil
-          while !(enrollment = @enrollment_batch.shift).nil? && tx_end_time > Time.now
+          while !@enrollment_batch.empty? && tx_end_time > Time.now
+            enrollment = @enrollment_batch.shift
             @logger.debug("Processing Enrollment #{enrollment.inspect}")
             course_id, section_id, user_id, role, status, start_date, end_date, associated_user_id = enrollment
 

lib/sis/user_importer.rb

@@ -83,7 +83,8 @@ module SIS
         User.transaction do
           tx_end_time = Time.now + transaction_timeout
           user_row = nil
-          while !(user_row = @batched_users.shift).nil? && tx_end_time > Time.now
+          while !@batched_users.empty? && tx_end_time > Time.now
+            user_row = @batched_users.shift
             @logger.debug("Processing User #{user_row.inspect}")
             user_id, login_id, status, first_name, last_name, email, password, ssha_password = user_row
 

spec/lib/sis_importer_spec.rb

@@ -0,0 +1,36 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas 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 Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+
+describe SIS do
+  it "should split into transactions based on time elapsed" do
+    account_model
+    messages = []
+    Setting.set('sis_transaction_seconds', '1')
+    # this is the fun bit where we get to stub User.new to insert a sleep into
+    # the transaction loop.
+    User.stub!(:new) { sleep 1; User.allocate.tap { |u| u.send(:initialize) } }
+    SIS::UserImporter.new(nil, @account, Rails.logger).process(2, messages) do |importer|
+      importer.add_user(*"U001,user1,active,User,One,user1@example.com".split(','))
+      importer.add_user(*"U002,user2,active,User,Two,user2@example.com".split(','))
+      importer.add_user(*"U003,user3,active,User,Three,user3@example.com".split(','))
+    end
+    Pseudonym.all.map(&:sis_user_id).sort.should == %w(U001 U002 U003)
+  end
+end

spec/models/account_spec.rb

@@ -45,22 +45,10 @@ describe Account do
     # @a.to_atom.should be_is_a(Atom::Entry)
   # end
 
-  def process_csv_data(account, lines)
-    tmp = Tempfile.new("sis_rspec")
-    path = "#{tmp.path}.csv"
-    tmp.close!
-    File.open(path, "w+") { |f| f.puts lines.join "\n" }
-    importer = SIS::CSV::Import.process(@account, :files => [path],
-        :allow_printing => false)
-    File.unlink path
-    importer.warnings.should == []
-    importer.errors.should == []
-  end
-
   context "course lists" do
     before(:each) do
       @account = Account.create!
-      process_csv_data(@account, [
+      process_csv_data_cleanly([
         "user_id,login_id,first_name,last_name,email,status",
         "U001,user1,User,One,user1@example.com,active",
         "U002,user2,User,Two,user2@example.com,active",
@@ -74,13 +62,13 @@ describe Account do
         "U010,user10,User,Ten,user10@example.com,active",
         "U011,user11,User,Eleven,user11@example.com,deleted"
       ])
-      process_csv_data(@account, [
+      process_csv_data_cleanly([
         "term_id,name,status,start_date,end_date",
         "T001,Term 1,active,,",
         "T002,Term 2,active,,",
         "T003,Term 3,active,,"
       ])
-      process_csv_data(@account, [
+      process_csv_data_cleanly([
         "course_id,short_name,long_name,account_id,term_id,status",
         "C001,C001,Test Course 1,,T001,active",
         "C002,C002,Test Course 2,,T001,deleted",
@@ -101,7 +89,7 @@ describe Account do
         "C008S,C008S,Test search Course 8,,T003,active",
         "C009S,C009S,Test search Course 9,,T003,active"
       ])
-      process_csv_data(@account, [
+      process_csv_data_cleanly([
         "section_id,course_id,name,start_date,end_date,status",
         "S001,C001,Sec1,,,active",
         "S002,C002,Sec2,,,active",
@@ -122,7 +110,7 @@ describe Account do
         "S008S,C001S,Sec8,,,deleted",
         "S009S,C008S,Sec9,,,active"
       ])
-      process_csv_data(@account, [
+      process_csv_data_cleanly([
         "course_id,user_id,role,section_id,status,associated_user_id",
         ",U001,student,S001,active,",
         ",U002,student,S002,active,",

spec/spec_helper.rb

@@ -312,7 +312,7 @@ Spec::Runner.configure do |config|
     tmp = Tempfile.new("sis_rspec")
     path = "#{tmp.path}.csv"
     tmp.close!
-    File.open(path, "w+") { |f| f.puts lines.join "\n" }
+    File.open(path, "w+") { |f| f.puts lines.flatten.join "\n" }
     
     importer = SIS::CSV::Import.process(@account, :files => [ path ], :allow_printing=>false)