From 4ced9721ff3ae0e0bf4ab9de639f0309cbb1c6bc Mon Sep 17 00:00:00 2001 From: Paul Sturgess Date: Fri, 18 Oct 2024 17:25:51 +0100 Subject: [PATCH] wip --- Appraisals | 9 ++++ Gemfile | 1 + README.md | 19 +++++++ lib/metricks/engine.rb | 6 +++ lib/metricks/lock.rb | 40 +++++++++++++++ lib/metricks/models/metric.rb | 4 +- metricks.gemspec | 1 - spec/spec_helper.rb | 4 ++ spec/specs/engine_spec.rb | 29 +++++++++++ spec/specs/lock_spec.rb | 85 ++++++++++++++++++++++++++++++++ spec/specs/models/metric_spec.rb | 5 ++ 11 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 lib/metricks/lock.rb create mode 100644 spec/specs/engine_spec.rb create mode 100644 spec/specs/lock_spec.rb diff --git a/Appraisals b/Appraisals index 3897b53..46596ae 100644 --- a/Appraisals +++ b/Appraisals @@ -3,17 +3,26 @@ if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('3.0.0') appraise 'activerecord-5-2' do gem 'activerecord', '~> 5.2.0' + gem 'rails', '~> 5.2.0' end end appraise 'activerecord-6-0' do gem 'activerecord', '~> 6.0.0' + gem 'rails', '~> 6.0.0' end appraise 'activerecord-6-1' do gem 'activerecord', '~> 6.1.0' + gem 'rails', '~> 6.1.0' end appraise 'activerecord-7-0' do gem 'activerecord', '~> 7.0.0' + gem 'rails', '~> 7.0.0' +end + +appraise 'activerecord-7-1' do + gem 'activerecord', '~> 7.1.0' + gem 'rails', '~> 7.1.0' end diff --git a/Gemfile b/Gemfile index 9312a50..6cc8ea1 100644 --- a/Gemfile +++ b/Gemfile @@ -6,4 +6,5 @@ group :development do gem 'appraisal', '~> 2.4' gem 'rspec' gem 'sqlite3', '~> 1.4' + gem 'with_advisory_lock' end diff --git a/README.md b/README.md index dc25b97..839e6f4 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ To get started, add the following to your Gemfile and run `bundle install`. ```ruby source "https://rubygems.pkg.github.com/krystal" do gem 'metricks', '>= 1.0.0', '< 2.0' + gem 'with_advisory_lock' # optional, see below end ``` @@ -36,6 +37,24 @@ rake metricks:install:migrations rake db:migrate ``` +### with_advisory_lock + +By default Metricks expects you to have the [with_advisory_lock gem](https://github.com/ClosureTree/with_advisory_lock/) installed in your application. This is used to ensure that metrics are stored accurately, refer to the gem itself for details of how the locking works. + +If you do not wish to use this gem, you can provide your own locking mechanism in an initializer. The arguments passed to `with_lock` will match the method signature of `with_advisory_lock`. + +```ruby +# config/initializers/metricks.rb +Rails.application.config.metricks.with_lock = proc do |key, opts, block| + opts ||= {} + timeout_seconds = opts[:timeout_seconds] || 60 + + MyCustomLock.with_lock(key: key, timeout_seconds: timeout_seconds, &block) +end +``` + +However if you're happy to use with_advisory_lock, you don't need to create an initializer. + ## Usage There are two key types of metric: evented or cumulative. By default, all metrics stored are evented which means they represent an event happening (for example an invoice being raised, a user being created or a product being sold). A cumulative metric stores values that increase or decrease (for example total revenue, MRR or number of active users). diff --git a/lib/metricks/engine.rb b/lib/metricks/engine.rb index 65f3c64..3afe515 100644 --- a/lib/metricks/engine.rb +++ b/lib/metricks/engine.rb @@ -3,9 +3,15 @@ class Engine < ::Rails::Engine engine_name 'metricks' + config.metricks = ActiveSupport::OrderedOptions.new + config.metricks.with_lock = nil + initializer 'metricks.initialize' do |app| ActiveSupport.on_load :active_record do require 'metricks/models/metric' + + Metricks::Lock.with_lock = app.config.metricks.with_lock + Metricks::Lock.validate! end end diff --git a/lib/metricks/lock.rb b/lib/metricks/lock.rb new file mode 100644 index 0000000..037ab2a --- /dev/null +++ b/lib/metricks/lock.rb @@ -0,0 +1,40 @@ +require 'with_advisory_lock' if defined?(WithAdvisoryLock) + +# The default locking mechanism is to use the with_advisory_lock gem +# But this can be overriden using an initializer in the host Rails application (refer to README.md) +# This is set in lib/metricks/engine.rb +# Because of this, the with_advisory_lock gem is not a hard dependency. +module Metricks + class Lock + + class << self + attr_accessor :with_lock + + def with_lock(key, opts = {}, &block) + with_lock_block = @with_lock || default_with_lock + + instance_exec(key, opts, block, &with_lock_block) + end + + def validate! + return if @with_lock.present? + return if defined?(WithAdvisoryLock) + + + raise Metricks::Error.new( + 'ConfigurationMissing', + message: 'By default Metricks requires with_advisory_lock gem to be installed. ' \ + 'Alternatively a custom locking mechanism can be configured via config.metricks.with_lock' + ) + end + + private + + def default_with_lock + proc do |key, opts, block| + ActiveRecord::Base.with_advisory_lock(key, opts, &block) + end + end + end + end +end diff --git a/lib/metricks/models/metric.rb b/lib/metricks/models/metric.rb index 07c4bc8..862cc1a 100644 --- a/lib/metricks/models/metric.rb +++ b/lib/metricks/models/metric.rb @@ -1,4 +1,4 @@ -require 'with_advisory_lock' +require 'metricks/lock' require 'metricks/gatherer' require 'metricks/error' require 'metricks/compared_set' @@ -42,7 +42,7 @@ def record(type, **options) metric.amount ||= options[:amount] || 1 if type.cumulative? - with_advisory_lock 'AddCumulativeMetric' do + ::Metricks::Lock.with_lock 'AddCumulativeMetric' do existing = self.last(type, after: metric.time, associations: options[:associations]) if existing.present? raise Metricks::Error.new('CannotAddHistoricalCumulativeMetrics', message: "Nope.") diff --git a/metricks.gemspec b/metricks.gemspec index 13fa024..4f2592c 100644 --- a/metricks.gemspec +++ b/metricks.gemspec @@ -15,5 +15,4 @@ Gem::Specification.new do |gem| gem.email = ['help@krystal.uk'] gem.required_ruby_version = '>= 2.7' gem.add_runtime_dependency 'activerecord', '>= 5.0' - gem.add_runtime_dependency 'with_advisory_lock', '>= 4.6', '< 5.0' end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index de5bc27..2cc031d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ $LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) +require 'rails' require 'active_record' ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: ':memory:' @@ -16,4 +17,7 @@ config.after(:each) do Metricks::Models::Metric.delete_all end + + config.full_backtrace = true + end diff --git a/spec/specs/engine_spec.rb b/spec/specs/engine_spec.rb new file mode 100644 index 0000000..b9d81b8 --- /dev/null +++ b/spec/specs/engine_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' +require 'metricks/engine' + +describe Metricks::Engine do + + let(:mock_app) do + Class.new(Rails::Application) do + config.eager_load = false + end + end + + it 'allows with_lock to be configured' do + success = false + + allow(mock_app.config.metricks).to receive(:with_lock) + .and_return(->(result, opts, block) { block.call(result, opts) }) + + expect { + mock_app.initialize! + }.not_to raise_error + + Metricks::Lock.with_lock(true, {}) do |result| + success = result + end + + expect(success).to be(true) + end + +end diff --git a/spec/specs/lock_spec.rb b/spec/specs/lock_spec.rb new file mode 100644 index 0000000..4abcf0f --- /dev/null +++ b/spec/specs/lock_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' +require 'with_advisory_lock' +require 'metricks/lock' + +describe Metricks::Lock do + + describe ".with_lock" do + before do + allow(ActiveRecord::Base).to receive(:with_advisory_lock).and_call_original + end + + context "when with_lock is set" do + before do + Metricks::Lock.with_lock = ->(key, opts, block) { block.call(key, opts) } + end + + it "calls the block with the args" do + success = false + passed_opts = {} + + Metricks::Lock.with_lock(true, {hi: 'there'}) do |result, opts| + success = result + passed_opts = opts + end + + expect(success).to be(true) + expect(passed_opts).to eq({hi: 'there'}) + expect(ActiveRecord::Base).not_to have_received(:with_advisory_lock) + end + end + + context "when with_lock is not set" do + before do + Metricks::Lock.with_lock = nil + end + + it "uses with_advisory_lock" do + success = false + + Metricks::Lock.with_lock(true, timeout_seconds: 5) do |result, opts| + success = true + end + + expect(success).to be(true) + expect(ActiveRecord::Base).to have_received(:with_advisory_lock) + .with(true, {timeout_seconds: 5}) + end + end + end + + describe ".validate!" do + context "when with_lock is set" do + before do + Metricks::Lock.with_lock = ->(key, opts, block) { block.call } + hide_const("WithAdvisoryLock") + end + + it "does not raise an error" do + expect { Metricks::Lock.validate! }.not_to raise_error + end + end + + context "when with_lock is not set and WithAdvisoryLock is defined" do + before do + stub_const("WithAdvisoryLock", true) + end + + it "does not raise an error" do + expect { Metricks::Lock.validate! }.not_to raise_error + end + end + + context "when with_lock is not set and WithAdvisoryLock is not defined" do + before do + Metricks::Lock.with_lock = nil + hide_const("WithAdvisoryLock") + end + + it "raises an error" do + expect { Metricks::Lock.validate! }.to raise_error(Metricks::Error) + end + end + end + +end diff --git a/spec/specs/models/metric_spec.rb b/spec/specs/models/metric_spec.rb index b79c923..50cb9f4 100644 --- a/spec/specs/models/metric_spec.rb +++ b/spec/specs/models/metric_spec.rb @@ -1,7 +1,12 @@ require 'spec_helper' require 'metricks/models/metric' +require 'with_advisory_lock' describe Metricks::Models::Metric do + before do + Metricks::Lock.with_lock = nil # ensure it's reset from other specs, set to nil will use the default locking + end + context 'a valid un-saved metric' do subject(:metric) do Metricks::Models::Metric.new(type: PotatoesPicked.id, amount: 10.0)