Merge pull request #238 from lnu-norge/index-aggregations

Minor performance improvements for aggregations

app/controllers/facility_reviews_controller.rb

@@ -29,9 +29,6 @@ def create
 
     return create_failed unless FacilityReview.create!(relevant_reviews)
 
-    # NB: Under the current model, this needs to happen before we add the descriptions with update_space_facilities,
-    # as aggregating DESTROYS all space_facilities, including the descriptions (wtf)
-    @space.aggregate_facility_reviews(facilities: @affected_facilities)
     update_space_facilities(reviews_and_descriptions[:space_facilities_attributes].values)
 
     create_succeeded

app/models/facility_review.rb

@@ -13,6 +13,10 @@ class FacilityReview < ApplicationRecord
 
   validates :space, uniqueness: { scope: [:user, :facility] }
 
+  after_commit do
+    space.aggregate_facility_reviews(facilities: [facility])
+  end
+
   def experience_icon
     ICON_FOR_EXPERIENCE[experience]
   end
@@ -46,6 +50,7 @@ def experience_icon
 #
 #  index_facility_reviews_on_facility_id                           (facility_id)
 #  index_facility_reviews_on_space_id                              (space_id)
+#  index_facility_reviews_on_space_id_and_facility_id              (space_id,facility_id)
 #  index_facility_reviews_on_space_id_and_user_id_and_facility_id  (space_id,user_id,facility_id) UNIQUE
 #  index_facility_reviews_on_user_id                               (user_id)
 #

app/models/space.rb

@@ -34,10 +34,14 @@ class Space < ApplicationRecord # rubocop:disable Metrics/ClassLength
           south_east_lat:,
           south_east_lng:)
   }
+
+  # This scope cuts the db calls when aggregating space_facilities
+  has_many :facility_reviews_ordered_by_newest_first, -> { order(created_at: :desc) },
+           class_name: "FacilityReview", dependent: :destroy, inverse_of: :space
   scope :with_aggregation_data, lambda {
     preload(
       :space_facilities,
-      :facility_reviews,
+      :facility_reviews_ordered_by_newest_first,
       space_types: [
         :facilities
       ]

app/models/space_types_facility.rb

@@ -4,13 +4,7 @@ class SpaceTypesFacility < ApplicationRecord
   belongs_to :space_type
   belongs_to :facility
 
-  after_create do
-    space_type.reload.spaces.each do |space|
-      space.reload.aggregate_facility_reviews(facilities: [facility])
-    end
-  end
-
-  after_destroy do
+  after_commit do
     space_type.reload.spaces.each do |space|
       space.reload.aggregate_facility_reviews(facilities: [facility])
     end
@@ -29,8 +23,9 @@ class SpaceTypesFacility < ApplicationRecord
 #
 # Indexes
 #
-#  index_space_types_facilities_on_facility_id    (facility_id)
-#  index_space_types_facilities_on_space_type_id  (space_type_id)
+#  index_space_types_facilities_on_facility_id                    (facility_id)
+#  index_space_types_facilities_on_space_type_id                  (space_type_id)
+#  index_space_types_facilities_on_space_type_id_and_facility_id  (space_type_id,facility_id) UNIQUE
 #
 # Foreign Keys
 #

app/models/space_types_relation.rb

@@ -4,11 +4,7 @@ class SpaceTypesRelation < ApplicationRecord
   belongs_to :space_type
   belongs_to :space
 
-  after_save do
-    space.reload.aggregate_facility_reviews(facilities: space_type.facilities)
-  end
-
-  after_destroy do
+  after_commit do
     space.reload.aggregate_facility_reviews(facilities: space_type.facilities)
   end
 end

app/services/spaces/aggregate_facility_reviews_service.rb

@@ -11,6 +11,7 @@ def initialize(space:, facilities: [])
       @facilities = facilities.any? ? facilities : Facility.order(:created_at)
       @space_facilities = @space.space_facilities
       @space_types = @space.space_types
+      @facility_reviews = @space.facility_reviews_ordered_by_newest_first
       group_recent_facility_reviews_by_facility(count: 5)
 
       super()

app/services/spaces/concerns/countable_reviews.rb

@@ -8,10 +8,9 @@ def most_recent_facility_reviews_for(facility)
   end
 
   def group_recent_facility_reviews_by_facility(count: 5)
-    @grouped_facility_reviews = @space.facility_reviews
-                                      .order(created_at: :desc)
-                                      .group_by(&:facility_id)
-                                      .transform_values { |reviews| reviews.first(count) }
+    @grouped_facility_reviews = @facility_reviews
+                                .group_by(&:facility_id)
+                                .transform_values { |reviews| reviews.first(count) }
   end
 
   def facility_reviews_for(facility)

db/migrate/20240302202011_add_indexes_for_aggregations.rb

@@ -0,0 +1,9 @@
+class AddIndexesForAggregations < ActiveRecord::Migration[7.1]
+  def change
+    # Add index for facility_reviews on space_id and facility_id
+    add_index :facility_reviews, %i[space_id facility_id]
+
+    # Add index for space types facilities on space_type_id and facility_id
+    add_index :space_types_facilities, [:space_type_id, :facility_id], unique: true
+  end
+end

spec/fabricators/facility_review_fabricator.rb

@@ -23,6 +23,7 @@
 #
 #  index_facility_reviews_on_facility_id                           (facility_id)
 #  index_facility_reviews_on_space_id                              (space_id)
+#  index_facility_reviews_on_space_id_and_facility_id              (space_id,facility_id)
 #  index_facility_reviews_on_space_id_and_user_id_and_facility_id  (space_id,user_id,facility_id) UNIQUE
 #  index_facility_reviews_on_user_id                               (user_id)
 #

spec/fabricators/space_types_facility_fabricator.rb

@@ -17,8 +17,9 @@
 #
 # Indexes
 #
-#  index_space_types_facilities_on_facility_id    (facility_id)
-#  index_space_types_facilities_on_space_type_id  (space_type_id)
+#  index_space_types_facilities_on_facility_id                    (facility_id)
+#  index_space_types_facilities_on_space_type_id                  (space_type_id)
+#  index_space_types_facilities_on_space_type_id_and_facility_id  (space_type_id,facility_id) UNIQUE
 #
 # Foreign Keys
 #

spec/services/spaces/aggregate_facility_reviews_service_spec.rb

@@ -198,12 +198,12 @@ def experience(experience, other_facility = nil)
   end
 
   it "is performant for a changing a single facility, even if there are a large amount of other facilities" do
-    Array.new(10) { Fabricate(:facility, space_types: [space_type]) }
+    Array.new(20) { Fabricate(:facility, space_types: [space_type]) }
     expect { space.aggregate_facility_reviews(facilities: [facility]) }.to perform_under(20).ms
   end
 
   it "is performant for a large amount of facilities" do
-    Array.new(10) { Fabricate(:facility, space_types: [space_type]) }
+    Array.new(20) { Fabricate(:facility, space_types: [space_type]) }
     expect { space.aggregate_facility_reviews }.to perform_under(30).ms
   end
 end