Skip to content

Commit

Permalink
Make aggregation a lot more performant, by skipping loads of steps to…
Browse files Browse the repository at this point in the history
… the db. Cutting from 200ms to 20ms mean time on 100 facilities.
  • Loading branch information
DanielJackson-Oslo committed Mar 1, 2024
1 parent 320c35f commit 1eac656
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 23 deletions.
13 changes: 12 additions & 1 deletion app/models/space.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ class Space < ApplicationRecord # rubocop:disable Metrics/ClassLength
south_east_lat:,
south_east_lng:)
}
scope :with_aggregation_data, lambda {
preload(
:space_facilities,
:facility_reviews,
space_types: [
:facilities
]
)
}

has_many :space_types_relations, dependent: :destroy
has_many :space_types, through: :space_types_relations, dependent: :destroy
Expand Down Expand Up @@ -80,7 +89,9 @@ def self.rect_of_spaces
end

def aggregate_facility_reviews(facilities: [])
Spaces::AggregateFacilityReviewsService.call(space: self, facilities:)
space = Space.with_aggregation_data.find(id)

Spaces::AggregateFacilityReviewsService.call(space:, facilities:)
end

def aggregate_star_rating
Expand Down
54 changes: 32 additions & 22 deletions app/services/spaces/aggregate_facility_reviews_service.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
# frozen_string_literal: true

require_relative "concerns/countable_reviews"

module Spaces
class AggregateFacilityReviewsService < ApplicationService
include CountableReviews

def initialize(space:, facilities: [])
@space = Space.includes(:space_facilities, space_types: :facilities, facility_reviews: :facility).find(space.id)
@space = space
@facilities = facilities.any? ? facilities : Facility.order(:created_at)
@space_facilities = @space.space_facilities
@space_types = @space.space_types
group_recent_facility_reviews_by_facility(count: 5)

super()
end
Expand All @@ -23,55 +30,58 @@ def aggregate_facilities
end
end

def aggregate_reviews(facility) # rubocop:disable Metrics/AbcSize
space_facility = SpaceFacility.find_or_create_by(space: @space, facility:)

reviews = @space.facility_reviews.where(facility:).order(created_at: :desc).limit(5)
count = reviews.count

def aggregate_reviews(facility)
space_facility = find_or_create_space_facility(facility)
reviews = most_recent_facility_reviews_for(facility)
belongs_to_space_type = facility_belongs_to_space_type(facility)

return handle_zero_facility_reviews(space_facility, belongs_to_space_type) if count.zero?
return handle_zero_facility_reviews(space_facility, belongs_to_space_type) if reviews.blank?

# Set criteria:
two_out_of_three = (count / 3.0 * 2.0).ceil
positive_threshold = reviews.positive.count >= two_out_of_three
impossible_threshold = reviews.impossible.count >= two_out_of_three
negative_threshold = reviews.negative.count >= two_out_of_three
two_out_of_three = (reviews.count / 3.0 * 2.0).ceil
positive_threshold = count_of_positive(reviews) >= two_out_of_three
impossible_threshold = count_of_impossible(reviews) >= two_out_of_three
negative_threshold = count_of_negative(reviews) >= two_out_of_three

set_relevance(space_facility, belongs_to_space_type, positive_threshold)

set_experience(space_facility, positive_threshold, impossible_threshold, negative_threshold)
end

# NB: Does not set relevance for the maybe scenario, that is set in set_experience
def set_relevance(space_facility, belongs_to_space_type, positive_threshold)
return space_facility.not_relevant! unless positive_threshold || belongs_to_space_type
return space_facility.update(relevant: false) unless positive_threshold || belongs_to_space_type

space_facility.relevant!
space_facility.update(relevant: true)
end

def set_experience(space_facility, positive_threshold, impossible_threshold, negative_threshold)
return space_facility.likely! if positive_threshold
return space_facility.impossible! if impossible_threshold
return space_facility.unlikely! if negative_threshold
return space_facility.update(experience: "likely") if positive_threshold
return space_facility.update(experience: "impossible") if impossible_threshold
return space_facility.update(experience: "unlikely") if negative_threshold

# Nothing else fits, so it's a _relevant_ maybe!
space_facility.maybe! && space_facility.relevant!
space_facility.update(experience: "maybe", relevant: true)
end

def handle_zero_facility_reviews(space_facility, belongs_to_space_type)
return space_facility.unknown! && space_facility.not_relevant! unless belongs_to_space_type
return space_facility.update(experience: "unknown", relevant: true) if belongs_to_space_type

space_facility.unknown! && space_facility.relevant!
space_facility.update(experience: "unknown", relevant: false)
end

def facility_belongs_to_space_type(facility)
@space.space_types.find do |space_type|
@space_types.find do |space_type|
space_type.facilities.include? facility
end.present?
end

def find_or_create_space_facility(facility)
space_facility = @space_facilities.find { |sf| sf.facility_id == facility.id }
return space_facility if space_facility

SpaceFacility.create(facility_id: facility.id, space_id: @space.id)
end

attr_reader :space
end
end
40 changes: 40 additions & 0 deletions app/services/spaces/concerns/countable_reviews.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

module CountableReviews
private

def most_recent_facility_reviews_for(facility)
@grouped_facility_reviews[facility.id] || []
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) }
end

def facility_reviews_for(facility)
@last_five_facility_reviews_grouped_by_facility.find { |r| r.facility_id == facility.id }
end

def count_of_positive(reviews)
# These keep performance high for this service, by keeping db calls to
# a minimum. We could use the review.positive scope, but that would
# require a db call for each review, which would be slow. We're only
# counting in the already fetched list of reviews.
count_of_reviews_with_experience(reviews, "was_allowed")
end

def count_of_negative(reviews)
count_of_reviews_with_experience(reviews, "was_not_allowed")
end

def count_of_impossible(reviews)
count_of_reviews_with_experience(reviews, "was_not_available")
end

def count_of_reviews_with_experience(reviews, experience)
reviews.count { |r| r.experience == experience }
end
end

0 comments on commit 1eac656

Please sign in to comment.