perf(cache): Optimize cache by removing null attributes

Author: groyoh

app/services/subscriptions/charge_cache_middleware.rb

@@ -12,28 +12,44 @@ def initialize(subscription:, charge:, to_datetime:, cache: true)
     def call(charge_filter:)
       return yield unless cache
 
-      json = Subscriptions::ChargeCacheService.call(subscription:, charge:, charge_filter:, expires_in: cache_expiration) do
-        yield
-          .map { |fee| fee.attributes.merge("pricing_unit_usage" => fee.pricing_unit_usage&.attributes) }
+      fees = nil
+      cached_fees = Subscriptions::ChargeCacheService.call(subscription:, charge:, charge_filter:, expires_in: cache_expiration) do
+        fees = yield
+        fees.map { |fee| fee.attributes.merge("pricing_unit_usage" => fee.pricing_unit_usage&.attributes) }
+          .then { |fees| deep_compact(fees) }
           .to_json
       end
 
-      JSON.parse(json).map do |j|
-        pricing_unit_usage = if j["pricing_unit_usage"].present?
-          PricingUnitUsage.new(j["pricing_unit_usage"].slice(*PricingUnitUsage.column_names))
-        end
+      return fees if fees # avoid parsing the JSON if we already have the fees
 
-        Fee.new(
-          **j.slice(*Fee.column_names),
-          pricing_unit_usage:
-        )
-      end
+      parse_cached_fees(cached_fees)
     end
 
     private
 
     attr_reader :subscription, :charge, :to_datetime, :cache
 
+    def parse_cached_fees(cached_fees)
+      JSON.parse(cached_fees).map do |fee_attributes|
+        pricing_unit_usage = fee_attributes["pricing_unit_usage"]
+        pricing_unit_usage = if pricing_unit_usage.present?
+          PricingUnitUsage.new(pricing_unit_usage.slice(*PricingUnitUsage.column_names))
+        end
+
+        Fee.new(**fee_attributes.slice(*Fee.column_names), pricing_unit_usage:)
+      end
+    end
+
+    def deep_compact(object)
+      if object.is_a?(Hash)
+        object.compact.transform_values { |v| deep_compact(v) }
+      elsif object.is_a?(Array)
+        object.map { |v| deep_compact(v) }
+      else
+        object
+      end
+    end
+
     def cache_expiration
       [(to_datetime - Time.current).to_i.seconds, 0].max
     end

config/environments/production.rb

@@ -61,7 +61,8 @@
         Rails.logger.error(exception.backtrace.join("\n"))
 
         Sentry.capture_exception(exception)
-      }
+      },
+      compress_threshold: 512 # 512 bytes
     }
 
     if ENV["LAGO_REDIS_CACHE_PASSWORD"].present? && !ENV["LAGO_REDIS_CACHE_PASSWORD"].empty?

spec/services/subscriptions/charge_cache_middleware_spec.rb

