From d2aa2c88e9e5603f1178a38105ce1b53c777953e Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sat, 13 Apr 2019 19:32:00 +0400 Subject: [PATCH 1/7] Move logic from rake task to service --- app/services/db_importer.rb | 34 ++++++++++++++++++++++++++++++++++ lib/tasks/utils.rake | 34 ++-------------------------------- 2 files changed, 36 insertions(+), 32 deletions(-) create mode 100644 app/services/db_importer.rb diff --git a/app/services/db_importer.rb b/app/services/db_importer.rb new file mode 100644 index 0000000..e5f19f5 --- /dev/null +++ b/app/services/db_importer.rb @@ -0,0 +1,34 @@ +class DbImporter + def call(source:) + json = JSON.parse(File.read(source)) + + ActiveRecord::Base.transaction do + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + + json.each do |trip| + from = City.find_or_create_by(name: trip['from']) + to = City.find_or_create_by(name: trip['to']) + services = [] + trip['bus']['services'].each do |service| + s = Service.find_or_create_by(name: service) + services << s + end + bus = Bus.find_or_create_by(number: trip['bus']['number']) + bus.update(model: trip['bus']['model'], services: services) + + Trip.create!( + from: from, + to: to, + bus: bus, + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'] + ) + end + end + end +end diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe87..ad1eba2 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,4 @@ -# Наивная загрузка данных из json-файла в БД -# rake reload_json[fixtures/small.json] +desc 'Import data from json file to DB: rake reload_json[fixtures/small.json]' task :reload_json, [:file_name] => :environment do |_task, args| - json = JSON.parse(File.read(args.file_name)) - - ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') - - json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s - end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: trip['start_time'], - duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) - end - end + DbImporter.new.call(source: args.file_name) end From beb3bf7a0bbc2421aa12943ffe04ac80202289ac Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sat, 13 Apr 2019 19:32:36 +0400 Subject: [PATCH 2/7] Add rubocop and fix offenses --- .rubocop.yml | 23 +++++++++++++++++++++++ Gemfile | 1 + Gemfile.lock | 18 ++++++++++++++++++ app/models/bus.rb | 22 +++++++++++----------- app/models/city.rb | 2 +- app/models/service.rb | 2 +- app/models/trip.rb | 6 +++--- test/application_system_test_case.rb | 2 +- 8 files changed, 59 insertions(+), 17 deletions(-) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..1b2270b --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,23 @@ +AllCops: + TargetRubyVersion: 2.6.1 + Exclude: + - "config/**/*" + - "db/**/*" + - "bin/**" + - "*Gemfile*" + - "tmp/**/*" + +Metrics/LineLength: + Max: 120 + +Metrics/MethodLength: + Max: 15 + +Style/ClassAndModuleChildren: + Enabled: false + +Style/Documentation: + Enabled: false + +Style/FrozenStringLiteralComment: + Enabled: false diff --git a/Gemfile b/Gemfile index 33017fd..99c4a2c 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,7 @@ gem 'bootsnap', '>= 1.1.0', require: false group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] + gem 'rubocop' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index eb22e16..11756a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -43,6 +43,7 @@ GEM minitest (~> 5.1) tzinfo (~> 1.1) arel (9.0.0) + ast (2.4.0) bindex (0.6.0) bootsnap (1.4.2) msgpack (~> 1.0) @@ -56,6 +57,7 @@ GEM activesupport (>= 4.2.0) i18n (1.6.0) concurrent-ruby (~> 1.0) + jaro_winkler (1.5.2) listen (3.1.5) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) @@ -76,7 +78,11 @@ GEM nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + parallel (1.14.0) + parser (2.6.0.0) + ast (~> 2.4.0) pg (1.1.4) + psych (3.1.0) puma (3.12.1) rack (2.0.6) rack-test (1.1.0) @@ -105,10 +111,20 @@ GEM method_source rake (>= 0.8.7) thor (>= 0.19.0, < 2.0) + rainbow (3.0.0) rake (12.3.2) rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) + rubocop (0.66.0) + jaro_winkler (~> 1.5.1) + parallel (~> 1.10) + parser (>= 2.5, != 2.5.1.1) + psych (>= 3.1.0) + rainbow (>= 2.2.2, < 4.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 1.4.0, < 1.6) + ruby-progressbar (1.10.0) ruby_dep (1.5.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) @@ -121,6 +137,7 @@ GEM thread_safe (0.3.6) tzinfo (1.2.5) thread_safe (~> 0.1) + unicode-display_width (1.5.0) web-console (3.7.0) actionview (>= 5.0) activemodel (>= 5.0) @@ -140,6 +157,7 @@ DEPENDENCIES pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) + rubocop tzinfo-data web-console (>= 3.3.0) diff --git a/app/models/bus.rb b/app/models/bus.rb index 1dcc54c..7cea181 100644 --- a/app/models/bus.rb +++ b/app/models/bus.rb @@ -1,15 +1,15 @@ class Bus < ApplicationRecord - MODELS = [ - 'Икарус', - 'Мерседес', - 'Сканиа', - 'Буханка', - 'УАЗ', - 'Спринтер', - 'ГАЗ', - 'ПАЗ', - 'Вольво', - 'Газель', + MODELS = %w[ + Икарус + Мерседес + Сканиа + Буханка + УАЗ + Спринтер + ГАЗ + ПАЗ + Вольво + Газель ].freeze has_many :trips diff --git a/app/models/city.rb b/app/models/city.rb index 19ec7f3..1a0f767 100644 --- a/app/models/city.rb +++ b/app/models/city.rb @@ -3,6 +3,6 @@ class City < ApplicationRecord validate :name_has_no_spaces def name_has_no_spaces - errors.add(:name, "has spaces") if name.include?(' ') + errors.add(:name, 'has spaces') if name.include?(' ') end end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a3..e2eb93e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -9,7 +9,7 @@ class Service < ApplicationRecord 'Телевизор общий', 'Телевизор индивидуальный', 'Стюардесса', - 'Можно не печатать билет', + 'Можно не печатать билет' ].freeze has_and_belongs_to_many :buses, join_table: :buses_services diff --git a/app/models/trip.rb b/app/models/trip.rb index 9d63dff..f28e291 100644 --- a/app/models/trip.rb +++ b/app/models/trip.rb @@ -1,5 +1,5 @@ class Trip < ApplicationRecord - HHMM_REGEXP = /([0-1][0-9]|[2][0-3]):[0-5][0-9]/ + HHMM_REGEXP = /([0-1][0-9]|[2][0-3]):[0-5][0-9]/.freeze belongs_to :from, class_name: 'City' belongs_to :to, class_name: 'City' @@ -25,8 +25,8 @@ def to_h bus: { number: bus.number, model: bus.model, - services: bus.services.map(&:name), - }, + services: bus.services.map(&:name) + } } end end diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index d19212a..23701b4 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -1,4 +1,4 @@ -require "test_helper" +require 'test_helper' class ApplicationSystemTestCase < ActionDispatch::SystemTestCase driven_by :selenium, using: :chrome, screen_size: [1400, 1400] From 604e10cef619dbe1bcfbc7ce23b276808ae8bf9c Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sat, 13 Apr 2019 20:08:02 +0400 Subject: [PATCH 3/7] Add unit test for db_importer service --- test/fixtures/files/trip_import.json | 1 + test/services/db_importer_test.rb | 31 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 test/fixtures/files/trip_import.json create mode 100644 test/services/db_importer_test.rb diff --git a/test/fixtures/files/trip_import.json b/test/fixtures/files/trip_import.json new file mode 100644 index 0000000..211d8b0 --- /dev/null +++ b/test/fixtures/files/trip_import.json @@ -0,0 +1 @@ +[{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":168,"from":"Москва","price_cents":474,"start_time":"11:00","to":"Самара"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":37,"from":"Самара","price_cents":173,"start_time":"17:30","to":"Москва"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":323,"from":"Москва","price_cents":672,"start_time":"12:00","to":"Самара"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":315,"from":"Самара","price_cents":969,"start_time":"18:30","to":"Москва"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":304,"from":"Москва","price_cents":641,"start_time":"13:00","to":"Самара"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":21,"from":"Самара","price_cents":663,"start_time":"19:30","to":"Москва"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":598,"from":"Москва","price_cents":629,"start_time":"14:00","to":"Самара"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":292,"from":"Самара","price_cents":22,"start_time":"20:30","to":"Москва"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":127,"from":"Москва","price_cents":795,"start_time":"15:00","to":"Самара"},{"bus":{"model":"Икарус","number":"123","services":["Туалет","WiFi"]},"duration_minutes":183,"from":"Самара","price_cents":846,"start_time":"21:30","to":"Москва"}] diff --git a/test/services/db_importer_test.rb b/test/services/db_importer_test.rb new file mode 100644 index 0000000..afea558 --- /dev/null +++ b/test/services/db_importer_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +class DbImporterTest < ActiveSupport::TestCase + test 'imports all presented records' do + assert_difference 'Trip.count', 10 do + import! + end + end + + test 'correctly populates records attributes' do + import! + + actual_trip = Trip.first.to_h + expected_trip = { + from: 'Москва', + to: 'Самара', + start_time: '11:00', + duration_minutes: 168, + price_cents: 474, + bus: { number: '123', model: 'Икарус', services: %w[Туалет WiFi] } + } + + assert_equal expected_trip, actual_trip + end + + private + + def import! + DbImporter.new.call(source: 'test/fixtures/files/trip_import.json') + end +end From 0f7985d5d16e97692ed3f6fc2d75498d227ef2ff Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sat, 13 Apr 2019 20:36:40 +0400 Subject: [PATCH 4/7] Add rake task for benchmark import --- lib/tasks/utils.rake | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index ad1eba2..4f20286 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,4 +1,14 @@ +require 'benchmark' + desc 'Import data from json file to DB: rake reload_json[fixtures/small.json]' task :reload_json, [:file_name] => :environment do |_task, args| + puts "Import data from file #{args.file_name}..." DbImporter.new.call(source: args.file_name) + puts 'Done!' +end + +desc 'Benchmark :reload_json task: rake reload_json_benchmark[fixtures/small.json]' +task :reload_json_benchmark, [:file_name] => :environment do |_task, args| + bm = Benchmark.measure { Rake::Task['reload_json'].invoke(*args) } + puts bm end From 27a580dc713e321ff93b4b078329039d5ecfca6a Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sat, 13 Apr 2019 22:01:38 +0400 Subject: [PATCH 5/7] Import optimization (wip) --- Gemfile | 2 + Gemfile.lock | 5 ++ app/services/db_importer.rb | 77 ++++++++++++------- .../20190413173437_add_indices_for_import.rb | 9 +++ db/schema.rb | 5 +- 5 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 db/migrate/20190413173437_add_indices_for_import.rb diff --git a/Gemfile b/Gemfile index 99c4a2c..d3d1078 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,8 @@ gem 'rails', '~> 5.2.3' gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' gem 'bootsnap', '>= 1.1.0', require: false +gem 'oj' +gem 'strong_migrations' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 11756a6..3a1caf0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -78,6 +78,7 @@ GEM nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + oj (3.7.11) parallel (1.14.0) parser (2.6.0.0) ast (~> 2.4.0) @@ -133,6 +134,8 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) + strong_migrations (0.3.1) + activerecord (>= 3.2.0) thor (0.20.3) thread_safe (0.3.6) tzinfo (1.2.5) @@ -154,10 +157,12 @@ DEPENDENCIES bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + oj pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) rubocop + strong_migrations tzinfo-data web-console (>= 3.3.0) diff --git a/app/services/db_importer.rb b/app/services/db_importer.rb index e5f19f5..4dd1cc1 100644 --- a/app/services/db_importer.rb +++ b/app/services/db_importer.rb @@ -1,34 +1,59 @@ +require 'oj' + class DbImporter def call(source:) - json = JSON.parse(File.read(source)) + json = Oj.load_file(source) ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') - - json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s - end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: trip['start_time'], - duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'] - ) + clear_db! + create_from_json!(json) + end + end + + def clear_db! + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + end + + def create_from_json!(json) + cities = Set.new + services = Set.new + + json.each do |trip| + cities << trip['from'] + cities << trip['to'] + + trip['bus']['services'].each do |service| + services << service end end + + cities.each do |city| + City.create(name: city) + end + + services.each do |service| + Service.create(name: service) + end + + json.each do |trip| + from = City.find_by(name: trip['from']) + to = City.find_by(name: trip['to']) + services = Service.where(name: trip['bus']['services']) + bus = Bus.find_or_create_by(number: trip['bus']['number']) + bus.update(model: trip['bus']['model'], services: services) + + Trip.create!( + from: from, + to: to, + bus: bus, + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'] + ) + end end end diff --git a/db/migrate/20190413173437_add_indices_for_import.rb b/db/migrate/20190413173437_add_indices_for_import.rb new file mode 100644 index 0000000..19a3bc4 --- /dev/null +++ b/db/migrate/20190413173437_add_indices_for_import.rb @@ -0,0 +1,9 @@ +class AddIndicesForImport < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :cities, :name, algorithm: :concurrently + add_index :services, :name, algorithm: :concurrently + add_index :buses, :number, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e4..119722a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_03_30_193044) do +ActiveRecord::Schema.define(version: 2019_04_13_173437) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -18,6 +18,7 @@ create_table "buses", force: :cascade do |t| t.string "number" t.string "model" + t.index ["number"], name: "index_buses_on_number" end create_table "buses_services", force: :cascade do |t| @@ -27,10 +28,12 @@ create_table "cities", force: :cascade do |t| t.string "name" + t.index ["name"], name: "index_cities_on_name" end create_table "services", force: :cascade do |t| t.string "name" + t.index ["name"], name: "index_services_on_name" end create_table "trips", force: :cascade do |t| From a71cf18ee6eb1e57c313d3ad80f7092be277acfa Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sun, 14 Apr 2019 13:08:08 +0400 Subject: [PATCH 6/7] Optimize import with activerecord-import gem --- .rubocop.yml | 3 ++ Gemfile | 1 + Gemfile.lock | 3 ++ app/models/bus.rb | 3 +- app/models/buses_service.rb | 4 +++ app/models/service.rb | 3 +- app/services/db_importer.rb | 55 +++++++++++++++++++++++++------------ 7 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 app/models/buses_service.rb diff --git a/.rubocop.yml b/.rubocop.yml index 1b2270b..7008ee7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,9 @@ AllCops: - "*Gemfile*" - "tmp/**/*" +Metrics/AbcSize: + Enabled: false + Metrics/LineLength: Max: 120 diff --git a/Gemfile b/Gemfile index d3d1078..8ce8b25 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem 'puma', '~> 3.11' gem 'bootsnap', '>= 1.1.0', require: false gem 'oj' gem 'strong_migrations' +gem 'activerecord-import' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 3a1caf0..f23f37b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,6 +33,8 @@ GEM activemodel (= 5.2.3) activesupport (= 5.2.3) arel (>= 9.0) + activerecord-import (1.0.1) + activerecord (>= 3.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -154,6 +156,7 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) diff --git a/app/models/bus.rb b/app/models/bus.rb index 7cea181..a08b7e4 100644 --- a/app/models/bus.rb +++ b/app/models/bus.rb @@ -13,7 +13,8 @@ class Bus < ApplicationRecord ].freeze has_many :trips - has_and_belongs_to_many :services, join_table: :buses_services + has_many :buses_services + has_many :services, through: :buses_services validates :number, presence: true, uniqueness: true validates :model, inclusion: { in: MODELS } diff --git a/app/models/buses_service.rb b/app/models/buses_service.rb new file mode 100644 index 0000000..6219d44 --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,4 @@ +class BusesService < ApplicationRecord + belongs_to :bus + belongs_to :service +end diff --git a/app/models/service.rb b/app/models/service.rb index e2eb93e..3d3b64b 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -12,7 +12,8 @@ class Service < ApplicationRecord 'Можно не печатать билет' ].freeze - has_and_belongs_to_many :buses, join_table: :buses_services + has_many :buses_services + has_many :buses, through: :buses_services validates :name, presence: true validates :name, inclusion: { in: SERVICES } diff --git a/app/services/db_importer.rb b/app/services/db_importer.rb index 4dd1cc1..139f0cc 100644 --- a/app/services/db_importer.rb +++ b/app/services/db_importer.rb @@ -1,6 +1,12 @@ require 'oj' class DbImporter + def initialize + @cities = {} + @services = {} + @buses = {} + end + def call(source:) json = Oj.load_file(source) @@ -10,6 +16,10 @@ def call(source:) end end + private + + attr_reader :cities, :services, :buses + def clear_db! City.delete_all Bus.delete_all @@ -19,41 +29,52 @@ def clear_db! end def create_from_json!(json) - cities = Set.new - services = Set.new + import_cities_and_services(json) + import_buses(json) + import_trips(json) + end + def import_cities_and_services(json) json.each do |trip| - cities << trip['from'] - cities << trip['to'] + cities[trip['from']] ||= City.new(name: trip['from']) + cities[trip['to']] ||= City.new(name: trip['to']) trip['bus']['services'].each do |service| - services << service + services[service] ||= Service.new(name: service) end end - cities.each do |city| - City.create(name: city) - end + City.import %i[name], cities.values, syncronize: true, raise_error: true + Service.import %i[name], services.values, syncronize: true, raise_error: true + end - services.each do |service| - Service.create(name: service) + def import_buses(json) + json.each do |trip| + bus = buses[trip['bus']['number']] || Bus.new(number: trip['bus']['number']) + bus.model = trip['bus']['model'] + bus.services = services.values_at(*trip['bus']['services']) + buses[trip['bus']['number']] = bus end + Bus.import buses.values, recursive: true, syncronize: true, raise_error: true + end + + def import_trips(json) + trips = [] json.each do |trip| - from = City.find_by(name: trip['from']) - to = City.find_by(name: trip['to']) - services = Service.where(name: trip['bus']['services']) - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) + from = cities[trip['from']] + to = cities[trip['to']] - Trip.create!( + trips << Trip.new( from: from, to: to, - bus: bus, + bus: buses[trip['bus']['number']], start_time: trip['start_time'], duration_minutes: trip['duration_minutes'], price_cents: trip['price_cents'] ) end + + Trip.import trips, raise_error: true end end From 2d90db858f8b03cca17fc1c23084e74b99c4636f Mon Sep 17 00:00:00 2001 From: Anna Selivanova Date: Sun, 14 Apr 2019 18:48:50 +0400 Subject: [PATCH 7/7] Optimize page load --- .gitignore | 2 + Gemfile | 7 +++- Gemfile.lock | 10 +++++ app/controllers/trips_controller.rb | 4 +- app/helpers/application_helper.rb | 3 ++ app/views/trips/_delimiter.html.erb | 1 - app/views/trips/_service.html.erb | 1 - app/views/trips/_services.html.erb | 6 --- app/views/trips/_trip.html.erb | 21 +++++++--- app/views/trips/index.html.erb | 12 +----- config/newrelic.yml | 41 +++++++++++++++++++ config/routes.rb | 2 + .../20190413173437_add_indices_for_import.rb | 9 ---- db/migrate/20190414123049_add_indices.rb | 16 ++++++++ db/schema.rb | 13 ++++-- test/integration/trips_index_test.rb | 28 +++++++++++++ 16 files changed, 138 insertions(+), 38 deletions(-) delete mode 100644 app/views/trips/_delimiter.html.erb delete mode 100644 app/views/trips/_service.html.erb delete mode 100644 app/views/trips/_services.html.erb create mode 100644 config/newrelic.yml delete mode 100644 db/migrate/20190413173437_add_indices_for_import.rb create mode 100644 db/migrate/20190414123049_add_indices.rb create mode 100644 test/integration/trips_index_test.rb diff --git a/.gitignore b/.gitignore index 18b43c9..931707d 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,5 @@ # Ignore master key for decrypting credentials and more. /config/master.key + +.env diff --git a/Gemfile b/Gemfile index 8ce8b25..a0ef96f 100644 --- a/Gemfile +++ b/Gemfile @@ -7,9 +7,14 @@ gem 'rails', '~> 5.2.3' gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' gem 'bootsnap', '>= 1.1.0', require: false + +gem 'activerecord-import' +gem 'dotenv-rails' +gem 'newrelic_rpm' gem 'oj' +gem 'pghero' gem 'strong_migrations' -gem 'activerecord-import' + group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index f23f37b..5189d9b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,6 +53,10 @@ GEM byebug (11.0.1) concurrent-ruby (1.1.5) crass (1.0.4) + dotenv (2.7.2) + dotenv-rails (2.7.2) + dotenv (= 2.7.2) + railties (>= 3.2, < 6.1) erubi (1.8.0) ffi (1.10.0) globalid (0.4.2) @@ -77,6 +81,7 @@ GEM mini_portile2 (2.4.0) minitest (5.11.3) msgpack (1.2.9) + newrelic_rpm (6.2.0.354) nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) @@ -85,6 +90,8 @@ GEM parser (2.6.0.0) ast (~> 2.4.0) pg (1.1.4) + pghero (2.2.0) + activerecord psych (3.1.0) puma (3.12.1) rack (2.0.6) @@ -159,9 +166,12 @@ DEPENDENCIES activerecord-import bootsnap (>= 1.1.0) byebug + dotenv-rails listen (>= 3.0.5, < 3.2) + newrelic_rpm oj pg (>= 0.18, < 2.0) + pghero puma (~> 3.11) rails (~> 5.2.3) rubocop diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be..29bdaf8 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -2,6 +2,8 @@ class TripsController < ApplicationController def index @from = City.find_by_name!(params[:from]) @to = City.find_by_name!(params[:to]) - @trips = Trip.where(from: @from, to: @to).order(:start_time) + @trips = Trip.eager_load(bus: :services) + .where(from: @from, to: @to) + .order(:start_time) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index de6be79..0bdce32 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,2 +1,5 @@ module ApplicationHelper + def trips_delimiter + '===================================================='.freeze + end end diff --git a/app/views/trips/_delimiter.html.erb b/app/views/trips/_delimiter.html.erb deleted file mode 100644 index 3f845ad..0000000 --- a/app/views/trips/_delimiter.html.erb +++ /dev/null @@ -1 +0,0 @@ -==================================================== diff --git a/app/views/trips/_service.html.erb b/app/views/trips/_service.html.erb deleted file mode 100644 index 178ea8c..0000000 --- a/app/views/trips/_service.html.erb +++ /dev/null @@ -1 +0,0 @@ -
  • <%= "#{service.name}" %>
    • diff --git a/app/views/trips/_services.html.erb b/app/views/trips/_services.html.erb deleted file mode 100644 index 2de639f..0000000 --- a/app/views/trips/_services.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
  • Сервисы в автобусе:
    • -
      - <% services.each do |service| %> - <%= render "service", service: service %> - <% end %> -
    diff --git a/app/views/trips/_trip.html.erb b/app/views/trips/_trip.html.erb index fa1de9a..3b86df4 100644 --- a/app/views/trips/_trip.html.erb +++ b/app/views/trips/_trip.html.erb @@ -1,5 +1,16 @@ -
  • <%= "Отправление: #{trip.start_time}" %>
    • -
  • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
    • -
  • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
    • -
  • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
    • -
  • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
    • +
      +
    • <%= "Отправление: #{trip.start_time}" %>
      • +
    • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
      • +
    • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
      • +
    • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
      • +
    • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
      • + <% if trip.bus.services.present? %> +
    • Сервисы в автобусе:
      • +
        + <% trip.bus.services.each do |service| %> +
      • <%= "#{service.name}" %>
        • + <% end %> +
      + <% end %> +
    +<%= trips_delimiter %> diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce4..f131334 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,15 +2,7 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

    - <%= "В расписании #{@trips.count} рейсов" %> + <%= "В расписании #{@trips.length} рейсов" %>

    -<% @trips.each do |trip| %> -
      - <%= render "trip", trip: trip %> - <% if trip.bus.services.present? %> - <%= render "services", services: trip.bus.services %> - <% end %> -
    - <%= render "delimiter" %> -<% end %> +<%= render partial: "trip", collection: @trips %> diff --git a/config/newrelic.yml b/config/newrelic.yml new file mode 100644 index 0000000..1a36bd9 --- /dev/null +++ b/config/newrelic.yml @@ -0,0 +1,41 @@ +# +# This file configures the New Relic Agent. New Relic monitors Ruby, Java, +# .NET, PHP, Python, Node, and Go applications with deep visibility and low +# overhead. For more information, visit www.newrelic.com. +# +# Generated April 14, 2019 +# +# This configuration file is custom generated for Self-employed_368 +# +# For full documentation of agent configuration options, please refer to +# https://docs.newrelic.com/docs/agents/ruby-agent/installation-configuration/ruby-agent-configuration + +common: &default_settings + # Required license key associated with your New Relic account. + license_key: ENV["NEWRELIC_LICENSE_KEY"] + + # Your application name. Renaming here affects where data displays in New + # Relic. For more details, see https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/renaming-applications + app_name: Task4 + + # To disable the agent regardless of other settings, uncomment the following: + # agent_enabled: false + + # Logging level for log/newrelic_agent.log + log_level: info + + +# Environment-specific settings are in this section. +# RAILS_ENV or RACK_ENV (as appropriate) is used to determine the environment. +# If your application has other named environments, configure them here. +development: + <<: *default_settings + app_name: Task4 (Development) + +test: + <<: *default_settings + # It doesn't make sense to report to New Relic from automated test runs. + monitor_mode: false + +production: + <<: *default_settings diff --git a/config/routes.rb b/config/routes.rb index a2da6a7..92916b5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,4 +2,6 @@ # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html get "/" => "statistics#index" get "автобусы/:from/:to" => "trips#index" + + mount PgHero::Engine, at: "pghero" end diff --git a/db/migrate/20190413173437_add_indices_for_import.rb b/db/migrate/20190413173437_add_indices_for_import.rb deleted file mode 100644 index 19a3bc4..0000000 --- a/db/migrate/20190413173437_add_indices_for_import.rb +++ /dev/null @@ -1,9 +0,0 @@ -class AddIndicesForImport < ActiveRecord::Migration[5.2] - disable_ddl_transaction! - - def change - add_index :cities, :name, algorithm: :concurrently - add_index :services, :name, algorithm: :concurrently - add_index :buses, :number, algorithm: :concurrently - end -end diff --git a/db/migrate/20190414123049_add_indices.rb b/db/migrate/20190414123049_add_indices.rb new file mode 100644 index 0000000..e29c457 --- /dev/null +++ b/db/migrate/20190414123049_add_indices.rb @@ -0,0 +1,16 @@ +class AddIndices < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :buses_services, [:bus_id, :service_id], unique: true, algorithm: :concurrently + + add_foreign_key :trips, :buses + add_foreign_key :trips, :cities, column: :from_id + add_foreign_key :trips, :cities, column: :to_id + + add_index :trips, :bus_id, algorithm: :concurrently + add_index :trips, :from_id, algorithm: :concurrently + add_index :trips, :to_id, algorithm: :concurrently + add_index :trips, :start_time, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 119722a..2c85f2b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_04_13_173437) do +ActiveRecord::Schema.define(version: 2019_04_14_123049) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -18,22 +18,20 @@ create_table "buses", force: :cascade do |t| t.string "number" t.string "model" - t.index ["number"], name: "index_buses_on_number" end create_table "buses_services", force: :cascade do |t| t.integer "bus_id" t.integer "service_id" + t.index ["bus_id", "service_id"], name: "index_buses_services_on_bus_id_and_service_id", unique: true end create_table "cities", force: :cascade do |t| t.string "name" - t.index ["name"], name: "index_cities_on_name" end create_table "services", force: :cascade do |t| t.string "name" - t.index ["name"], name: "index_services_on_name" end create_table "trips", force: :cascade do |t| @@ -43,6 +41,13 @@ t.integer "duration_minutes" t.integer "price_cents" t.integer "bus_id" + t.index ["bus_id"], name: "index_trips_on_bus_id" + t.index ["from_id"], name: "index_trips_on_from_id" + t.index ["start_time"], name: "index_trips_on_start_time" + t.index ["to_id"], name: "index_trips_on_to_id" end + add_foreign_key "trips", "buses" + add_foreign_key "trips", "cities", column: "from_id" + add_foreign_key "trips", "cities", column: "to_id" end diff --git a/test/integration/trips_index_test.rb b/test/integration/trips_index_test.rb new file mode 100644 index 0000000..b552e4a --- /dev/null +++ b/test/integration/trips_index_test.rb @@ -0,0 +1,28 @@ +require 'test_helper' + +class TripsIndexTest < ActionDispatch::IntegrationTest + def setup + DbImporter.new.call(source: 'test/fixtures/files/trip_import.json') + end + + test 'responds with success status' do + make_request + + assert_response :success + end + + test 'displays all relevant records' do + make_request + + assert response.body.include?('В расписании 5 рейсов') + + Trip.where(from: 'Самара', to: 'Москва').find_each do |trip| + assert response.body.include?("Отправление: #{trip.start_time}") + assert response.body.include?("Автобус: #{trip.bus.model} №#{trip.bus.number}") + end + end + + def make_request + get URI.encode('/автобусы/Самара/Москва') + end +end