From b56cd67958af900954d3b1ea7e3e28a275be8714 Mon Sep 17 00:00:00 2001 From: Yury Farias Date: Mon, 2 Jun 2025 11:56:32 -0300 Subject: [PATCH] fix(analytics): optimizing n+1 queries --- README.md | 6 +- docs/src/content/docs/usage/analytics.mdx | 6 +- lib/togglefy/analytics.rb | 92 +++++---------- spec/togglefy/analytics_spec.rb | 131 +++++++++++++--------- 4 files changed, 110 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index e121980..f79d0f7 100644 --- a/README.md +++ b/README.md @@ -469,7 +469,7 @@ Here’s what the result might look like: last_created: 2025-05-21 19:18:37.772172000 UTC +00:00, past_7: 9, past_14: 11, - past_30: 0 + past_31: 0 }, { assignable: "Account", @@ -483,7 +483,7 @@ Here’s what the result might look like: last_created: 2025-05-21 19:18:37.772172000 UTC +00:00, past_7: 2, past_14: 0, - past_30: 0 + past_31: 0 } ] ``` @@ -503,7 +503,7 @@ Here’s what each field means: * `last_created`: The most recent time the feature was toggled on for this assignable. * `past_7`: Number of times the feature was toggled on in the past 7 days. * `past_14`: Number of times the feature was toggled on in the past 14 days. -* `past_30`: Number of times the feature was toggled on in the past 30 days. +* `past_31`: Number of times the feature was toggled on in the past 30 days. ## Aliases diff --git a/docs/src/content/docs/usage/analytics.mdx b/docs/src/content/docs/usage/analytics.mdx index 320f8f4..003b3b1 100644 --- a/docs/src/content/docs/usage/analytics.mdx +++ b/docs/src/content/docs/usage/analytics.mdx @@ -39,7 +39,7 @@ Here’s what the result might look like: last_created: 2025-05-21 19:18:37.772172000 UTC +00:00, past_7: 9, past_14: 11, - past_30: 0 + past_31: 0 }, { assignable: "Account", @@ -53,7 +53,7 @@ Here’s what the result might look like: last_created: 2025-05-21 19:18:37.772172000 UTC +00:00, past_7: 2, past_14: 0, - past_30: 0 + past_31: 0 } ] ``` @@ -73,5 +73,5 @@ Here’s what each field means: - `last_created`: The most recent time the feature was toggled on for this assignable. - `past_7`: Number of times the feature was toggled on in the past 7 days. - `past_14`: Number of times the feature was toggled on in the past 14 days. -- `past_30`: Number of times the feature was toggled on in the past 30 days. +- `past_31`: Number of times the feature was toggled on in the past 30 days. diff --git a/lib/togglefy/analytics.rb b/lib/togglefy/analytics.rb index 145d101..5cd4154 100644 --- a/lib/togglefy/analytics.rb +++ b/lib/togglefy/analytics.rb @@ -29,7 +29,7 @@ class Analytics # @param identifier [String, Symbol, nil] The unique feature identifier. def initialize(identifier = nil) @identifier = identifier - @feature = fetch_feature + @feature_data = fetch_feature_data end # Generates analytics data for the feature. @@ -41,47 +41,38 @@ def initialize(identifier = nil) # - percentages # - assignment metadata (created_at timestamps, activity windows) def track - return [] unless @feature + return [] unless @feature_data build_tracking_data end private - # Attempts to fetch the feature based on the identifier. + # Attempts to fetch the feature data based on the identifier. # # @return [Togglefy::Feature, nil] The feature if found, else nil. - def fetch_feature - Togglefy.feature(@identifier) || nil + def fetch_feature_data + Togglefy::Feature.includes(feature_assignments: :assignable) + .find_by(identifier: @identifier) end # Builds the full tracking data array. # # @return [Array] Tracking data for each assignable. def build_tracking_data - assignables.map do |assignable| - assignable_class = safe_constantize(assignable) + @feature_data.feature_assignments.group_by(&:assignable_type).map do |assignable_type, assignments| + assignable_class = safe_constantize(assignable_type) next unless assignable_class - assignable_data = build_assignables_data(assignable_class) - assignments_data = build_assignments_data + total_count = assignable_class.count + enabled_count = assignments.count + disabled_count = total_count - enabled_count - [{ assignable: assignable, feature: @identifier }, assignable_data, assignments_data].reduce(:merge) - end - end + assignable_data = assignables_data(enabled_count, disabled_count, total_count) + assignments_data = build_assignments_data(assignments) - # Returns a list of assignable class names related to the feature. - # - # @return [Array] The list of assignable types (e.g., ["User", "Admin"]). - def assignables - @assignables ||= assignments.map(&:assignable_type).uniq - end - - # Returns all assignments for the current feature. - # - # @return [ActiveRecord::Relation] The assignment records (enabled/disabled toggles). - def assignments - @assignments ||= @feature ? Togglefy::FeatureAssignment.where(feature_id: @feature.id) : [] + [{ assignable: assignable_type, feature: @identifier }, assignable_data, assignments_data].reduce(:merge) + end.compact end # Safely converts a string class name to a constant. @@ -95,25 +86,6 @@ def safe_constantize(assignable) nil end - # Builds metrics for how the feature is distributed across assignables. - # - # @param klass [Class] The assignable class. - # @return [Hash] Count and percentage breakdown. - def build_assignables_data(klass) - return assignables_data unless klass.respond_to?(:with_features) && klass.respond_to?(:without_features) - - with_features = klass.with_features(@feature.id) - without_features = klass.without_features(@feature.id) - - enabled_count = with_features.count - disabled_count = without_features.count - total_count = enabled_count + disabled_count - - return assignables_data if total_count.zero? - - assignables_data(enabled_count, disabled_count, total_count) - end - # Builds a base hash of assignment data counts and percentages. # # @param enabled_count [Integer] Number of enabled assignments. @@ -144,40 +116,26 @@ def calculate_percentage(count, total) # Builds time-based metrics for feature assignment activity. # + # @param assignments [ActiveRecord::Relation] The feature assignment records. # @return [Hash] Assignment activity data. - def build_assignments_data - return default_assignments_data if assignments.empty? - + def build_assignments_data(assignments) # rubocop:disable Naming/VariableNumber { - last_created: assignments.maximum(:created_at), - first_created: assignments.minimum(:created_at), - past_7: count_assignments_in_period(7.days.ago), - past_14: count_assignments_in_period(14.days.ago), - past_30: count_assignments_in_period(30.days.ago) + last_created: assignments.map(&:created_at).max || nil, + first_created: assignments.map(&:created_at).min || nil, + past_7: count_assignments_in(assignments, 7.days.ago), + past_14: count_assignments_in(assignments, 14.days.ago), + past_31: count_assignments_in(assignments, 31.days.ago) } # rubocop:enable Naming/VariableNumber end # Counts how many assignments occurred since a given date. # - # @param start_date [Date, Time] The starting date for the activity window. + # @param period [Date, Time] Range period which the method will count the assignments. # @return [Integer] The number of assignments created since that date. - def count_assignments_in_period(start_date) - assignments.where(created_at: start_date..Time.current).count - end - - # Returns default assignment tracking data for a feature. - # - # @return [Hash] A hash with default values for assignment metrics. - def default_assignments_data - { - last_created: nil, - first_created: nil, - past7: 0, - past14: 0, - past30: 0 - } + def count_assignments_in(assignments, period) + assignments.count { |a| a.created_at >= period } || 0 end end end diff --git a/spec/togglefy/analytics_spec.rb b/spec/togglefy/analytics_spec.rb index cbf05cf..c966462 100644 --- a/spec/togglefy/analytics_spec.rb +++ b/spec/togglefy/analytics_spec.rb @@ -14,8 +14,10 @@ describe "#initialize" do context "with valid identifier" do it "sets the identifier and fetches the feature" do - expect(Togglefy).to receive(:feature).with(feature_identifier) - described_class.new(feature_identifier) + analytics = described_class.new(feature_identifier) + + expect(analytics.instance_variable_get(:@identifier)).to eq(feature_identifier) + expect(analytics.instance_variable_get(:@feature_data)).to be_present end end end @@ -24,20 +26,25 @@ before do user_one.add_feature(feature_identifier) user_two.add_feature(feature_identifier) + + # Create additional users without the feature 3.times { User.create! } end context "when feature exists" do it "returns tracking data consistent" do - result = analytics.track + result = described_class.new(feature_identifier).track + expect(result).to be_an(Array) - expect(result.first).to be_an(Hash) + expect(result).not_to be_empty + expect(result.first).to be_a(Hash) end it "returns tracking data correctly" do - result = analytics.track - user_tracking = result.first + result = described_class.new(feature_identifier).track + expect(result).not_to be_empty + user_tracking = result.first expect(user_tracking[:assignable]).to eq("User") expect(user_tracking[:total]).to eq(5) expect(user_tracking[:enabled_count]).to eq(2) @@ -49,7 +56,7 @@ # rubocop:disable Naming/VariableNumber expect(user_tracking[:past_7]).to eq(2) expect(user_tracking[:past_14]).to eq(2) - expect(user_tracking[:past_30]).to eq(2) + expect(user_tracking[:past_31]).to eq(2) # rubocop:enable Naming/VariableNumber end end @@ -71,15 +78,9 @@ end end - describe "#build_assignables_data" do - before do - user_one.add_feature(feature_identifier) - user_two.add_feature(feature_identifier) - 3.times { User.create! } - end - + describe "#assignables_data" do it "builds assignable data correctly" do - result = analytics.send(:build_assignables_data, user_class) + result = analytics.send(:assignables_data, 2, 3, 5) expect(result[:enabled_count]).to eq(2) expect(result[:disabled_count]).to eq(3) @@ -88,60 +89,86 @@ expect(result[:percentage_disabled]).to eq("60.0%") end - context "when assignable doesnt respond to Togglefy methods" do - before do - allow(user_class).to receive(:respond_to?).with(:with_features).and_return(false) - end - - it "returns default assignable data" do - result = analytics.send(:build_assignables_data, user_class) + it "handles zero totals gracefully" do + result = analytics.send(:assignables_data) - expect(result[:enabled_count]).to eq(0) - expect(result[:disabled_count]).to eq(0) - expect(result[:total]).to eq(0) - expect(result[:percentage_enabled]).to eq("0.00%") - expect(result[:percentage_disabled]).to eq("0.00%") - end + expect(result[:enabled_count]).to eq(0) + expect(result[:disabled_count]).to eq(0) + expect(result[:total]).to eq(0) + expect(result[:percentage_enabled]).to eq("0.00%") + expect(result[:percentage_disabled]).to eq("0.00%") end end describe "#build_assignments_data" do - let(:mock_assignments) { double("ActiveRecord::Relation") } - let(:past_7_assignments) { double("Relation", count: 3) } - let(:past_14_assignments) { double("Relation", count: 7) } - let(:past_30_assignments) { double("Relation", count: 15) } + let(:created_times) do + [ + Time.current - 1.day, + Time.current - 5.days, + Time.current - 10.days, + Time.current - 20.days + ] + end + + let(:mock_assignments) do + created_times.map do |time| + instance_double("Togglefy::FeatureAssignment", created_at: time) + end + end before do travel_to Time.new(2024, 12, 4, 16, 30) # Mon, 04 Dec 2024 16:30:00 + end - allow(analytics).to receive(:assignments).and_return(mock_assignments) - allow(mock_assignments).to receive(:empty?).and_return(false) - allow(mock_assignments).to receive(:maximum).with(:created_at).and_return(1.day.ago) - allow(mock_assignments).to receive(:minimum).with(:created_at).and_return(30.days.ago) + it "builds assignments data correctly" do + result = analytics.send(:build_assignments_data, mock_assignments) - allow(mock_assignments).to receive(:where) - .with(created_at: 7.days.ago..Time.current) - .and_return(past_7_assignments) - allow(mock_assignments).to receive(:where) - .with(created_at: 14.days.ago..Time.current) - .and_return(past_14_assignments) - allow(mock_assignments).to receive(:where) - .with(created_at: 30.days.ago..Time.current) - .and_return(past_30_assignments) + expect(result[:last_created]).to eq(created_times.first) + expect(result[:first_created]).to eq(created_times.last) + # rubocop:disable Naming/VariableNumber + expect(result[:past_7]).to eq(2) # 1 day and 5 days ago + expect(result[:past_14]).to eq(3) # 1, 5, and 10 days ago + expect(result[:past_31]).to eq(4) # all assignments + # rubocop:enable Naming/VariableNumber end - it "builds assignments data correctly" do - result = analytics.send(:build_assignments_data) + it "handles empty assignments gracefully" do + result = analytics.send(:build_assignments_data, []) - expect(result[:last_created]).to eq(1.day.ago) - expect(result[:first_created]).to eq(30.days.ago) + expect(result[:last_created]).to be_nil + expect(result[:first_created]).to be_nil # rubocop:disable Naming/VariableNumber - expect(result[:past_7]).to eq(15) - expect(result[:past_14]).to eq(15) - expect(result[:past_30]).to eq(15) + expect(result[:past_7]).to eq(0) + expect(result[:past_14]).to eq(0) + expect(result[:past_31]).to eq(0) # rubocop:enable Naming/VariableNumber end end + + describe "#count_assignments_in" do + let(:assignments) do + [ + instance_double("Togglefy::FeatureAssignment", created_at: Time.current - 1.day), + instance_double("Togglefy::FeatureAssignment", created_at: Time.current - 5.days), + instance_double("Togglefy::FeatureAssignment", created_at: Time.current - 10.days), + instance_double("Togglefy::FeatureAssignment", created_at: Time.current - 20.days) + ] + end + + before do + travel_to Time.new(2024, 12, 4, 16, 30) # Mon, 04 Dec 2024 16:30:00 + end + + it "counts assignments correctly for different periods" do + expect(analytics.send(:count_assignments_in, assignments, 7.days.ago)).to eq(2) + expect(analytics.send(:count_assignments_in, assignments, 14.days.ago)).to eq(3) + expect(analytics.send(:count_assignments_in, assignments, 31.days.ago)).to eq(4) + end + + it "returns 0 for empty assignments" do + expect(analytics.send(:count_assignments_in, [], 7.days.ago)).to eq(0) + end + end end end # rubocop:enable Metrics/BlockLength