optimization

.gitignore

@@ -7,6 +7,8 @@
 # Ignore bundler config.
 /.bundle
 
+/vendor/bundle
+/bundle
 # Ignore all logfiles and tempfiles.
 /log/*
 /tmp/*

.rspec

@@ -0,0 +1 @@
+--require spec_helper

Gemfile

@@ -7,10 +7,16 @@ 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 'strong_migrations'
 
 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 'rspec-rails', '~> 3.8'
+  gem "rspec-sqlimit"
+  gem 'rails-controller-testing'
+  gem 'capybara'
 end
 
 group :development do
@@ -20,6 +26,7 @@ group :development do
 end
 
 group :test do
+  gem 'database_cleaner'
 end
 
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem

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)
@@ -42,14 +44,26 @@ GEM
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
       tzinfo (~> 1.1)
+    addressable (2.6.0)
+      public_suffix (>= 2.0.2, < 4.0)
     arel (9.0.0)
     bindex (0.6.0)
     bootsnap (1.4.2)
       msgpack (~> 1.0)
     builder (3.2.3)
     byebug (11.0.1)
+    capybara (3.16.1)
+      addressable
+      mini_mime (>= 0.1.3)
+      nokogiri (~> 1.8)
+      rack (>= 1.6.0)
+      rack-test (>= 0.6.3)
+      regexp_parser (~> 1.2)
+      xpath (~> 3.2)
     concurrent-ruby (1.1.5)
     crass (1.0.4)
+    database_cleaner (1.7.0)
+    diff-lcs (1.3)
     erubi (1.8.0)
     ffi (1.10.0)
     globalid (0.4.2)
@@ -77,6 +91,7 @@ GEM
     nokogiri (1.10.2)
       mini_portile2 (~> 2.4.0)
     pg (1.1.4)
+    public_suffix (3.0.3)
     puma (3.12.1)
     rack (2.0.6)
     rack-test (1.1.0)
@@ -94,6 +109,10 @@ GEM
       bundler (>= 1.3.0)
       railties (= 5.2.3)
       sprockets-rails (>= 2.0.0)
+    rails-controller-testing (1.0.4)
+      actionpack (>= 5.0.1.x)
+      actionview (>= 5.0.1.x)
+      activesupport (>= 5.0.1.x)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
       nokogiri (>= 1.6)
@@ -109,6 +128,31 @@ GEM
     rb-fsevent (0.10.3)
     rb-inotify (0.10.0)
       ffi (~> 1.0)
+    regexp_parser (1.4.0)
+    rspec (3.8.0)
+      rspec-core (~> 3.8.0)
+      rspec-expectations (~> 3.8.0)
+      rspec-mocks (~> 3.8.0)
+    rspec-core (3.8.0)
+      rspec-support (~> 3.8.0)
+    rspec-expectations (3.8.2)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.8.0)
+    rspec-mocks (3.8.0)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.8.0)
+    rspec-rails (3.8.2)
+      actionpack (>= 3.0)
+      activesupport (>= 3.0)
+      railties (>= 3.0)
+      rspec-core (~> 3.8.0)
+      rspec-expectations (~> 3.8.0)
+      rspec-mocks (~> 3.8.0)
+      rspec-support (~> 3.8.0)
+    rspec-sqlimit (0.0.2)
+      rails (> 4.0, < 6.0)
+      rspec (~> 3.0)
+    rspec-support (3.8.0)
     ruby_dep (1.5.0)
     sprockets (3.7.2)
       concurrent-ruby (~> 1.0)
@@ -117,6 +161,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)
@@ -129,17 +175,26 @@ GEM
     websocket-driver (0.7.0)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.3)
+    xpath (3.2.0)
+      nokogiri (~> 1.8)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
+  activerecord-import
   bootsnap (>= 1.1.0)
   byebug
+  capybara
+  database_cleaner
   listen (>= 3.0.5, < 3.2)
   pg (>= 0.18, < 2.0)
   puma (~> 3.11)
   rails (~> 5.2.3)
+  rails-controller-testing
+  rspec-rails (~> 3.8)
+  rspec-sqlimit
+  strong_migrations
   tzinfo-data
   web-console (>= 3.3.0)
 

app/controllers/trips_controller.rb

@@ -2,6 +2,6 @@ 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.where(from: @from, to: @to).order(:start_time).eager_load(bus: :services)
   end
 end

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, source: :service
 
   validates :number, presence: true, uniqueness: true
   validates :model, inclusion: { in: MODELS }

app/models/buses_service.rb

@@ -0,0 +1,4 @@
+class BusesService < ApplicationRecord
+  belongs_to :bus
+  belongs_to :service
+end

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, source: :bus
 
   validates :name, presence: true
   validates :name, inclusion: { in: SERVICES }

app/views/trips/_service.html.erb

@@ -1 +1,3 @@
-<li><%= "#{service.name}" %></li>
+<div class='service-item'>
+  <li><%= "#{service.name}" %></li>
+</div>

app/views/trips/_trip.html.erb

@@ -1,5 +1,7 @@
-<li><%= "Отправление: #{trip.start_time}" %></li>
-<li><%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %></li>
-<li><%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %></li>
-<li><%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %></li>
-<li><%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %></li>
+<div class="trip-item">
+  <li><%= "Отправление: #{trip.start_time}" %></li>
+  <li><%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %></li>
+  <li><%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %></li>
+  <li><%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %></li>
+  <li><%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %></li>
+</div>

app/views/trips/index.html.erb

@@ -2,7 +2,7 @@
   <%= "Автобусы #{@from.name} – #{@to.name}" %>
 </h1>
 <h2>
-  <%= "В расписании #{@trips.count} рейсов" %>
+  <%= "В расписании #{@trips.length} рейсов" %>
 </h2>
 
 <% @trips.each do |trip| %>

bin/setup

@@ -28,8 +28,8 @@ chdir APP_ROOT do
   puts "\n== Preparing database =="
   system! 'bin/rails db:setup'
 
-  puts "\n== Loading data from fixtures/small.json =="
-  system! 'bin/rake reload_json[fixtures/small.json]'
+  puts "\n== Loading data from fixtures/large.json =="
+  system! 'bin/rake reload_json[fixtures/large.json]'
 
   puts "\n== Removing old logs and tempfiles =="
   system! 'bin/rails log:clear tmp:clear'

db/migrate/20190403122705_add_indexes_to_trip.rb

@@ -0,0 +1,10 @@
+class AddIndexesToTrip < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :trips, [:from_id, :to_id], algorithm: :concurrently
+    add_index :trips, :bus_id, algorithm: :concurrently
+    add_index :buses_services, :bus_id, algorithm: :concurrently
+    add_index :buses_services, :service_id, algorithm: :concurrently
+  end
+end

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_03_122705) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -23,6 +23,8 @@
   create_table "buses_services", force: :cascade do |t|
     t.integer "bus_id"
     t.integer "service_id"
+    t.index ["bus_id"], name: "index_buses_services_on_bus_id"
+    t.index ["service_id"], name: "index_buses_services_on_service_id"
   end
 
   create_table "cities", force: :cascade do |t|
@@ -40,6 +42,8 @@
     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", "to_id"], name: "index_trips_on_from_id_and_to_id"
   end
 
 end

lib/tasks/utils.rake

@@ -10,25 +10,43 @@ task :reload_json, [:file_name] => :environment do |_task, args|
     Trip.delete_all
     ActiveRecord::Base.connection.execute('delete from buses_services;')
 
+    cities = {}
+    services = {}
+
+    json.each do |trip|
+      cities[trip['from']] = City.new(name: trip['from']) if trip['from']
+      cities[trip['to']] = City.new(name: trip['to']) if trip['to']
+      trip['bus']['services'].each { |s| services.merge!(s => Service.new(name: s)) }
+    end
+
+    City.import cities.values
+    Service.import services.values
+
+    buses = {}
     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,
+      bus = Bus.new(number: trip['bus']['number'], model: trip['bus']['model'])
+      buses.merge!(trip['bus']['number'] => [bus, services.values_at(*trip['bus']['services'])])
+    end
+
+    Bus.import buses.values.map(&:first)
+
+    buses_services = []
+    buses.values.each do |bus_with_service|
+      bus_with_service.second.each { |s| buses_services << { bus_id: bus_with_service.first.id, service_id: s.id } }
+    end
+    BusesService.import buses_services
+
+    trips = json.map! do |trip|
+      {
+        from_id: cities.fetch(trip['from']).id,
+        to_id: cities.fetch(trip['to']).id,
+        bus_id: buses.fetch(trip['bus']['number']).first.id,
         start_time: trip['start_time'],
         duration_minutes: trip['duration_minutes'],
-        price_cents: trip['price_cents'],
-      )
+        price_cents: trip['price_cents']
+      }
     end
+
+    Trip.import trips
   end
 end

spec/controllers/trips_controller_spec.rb

@@ -0,0 +1,35 @@
+require 'rails_helper'
+require 'rspec-sqlimit'
+require 'rake'
+require 'benchmark'
+load 'Rakefile'
+
+RSpec.describe TripsController, type: :controller do
+  before do
+    Rake::Task['reload_json'].reenable
+    Rake::Task['reload_json'].invoke('fixtures/small.json')
+  end
+  let!(:trip) { Trip.preload(:from, :to).take }
+
+  it 'correct trips count' do
+    get :index, params: { from: trip.from.name , to: trip.to.name }
+    expect(assigns(:trips).size).to eq(9)
+    expect(assigns(:from).name).to eq(trip.from.name)
+    expect(assigns(:to).name).to eq(trip.to.name)
+    end
+
+
+  it 'run in time' do
+    time = Benchmark.realtime do
+      get :index, params: { from: trip.from.name , to: trip.to.name }
+    end
+
+    puts "Controller time: #{time}"
+
+    expect(time).to be <  0.006
+  end
+
+  it "doesn't send unnecessary requests to db" do
+    expect { get :index, params: { from: trip.from.name , to: trip.to.name } }.not_to exceed_query_limit(2)
+  end
+end