From 7e99cde2b89ddaad9f2859e1cdbb6c1922b7cebf Mon Sep 17 00:00:00 2001 From: Julian Rubisch Date: Wed, 1 Mar 2023 09:04:10 +0100 Subject: [PATCH 1/5] test: Add Site dummy model for debounce testing --- test/dummy/app/models/site.rb | 5 +++++ test/dummy/db/migrate/20230301080311_create_sites.rb | 9 +++++++++ test/dummy/db/schema.rb | 10 +++++++--- test/dummy/test/fixtures/sites.yml | 7 +++++++ test/dummy/test/models/site_test.rb | 7 +++++++ 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 test/dummy/app/models/site.rb create mode 100644 test/dummy/db/migrate/20230301080311_create_sites.rb create mode 100644 test/dummy/test/fixtures/sites.yml create mode 100644 test/dummy/test/models/site_test.rb diff --git a/test/dummy/app/models/site.rb b/test/dummy/app/models/site.rb new file mode 100644 index 00000000..26c580a5 --- /dev/null +++ b/test/dummy/app/models/site.rb @@ -0,0 +1,5 @@ +class Site < ApplicationRecord + include CableReady::Updatable + + enable_cable_ready_updates +end diff --git a/test/dummy/db/migrate/20230301080311_create_sites.rb b/test/dummy/db/migrate/20230301080311_create_sites.rb new file mode 100644 index 00000000..e8e449de --- /dev/null +++ b/test/dummy/db/migrate/20230301080311_create_sites.rb @@ -0,0 +1,9 @@ +class CreateSites < ActiveRecord::Migration[6.1] + def change + create_table :sites do |t| + t.string :name + + t.timestamps + end + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 4fe8e204..a4629f35 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -12,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_05_18_072701) do +ActiveRecord::Schema.define(version: 2023_03_01_080311) do create_table "accounts", force: :cascade do |t| t.string "account_number" @@ -78,6 +76,12 @@ t.datetime "updated_at", precision: 6, null: false end + create_table "sites", force: :cascade do |t| + t.string "name" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + end + create_table "suppliers", force: :cascade do |t| t.string "name" t.datetime "created_at", precision: 6, null: false diff --git a/test/dummy/test/fixtures/sites.yml b/test/dummy/test/fixtures/sites.yml new file mode 100644 index 00000000..7d412240 --- /dev/null +++ b/test/dummy/test/fixtures/sites.yml @@ -0,0 +1,7 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +one: + name: MyString + +two: + name: MyString diff --git a/test/dummy/test/models/site_test.rb b/test/dummy/test/models/site_test.rb new file mode 100644 index 00000000..f373949e --- /dev/null +++ b/test/dummy/test/models/site_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class SiteTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end From 6bc8fda5a3912ce3191bc7395e520052072d87b7 Mon Sep 17 00:00:00 2001 From: Julian Rubisch Date: Wed, 1 Mar 2023 10:20:39 +0100 Subject: [PATCH 2/5] feat: Implement server side debouncing with memory debounce adapter --- app/models/concerns/cable_ready/updatable.rb | 36 ++++++++++++++++++-- test/dummy/app/models/site.rb | 2 +- test/lib/cable_ready/updatable_test.rb | 17 +++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/cable_ready/updatable.rb b/app/models/concerns/cable_ready/updatable.rb index 46f0b107..f740bd37 100644 --- a/app/models/concerns/cable_ready/updatable.rb +++ b/app/models/concerns/cable_ready/updatable.rb @@ -6,6 +6,18 @@ module CableReady module Updatable extend ::ActiveSupport::Concern + class MemoryDebounceAdapter + include Singleton + + delegate_missing_to :@keys + + def initialize + @keys = {} + end + end + + mattr_accessor :debounce_adapter, default: MemoryDebounceAdapter.instance + included do |base| if defined?(ActiveRecord) && base < ActiveRecord::Base include ExtendHasMany @@ -30,11 +42,14 @@ def self.enable_cable_ready_updates(*options) options = options.extract_options! options = { on: [:create, :update, :destroy], - if: -> { true } + if: -> { true }, + debounce: 0.seconds }.merge(options) enabled_operations = Array(options[:on]) + @debounce_time = options[:debounce] + after_commit(ModelUpdatableCallbacks.new(:create, enabled_operations), {on: :create, if: options[:if]}) after_commit(ModelUpdatableCallbacks.new(:update, enabled_operations), {on: :update, if: options[:if]}) after_commit(ModelUpdatableCallbacks.new(:destroy, enabled_operations), {on: :destroy, if: options[:if]}) @@ -52,6 +67,8 @@ def self.skip_cable_ready_updates private module ClassMethods + include Compoundable + def has_many(name, scope = nil, **options, &extension) option = if options.has_key?(:enable_updates) warn "DEPRECATED: please use `enable_cable_ready_updates` instead. The `enable_updates` option will be removed from a future version of CableReady 5" @@ -180,7 +197,22 @@ def build_options(option) def broadcast_updates(model_class, options) return if skip_updates_classes.any? { |klass| klass >= self } raise("ActionCable must be enabled to use Updatable") unless defined?(ActionCable) - ActionCable.server.broadcast(model_class, options) + + @debounce_time ||= 0.seconds + + if @debounce_time > 0.seconds + key = compound([model_class, *options]) + old_wait_until = CableReady::Updatable.debounce_adapter[key] + now = Time.now.to_f + + if old_wait_until.nil? || old_wait_until < now + new_wait_until = now + @debounce_time.to_f + CableReady::Updatable.debounce_adapter[key] = new_wait_until + ActionCable.server.broadcast(model_class, options) + end + else + ActionCable.server.broadcast(model_class, options) + end end def skip_updates_classes diff --git a/test/dummy/app/models/site.rb b/test/dummy/app/models/site.rb index 26c580a5..3152f9f0 100644 --- a/test/dummy/app/models/site.rb +++ b/test/dummy/app/models/site.rb @@ -1,5 +1,5 @@ class Site < ApplicationRecord include CableReady::Updatable - enable_cable_ready_updates + enable_cable_ready_updates debounce: 3.seconds end diff --git a/test/lib/cable_ready/updatable_test.rb b/test/lib/cable_ready/updatable_test.rb index 4d63b80b..f01890a7 100644 --- a/test/lib/cable_ready/updatable_test.rb +++ b/test/lib/cable_ready/updatable_test.rb @@ -229,4 +229,21 @@ class TestEnableUpdatesOption < ActiveRecord::Base end end # standard:enable Lint/ConstantDefinitionInBlock + + test "only sends out a ping once per debounce period" do + site = Site.create(name: "Front Page") + + mock_server = mock("server") + mock_server.expects(:broadcast).with(Site, {changed: ["name", "updated_at"]}).twice + mock_server.expects(:broadcast).with(site.to_global_id, {changed: ["name", "updated_at"]}).twice + + ActionCable.stubs(:server).returns(mock_server) + + # debounce time is 3 seconds, so the last update should trigger its own broadcast + site.update(name: "Landing Page 1") + travel(1.second) + site.update(name: "Landing Page 2") + travel(3.seconds) + site.update(name: "Landing Page 3") + end end From cbeefc34d9fdbec35cb4d66ed6b9aa744dfc5bee Mon Sep 17 00:00:00 2001 From: Julian Rubisch Date: Wed, 1 Mar 2023 10:25:54 +0100 Subject: [PATCH 3/5] chore: Use MonitorMixin to make debounce store threadsafe --- app/models/concerns/cable_ready/updatable.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/cable_ready/updatable.rb b/app/models/concerns/cable_ready/updatable.rb index f740bd37..375476f4 100644 --- a/app/models/concerns/cable_ready/updatable.rb +++ b/app/models/concerns/cable_ready/updatable.rb @@ -8,11 +8,19 @@ module Updatable class MemoryDebounceAdapter include Singleton + include MonitorMixin - delegate_missing_to :@keys + delegate_missing_to :@dict def initialize - @keys = {} + super + @dict = {} + end + + def []=(key, value) + synchronize do + @dict[key] = value + end end end From 68ae3c73b980ae8bc5d32063d1d11868d1e43bc0 Mon Sep 17 00:00:00 2001 From: Julian Rubisch Date: Wed, 1 Mar 2023 10:36:07 +0100 Subject: [PATCH 4/5] chore: Memoize debounce time --- app/models/concerns/cable_ready/updatable.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/cable_ready/updatable.rb b/app/models/concerns/cable_ready/updatable.rb index 375476f4..9814e9e7 100644 --- a/app/models/concerns/cable_ready/updatable.rb +++ b/app/models/concerns/cable_ready/updatable.rb @@ -206,15 +206,13 @@ def broadcast_updates(model_class, options) return if skip_updates_classes.any? { |klass| klass >= self } raise("ActionCable must be enabled to use Updatable") unless defined?(ActionCable) - @debounce_time ||= 0.seconds - - if @debounce_time > 0.seconds + if debounce_time > 0.seconds key = compound([model_class, *options]) old_wait_until = CableReady::Updatable.debounce_adapter[key] now = Time.now.to_f if old_wait_until.nil? || old_wait_until < now - new_wait_until = now + @debounce_time.to_f + new_wait_until = now + debounce_time.to_f CableReady::Updatable.debounce_adapter[key] = new_wait_until ActionCable.server.broadcast(model_class, options) end @@ -223,6 +221,10 @@ def broadcast_updates(model_class, options) end end + def debounce_time + @debounce_time ||= 0.seconds + end + def skip_updates_classes Thread.current[:skip_updates_classes] ||= [] end From 0288894712c94d53f1cb8e8928428c0116a88b41 Mon Sep 17 00:00:00 2001 From: Julian Rubisch Date: Wed, 1 Mar 2023 17:18:35 +0100 Subject: [PATCH 5/5] feat: Synchronize debounce store reads --- app/models/concerns/cable_ready/updatable.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/concerns/cable_ready/updatable.rb b/app/models/concerns/cable_ready/updatable.rb index 9814e9e7..e1d22203 100644 --- a/app/models/concerns/cable_ready/updatable.rb +++ b/app/models/concerns/cable_ready/updatable.rb @@ -22,6 +22,12 @@ def []=(key, value) @dict[key] = value end end + + def [](key) + synchronize do + @dict[key] + end + end end mattr_accessor :debounce_adapter, default: MemoryDebounceAdapter.instance