Turn on strict_loading to catch n+1 queries

app/controllers/api/v4/application_controller.rb

@@ -71,9 +71,9 @@ def set_runner
 
   def set_run
     @run = if params[:historic] == "1"
-        Run.includes(:game, :category, :user, :histories, segments: [:histories]).find36(params[:run])
+        Run.includes(:user, :histories, :video, category: [:srdc], game: [:srdc], segments: [:histories]).find36(params[:run])
       else
-        Run.includes(:game, :category, :user, :segments).find36(params[:run])
+        Run.includes(:user, :segments, :video, category: [:srdc], game: [:srdc]).find36(params[:run])
       end
   rescue ActiveRecord::RecordNotFound
     render not_found(:run)
@@ -110,7 +110,7 @@ def validate_user
   end
 
   def set_race(param: :race_id) # rubocop:disable Naming/AccessorMethodName
-    @race = Race.find(params[param])
+    @race = Race.includes(category: [:srdc], entries: [:creator], game: [:srdc]).find(params[param])
     return unless @race.secret_visibility? && !@race.joinable?(user: current_user, token: params[:join_token])
 
     render status: :forbidden, json: {

app/controllers/api/v4/games_controller.rb

@@ -2,11 +2,12 @@ class Api::V4::GamesController < Api::V4::ApplicationController
   before_action :set_game, only: [:show]
 
   def index
+    games = Game.includes(:srdc, categories: [:srdc])
     if params[:search].blank?
-      @games = Game.joins(:srdc).includes(:srdc, categories: [:srdc]).order(:name)
+      @games = games.joins(:srdc).order(:name)
     else
-      id_search = Game.find_by(id: params[:search])
-      @games = Game.search(params[:search]).includes(:srdc, categories: [:srdc])
+      id_search = games.find_by(id: params[:search])
+      @games = games.search(params[:search])
       @games = @games.to_ary.unshift(id_search) if id_search.present?
     end
     render json: Api::V4::GameBlueprint.render(@games, root: :games, toplevel: :game)

app/controllers/api/v4/races/entries_controller.rb

@@ -112,7 +112,7 @@ def check_permission
   end
 
   def set_entry(param: :id) # rubocop:disable Naming/AccessorMethodName
-    @entry = @race.entries.find(params[param])
+    @entry = @race.entries.includes(:creator).find(params[param])
     return if @entry.creator == current_user
 
     render status: :forbidden, json: {

app/controllers/games_controller.rb

@@ -22,8 +22,9 @@ def authorize
   end
 
   def set_game
-    @game = Game.joins(:srdc).find_by(speedrun_dot_com_games: {shortname: params[:game]}) ||
-            Game.includes(:srdc).find(id: params[:game])
+    games = Game.includes(:srdc, :srl)
+    @game = games.joins(:srdc).find_by(speedrun_dot_com_games: {shortname: params[:game]}) ||
+            games.find(id: params[:game])
 
     redirect_to game_path(@game) if @game.srdc.try(:shortname).present? && params[:game] == @game.id.to_s
   rescue ActiveRecord::RecordNotFound

app/controllers/runs_controller.rb

@@ -115,8 +115,8 @@ def destroy
   private
 
   def set_run
-    @run = Run.includes(segments: {icon_attachment: :blob}).find_by(id: params[:id].to_i(36)) ||
-           Run.includes(segments: {icon_attachment: :blob}).find_by!(nick: params[:id])
+    runs = Run.includes(:entry, :highlight_suggestion, :segments, :user, :video, category: [:srdc], game: [:srdc], segments: {icon_attachment: :blob})
+    @run = runs.find_by(id: params[:id].to_i(36)) || runs.find_by!(nick: params[:id])
     timing = params[:timing] || @run.default_timing
     unless [Run::REAL, Run::GAME].include?(timing)
       redirect_to request.path, alert: 'Timing can only be real or game.'

app/controllers/sessions_controller.rb

@@ -4,7 +4,7 @@ def new
   end
 
   def create
-    @user = User.find_by(email: session_params[:email])
+    @user = User.includes(:google, :twitch).find_by(email: session_params[:email])
     if @user.nil?
       redirect_back(
         alert: "No account exists with that email address.",
@@ -85,7 +85,7 @@ def failure
 
   def sign_in(user)
     create_auth_session(user)
-    auth_session.persist!
+    auth_session.persist
   end
 
   def redirect_path

app/models/category.rb

@@ -93,16 +93,15 @@ def merge_into!(category)
   # route returns a run whose route is the most popular in this category, as determined by the number of runs sharing
   # its segment names and order. Returns nil if no runs exist in this category.
   def route
-    result = Run.select('MIN(runs.id) as id')
-                .joins(:segments)
-                .where(category: self)
-                .group(:segment_number, :name)
-                .order(Arel.sql('COUNT(*) DESC'))
-                .first
+    result = runs.select('MIN(runs.id) as id')
+                 .joins(:segments)
+                 .group(:segment_number, :name)
+                 .order(Arel.sql('COUNT(*) DESC'))
+                 .first
 
     return nil if result.nil?
 
-    Run.find(result.id)
+    Run.includes(:user).find(result.id)
   end
 
   # median_duration returns the median duration of completed attempts for this category. If attempt_number is given, it

app/models/concerns/forgetful_persons_run.rb

@@ -8,36 +8,43 @@ module ForgetfulPersonsRun
     # is not complete (i.e. the last segment and any # segments directly before it have nil durations), we don't
     # return that last stretch of segments at all.
     def collapsed_segments(timing)
-      in_progress_segments = segments.reverse.take_while { |segment| segment.duration(timing).nil? }
-      segments[0..segments.count - 1 - in_progress_segments.count].reduce([]) do |segs, seg|
-        if segs.any? && segs.last.try(:duration, timing).nil?
-          skipped_seg = segs.last
-          segs + [Segment.new(
-            segs.pop.attributes.merge(
-              name: "#{skipped_seg.name} + #{seg.name}",
-
-              realtime_start_ms:             skipped_seg.start(Run::REAL).to_ms,
-              realtime_end_ms:               seg.end(Run::REAL).to_ms,
-              realtime_duration_ms:          seg.duration(Run::REAL).to_ms,
-              realtime_shortest_duration_ms: [skipped_seg, seg].map { |s| s.shortest_duration(Run::REAL).to_ms }.compact.sum(0),
-
-              gametime_start_ms:             skipped_seg.start(Run::GAME).to_ms,
-              gametime_end_ms:               skipped_seg.end(Run::GAME).to_ms,
-              gametime_duration_ms:          seg.duration(Run::GAME).to_ms,
-              gametime_shortest_duration_ms: [skipped_seg, seg].map { |s| s.shortest_duration(Run::GAME).to_ms }.compact.sum(0),
-            ).merge(
-              case timing
-              when Run::REAL
-                {realtime_reduced: true}
-              when Run::GAME
-                {gametime_reduced: true}
-              end
-            )
-          )]
-        else
-          segs + [seg]
-        end
-      end.push(*in_progress_segments)
+      @collapsed_segments ||= begin
+        in_progress_segments = segments.reverse.take_while { |segment| segment.duration(timing).nil? }
+        segments[0..segments.count - 1 - in_progress_segments.count].reduce([]) do |segs, next_seg|
+          if segs.any? && segs.last.try(:duration, timing).nil?
+            skipped_seg = segs.last
+
+            # We're about to modify this segment in memory for display purposes,
+            # but we don't want to persist it to the database.
+            # We also don't want to use Segment.new,
+            # because its associations (e.g. segment.run) won't be cached so would cause duplicate queries.
+            skipped_seg.readonly!
+
+            skipped_seg.name = "#{skipped_seg.name} + #{next_seg.name}"
+
+            skipped_seg.realtime_start_ms = skipped_seg.start(Run::REAL).to_ms
+            skipped_seg.realtime_end_ms = next_seg.end(Run::REAL).to_ms
+            skipped_seg.realtime_duration_ms = next_seg.duration(Run::REAL).to_ms
+            skipped_seg.realtime_shortest_duration_ms = [skipped_seg, next_seg].map { |s| s.shortest_duration(Run::REAL).to_ms }.compact.sum(0)
+
+            skipped_seg.gametime_start_ms = skipped_seg.start(Run::GAME).to_ms
+            skipped_seg.gametime_end_ms = next_seg.end(Run::GAME).to_ms
+            skipped_seg.gametime_duration_ms = next_seg.duration(Run::GAME).to_ms
+            skipped_seg.gametime_shortest_duration_ms = [skipped_seg, next_seg].map { |s| s.shortest_duration(Run::GAME).to_ms }.compact.sum(0)
+
+            case timing
+            when Run::REAL
+              skipped_seg.realtime_reduced = true
+            when Run::GAME
+              skipped_seg.gametime_reduced = true
+            end
+
+            segs
+          else
+            segs + [next_seg]
+          end
+        end.push(*in_progress_segments)
+      end
     end
 
     def skipped_splits(timing)

app/models/entry.rb

@@ -1,6 +1,15 @@
 class Entry < ApplicationRecord
   belongs_to :race, touch: true
-  belongs_to :runner, class_name: 'User', optional: true
+  # TODO: RacesController#set_race uses FriendlyUUID to find the record,
+  # which currently has a bug preventing it from being found when using .includes.
+  # We should either fix that bug,
+  # or at least when we upgrade to Rails 7,
+  # update that method to override strict_loading
+  # (.strict_loading!(false))
+  # so we can remove the override from the entire association here.
+  #
+  # See: https://github.com/glacials/friendly_uuid/issues/16
+  belongs_to :runner, class_name: 'User', optional: true, strict_loading: false
   belongs_to :creator, class_name: 'User'
   belongs_to :run, dependent: :destroy, optional: true
 

app/models/race.rb

@@ -7,13 +7,41 @@ class Race < ApplicationRecord
 
   belongs_to :user
 
-  belongs_to :game, optional: true
-  belongs_to :category, optional: true
+  # TODO: Race.unfinished uses a union query,
+  # which does not support calls to .includes.
+  # When we upgrade to Rails 7,
+  # update that method to override strict_loading
+  # (.strict_loading!(false))
+  # so we can remove the override from the entire association here.
+  belongs_to :game, optional: true, strict_loading: false
+  # TODO: Race.unfinished uses a union query,
+  # which does not support calls to .includes.
+  # When we upgrade to Rails 7,
+  # update that method to override strict_loading
+  # (.strict_loading!(false))
+  # so we can remove the override from the entire association here.
+  belongs_to :category, optional: true, strict_loading: false
 
   enum visibility: { public: 0, invite_only: 1, secret: 2 }, _suffix: true
 
-  belongs_to :owner, foreign_key: :user_id, class_name: "User"
-  has_many :entries, dependent: :destroy
+  # TODO: Race.unfinished uses a union query,
+  # which does not support calls to .includes.
+  # When we upgrade to Rails 7,
+  # update that method to override strict_loading
+  # (.strict_loading!(false))
+  # so we can remove the override from the entire association here.
+  belongs_to :owner, foreign_key: :user_id, class_name: "User", strict_loading: false
+
+  # TODO: RacesController#create renders the created race as JSON,
+  # but it is impossible
+  # (and nonsensical))
+  # to preload associations for a race just created.
+  # When we upgrade to Rails 7,
+  # update that method to override strict_loading
+  # (.strict_loading!(false))
+  # so we can remove the override from the entire association here.
+  has_many :entries, dependent: :destroy, strict_loading: false
+
   has_many :runners, through: :entries
   has_many :chat_messages, dependent: :destroy
 

app/models/user.rb

@@ -4,6 +4,13 @@ class User < ApplicationRecord
   include RivalUser
   include RunnerUser
 
+  # current_user is controlled by Authie and is difficult to inject .includes calls into.
+  # Relevant issue: https://github.com/adamcooke/authie/issues/42
+  #
+  # Can also turn this off when upgrading to Rails 7,
+  # which allows calling .strict_loading!(false) to bypass the requirement on individual record objects.
+  self.strict_loading_by_default = false
+
   has_many :runs, dependent: :nullify
   has_many :categories, -> { distinct }, through: :runs
   has_many :games, -> { distinct }, through: :runs
@@ -81,9 +88,9 @@ def to_param
   def pb_for(timing, category)
     case timing
     when Run::REAL
-      runs.where(category: category).order(realtime_duration_ms: :asc).first
+      runs.includes(:segments).where(category: category).order(realtime_duration_ms: :asc).first
     when Run::GAME
-      runs.where(category: category).order(gametime_duration_ms: :asc).first
+      runs.includes(:segments).where(category: category).order(gametime_duration_ms: :asc).first
     end
   end
 

app/validators/entry_validator.rb

@@ -7,7 +7,7 @@ def validate(record)
     validate_pre_start_race(record) unless record.race.started?
     validate_times(record)
     validate_in_progress_race(record) if record.race.in_progress?
-    record.errors[:base] << 'Cannot modify entry in finished race' if record.race.finished?
+    record.errors.add(:base, 'Cannot modify entry in finished race') if record.race.finished?
   end
 
   private
@@ -22,26 +22,26 @@ def validate_run_id(record)
       readied_at: Time.now.utc
     )
   rescue ActiveRecord::RecordNotFound
-    record.errors[:run_id] << 'No run with that ID exists'
+    record.errors.add(:run_id, 'No run with that ID exists')
   end
 
   def validate_new_record(record)
     # Reject new entry if race has already started
     if record.race.started?
-      record.errors[:base] << 'Cannot join race that has already started'
+      record.errors.add(:base, 'Cannot join race that has already started')
     end
 
     # Reject if entry's user is in another active race
     if !record.ghost? && record.runner.in_race?
-      record.errors[:base] << 'Cannot join more than one race at a time'
+      record.errors.add(:base, 'Cannot join more than one race at a time')
     end
   end
 
   # Rejects setting finish/forfeit times before a race starts
   def validate_pre_start_race(record)
     return unless record.finished_at_changed? || record.forfeited_at_changed?
 
-    record.errors[:base] << 'Cannot finish/forfeit before a race starts'
+    record.errors.add(:base, 'Cannot finish/forfeit before a race starts')
   end
 
   # Rejects times before the race start time
@@ -51,19 +51,19 @@ def validate_times(record)
     [record.finished_at, record.forfeited_at].each do |time|
       next unless time.present? && time < record.race.started_at
 
-      record.errors[:base] << "#{time} cannot be before race start time"
+      record.errors.add(:base, "#{time} cannot be before race start time")
     end
   end
 
   def validate_in_progress_race(record)
     # Reject changing ready time after race has started
     if record.readied_at_changed?
-      record.errors[:base] << 'Cannot change ready status once race has started'
+      record.errors.add(:base, 'Cannot change ready status once race has started')
     end
 
     # Reject both finished_at and forfeited_at being set
     if record.finished_at.present? && record.forfeited_at.present?
-      record.errors[:base] << 'Cannot finish and forfeit from the same race'
+      record.errors.add(:base, 'Cannot finish and forfeit from the same race')
     end
   end
 end

app/validators/video_validator.rb

@@ -17,7 +17,7 @@ def validate_url(record)
     end
 
     unless valid_domain?(record.url)
-      record.errors.add 'Your video URL must be a link to a Twitch or YouTube video.'
+      record.errors.add(:url, 'must be a link to a Twitch or YouTube video')
     end
 
     # Embeds break for URLs like https://www.twitch.tv/videos/29447340?filter=highlights&sort=time, which is what Twitch
@@ -26,7 +26,7 @@ def validate_url(record)
       record.url = URI(record.url).tap { |u| u.query = nil }.to_s
     end
   rescue URI::InvalidURIError
-    record.errors.add 'Your video URL must be a link to a Twitch or YouTube video.'
+    record.errors.add(:url, 'must be a link to a Twitch or YouTube video')
   end
 
   def valid_domain?(url)

app/views/layouts/application.slim

@@ -120,7 +120,7 @@ html lang='en'
             small: #hide-survey-button.text-secondary style='cursor: pointer' no thanks
           a#survey-button.btn.btn-outline-warning.btn-block.mb-0.text-center href=survey_url
             ' Help us out by taking the Splits.io survey!
-        - entry = current_user&.entries&.nonghosts&.active&.first
+        - entry = current_user&.entries&.includes(:race)&.nonghosts&.active&.first
         - if entry.present? && request.path != race_path(entry.race)
           .p-1.col-md-6.mx-auto
             a.btn.btn-success.btn-block.mb-0.text-center.glow href=race_path(entry.race)

app/views/runs/_compare_button.slim

@@ -14,7 +14,7 @@
         ' Compare
     .dropdown-menu aria={labelledby: 'compare'} style='height: auto; max-height: 400px; overflow-x: hidden'
         .dropdown-header Mine
-        - comparable_runs = current_user.present? ? current_user.comparable_runs(timing, run).where.not(id: run.id) : []
+        - comparable_runs = current_user.present? ? current_user.comparable_runs(timing, run).where.not(id: run.id).includes(:segments, :video) : []
         - if comparable_runs.none?
           .dropdown-item.disabled: i No comparable runs by me
         - comparable_runs.each do |comparable_run|

app/views/runs/_title.slim

@@ -7,7 +7,7 @@ h5
       => icon('fas', 'check')
       span Personal Best
   span.mr-2 = render 'runs/timing_badge', run: run, timing: timing
-  - old_runs = run.user.non_pbs([run.category_id]).group_by(&:category_id)[run.category_id]&.sort_by(&:created_at)&.reverse if run.user
+  - old_runs = run.user.non_pbs([run.category_id]).includes(:segments).group_by(&:category_id)[run.category_id]&.sort_by(&:created_at)&.reverse if run.user
   - if old_runs.present?
     span.dropdown-toggle.mr-2.cursor-pointer(
       content='Previous times'

config/application.rb

@@ -38,5 +38,9 @@ class Application < Rails::Application
     config.action_cable.mount_path = "/api/cable"
 
     config.active_storage.variant_processor = :vips
+
+    # Help detect n+1 queries,
+    # but get out of the way in the Rails console.
+    config.active_record.strict_loading_by_default = !Rails.const_defined?('Console')
   end
 end

config/environments/production.rb

@@ -122,4 +122,8 @@
 
   config.stripe.secret_key = ENV['STRIPE_PUBLISHABLE_KEY']
   config.stripe.publishable_key = ENV['STRIPE_SECRET_KEY']
+
+  # For strict loading violations, don't error; only log.
+  # See config/application.rb for the config.active_record.strict_loading_by_default setting.
+  config.active_record.action_on_strict_loading_violation = :log
 end