@@ -0,0 +1,217 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Subscriptions::ChargeCacheMiddleware, cache: :redis do
+  subject(:middleware) { described_class.new(subscription:, charge:, to_datetime:, cache:) }
+
+  let(:organization) { create(:organization, premium_integrations: zero_amount_fees_enabled ? ["zero_amount_fees"] : []) }
+  let(:billing_entity) { organization.billing_entities.first }
+  let(:zero_amount_fees_enabled) { false }
+  let(:plan) { create(:plan, organization:) }
+  let(:subscription) { create(:subscription, plan:, organization:) }
+  let(:charge) { create(:standard_charge, plan: plan, organization:) }
+  let(:pricing_unit) { create(:pricing_unit, organization:, short_name: "CAR") }
+  let(:to_datetime) { Time.current + 1.hour }
+  let(:cache) { true }
+  let(:charge_filter) { nil }
+  let(:cache_key) do
+    [
+      "charge-usage",
+      1,
+      charge.id,
+      subscription.id,
+      charge.updated_at.iso8601,
+      charge_filter&.id,
+      charge_filter&.updated_at&.iso8601
+    ].compact.join("/")
+  end
+
+  def fee(amount_cents:, with_pricing_unit_usage: false)
+    Fee.new(
+      amount_cents:,
+      amount_currency: "USD",
+      fee_type: "charge",
+      pricing_unit_usage: with_pricing_unit_usage ? PricingUnitUsage.new(
+        organization: organization,
+        pricing_unit:,
+        short_name: "CAR",
+        conversion_rate: 1.5,
+        amount_cents: 150,
+        precise_amount_cents: 150.0,
+        unit_amount_cents: 10,
+        precise_unit_amount: 10.0
+      ) : nil,
+      charge: charge,
+      charge_filter: charge_filter,
+      organization: organization,
+      billing_entity: billing_entity,
+      subscription: subscription
+    )
+  end
+
+  def cached_fee_payload(amount_cents:, overrides: {})
+    {
+      "amount_cents" => amount_cents,
+      "amount_currency" => "USD",
+      "amount_details" => {},
+      "fee_type" => "charge",
+      "charge_id" => charge.id,
+      "organization_id" => organization.id,
+      "subscription_id" => subscription.id,
+      "billing_entity_id" => billing_entity.id,
+      "grouped_by" => {},
+      "payment_status" => "pending",
+      "precise_amount_cents" => "0.0",
+      "precise_coupons_amount_cents" => "0.0",
+      "precise_credit_notes_amount_cents" => "0.0",
+      "precise_unit_amount" => "0.0",
+      "pay_in_advance" => false,
+      "properties" => {},
+      "taxes_base_rate" => 1.0,
+      "taxes_precise_amount_cents" => "0.0",
+      "taxes_rate" => 0.0,
+      "unit_amount_cents" => 0,
+      "units" => "0.0",
+      **(charge_filter ? {"charge_filter_id" => charge_filter.id} : {})
+    }.merge(overrides)
+  end
+
+  def charge_cache_key
+    "charge-usage/1/#{charge.id}/#{subscription.id}/#{charge.updated_at.iso8601}"
+  end
+
+  def charge_filter_cache_key
+    "charge-usage/1/#{charge.id}/#{subscription.id}/#{charge.updated_at.iso8601}/#{charge_filter.id}/#{charge_filter.updated_at.iso8601}"
+  end
+
+  def fetch_cache(key)
+    value = Rails.cache.read(key)
+    return nil if value.nil?
+
+    JSON.parse(value)
+  end
+
+  def expire_time(key)
+    Rails.cache.redis.with { |r| r.pexpiretime(key).to_f / 1000 }
+  end
+
+  # We have to compare attributes as ActiveRecord compares objects by their id
+  def expect_to_match_fees(actual, expected)
+    expect(actual).to all(be_a(Fee))
+    expect(expected).to all(be_a(Fee))
+
+    expect(actual.map(&:attributes)).to eq(expected.map(&:attributes))
+  end
+
+  describe "#call" do
+    let(:fees) { [fee(amount_cents: 100), fee(amount_cents: 200)] }
+    let(:other_fees) { [fee(amount_cents: 300)] }
+
+    context "when cache is disabled" do
+      let(:cache) { false }
+
+      it "yields and returns the block result without caching" do
+        result = middleware.call(charge_filter:) { fees }
+        expect_to_match_fees(result, fees)
+
+        result = middleware.call(charge_filter:) { other_fees }
+        expect_to_match_fees(result, other_fees)
+
+        expect(fetch_cache(charge_cache_key)).to be_nil
+      end
+    end
+
+    context "when cache is enabled" do
+      it "caches and returns the fees" do
+        result = middleware.call(charge_filter:) { fees }
+
+        expect_to_match_fees(result, fees)
+
+        expect(fetch_cache(charge_cache_key)).to eq([
+          cached_fee_payload(amount_cents: 100),
+          cached_fee_payload(amount_cents: 200)
+        ])
+        key_expire_time = expire_time(charge_cache_key)
+        expect(key_expire_time).to be_within(10.seconds).of(to_datetime.to_i)
+
+        result = middleware.call(charge_filter:) { other_fees }
+        expect_to_match_fees(result, fees)
+
+        expect(fetch_cache(charge_cache_key)).to eq([
+          cached_fee_payload(amount_cents: 100),
+          cached_fee_payload(amount_cents: 200)
+        ])
+        # Key expire time should not be updated
+        expect(expire_time(charge_cache_key)).to eq(key_expire_time)
+      end
+
+      context "with a charge filter" do
+        let(:charge_filter) { create(:charge_filter) }
+
+        it "passes the charge filter to the cache service" do
+          result = middleware.call(charge_filter:) { fees }
+
+          expect_to_match_fees(result, fees)
+
+          expect(fetch_cache(charge_filter_cache_key)).to eq([
+            cached_fee_payload(amount_cents: 100),
+            cached_fee_payload(amount_cents: 200)
+          ])
+
+          result = middleware.call(charge_filter:) { other_fees }
+          expect_to_match_fees(result, fees)
+
+          expect(fetch_cache(charge_filter_cache_key)).to eq([
+            cached_fee_payload(amount_cents: 100),
+            cached_fee_payload(amount_cents: 200)
+          ])
+        end
+      end
+
+      context "with pricing unit usage" do
+        let(:fees) { [fee(amount_cents: 100, with_pricing_unit_usage: true)] }
+
+        it "caches and reconstructs fees with pricing unit usage" do
+          result = middleware.call(charge_filter:) { fees }
+          expect_to_match_fees(result, fees)
+
+          expect(fetch_cache(charge_cache_key)).to eq([
+            cached_fee_payload(
+              amount_cents: 100,
+              overrides: {"pricing_unit_usage" => {
+                "amount_cents" => 150,
+                "conversion_rate" => "1.5",
+                "organization_id" => organization.id,
+                "precise_amount_cents" => "150.0",
+                "precise_unit_amount" => "10.0",
+                "pricing_unit_id" => pricing_unit.id,
+                "short_name" => "CAR",
+                "unit_amount_cents" => 10
+              }}
+            )
+          ])
+
+          result = middleware.call(charge_filter:) { other_fees }
+          expect_to_match_fees(result, fees)
+
+          expect(fetch_cache(charge_cache_key)).to eq([
+            cached_fee_payload(
+              amount_cents: 100,
+              overrides: {"pricing_unit_usage" => {
+                "amount_cents" => 150,
+                "conversion_rate" => "1.5",
+                "organization_id" => organization.id,
+                "precise_amount_cents" => "150.0",
+                "precise_unit_amount" => "10.0",
+                "pricing_unit_id" => pricing_unit.id,
+                "short_name" => "CAR",
+                "unit_amount_cents" => 10
+              }}
+            )
+          ])
+        end
+      end
+    end
+  end
+end