diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | Gemfile.lock | 2 | ||||
| -rw-r--r-- | app/models/import.rb | 39 | ||||
| -rw-r--r-- | app/services/parent_import_notifier.rb | 15 | ||||
| -rw-r--r-- | config/schedule.rb | 4 | ||||
| -rw-r--r-- | db/migrate/20170724094628_add_notified_parent_at_to_imports.rb | 5 | ||||
| -rw-r--r-- | db/schema.rb | 23 | ||||
| -rw-r--r-- | lib/tasks/imports.rake | 6 | ||||
| -rw-r--r-- | spec/factories/imports.rb | 7 | ||||
| -rw-r--r-- | spec/factories/netex_imports.rb | 5 | ||||
| -rw-r--r-- | spec/factories/workbench_imports.rb | 5 | ||||
| -rw-r--r-- | spec/models/import_spec.rb | 148 | ||||
| -rw-r--r-- | spec/services/parent_import_notifier_spec.rb | 90 |
13 files changed, 331 insertions, 19 deletions
@@ -163,6 +163,7 @@ group :test do gem 'simplecov', :require => false gem 'simplecov-rcov', :require => false gem 'htmlbeautifier' + gem 'timecop' end group :test, :development, :dev do diff --git a/Gemfile.lock b/Gemfile.lock index 2239cf853..d03a26d18 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -525,6 +525,7 @@ GEM thread (0.2.2) thread_safe (0.3.6) tilt (1.4.1) + timecop (0.9.1) transpec (3.3.0) activesupport (>= 3.0, < 6.0) astrolabe (~> 1.2) @@ -669,6 +670,7 @@ DEPENDENCIES squeel! teaspoon-jasmine therubyracer (~> 0.12) + timecop transpec uglifier (~> 2.7.2) webmock diff --git a/app/models/import.rb b/app/models/import.rb index c932ecdd9..f8702c115 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -3,7 +3,7 @@ class Import < ActiveRecord::Base belongs_to :workbench belongs_to :referential - belongs_to :parent, class_name: to_s + belongs_to :parent, polymorphic: true extend Enumerize enumerize :status, in: %i(new pending successful failed running aborted canceled) @@ -11,8 +11,43 @@ class Import < ActiveRecord::Base validates :file, presence: true validates_presence_of :referential, :workbench - before_create do + before_create :initialize_fields + + def self.failing_statuses + symbols_with_indifferent_access(%i(failed aborted canceled)) + end + + def self.finished_statuses + symbols_with_indifferent_access(%i(successful failed aborted canceled)) + end + + def notify_parent + parent.child_change(self) + update(notified_parent_at: DateTime.now) + end + + def child_change(child) + return if self.class.finished_statuses.include?(status) + + if self.class.failing_statuses.include?(child.status) + return update(status: 'failed') + end + + update(status: 'successful') if ready? + end + + def ready? + current_step == total_steps + end + + private + + def initialize_fields self.token_download = SecureRandom.urlsafe_base64 self.status = Import.status.new end + + def self.symbols_with_indifferent_access(array) + array.flat_map { |symbol| [symbol, symbol.to_s] } + end end diff --git a/app/services/parent_import_notifier.rb b/app/services/parent_import_notifier.rb new file mode 100644 index 000000000..47e6755e4 --- /dev/null +++ b/app/services/parent_import_notifier.rb @@ -0,0 +1,15 @@ +class ParentImportNotifier + def self.notify_when_finished(imports = nil) + imports ||= imports_pending_notification + imports.each(&:notify_parent) + end + + def self.imports_pending_notification + Import + .where( + notified_parent_at: nil, + status: Import.finished_statuses + ) + .where.not(parent: nil) + end +end diff --git a/config/schedule.rb b/config/schedule.rb index 83c4d7388..8aa21076f 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -38,3 +38,7 @@ end every :day, :at => '4:00 am' do rake "codifligne:sync" end + +every 5.minutes do + rake "import:notify_parent" +end diff --git a/db/migrate/20170724094628_add_notified_parent_at_to_imports.rb b/db/migrate/20170724094628_add_notified_parent_at_to_imports.rb new file mode 100644 index 000000000..7c6484cfc --- /dev/null +++ b/db/migrate/20170724094628_add_notified_parent_at_to_imports.rb @@ -0,0 +1,5 @@ +class AddNotifiedParentAtToImports < ActiveRecord::Migration + def change + add_column :imports, :notified_parent_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 05a024e1d..05f5c2e87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -284,11 +284,12 @@ ActiveRecord::Schema.define(version: 20170727130705) do t.datetime "started_at" t.datetime "ended_at" t.string "token_download" - t.string "type", limit: 255 + t.string "type" t.integer "parent_id", limit: 8 t.string "parent_type" - t.integer "current_step", default: 0 - t.integer "total_steps", default: 0 + t.datetime "notified_parent_at" + t.integer "current_step", default: 0 + t.integer "total_steps", default: 0 end add_index "imports", ["referential_id"], name: "index_imports_on_referential_id", using: :btree @@ -606,7 +607,7 @@ ActiveRecord::Schema.define(version: 20170727130705) do create_table "stop_areas", id: :bigserial, force: :cascade do |t| t.integer "parent_id", limit: 8 - t.string "objectid", null: false + t.string "objectid", null: false t.integer "object_version", limit: 8 t.string "creator_id" t.string "name" @@ -615,8 +616,8 @@ ActiveRecord::Schema.define(version: 20170727130705) do t.string "registration_number" t.string "nearest_topic_name" t.integer "fare_code" - t.decimal "longitude", precision: 19, scale: 16 - t.decimal "latitude", precision: 19, scale: 16 + t.decimal "longitude", precision: 19, scale: 16 + t.decimal "latitude", precision: 19, scale: 16 t.string "long_lat_type" t.string "country_code" t.string "street_name" @@ -634,7 +635,7 @@ ActiveRecord::Schema.define(version: 20170727130705) do t.datetime "deleted_at" t.datetime "created_at" t.datetime "updated_at" - t.string "stif_type", limit: 255 + t.string "stif_type" end add_index "stop_areas", ["name"], name: "index_stop_areas_on_name", using: :btree @@ -701,18 +702,18 @@ ActiveRecord::Schema.define(version: 20170727130705) do add_index "time_table_periods", ["time_table_id"], name: "index_time_table_periods_on_time_table_id", using: :btree create_table "time_tables", id: :bigserial, force: :cascade do |t| - t.string "objectid", null: false - t.integer "object_version", limit: 8, default: 1 + t.string "objectid", null: false + t.integer "object_version", limit: 8, default: 1 t.string "creator_id" t.string "version" t.string "comment" - t.integer "int_day_types", default: 0 + t.integer "int_day_types", default: 0 t.date "start_date" t.date "end_date" t.integer "calendar_id", limit: 8 t.datetime "created_at" t.datetime "updated_at" - t.string "color", limit: 255 + t.string "color" t.integer "created_from_id" end diff --git a/lib/tasks/imports.rake b/lib/tasks/imports.rake new file mode 100644 index 000000000..6bc84acc8 --- /dev/null +++ b/lib/tasks/imports.rake @@ -0,0 +1,6 @@ +namespace :import do + desc "Notify parent imports when children finish" + task notify_parent: :environment do + ParentImportNotifier.notify_when_finished + end +end diff --git a/spec/factories/imports.rb b/spec/factories/imports.rb index aa9288fe9..afced3b57 100644 --- a/spec/factories/imports.rb +++ b/spec/factories/imports.rb @@ -10,11 +10,8 @@ FactoryGirl.define do started_at nil ended_at nil - factory :netex_import, class: NetexImport do - file {File.open(Rails.root.join('spec', 'fixtures', 'terminated_job.json'))} - end - factory :workbench_import, class: WorkbenchImport do - file {File.open(Rails.root.join('spec', 'fixtures', 'terminated_job.json'))} + after(:build) do |import| + import.class.skip_callback(:create, :before, :initialize_fields) end end end diff --git a/spec/factories/netex_imports.rb b/spec/factories/netex_imports.rb new file mode 100644 index 000000000..057e47730 --- /dev/null +++ b/spec/factories/netex_imports.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :netex_import, class: NetexImport, parent: :import do + file { File.open(Rails.root.join('spec', 'fixtures', 'terminated_job.json')) } + end +end diff --git a/spec/factories/workbench_imports.rb b/spec/factories/workbench_imports.rb new file mode 100644 index 000000000..5cdcfd15f --- /dev/null +++ b/spec/factories/workbench_imports.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :workbench_import, class: WorkbenchImport, parent: :import do + file { File.open(Rails.root.join('spec', 'fixtures', 'terminated_job.json')) } + end +end diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb index c56858b44..164769fd5 100644 --- a/spec/models/import_spec.rb +++ b/spec/models/import_spec.rb @@ -2,11 +2,157 @@ RSpec.describe Import, :type => :model do it { should belong_to(:referential) } it { should belong_to(:workbench) } - it { should belong_to(:parent).class_name(described_class.to_s) } + it { should belong_to(:parent) } it { should enumerize(:status).in("aborted", "canceled", "failed", "new", "pending", "running", "successful") } it { should validate_presence_of(:file) } it { should validate_presence_of(:referential) } it { should validate_presence_of(:workbench) } + + let(:workbench_import) { build_stubbed(:workbench_import) } + let(:workbench_import_with_completed_steps) do + workbench_import = build_stubbed( + :workbench_import, + total_steps: 2, + current_step: 2 + ) + end + + let(:netex_import) do + netex_import = build_stubbed( + :netex_import, + parent: workbench_import + ) + end + + describe "#notify_parent" do + it "must call #child_change on its parent" do + allow(netex_import).to receive(:update) + + expect(workbench_import).to receive(:child_change).with(netex_import) + + netex_import.notify_parent + end + + it "must update the :notified_parent_at field of the child import" do + allow(workbench_import).to receive(:child_change) + + Timecop.freeze(DateTime.now) do + expect(netex_import).to receive(:update).with( + notified_parent_at: DateTime.now + ) + + netex_import.notify_parent + end + end + end + + describe "#child_change" do + shared_examples( + "updates :status to failed when child status indicates failure" + ) do |failure_status| + it "updates :status to failed when child status indicates failure" do + allow(workbench_import).to receive(:update) + + netex_import = build_stubbed( + :netex_import, + parent: workbench_import, + status: failure_status + ) + + expect(workbench_import).to receive(:update).with(status: 'failed') + + workbench_import.child_change(netex_import) + end + end + + include_examples( + "updates :status to failed when child status indicates failure", + "failed" + ) + include_examples( + "updates :status to failed when child status indicates failure", + "aborted" + ) + include_examples( + "updates :status to failed when child status indicates failure", + "canceled" + ) + + it "updates :status to successful when #ready?" do + expect(workbench_import).to receive(:update).with(status: 'successful') + + workbench_import.child_change(netex_import) + end + + it "updates :status to failed when #ready? and child is failed" do + netex_import = build_stubbed( + :netex_import, + parent: workbench_import, + status: :failed + ) + + expect(workbench_import).to receive(:update).with(status: 'failed') + + workbench_import.child_change(netex_import) + end + + shared_examples( + "doesn't update :status if parent import status is finished" + ) do |finished_status| + it "doesn't update :status if parent import status is finished" do + workbench_import = build_stubbed( + :workbench_import, + total_steps: 2, + current_step: 2, + status: finished_status + ) + child = double('Import') + + expect(workbench_import).not_to receive(:update) + + workbench_import.child_change(child) + end + end + + include_examples( + "doesn't update :status if parent import status is finished", + "successful" + ) + include_examples( + "doesn't update :status if parent import status is finished", + "failed" + ) + include_examples( + "doesn't update :status if parent import status is finished", + "aborted" + ) + include_examples( + "doesn't update :status if parent import status is finished", + "canceled" + ) + end + + describe "#ready?" do + it "returns true if #current_step == #total_steps" do + import = build_stubbed( + :import, + total_steps: 4, + current_step: 4 + ) + + expect(import.ready?).to be true + end + + it "returns false if #current_step != #total_steps" do + import = build_stubbed( + :import, + total_steps: 6, + current_step: 3 + ) + + expect(import.ready?).to be false + end + end end diff --git a/spec/services/parent_import_notifier_spec.rb b/spec/services/parent_import_notifier_spec.rb new file mode 100644 index 000000000..3ab505f88 --- /dev/null +++ b/spec/services/parent_import_notifier_spec.rb @@ -0,0 +1,90 @@ +RSpec.describe ParentImportNotifier do + let(:workbench_import) { create(:workbench_import) } + + describe ".notify_when_finished" do + it "calls #notify_parent on finished imports" do + workbench_import = build_stubbed(:workbench_import) + + finished_statuses = [:failed, :successful, :aborted, :canceled] + netex_imports = [] + + finished_statuses.each do |status| + netex_imports << build_stubbed( + :netex_import, + parent: workbench_import, + status: status + ) + end + + netex_imports.each do |netex_import| + expect(netex_import).to receive(:notify_parent) + end + + ParentImportNotifier.notify_when_finished(netex_imports) + end + + it "doesn't call #notify_parent if its `notified_parent_at` is set" do + netex_import = create( + :netex_import, + parent: workbench_import, + status: :failed, + notified_parent_at: DateTime.now + ) + + expect(netex_import).not_to receive(:notify_parent) + + ParentImportNotifier.notify_when_finished + end + end + + describe ".imports_pending_notification" do + it "includes imports with a parent and `notified_parent_at` unset" do + netex_import = create( + :netex_import, + parent: workbench_import, + status: :successful, + notified_parent_at: nil + ) + + expect( + ParentImportNotifier.imports_pending_notification + ).to eq([netex_import]) + end + + it "doesn't include imports without a parent" do + create(:import, parent: nil) + + expect( + ParentImportNotifier.imports_pending_notification + ).to be_empty + end + + it "doesn't include imports that aren't finished" do + [:new, :pending, :running].each do |status| + create( + :netex_import, + parent: workbench_import, + status: status, + notified_parent_at: nil + ) + end + + expect( + ParentImportNotifier.imports_pending_notification + ).to be_empty + end + + it "doesn't include imports that have already notified their parent" do + create( + :netex_import, + parent: workbench_import, + status: :successful, + notified_parent_at: DateTime.now + ) + + expect( + ParentImportNotifier.imports_pending_notification + ).to be_empty + end + end +end |