From e5b4735258430cf93ebac6d48d1d3096597a4ea1 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:18:19 +0400 Subject: [PATCH 1/7] refactor: move cache stores under Stores namespace MemoryCacheStore, RedisCacheStore and ActiveRecordCacheStore become TranslationDiff::Stores::Memory/Redis/ActiveRecord, living beside the Stores registry the way ruby_llm nests providers under Providers. Stores.register(:redis, Stores::Redis) now reads the way it means. Stores::Redis qualifies Redis::Namespace with :: since it now shares a namespace with the class of the same name; also rewires lib/translation_diff.rb's require order for the whole cache/limiter/ active_record/configuration reorganisation landing across this and the next few commits. --- lib/translation_diff.rb | 15 ++++++------- lib/translation_diff/stores.rb | 9 +++++++- .../active_record.rb} | 6 ++--- .../memory.rb} | 4 ++-- .../{redis_cache_store.rb => stores/redis.rb} | 6 ++--- test/translation_diff/previewer_test.rb | 2 +- test/translation_diff/prune_task_test.rb | 2 +- .../active_record_mysql_text_limit_test.rb} | 8 +++---- .../active_record_redaction_test.rb | 20 ++++++++--------- .../active_record_test.rb} | 18 +++++++-------- .../active_record_transaction_test.rb | 22 +++++++++---------- .../active_record_write_multi_dedupe_test.rb | 4 ++-- .../memory_test.rb} | 8 +++---- .../redis_test.rb} | 4 ++-- .../translator_cache_failure_test.rb | 10 ++++----- 15 files changed, 72 insertions(+), 66 deletions(-) rename lib/translation_diff/{active_record_cache_store.rb => stores/active_record.rb} (97%) rename lib/translation_diff/{memory_cache_store.rb => stores/memory.rb} (84%) rename lib/translation_diff/{redis_cache_store.rb => stores/redis.rb} (87%) rename test/translation_diff/{active_record_cache_store_mysql_text_limit_test.rb => stores/active_record_mysql_text_limit_test.rb} (88%) rename test/translation_diff/{ => stores}/active_record_redaction_test.rb (67%) rename test/translation_diff/{active_record_cache_store_test.rb => stores/active_record_test.rb} (89%) rename test/translation_diff/{ => stores}/active_record_transaction_test.rb (82%) rename test/translation_diff/{ => stores}/active_record_write_multi_dedupe_test.rb (90%) rename test/translation_diff/{memory_cache_store_test.rb => stores/memory_test.rb} (86%) rename test/translation_diff/{redis_cache_store_test.rb => stores/redis_test.rb} (96%) diff --git a/lib/translation_diff.rb b/lib/translation_diff.rb index dbeb620..60a6d1f 100644 --- a/lib/translation_diff.rb +++ b/lib/translation_diff.rb @@ -28,8 +28,6 @@ require "translation_diff/fragment" require "translation_diff/passage" require "translation_diff/sentence_cache" -require "translation_diff/cache_ttl_option" -require "translation_diff/cache_guard_options" require "translation_diff/configuration" require "translation_diff/configuration/provider_option_owners" @@ -49,13 +47,14 @@ require "translation_diff/segmenters/simple" require "translation_diff/segmenters/pragmatic" require "translation_diff/stores" -require "translation_diff/memory_cache_store" -require "translation_diff/redis_cache_store" -require "translation_diff/active_record_support" -require "translation_diff/active_record_cache_store" +require "translation_diff/stores/memory" +require "translation_diff/stores/redis" +require "translation_diff/active_record" +require "translation_diff/active_record/support" +require "translation_diff/stores/active_record" require "translation_diff/rate_limiters" -require "translation_diff/redis_rate_limiter" -require "translation_diff/active_record_rate_limiter" +require "translation_diff/rate_limiters/redis" +require "translation_diff/rate_limiters/active_record" require "translation_diff/instrumentation" require "translation_diff/call_preparation" require "translation_diff/dispatcher" diff --git a/lib/translation_diff/stores.rb b/lib/translation_diff/stores.rb index 5193f94..9528c41 100644 --- a/lib/translation_diff/stores.rb +++ b/lib/translation_diff/stores.rb @@ -1,2 +1,9 @@ # Cache stores, by name; assigning an object to `config.cache` bypasses this entirely. -TranslationDiff::Stores = TranslationDiff::Registry.new("cache store") +module TranslationDiff::Stores + def self.register(name, klass) = registry.register(name, klass) + def self.build(name, config) = registry.build(name, config) + def self.registered?(name) = registry.registered?(name) + def self.names = registry.names + def self.classes = registry.classes + def self.registry = @registry ||= TranslationDiff::Registry.new("cache store") +end diff --git a/lib/translation_diff/active_record_cache_store.rb b/lib/translation_diff/stores/active_record.rb similarity index 97% rename from lib/translation_diff/active_record_cache_store.rb rename to lib/translation_diff/stores/active_record.rb index a4c4fc6..77eda18 100644 --- a/lib/translation_diff/active_record_cache_store.rb +++ b/lib/translation_diff/stores/active_record.rb @@ -1,6 +1,6 @@ # Caches translations in the application's own database; ActiveRecord is required on first use, never at load. -class TranslationDiff::ActiveRecordCacheStore - include TranslationDiff::ActiveRecordSupport +class TranslationDiff::Stores::ActiveRecord + include TranslationDiff::ActiveRecord::Support def self.build(config) new(namespace: config.cache_namespace, ttl: config.cache_ttl, @@ -103,4 +103,4 @@ def active_record_component = "ActiveRecord cache store" def active_record_upsert_detail = "upsert_all takes unique_by and record_timestamps there." end -TranslationDiff::Stores.register(:active_record, TranslationDiff::ActiveRecordCacheStore) +TranslationDiff::Stores.register(:active_record, TranslationDiff::Stores::ActiveRecord) diff --git a/lib/translation_diff/memory_cache_store.rb b/lib/translation_diff/stores/memory.rb similarity index 84% rename from lib/translation_diff/memory_cache_store.rb rename to lib/translation_diff/stores/memory.rb index be7ba76..ac14521 100644 --- a/lib/translation_diff/memory_cache_store.rb +++ b/lib/translation_diff/stores/memory.rb @@ -1,5 +1,5 @@ # The default cache, a bounded in-process LRU. NOT thread-safe, deliberately -- set `redis_url` for that. -class TranslationDiff::MemoryCacheStore +class TranslationDiff::Stores::Memory def self.build(config) = new(max_size: config.cache_max_size) def initialize(max_size:) @@ -31,4 +31,4 @@ def touch(key) end end -TranslationDiff::Stores.register(:memory, TranslationDiff::MemoryCacheStore) +TranslationDiff::Stores.register(:memory, TranslationDiff::Stores::Memory) diff --git a/lib/translation_diff/redis_cache_store.rb b/lib/translation_diff/stores/redis.rb similarity index 87% rename from lib/translation_diff/redis_cache_store.rb rename to lib/translation_diff/stores/redis.rb index cf3dabc..7773bd6 100644 --- a/lib/translation_diff/redis_cache_store.rb +++ b/lib/translation_diff/stores/redis.rb @@ -1,4 +1,4 @@ -class TranslationDiff::RedisCacheStore +class TranslationDiff::Stores::Redis ONE_WEEK = 60 * 60 * 24 * 7 DEFAULT_NAMESPACE = "translation-diff".freeze @@ -35,7 +35,7 @@ def write_multi(pairs) def redis connection_pool.with do |redis| - yield Redis::Namespace.new(namespace, redis: redis) + yield ::Redis::Namespace.new(namespace, redis: redis) end end @@ -46,4 +46,4 @@ def write_one(redis, key, value) def expiring? = timeout.is_a?(Numeric) && timeout.positive? end -TranslationDiff::Stores.register(:redis, TranslationDiff::RedisCacheStore) +TranslationDiff::Stores.register(:redis, TranslationDiff::Stores::Redis) diff --git a/test/translation_diff/previewer_test.rb b/test/translation_diff/previewer_test.rb index c43fa2d..454690b 100644 --- a/test/translation_diff/previewer_test.rb +++ b/test/translation_diff/previewer_test.rb @@ -47,7 +47,7 @@ class WriteTrackingStore attr_reader :write_calls def initialize - @inner = TranslationDiff::MemoryCacheStore.new(max_size: 100) + @inner = TranslationDiff::Stores::Memory.new(max_size: 100) @write_calls = 0 end diff --git a/test/translation_diff/prune_task_test.rb b/test/translation_diff/prune_task_test.rb index 020d944..3406333 100644 --- a/test/translation_diff/prune_task_test.rb +++ b/test/translation_diff/prune_task_test.rb @@ -41,7 +41,7 @@ def test_reports_when_the_cache_store_does_not_support_pruning out, = capture_io { Rake::Task["translation_diff:prune"].invoke } - assert_includes out, "the configured cache store (TranslationDiff::MemoryCacheStore) does not support pruning" + assert_includes out, "the configured cache store (TranslationDiff::Stores::Memory) does not support pruning" end def test_reports_when_there_is_no_rate_limiter_configured diff --git a/test/translation_diff/active_record_cache_store_mysql_text_limit_test.rb b/test/translation_diff/stores/active_record_mysql_text_limit_test.rb similarity index 88% rename from test/translation_diff/active_record_cache_store_mysql_text_limit_test.rb rename to test/translation_diff/stores/active_record_mysql_text_limit_test.rb index 2b862bc..e2f4e6a 100644 --- a/test/translation_diff/active_record_cache_store_mysql_text_limit_test.rb +++ b/test/translation_diff/stores/active_record_mysql_text_limit_test.rb @@ -5,7 +5,7 @@ ActiveRecordDatabase.connect! # MySQL's TEXT column tops out at 65,535 bytes; only a real MySQL server proves the migration raised that ceiling. - class ActiveRecordCacheStoreMysqlTextLimitTest < Minitest::Test + class ActiveRecordStoreMysqlTextLimitTest < Minitest::Test class RecordingProvider < TranslationDiff::Provider def self.capabilities TranslationDiff::Capabilities.new(max_request_size: 100_000_000, max_batch_size: 1_000, @@ -64,12 +64,12 @@ def test_a_translator_still_returns_a_translation_the_store_cannot_hold private def build_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: 60, - table_name: "translation_diff_translations") + TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: 60, + table_name: "translation_diff_translations") end end else - class ActiveRecordCacheStoreMysqlTextLimitTest < Minitest::Test + class ActiveRecordStoreMysqlTextLimitTest < Minitest::Test def test_mysql_is_unavailable skip "TRANSLATION_DIFF_DATABASE_URL does not name a MySQL database; " \ "only a real MySQL server enforces the TEXT column's byte ceiling" diff --git a/test/translation_diff/active_record_redaction_test.rb b/test/translation_diff/stores/active_record_redaction_test.rb similarity index 67% rename from test/translation_diff/active_record_redaction_test.rb rename to test/translation_diff/stores/active_record_redaction_test.rb index d9e3b62..b06d0fc 100644 --- a/test/translation_diff/active_record_redaction_test.rb +++ b/test/translation_diff/stores/active_record_redaction_test.rb @@ -6,7 +6,7 @@ # upsert_all inlines values into the SQL it sends, so PostgreSQL's own error detail can carry a whole row; # only a real constraint violation against a real server reproduces that, hence the PostgreSQL gate. - class ActiveRecordCacheStoreRedactionTest < Minitest::Test + class ActiveRecordStoreRedactionTest < Minitest::Test CONSTRAINT = "no_forbidden_namespace_in_redaction_test".freeze def setup @@ -20,8 +20,8 @@ def teardown end def test_a_statement_invalid_never_carries_the_translated_content - store = TranslationDiff::ActiveRecordCacheStore.new(namespace: "forbidden-namespace", ttl: 60, - table_name: "translation_diff_translations") + store = TranslationDiff::Stores::ActiveRecord.new(namespace: "forbidden-namespace", ttl: 60, + table_name: "translation_diff_translations") error = assert_raises(TranslationDiff::Error) { store.write("a", "SECRET-PATIENT-NOTE-12345") } @@ -29,8 +29,8 @@ def test_a_statement_invalid_never_carries_the_translated_content end def test_the_redacted_error_names_the_adapters_own_error_class - store = TranslationDiff::ActiveRecordCacheStore.new(namespace: "forbidden-namespace", ttl: 60, - table_name: "translation_diff_translations") + store = TranslationDiff::Stores::ActiveRecord.new(namespace: "forbidden-namespace", ttl: 60, + table_name: "translation_diff_translations") error = assert_raises(TranslationDiff::Error) { store.write("a", "one") } @@ -39,8 +39,8 @@ def test_the_redacted_error_names_the_adapters_own_error_class # Ruby attaches the rescued original as #cause unless the raise says otherwise -- and #cause carries the row. def test_the_redacted_error_severs_the_cause_chain - store = TranslationDiff::ActiveRecordCacheStore.new(namespace: "forbidden-namespace", ttl: 60, - table_name: "translation_diff_translations") + store = TranslationDiff::Stores::ActiveRecord.new(namespace: "forbidden-namespace", ttl: 60, + table_name: "translation_diff_translations") error = assert_raises(TranslationDiff::Error) { store.write("a", "SECRET-PATIENT-NOTE-12345") } @@ -51,12 +51,12 @@ def test_the_redacted_error_severs_the_cause_chain private def connection - TranslationDiff::ActiveRecordCacheStore.new(namespace: "harness", ttl: 60, - table_name: "translation_diff_translations").model.connection + TranslationDiff::Stores::ActiveRecord.new(namespace: "harness", ttl: 60, + table_name: "translation_diff_translations").model.connection end end else - class ActiveRecordCacheStoreRedactionTest < Minitest::Test + class ActiveRecordStoreRedactionTest < Minitest::Test def test_postgres_is_unavailable skip "TRANSLATION_DIFF_DATABASE_URL does not name a PostgreSQL database; " \ "a check violation's row detail is what this test reproduces" diff --git a/test/translation_diff/active_record_cache_store_test.rb b/test/translation_diff/stores/active_record_test.rb similarity index 89% rename from test/translation_diff/active_record_cache_store_test.rb rename to test/translation_diff/stores/active_record_test.rb index 34737e3..f30d95a 100644 --- a/test/translation_diff/active_record_cache_store_test.rb +++ b/test/translation_diff/stores/active_record_test.rb @@ -6,7 +6,7 @@ if ActiveRecordDatabase.available? ActiveRecordDatabase.connect! - class ActiveRecordCacheStoreTest < Minitest::Test + class ActiveRecordStoreTest < Minitest::Test include CacheStoreContract include BatchingCacheStoreContract @@ -56,7 +56,7 @@ def test_a_nil_cache_ttl_writes_a_row_that_never_expires config.cache_table_name = "translation_diff_translations" config.cache_ttl = nil - TranslationDiff::ActiveRecordCacheStore.build(config).write("a", "one") + TranslationDiff::Stores::ActiveRecord.build(config).write("a", "one") assert_nil model.first.expires_at end @@ -67,7 +67,7 @@ def test_a_zero_cache_ttl_writes_a_row_that_never_expires_instead_of_already_exp config.cache_table_name = "translation_diff_translations" config.cache_ttl = 0 - TranslationDiff::ActiveRecordCacheStore.build(config).write("a", "one") + TranslationDiff::Stores::ActiveRecord.build(config).write("a", "one") assert_nil model.first.expires_at end @@ -99,7 +99,7 @@ def test_build_takes_its_settings_from_the_configuration config.cache_namespace = "from-config" config.cache_table_name = "translation_diff_translations" - built = TranslationDiff::ActiveRecordCacheStore.build(config) + built = TranslationDiff::Stores::ActiveRecord.build(config) built.write("a", "one") assert_equal "from-config", built.model.first.namespace @@ -111,7 +111,7 @@ def test_a_string_cache_ttl_from_env_does_not_raise_on_write config.cache_table_name = "translation_diff_translations" config.cache_ttl = "3600" - built = TranslationDiff::ActiveRecordCacheStore.build(config) + built = TranslationDiff::Stores::ActiveRecord.build(config) built.write("a", "one") refute_nil built.model.first.expires_at @@ -123,7 +123,7 @@ def test_a_string_cache_prune_probability_from_env_does_not_raise_on_write config.cache_table_name = "translation_diff_translations" config.cache_prune_probability = "0.5" - TranslationDiff::ActiveRecordCacheStore.build(config).write("a", "one") + TranslationDiff::Stores::ActiveRecord.build(config).write("a", "one") end def test_write_multi_omits_unique_by_when_the_connection_does_not_support_a_conflict_target @@ -172,8 +172,8 @@ def test_a_readonly_error_never_carries_the_translated_content private def build_store(namespace: "translation-diff", ttl: 604_800) - TranslationDiff::ActiveRecordCacheStore.new(namespace: namespace, ttl: ttl, - table_name: "translation_diff_translations") + TranslationDiff::Stores::ActiveRecord.new(namespace: namespace, ttl: ttl, + table_name: "translation_diff_translations") end def expire(store, key) @@ -182,7 +182,7 @@ def expire(store, key) end end else - class ActiveRecordCacheStoreTest < Minitest::Test + class ActiveRecordStoreTest < Minitest::Test def test_active_record_is_unavailable skip "active_record could not be loaded on this Ruby; the SQL cache store suite is skipped" end diff --git a/test/translation_diff/active_record_transaction_test.rb b/test/translation_diff/stores/active_record_transaction_test.rb similarity index 82% rename from test/translation_diff/active_record_transaction_test.rb rename to test/translation_diff/stores/active_record_transaction_test.rb index 1a036a7..f49b55a 100644 --- a/test/translation_diff/active_record_transaction_test.rb +++ b/test/translation_diff/stores/active_record_transaction_test.rb @@ -5,7 +5,7 @@ ActiveRecordDatabase.connect! # PostgreSQL aborts the whole transaction on a statement error; only there can poisoning actually be measured. - class ActiveRecordCacheStoreTransactionTest < Minitest::Test + class ActiveRecordStoreTransactionTest < Minitest::Test def setup ActiveRecordDatabase.truncate end @@ -28,8 +28,8 @@ def test_a_failing_write_leaves_the_callers_transaction_usable end def test_a_successful_write_still_lands - store = TranslationDiff::ActiveRecordCacheStore.new(namespace: "harness", ttl: 60, - table_name: "translation_diff_translations") + store = TranslationDiff::Stores::ActiveRecord.new(namespace: "harness", ttl: 60, + table_name: "translation_diff_translations") harness_model.transaction { store.write("a", "one") } @@ -87,9 +87,9 @@ def store_with_a_pending_prune end def pruning_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: 60, - table_name: "translation_diff_translations", - prune_probability: 1.0) + TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: 60, + table_name: "translation_diff_translations", + prune_probability: 1.0) end def expire(store, key) @@ -122,18 +122,18 @@ def remove_failing_delete_trigger end def harness_model - TranslationDiff::ActiveRecordCacheStore.new(namespace: "harness", ttl: 60, - table_name: "translation_diff_translations").model + TranslationDiff::Stores::ActiveRecord.new(namespace: "harness", ttl: 60, + table_name: "translation_diff_translations").model end # A namespace past the column's 64-character limit is a statement PostgreSQL always rejects. def failing_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "x" * 100, ttl: 60, - table_name: "translation_diff_translations") + TranslationDiff::Stores::ActiveRecord.new(namespace: "x" * 100, ttl: 60, + table_name: "translation_diff_translations") end end else - class ActiveRecordCacheStoreTransactionTest < Minitest::Test + class ActiveRecordStoreTransactionTest < Minitest::Test def test_postgres_is_unavailable skip "TRANSLATION_DIFF_DATABASE_URL does not name a PostgreSQL database; " \ "only PostgreSQL aborts a transaction on a statement error" diff --git a/test/translation_diff/active_record_write_multi_dedupe_test.rb b/test/translation_diff/stores/active_record_write_multi_dedupe_test.rb similarity index 90% rename from test/translation_diff/active_record_write_multi_dedupe_test.rb rename to test/translation_diff/stores/active_record_write_multi_dedupe_test.rb index 02a1716..f4f3752 100644 --- a/test/translation_diff/active_record_write_multi_dedupe_test.rb +++ b/test/translation_diff/stores/active_record_write_multi_dedupe_test.rb @@ -38,8 +38,8 @@ def test_without_the_dedupe_postgresql_raises_on_a_repeated_key_in_one_batch private def build_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: 60, - table_name: "translation_diff_translations") + TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: 60, + table_name: "translation_diff_translations") end end else diff --git a/test/translation_diff/memory_cache_store_test.rb b/test/translation_diff/stores/memory_test.rb similarity index 86% rename from test/translation_diff/memory_cache_store_test.rb rename to test/translation_diff/stores/memory_test.rb index c6cacf3..b074fca 100644 --- a/test/translation_diff/memory_cache_store_test.rb +++ b/test/translation_diff/stores/memory_test.rb @@ -2,14 +2,14 @@ require "support/cache_store_contract" require "support/batching_cache_store_contract" -class MemoryCacheStoreTest < Minitest::Test +class MemoryStoreTest < Minitest::Test include CacheStoreContract include BatchingCacheStoreContract attr_reader :store def setup - @store = TranslationDiff::MemoryCacheStore.new(max_size: 3) + @store = TranslationDiff::Stores::Memory.new(max_size: 3) end def test_it_evicts_the_oldest_entry_once_the_bound_is_reached @@ -39,7 +39,7 @@ def test_build_ignores_a_nil_cache_ttl_and_writes_normally config = TranslationDiff::Configuration.new config.cache_ttl = nil - built = TranslationDiff::MemoryCacheStore.build(config) + built = TranslationDiff::Stores::Memory.build(config) built.write("a", "one") assert_equal ["one"], built.read_multi(["a"]) @@ -49,7 +49,7 @@ def test_build_takes_its_bound_from_the_configuration config = TranslationDiff::Configuration.new config.cache_max_size = 1 - built = TranslationDiff::MemoryCacheStore.build(config) + built = TranslationDiff::Stores::Memory.build(config) built.write("a", "one") built.write("b", "two") diff --git a/test/translation_diff/redis_cache_store_test.rb b/test/translation_diff/stores/redis_test.rb similarity index 96% rename from test/translation_diff/redis_cache_store_test.rb rename to test/translation_diff/stores/redis_test.rb index 5b15d09..727b3a8 100644 --- a/test/translation_diff/redis_cache_store_test.rb +++ b/test/translation_diff/stores/redis_test.rb @@ -28,7 +28,7 @@ def pipelined end end -class RedisCacheStoreTest < Minitest::Test +class RedisStoreTest < Minitest::Test include CacheStoreContract include BatchingCacheStoreContract @@ -152,6 +152,6 @@ def test_write_multi_with_a_nil_timeout_never_expires private def build_store(redis, **) - TranslationDiff::RedisCacheStore.new(FakeConnectionPool.new(redis), **) + TranslationDiff::Stores::Redis.new(FakeConnectionPool.new(redis), **) end end diff --git a/test/translation_diff/translator_cache_failure_test.rb b/test/translation_diff/translator_cache_failure_test.rb index 8c4876b..d01044a 100644 --- a/test/translation_diff/translator_cache_failure_test.rb +++ b/test/translation_diff/translator_cache_failure_test.rb @@ -69,9 +69,9 @@ def store_with_a_pending_prune end def pruning_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: 60, - table_name: "translation_diff_translations", - prune_probability: 1.0) + TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: 60, + table_name: "translation_diff_translations", + prune_probability: 1.0) end def expire(store, key) @@ -97,8 +97,8 @@ def remove_failing_delete_trigger end def harness_model - TranslationDiff::ActiveRecordCacheStore.new(namespace: "harness", ttl: 60, - table_name: "translation_diff_translations").model + TranslationDiff::Stores::ActiveRecord.new(namespace: "harness", ttl: 60, + table_name: "translation_diff_translations").model end end else From 0266f0a0536de406a6a9ff7cb733ea4e387b6439 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:18:26 +0400 Subject: [PATCH 2/7] refactor: move rate limiters under RateLimiters namespace RedisRateLimiter and ActiveRecordRateLimiter become TranslationDiff::RateLimiters::Redis/ActiveRecord, matching Stores' new shape and the registry name each is registered under. --- .../configuration/option_table.rb | 2 +- lib/translation_diff/rate_limiters.rb | 9 ++++++++- .../active_record.rb} | 6 +++--- .../redis.rb} | 4 ++-- test/support/rate_limiter_contract.rb | 2 +- .../active_record_concurrency_test.rb | 10 +++++----- .../active_record_test.rb} | 17 +++++++++-------- .../redis_test.rb} | 18 +++++++++--------- 8 files changed, 38 insertions(+), 30 deletions(-) rename lib/translation_diff/{active_record_rate_limiter.rb => rate_limiters/active_record.rb} (96%) rename lib/translation_diff/{redis_rate_limiter.rb => rate_limiters/redis.rb} (97%) rename test/translation_diff/{active_record_rate_limiter_test.rb => rate_limiters/active_record_test.rb} (91%) rename test/translation_diff/{redis_rate_limiter_test.rb => rate_limiters/redis_test.rb} (86%) diff --git a/lib/translation_diff/configuration/option_table.rb b/lib/translation_diff/configuration/option_table.rb index 82eba76..f4f3d29 100644 --- a/lib/translation_diff/configuration/option_table.rb +++ b/lib/translation_diff/configuration/option_table.rb @@ -7,7 +7,7 @@ module TranslationDiff::Configuration::OptionTable [:provider, :deepl, :provider_instance], # rubocop:disable Style/SymbolArray -- stays [key, default, invalidates] [:cache, nil, :cache_store], [:cache_ttl, 604_800, :cache_store], - # Also the rate limiter's own namespace (RedisRateLimiter, ActiveRecordRateLimiter both read it). + # Also the rate limiter's own namespace (RateLimiters::Redis, RateLimiters::ActiveRecord both read it). [:cache_namespace, "translation-diff", %i[cache_store rate_limiter_instance]], [:cache_max_size, 1_000, :cache_store], [:cache_table_name, "translation_diff_translations", :cache_store], diff --git a/lib/translation_diff/rate_limiters.rb b/lib/translation_diff/rate_limiters.rb index 5ae854e..814c3da 100644 --- a/lib/translation_diff/rate_limiters.rb +++ b/lib/translation_diff/rate_limiters.rb @@ -1,2 +1,9 @@ # Rate limiters, by name; assigning an object to `config.rate_limiter` bypasses this entirely. -TranslationDiff::RateLimiters = TranslationDiff::Registry.new("rate limiter") +module TranslationDiff::RateLimiters + def self.register(name, klass) = registry.register(name, klass) + def self.build(name, config) = registry.build(name, config) + def self.registered?(name) = registry.registered?(name) + def self.names = registry.names + def self.classes = registry.classes + def self.registry = @registry ||= TranslationDiff::Registry.new("rate limiter") +end diff --git a/lib/translation_diff/active_record_rate_limiter.rb b/lib/translation_diff/rate_limiters/active_record.rb similarity index 96% rename from lib/translation_diff/active_record_rate_limiter.rb rename to lib/translation_diff/rate_limiters/active_record.rb index b37b1aa..e54f39c 100644 --- a/lib/translation_diff/active_record_rate_limiter.rb +++ b/lib/translation_diff/rate_limiters/active_record.rb @@ -1,6 +1,6 @@ # Throttles by counting characters into namespaced, time-bucketed rows in the application's own database. -class TranslationDiff::ActiveRecordRateLimiter - include TranslationDiff::ActiveRecordSupport +class TranslationDiff::RateLimiters::ActiveRecord + include TranslationDiff::ActiveRecord::Support class RateLimitExceeded < TranslationDiff::Error; end @@ -91,4 +91,4 @@ def active_record_component = "ActiveRecord rate limiter" def active_record_upsert_detail = "upsert_all takes unique_by there." end -TranslationDiff::RateLimiters.register(:active_record, TranslationDiff::ActiveRecordRateLimiter) +TranslationDiff::RateLimiters.register(:active_record, TranslationDiff::RateLimiters::ActiveRecord) diff --git a/lib/translation_diff/redis_rate_limiter.rb b/lib/translation_diff/rate_limiters/redis.rb similarity index 97% rename from lib/translation_diff/redis_rate_limiter.rb rename to lib/translation_diff/rate_limiters/redis.rb index edbc827..c843eb4 100644 --- a/lib/translation_diff/redis_rate_limiter.rb +++ b/lib/translation_diff/rate_limiters/redis.rb @@ -1,4 +1,4 @@ -class TranslationDiff::RedisRateLimiter +class TranslationDiff::RateLimiters::Redis class RateLimitExceeded < TranslationDiff::Error; end DEFAULT_THRESHOLD = 8000 @@ -58,4 +58,4 @@ def ratelimit_class end end -TranslationDiff::RateLimiters.register(:redis, TranslationDiff::RedisRateLimiter) +TranslationDiff::RateLimiters.register(:redis, TranslationDiff::RateLimiters::Redis) diff --git a/test/support/rate_limiter_contract.rb b/test/support/rate_limiter_contract.rb index 5e41e84..5dd1984 100644 --- a/test/support/rate_limiter_contract.rb +++ b/test/support/rate_limiter_contract.rb @@ -25,5 +25,5 @@ def test_the_refusal_names_the_limit_it_hit_and_no_content end # Rollover is not in this contract: proving it means waiting for a bucket to turn over, and only - # ActiveRecordRateLimiter can be made to turn one over without a real sleep. See its own test file. + # RateLimiters::ActiveRecord can be made to turn one over without a real sleep. See its own test file. end diff --git a/test/translation_diff/active_record_concurrency_test.rb b/test/translation_diff/active_record_concurrency_test.rb index 3b9d430..ec3122b 100644 --- a/test/translation_diff/active_record_concurrency_test.rb +++ b/test/translation_diff/active_record_concurrency_test.rb @@ -35,15 +35,15 @@ def test_two_limiters_racing_the_same_bucket_lose_no_increment private def build_cache_store - TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: 60, - table_name: "translation_diff_translations") + TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: 60, + table_name: "translation_diff_translations") end # A shared, frozen clock keeps both limiters in the same bucket for the length of the test. def build_rate_limiter(now) - TranslationDiff::ActiveRecordRateLimiter.new(namespace: "translation-diff", threshold: 1_000_000, - interval: 60, table_name: "translation_diff_rate_limits", - clock: -> { now }) + TranslationDiff::RateLimiters::ActiveRecord.new(namespace: "translation-diff", threshold: 1_000_000, + interval: 60, table_name: "translation_diff_rate_limits", + clock: -> { now }) end end else diff --git a/test/translation_diff/active_record_rate_limiter_test.rb b/test/translation_diff/rate_limiters/active_record_test.rb similarity index 91% rename from test/translation_diff/active_record_rate_limiter_test.rb rename to test/translation_diff/rate_limiters/active_record_test.rb index eb4c989..987a61b 100644 --- a/test/translation_diff/active_record_rate_limiter_test.rb +++ b/test/translation_diff/rate_limiters/active_record_test.rb @@ -34,8 +34,8 @@ def test_a_window_that_has_rolled_over_passes_again end def model - TranslationDiff::ActiveRecordRateLimiter.new(namespace: "translation-diff", - table_name: "translation_diff_rate_limits").model + TranslationDiff::RateLimiters::ActiveRecord.new(namespace: "translation-diff", + table_name: "translation_diff_rate_limits").model end def test_two_limiters_sharing_a_namespace_see_each_others_characters @@ -162,7 +162,7 @@ def test_build_takes_its_settings_from_the_configuration config.rate_limit = 100 config.rate_interval = 60 - built = TranslationDiff::ActiveRecordRateLimiter.build(config) + built = TranslationDiff::RateLimiters::ActiveRecord.build(config) built.check(1) assert_equal ["from-config"], built.model.pluck(:namespace) @@ -173,9 +173,10 @@ def test_build_falls_back_to_the_default_threshold_when_rate_limit_is_unset config = TranslationDiff::Configuration.new config.rate_limiter = :active_record - built = TranslationDiff::ActiveRecordRateLimiter.build(config) + built = TranslationDiff::RateLimiters::ActiveRecord.build(config) - assert_equal TranslationDiff::ActiveRecordRateLimiter::DEFAULT_THRESHOLD, built.instance_variable_get(:@threshold) + assert_equal TranslationDiff::RateLimiters::ActiveRecord::DEFAULT_THRESHOLD, + built.instance_variable_get(:@threshold) built.check(1) end @@ -214,14 +215,14 @@ def test_add_quotes_the_table_name_in_the_on_duplicate_fragment private - def rate_limit_exceeded_error = TranslationDiff::ActiveRecordRateLimiter::RateLimitExceeded + def rate_limit_exceeded_error = TranslationDiff::RateLimiters::ActiveRecord::RateLimitExceeded # Held still, so a five-second bucket boundary cannot fall between two reads of the clock. def frozen_clock = MutableClock.new(Time.at(1_700_000_000)) def build_limiter(threshold:, interval:, namespace: "translation-diff", clock: -> { Time.now }) - TranslationDiff::ActiveRecordRateLimiter.new(namespace: namespace, threshold: threshold, interval: interval, - table_name: "translation_diff_rate_limits", clock: clock) + TranslationDiff::RateLimiters::ActiveRecord.new(namespace: namespace, threshold: threshold, interval: interval, + table_name: "translation_diff_rate_limits", clock: clock) end end else diff --git a/test/translation_diff/redis_rate_limiter_test.rb b/test/translation_diff/rate_limiters/redis_test.rb similarity index 86% rename from test/translation_diff/redis_rate_limiter_test.rb rename to test/translation_diff/rate_limiters/redis_test.rb index 34ed58f..5c86307 100644 --- a/test/translation_diff/redis_rate_limiter_test.rb +++ b/test/translation_diff/rate_limiters/redis_test.rb @@ -86,7 +86,7 @@ def test_check_raises_once_the_threshold_is_passed limiter(server, threshold: 100).check(100) - assert_raises(TranslationDiff::RedisRateLimiter::RateLimitExceeded) do + assert_raises(TranslationDiff::RateLimiters::Redis::RateLimitExceeded) do limiter(server, threshold: 100).check(1) end assert_equal({ "ratelimit:translation-diff:call" => 100 }, server.totals) @@ -95,9 +95,9 @@ def test_check_raises_once_the_threshold_is_passed def test_check_uses_the_default_threshold server = FakeRedisServer.new - limiter(server).check(TranslationDiff::RedisRateLimiter::DEFAULT_THRESHOLD) + limiter(server).check(TranslationDiff::RateLimiters::Redis::DEFAULT_THRESHOLD) - assert_raises(TranslationDiff::RedisRateLimiter::RateLimitExceeded) { limiter(server).check(1) } + assert_raises(TranslationDiff::RateLimiters::Redis::RateLimitExceeded) { limiter(server).check(1) } end # Ratelimit buckets five seconds at a time, so buckets swept is the interval divided by five. @@ -106,7 +106,7 @@ def test_check_looks_back_over_the_default_interval limiter(server).check(1) - assert_equal [TranslationDiff::RedisRateLimiter::DEFAULT_INTERVAL / 5], server.count_spans + assert_equal [TranslationDiff::RateLimiters::Redis::DEFAULT_INTERVAL / 5], server.count_spans end def test_check_looks_back_over_a_custom_interval @@ -120,7 +120,7 @@ def test_check_looks_back_over_a_custom_interval # The other half of a window: what fell out of it stops counting, or a limiter never recovers. def test_a_bucket_older_than_the_interval_is_not_counted server = FakeRedisServer.new - stale = (Time.now.to_i / 5) - (TranslationDiff::RedisRateLimiter::DEFAULT_INTERVAL / 5) - 1 + stale = (Time.now.to_i / 5) - (TranslationDiff::RateLimiters::Redis::DEFAULT_INTERVAL / 5) - 1 server.hashes["ratelimit:translation-diff:call"][stale.to_s] = 10_000 limiter(server, threshold: 100).check(1) @@ -144,9 +144,9 @@ def test_build_falls_back_to_the_default_threshold_when_rate_limit_is_unset config.rate_limiter = :redis config.instance_variable_set(:@redis_pool, FakeConnectionPool.new(server)) - built = TranslationDiff::RedisRateLimiter.build(config) + built = TranslationDiff::RateLimiters::Redis.build(config) - assert_equal TranslationDiff::RedisRateLimiter::DEFAULT_THRESHOLD, built.send(:threshold) + assert_equal TranslationDiff::RateLimiters::Redis::DEFAULT_THRESHOLD, built.send(:threshold) built.check(1) end @@ -167,10 +167,10 @@ def test_changing_the_cache_namespace_moves_the_limiter_to_the_new_redis_namespa private def limiter(server, **) - TranslationDiff::RedisRateLimiter.new(FakeConnectionPool.new(server), **) + TranslationDiff::RateLimiters::Redis.new(FakeConnectionPool.new(server), **) end - def rate_limit_exceeded_error = TranslationDiff::RedisRateLimiter::RateLimitExceeded + def rate_limit_exceeded_error = TranslationDiff::RateLimiters::Redis::RateLimitExceeded def build_limiter(threshold:, interval:) @contract_server ||= FakeRedisServer.new From a955f94258cf3ce086f058cd4519c063913d7216 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:18:33 +0400 Subject: [PATCH 3/7] refactor: move ActiveRecord support under its own namespace ActiveRecordSupport becomes TranslationDiff::ActiveRecord::Support, the mixin the cache store and rate limiter both include. Since TranslationDiff::ActiveRecord now shares its name with ::ActiveRecord, the two references it makes to the real gem across a genuinely missing constant (ar_error?'s defined?/is_a? pair) are :: qualified; the version floor check and the anonymous model's base class already were. --- lib/translation_diff/active_record.rb | 3 ++ .../support.rb} | 4 +-- .../active_record/support_test.rb | 30 +++++++++++++++++++ .../active_record_support_test.rb | 30 ------------------- 4 files changed, 35 insertions(+), 32 deletions(-) create mode 100644 lib/translation_diff/active_record.rb rename lib/translation_diff/{active_record_support.rb => active_record/support.rb} (88%) create mode 100644 test/translation_diff/active_record/support_test.rb delete mode 100644 test/translation_diff/active_record_support_test.rb diff --git a/lib/translation_diff/active_record.rb b/lib/translation_diff/active_record.rb new file mode 100644 index 0000000..ffa30c9 --- /dev/null +++ b/lib/translation_diff/active_record.rb @@ -0,0 +1,3 @@ +# The namespace the cache store and the rate limiter's ActiveRecord support live under; see active_record/support. +module TranslationDiff::ActiveRecord +end diff --git a/lib/translation_diff/active_record_support.rb b/lib/translation_diff/active_record/support.rb similarity index 88% rename from lib/translation_diff/active_record_support.rb rename to lib/translation_diff/active_record/support.rb index d25c6b0..9db8e7f 100644 --- a/lib/translation_diff/active_record_support.rb +++ b/lib/translation_diff/active_record/support.rb @@ -1,5 +1,5 @@ # The lazy require, the version floor and the anonymous model class, shared by the cache store and the limiter. -module TranslationDiff::ActiveRecordSupport +module TranslationDiff::ActiveRecord::Support MINIMUM_ACTIVE_RECORD = "7.1".freeze def model @@ -10,7 +10,7 @@ def model # Any ActiveRecordError, not just StatementInvalid -- ReadOnlyError carries a whole write statement too. def ar_error?(error) - defined?(ActiveRecord::ActiveRecordError) && error.is_a?(ActiveRecord::ActiveRecordError) + defined?(::ActiveRecord::ActiveRecordError) && error.is_a?(::ActiveRecord::ActiveRecordError) end def build_model diff --git a/test/translation_diff/active_record/support_test.rb b/test/translation_diff/active_record/support_test.rb new file mode 100644 index 0000000..f0d6db5 --- /dev/null +++ b/test/translation_diff/active_record/support_test.rb @@ -0,0 +1,30 @@ +require "test_helper" +require "support/active_record_database" + +if ActiveRecordDatabase.available? + ActiveRecordDatabase.connect! + + class ActiveRecordSupportTest < Minitest::Test + def test_the_cache_store_and_the_rate_limiter_share_the_same_active_record_plumbing + assert_includes TranslationDiff::Stores::ActiveRecord.ancestors, TranslationDiff::ActiveRecord::Support + assert_includes TranslationDiff::RateLimiters::ActiveRecord.ancestors, TranslationDiff::ActiveRecord::Support + end + + def test_the_version_floor_is_declared_once_and_shared + assert_same TranslationDiff::ActiveRecord::Support::MINIMUM_ACTIVE_RECORD, + TranslationDiff::Stores::ActiveRecord::MINIMUM_ACTIVE_RECORD + assert_same TranslationDiff::ActiveRecord::Support::MINIMUM_ACTIVE_RECORD, + TranslationDiff::RateLimiters::ActiveRecord::MINIMUM_ACTIVE_RECORD + end + + def test_each_store_instance_memoises_its_own_model_rather_than_sharing_one + first = TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: nil, + table_name: "translation_diff_translations") + second = TranslationDiff::Stores::ActiveRecord.new(namespace: "translation-diff", ttl: nil, + table_name: "translation_diff_translations") + + assert_same first.model, first.model + refute_same first.model, second.model + end + end +end diff --git a/test/translation_diff/active_record_support_test.rb b/test/translation_diff/active_record_support_test.rb deleted file mode 100644 index af353dd..0000000 --- a/test/translation_diff/active_record_support_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require "test_helper" -require "support/active_record_database" - -if ActiveRecordDatabase.available? - ActiveRecordDatabase.connect! - - class ActiveRecordSupportTest < Minitest::Test - def test_the_cache_store_and_the_rate_limiter_share_the_same_active_record_plumbing - assert_includes TranslationDiff::ActiveRecordCacheStore.ancestors, TranslationDiff::ActiveRecordSupport - assert_includes TranslationDiff::ActiveRecordRateLimiter.ancestors, TranslationDiff::ActiveRecordSupport - end - - def test_the_version_floor_is_declared_once_and_shared - assert_same TranslationDiff::ActiveRecordSupport::MINIMUM_ACTIVE_RECORD, - TranslationDiff::ActiveRecordCacheStore::MINIMUM_ACTIVE_RECORD - assert_same TranslationDiff::ActiveRecordSupport::MINIMUM_ACTIVE_RECORD, - TranslationDiff::ActiveRecordRateLimiter::MINIMUM_ACTIVE_RECORD - end - - def test_each_store_instance_memoises_its_own_model_rather_than_sharing_one - first = TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: nil, - table_name: "translation_diff_translations") - second = TranslationDiff::ActiveRecordCacheStore.new(namespace: "translation-diff", ttl: nil, - table_name: "translation_diff_translations") - - assert_same first.model, first.model - refute_same first.model, second.model - end - end -end From 43182eb1531229f999cc696feb162b94f8d6d83b Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:18:39 +0400 Subject: [PATCH 4/7] refactor: move Configuration's option mixins under its own namespace CacheTtlOption and CacheGuardOptions become TranslationDiff::Configuration::CacheTtlOption/CacheGuardOptions, prepended pieces of Configuration rather than loose top-level modules -- the same shape option_table.rb already had. Their requires move inside Configuration's own class body, next to option_table's, since the modules they define now nest under a class that has to exist first. --- lib/translation_diff/configuration.rb | 8 +++++--- .../{ => configuration}/cache_guard_options.rb | 2 +- .../{ => configuration}/cache_ttl_option.rb | 2 +- test/translation_diff/configuration_test.rb | 12 ++++++------ 4 files changed, 13 insertions(+), 11 deletions(-) rename lib/translation_diff/{ => configuration}/cache_guard_options.rb (96%) rename lib/translation_diff/{ => configuration}/cache_ttl_option.rb (94%) diff --git a/lib/translation_diff/configuration.rb b/lib/translation_diff/configuration.rb index 9deec84..0358ed9 100644 --- a/lib/translation_diff/configuration.rb +++ b/lib/translation_diff/configuration.rb @@ -54,12 +54,14 @@ def normalise_declarations(declared) def provider_option_owners = @provider_option_owners ||= ProviderOptionOwners.new end - # Required here, not centrally: the module it defines nests under this class, which must exist first. + # Required here, not centrally: the modules they define nest under this class, which must exist first. require "translation_diff/configuration/option_table" TranslationDiff::Configuration::OptionTable.declare_on(self) - prepend TranslationDiff::CacheTtlOption - prepend TranslationDiff::CacheGuardOptions + require "translation_diff/configuration/cache_ttl_option" + require "translation_diff/configuration/cache_guard_options" + prepend TranslationDiff::Configuration::CacheTtlOption + prepend TranslationDiff::Configuration::CacheGuardOptions # Credentials are filtered by name; everything else is shown, or an inspect is one nobody reads. def inspect = "#<#{self.class.name} #{TranslationDiff::Redaction.render(self).join(' ')}>" diff --git a/lib/translation_diff/cache_guard_options.rb b/lib/translation_diff/configuration/cache_guard_options.rb similarity index 96% rename from lib/translation_diff/cache_guard_options.rb rename to lib/translation_diff/configuration/cache_guard_options.rb index 1c4809c..71a0919 100644 --- a/lib/translation_diff/cache_guard_options.rb +++ b/lib/translation_diff/configuration/cache_guard_options.rb @@ -1,5 +1,5 @@ # Prepended onto Configuration: fails cache_prune_probability and cache_namespace at configure time, not later. -module TranslationDiff::CacheGuardOptions +module TranslationDiff::Configuration::CacheGuardOptions CACHE_NAMESPACE_LIMIT = 64 # An ENV var arrives as a String; coerced here so a translate call never meets a bare String's missing diff --git a/lib/translation_diff/cache_ttl_option.rb b/lib/translation_diff/configuration/cache_ttl_option.rb similarity index 94% rename from lib/translation_diff/cache_ttl_option.rb rename to lib/translation_diff/configuration/cache_ttl_option.rb index 067aaa1..4269cb3 100644 --- a/lib/translation_diff/cache_ttl_option.rb +++ b/lib/translation_diff/configuration/cache_ttl_option.rb @@ -1,5 +1,5 @@ # Prepended onto Configuration: nil sticks here as "never expires", unlike the generic option rule. -module TranslationDiff::CacheTtlOption +module TranslationDiff::Configuration::CacheTtlOption NEVER_ASSIGNED = Object.new.freeze def initialize diff --git a/test/translation_diff/configuration_test.rb b/test/translation_diff/configuration_test.rb index ec952b1..565e196 100644 --- a/test/translation_diff/configuration_test.rb +++ b/test/translation_diff/configuration_test.rb @@ -364,7 +364,7 @@ def test_cache_store_defaults_to_memory_when_no_redis_url_is_set original = ENV.fetch("REDIS_URL", nil) ENV["REDIS_URL"] = nil - assert_instance_of TranslationDiff::MemoryCacheStore, @config.cache_store + assert_instance_of TranslationDiff::Stores::Memory, @config.cache_store ensure ENV["REDIS_URL"] = original end @@ -372,7 +372,7 @@ def test_cache_store_defaults_to_memory_when_no_redis_url_is_set def test_cache_store_defaults_to_redis_when_a_redis_url_is_set @config.redis_url = "redis://localhost:6379" - assert_instance_of TranslationDiff::RedisCacheStore, @config.cache_store + assert_instance_of TranslationDiff::Stores::Redis, @config.cache_store end def test_an_assigned_cache_object_wins_over_every_value @@ -395,21 +395,21 @@ def test_a_rate_limit_builds_a_redis_rate_limiter @config.rate_limit = 100 @config.redis_url = "redis://localhost:6379" - assert_instance_of TranslationDiff::RedisRateLimiter, @config.rate_limiter_instance + assert_instance_of TranslationDiff::RateLimiters::Redis, @config.rate_limiter_instance end def test_a_symbol_rate_limiter_resolves_through_the_registry @config.rate_limit = 100 @config.rate_limiter = :active_record - assert_instance_of TranslationDiff::ActiveRecordRateLimiter, @config.rate_limiter_instance + assert_instance_of TranslationDiff::RateLimiters::ActiveRecord, @config.rate_limiter_instance end def test_a_string_rate_limiter_resolves_through_the_registry @config.rate_limit = 100 @config.rate_limiter = "active_record" - assert_instance_of TranslationDiff::ActiveRecordRateLimiter, @config.rate_limiter_instance + assert_instance_of TranslationDiff::RateLimiters::ActiveRecord, @config.rate_limiter_instance end def test_an_unknown_rate_limiter_name_raises_listing_what_is_registered @@ -608,7 +608,7 @@ def test_changing_the_cache_namespace_rebuilds_the_cache_store end # The scenario the bug actually costs: a per-request `configure { |c| c.cache_namespace = tenant }` re-writing - # the same tenant on every request must never rebuild the store -- on MemoryCacheStore a rebuild is a brand + # the same tenant on every request must never rebuild the store -- on Stores::Memory a rebuild is a brand # new empty Hash, so the application would pay the provider again for its whole warm cache. def test_writing_the_same_cache_namespace_again_leaves_the_cache_store_in_place @config.cache_namespace = "tenant-1" From 04ecb4b6dd11149288fda8ba15f8fdd0b6d656f6 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:18:44 +0400 Subject: [PATCH 5/7] docs: catch up class names with the Stores/RateLimiters/ActiveRecord move MemoryCacheStore, RedisCacheStore, ActiveRecordCacheStore, RedisRateLimiter, ActiveRecordRateLimiter and ActiveRecordSupport are gone; every mention across README, docs/ and the Gemfile's dependency comments now names the class actually in lib/. Adds an Unreleased CHANGELOG entry naming the renames, since the source now shows them even though behaviour didn't change. --- CHANGELOG.md | 12 ++++++++++++ Gemfile | 8 ++++---- README.md | 2 +- docs/caching.md | 24 ++++++++++++------------ docs/configuration.md | 22 +++++++++++----------- docs/contracts.md | 14 +++++++------- docs/errors.md | 6 +++--- docs/instrumentation.md | 4 ++-- docs/sql-cache.md | 24 ++++++++++++------------ 9 files changed, 64 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45b8581..4a00521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Rescue `TranslationDiff::Error` to catch both. See [Errors](docs/errors.md). +- **The cache stores, rate limiters and ActiveRecord plumbing moved under + their registry's own namespace; nothing was left behind at the old + name.** `TranslationDiff::MemoryCacheStore`, `RedisCacheStore` and + `ActiveRecordCacheStore` are now `TranslationDiff::Stores::Memory`, + `Stores::Redis` and `Stores::ActiveRecord`; `RedisRateLimiter` and + `ActiveRecordRateLimiter` are now `TranslationDiff::RateLimiters::Redis` + and `RateLimiters::ActiveRecord`; `TranslationDiff::ActiveRecordSupport` + is now `TranslationDiff::ActiveRecord::Support`. `config.cache = :redis` + and the rest of the symbol-keyed configuration are unaffected -- only + the constant a name resolves to changed. See + [Caching](docs/caching.md) and [SQL cache](docs/sql-cache.md). + ### Added - **Every event from one `translate` call now shares a `call_id`.** Generated diff --git a/Gemfile b/Gemfile index 35790f4..f99081f 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ source "https://rubygems.org" gemspec # Not runtime dependencies of the gem (see the gemspec) -- Configuration# -# redis_pool requires them lazily, and RedisCacheStore/RedisRateLimiter +# redis_pool requires them lazily, and Stores::Redis/RateLimiters::Redis # duck-type against whatever a caller's connection pool yields. They are # only here so the test suite, which builds real pools against these # classes, has them available. @@ -11,7 +11,7 @@ gem "connection_pool", "~> 2.4", require: false gem "redis", "~> 5.0", require: false gem "redis-namespace", "~> 1.11", require: false -# Not a runtime dependency of the gem (see the gemspec) -- RedisRateLimiter +# Not a runtime dependency of the gem (see the gemspec) -- RateLimiters::Redis # requires it lazily on the first check, so an application that configures no # rate limit never needs it installed. It is only here so the test suite, # which exercises the limiter against the real Ratelimit class rather than a @@ -32,8 +32,8 @@ gem "cgi", "~> 0.5", require: false # stand-in, has it available. gem "aws-sigv4", "~> 1.12", require: false -# Not runtime dependencies of the gem (see the gemspec) -- ActiveRecordCacheStore -# and ActiveRecordRateLimiter require active_record lazily on first use, so an +# Not runtime dependencies of the gem (see the gemspec) -- TranslationDiff::Stores::ActiveRecord +# and TranslationDiff::RateLimiters::ActiveRecord require active_record lazily on first use, so an # application caching in Redis never needs it installed. They are here so the # suite can exercise the stores against a real database rather than a stand-in. gem "activerecord", "~> 8.1", require: false diff --git a/README.md b/README.md index 1a85048..b269c8c 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ See [Providers](docs/providers.md) for configuring each one, the full capabiliti - **Six built-in providers** -- DeepL, Google Cloud Translation, Azure AI Translator, ModernMT, LibreTranslate, Amazon Translate -- or bring your own by subclassing a small base class - **HTML aware:** markup is preserved, and `class="notranslate"` can protect a span (provider support varies -- see the caveats below) - **Any shape:** strings, arrays, and deep hashes go in and come back translated in the same shape -- **Three cache stores:** `MemoryCacheStore` out of the box, `RedisCacheStore` once you configure `redis_url`, `ActiveRecordCacheStore` to cache in your own database instead -- see [SQL cache](docs/sql-cache.md) +- **Three cache stores:** `Stores::Memory` out of the box, `Stores::Redis` once you configure `redis_url`, `Stores::ActiveRecord` to cache in your own database instead -- see [SQL cache](docs/sql-cache.md) - **Isolated contexts:** `TranslationDiff.context` for multi-tenant apps and per-request provider overrides, without touching the global configuration - **Pluggable sentence segmenter:** `pragmatic_segmenter` by default, with a zero-dependency `Simple` alternative - **HTTP retries, timeouts, and backoff** on every REST-backed provider, via `faraday` and `faraday-retry` diff --git a/docs/caching.md b/docs/caching.md index 8b439c7..330b9d2 100644 --- a/docs/caching.md +++ b/docs/caching.md @@ -5,7 +5,7 @@ One entry per sentence, keyed by the provider's `cache_key`, the lowercased source and target language codes, a digest of the provider options that call passed (`formality:`, a glossary id, ...), and a digest of the sentence -itself. `RedisCacheStore` prefixes all of that with `cache_namespace`. +itself. `Stores::Redis` prefixes all of that with `cache_namespace`. **No provider's `*_api_base` option is part of the key.** Two configurations pointing `deepl_api_base` (or any other provider's `_api_base`) at different @@ -125,15 +125,15 @@ methods -- so a store that implements only those two still passes it. include `BatchingCacheStoreContract` too, alongside `CacheStoreContract`, once `#store` also implements `write_multi`. -Three stores ship with this gem: `TranslationDiff::MemoryCacheStore`, the +Three stores ship with this gem: `TranslationDiff::Stores::Memory`, the default -- a bounded, in-process LRU, not thread-safe by design, evicting by -`cache_max_size` rather than by time; `TranslationDiff::RedisCacheStore`, +`cache_max_size` rather than by time; `TranslationDiff::Stores::Redis`, built from `redis_url` when that is set, expiring entries after `cache_ttl` and namespacing every key under `cache_namespace`; and -`TranslationDiff::ActiveRecordCacheStore`, opt-in, caching in the +`TranslationDiff::Stores::ActiveRecord`, opt-in, caching in the application's own database -- see [SQL cache](sql-cache.md). Neither `redis` nor `connection_pool` nor `redis-namespace` is a dependency of this gem -- -`RedisCacheStore` takes anything answering to `#with` the way +`Stores::Redis` takes anything answering to `#with` the way `ConnectionPool` does, and yields anything `Redis::Namespace` accepts. ## `write_multi` is optional @@ -145,9 +145,9 @@ from the batch; a store that does not is called once per sentence through against the contract before `write_multi` existed keeps working unchanged -- that is what "optional" means here. -All three shipped stores implement it: `MemoryCacheStore` loops over the -pairs (there is no round trip to save in-process); `RedisCacheStore` -pipelines the writes; `ActiveRecordCacheStore` upserts the whole batch in +All three shipped stores implement it: `Stores::Memory` loops over the +pairs (there is no round trip to save in-process); `Stores::Redis` +pipelines the writes; `Stores::ActiveRecord` upserts the whole batch in one statement. ### The three write paths fail differently @@ -155,15 +155,15 @@ one statement. Nobody had written this down before: what a partial failure leaves cached depends on which of these shapes wrote it. -- **No `write_multi` (the per-key path), and `MemoryCacheStore`'s loop.** +- **No `write_multi` (the per-key path), and `Stores::Memory`'s loop.** Sentences are written one at a time, in order. A failure at sentence N leaves 1..N-1 written, N failed, and N+1.. never attempted. -- **`RedisCacheStore#write_multi`.** A Redis pipeline is not a +- **`Stores::Redis#write_multi`.** A Redis pipeline is not a transaction: each `SETEX` in it runs independently of the others, so a failure in one does not stop its siblings from landing. Which of the batch actually landed does not follow the sentence order the way the per-key path's does. -- **`ActiveRecordCacheStore#write_multi`.** One `upsert_all` statement for +- **`Stores::ActiveRecord#write_multi`.** One `upsert_all` statement for the whole batch. It either lands as a whole or it does not -- there is no partial batch to reason about. @@ -177,7 +177,7 @@ paid for at the provider: `Translator#fill` rescues whatever error surfaces here, logs it, fires a `cache_error` event (provider and error class only, never the text -- see [Instrumentation](instrumentation.md)), and returns the translation regardless. This holds for all three shapes and every -store, not only `ActiveRecordCacheStore` -- a `MemoryCacheStore` bug, a +store, not only `Stores::ActiveRecord` -- a `Stores::Memory` bug, a dropped Redis connection, a SQL write blocked by a read-only replica (see [Rails replica routing](sql-cache.md#rails-replica-routing)) all behave the same way from the caller's side. What differs between the three shapes diff --git a/docs/configuration.md b/docs/configuration.md index 50db04f..4e1f87e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -55,13 +55,13 @@ at all, so an unset environment variable never has to be special-cased. | --- | --- | --- | | `provider` | `:deepl` | The translation provider: a registered name or a `TranslationDiff::Provider` of your own. See [Providers](providers.md). | | `cache` | `nil` | The cache store: a registered name or an object satisfying the [cache store contract](caching.md#the-cache-store-contract). `nil` means "choose for me" -- see below. | -| `cache_ttl` | `604_800` (one week) | Seconds an entry is kept before it expires. Read by `RedisCacheStore` (a `SETEX`) and by `ActiveRecordCacheStore` (written into each row's `expires_at`); `MemoryCacheStore` evicts by size instead and ignores it. A non-positive value (`0` or less, or `nil`) means never expires. A String is coerced, so an environment variable works; a value that is not a number is refused at `configure` time rather than mid-translation. See [SQL cache](sql-cache.md#cache_ttl-becomes-expires_at). | -| `cache_namespace` | `"translation-diff"` | Prefix applied to every Redis key this gem writes -- both cache entries and the rate limiter's own bookkeeping. Also the `namespace` column both SQL tables share and the unit `ActiveRecordCacheStore#prune` operates on. At most 64 characters -- longer is refused at `configure` time. See [SQL cache](sql-cache.md#the-tables). | -| `cache_max_size` | `1_000` | Maximum number of entries `MemoryCacheStore` keeps before evicting the least recently used one. | -| `cache_table_name` | `"translation_diff_translations"` | Table `ActiveRecordCacheStore` reads and writes. For a host with its own table-naming convention. See [SQL cache](sql-cache.md). | -| `rate_limit_table_name` | `"translation_diff_rate_limits"` | Table `ActiveRecordRateLimiter` reads and writes. As above. | -| `active_record_base` | `nil` (`::ActiveRecord::Base`) | The class `ActiveRecordCacheStore` and `ActiveRecordRateLimiter` build their model from -- point this at a second database. It does not exempt this store from a Rails application's own read-replica routing; see [Rails replica routing](sql-cache.md#rails-replica-routing). See [SQL cache](sql-cache.md#active_record_base-a-second-database). | -| `cache_prune_probability` | `0.0` | Chance, per write, that `ActiveRecordCacheStore` prunes expired rows before returning. `0.0` is off, and a value outside `0.0..1.0` is refused at `configure` time; `rake translation_diff:prune` is the other way to prune. See [SQL cache](sql-cache.md#pruning-three-answers-none-imposed). | +| `cache_ttl` | `604_800` (one week) | Seconds an entry is kept before it expires. Read by `Stores::Redis` (a `SETEX`) and by `Stores::ActiveRecord` (written into each row's `expires_at`); `Stores::Memory` evicts by size instead and ignores it. A non-positive value (`0` or less, or `nil`) means never expires. A String is coerced, so an environment variable works; a value that is not a number is refused at `configure` time rather than mid-translation. See [SQL cache](sql-cache.md#cache_ttl-becomes-expires_at). | +| `cache_namespace` | `"translation-diff"` | Prefix applied to every Redis key this gem writes -- both cache entries and the rate limiter's own bookkeeping. Also the `namespace` column both SQL tables share and the unit `Stores::ActiveRecord#prune` operates on. At most 64 characters -- longer is refused at `configure` time. See [SQL cache](sql-cache.md#the-tables). | +| `cache_max_size` | `1_000` | Maximum number of entries `Stores::Memory` keeps before evicting the least recently used one. | +| `cache_table_name` | `"translation_diff_translations"` | Table `Stores::ActiveRecord` reads and writes. For a host with its own table-naming convention. See [SQL cache](sql-cache.md). | +| `rate_limit_table_name` | `"translation_diff_rate_limits"` | Table `RateLimiters::ActiveRecord` reads and writes. As above. | +| `active_record_base` | `nil` (`::ActiveRecord::Base`) | The class `Stores::ActiveRecord` and `RateLimiters::ActiveRecord` build their model from -- point this at a second database. It does not exempt this store from a Rails application's own read-replica routing; see [Rails replica routing](sql-cache.md#rails-replica-routing). See [SQL cache](sql-cache.md#active_record_base-a-second-database). | +| `cache_prune_probability` | `0.0` | Chance, per write, that `Stores::ActiveRecord` prunes expired rows before returning. `0.0` is off, and a value outside `0.0..1.0` is refused at `configure` time; `rake translation_diff:prune` is the other way to prune. See [SQL cache](sql-cache.md#pruning-three-answers-none-imposed). | | `redis_url` | `ENV["REDIS_URL"]` | Where to connect for the Redis-backed cache store and rate limiter. Setting this is what makes `cache` default to `:redis` instead of `:memory`. | | `redis_pool_size` | `5` | Size of the connection pool built from `redis_url`. | | `redis_pool_timeout` | `5` | Seconds to wait for a connection from that pool before raising. | @@ -148,7 +148,7 @@ makes a per-request `TranslationDiff.configure { |c| c.cache_namespace = current_tenant }` safe: writing the same tenant on every request no longer rebuilds the cache store on every request. Writing a genuinely *different* value still rebuilds the store exactly as before, though, and if that store -is the default `MemoryCacheStore`, a rebuilt store is a fresh, empty Hash -- +is the default `Stores::Memory`, a rebuilt store is a fresh, empty Hash -- its contents are gone, and whatever it held has to be paid for again at the provider. @@ -159,8 +159,8 @@ from has already built. ## Choosing the cache store -`cache` unset means "choose for me": `RedisCacheStore` when `redis_url` is -configured, `MemoryCacheStore` otherwise, so the library works before any +`cache` unset means "choose for me": `Stores::Redis` when `redis_url` is +configured, `Stores::Memory` otherwise, so the library works before any infrastructure does. Set `cache` explicitly (`:redis`, `:memory`, or your own object) to override that choice. @@ -190,7 +190,7 @@ option *values* over, but deliberately not the collaborators already built from them -- each context resolves its own provider, cache store, segmenter and rate limiter from its own values, independently of whatever the configuration it was copied from had already built. When `cache` is left -unset, that resolves to `MemoryCacheStore`, an in-process store, so a freshly +unset, that resolves to `Stores::Memory`, an in-process store, so a freshly built context's store starts empty every time -- a short-lived, per-request context therefore caches nothing across requests. Configure `redis_url` (or assign one shared cache object explicitly) if contexts need to share a diff --git a/docs/contracts.md b/docs/contracts.md index 55ca286..84ddf34 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -12,7 +12,7 @@ through its own registry, `TranslationDiff::RateLimiters` -- `:redis` and and `Dispatcher#throttle` checks for that `nil` and skips rate limiting entirely, so the common case costs nothing; - otherwise the registered limiter named by `config.rate_limiter`, or - `TranslationDiff::RedisRateLimiter` when `rate_limiter` is left unset but + `TranslationDiff::RateLimiters::Redis` when `rate_limiter` is left unset but `rate_limit` is set -- built from `rate_interval`, `cache_namespace`, and either `redis_url` (`:redis`) or `active_record_base` and `rate_limit_table_name` (`:active_record`; see [SQL cache](sql-cache.md)). @@ -29,10 +29,10 @@ An object assigned to `rate_limiter` must implement: def check(size); end ``` -`TranslationDiff::RedisRateLimiter` raises -`TranslationDiff::RedisRateLimiter::RateLimitExceeded` when its threshold is +`TranslationDiff::RateLimiters::Redis` raises +`TranslationDiff::RateLimiters::Redis::RateLimitExceeded` when its threshold is exceeded within its interval; -`TranslationDiff::ActiveRecordRateLimiter` raises its own +`TranslationDiff::RateLimiters::ActiveRecord` raises its own `RateLimitExceeded`, a distinct class under the same name. Both raise with a message naming the namespace, the threshold and the interval that were hit (`"rate limit reached for translation-diff: 8000 characters per 60 @@ -44,7 +44,7 @@ configures no `rate_limit` never needs it, and its absence raises dependency either -- see [SQL cache](sql-cache.md#the-activerecord-version-floor). **Upgrading to 3.1.0: re-validate your `rate_limit` threshold.** Before this -release, `RedisRateLimiter` never actually limited anything -- a signature +release, `RateLimiters::Redis` never actually limited anything -- a signature mismatch with the `ratelimit` gem meant it recorded hits under a subject `exceeded?` never read, so the threshold could never be reached. That bug shipped in every release since `v1.0.2` (2023-02-16). If you have @@ -64,8 +64,8 @@ Keep `rate_interval` within 5-600 seconds if you want the configured number to be the enforced one. Both the clamp above and the upgrade note before it are about -`RedisRateLimiter`, which delegates its bucketing to the `ratelimit` gem. -`ActiveRecordRateLimiter` owns its own bucketing instead, and its window is +`RateLimiters::Redis`, which delegates its bucketing to the `ratelimit` gem. +`RateLimiters::ActiveRecord` owns its own bucketing instead, and its window is sliding rather than tumbling: buckets are `rate_interval / 12` seconds wide (floored at 1 second), and a check sums every bucket touching the trailing `rate_interval` seconds -- including the oldest one, which is only ever diff --git a/docs/errors.md b/docs/errors.md index c03fa0c..b8bf111 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -41,14 +41,14 @@ TranslationDiff::Error │ # Pragmatic computed offsets that │ # violate its own postcondition -- │ # not raised by ordinary use -├── TranslationDiff::RedisRateLimiter::RateLimitExceeded +├── TranslationDiff::RateLimiters::Redis::RateLimitExceeded │ # the configured rate_limit was exceeded, │ # raised by the Redis-backed limiter -└── TranslationDiff::ActiveRecordRateLimiter::RateLimitExceeded +└── TranslationDiff::RateLimiters::ActiveRecord::RateLimitExceeded # the same condition, raised by the SQL-backed # limiter -- a distinct class under its own # namespace, not the class above. Rescuing - # `RedisRateLimiter::RateLimitExceeded` + # `RateLimiters::Redis::RateLimitExceeded` # specifically and switching `rate_limiter` to # `:active_record` stops catching it; rescue # `TranslationDiff::Error` to catch both. diff --git a/docs/instrumentation.md b/docs/instrumentation.md index 88ed498..949122e 100644 --- a/docs/instrumentation.md +++ b/docs/instrumentation.md @@ -41,9 +41,9 @@ caller -- see `error` is the exception's class name, never its message, which could echo the row it failed to write. Which class you see depends on the store: a store that redacts its own failures reports that redaction, so -`ActiveRecordCacheStore` always gives `"TranslationDiff::Error"` -- the +`Stores::ActiveRecord` always gives `"TranslationDiff::Error"` -- the adapter's own class is named inside that error's (content-free) message, -not in this payload. `RedisCacheStore` does not wrap, so it gives the +not in this payload. `Stores::Redis` does not wrap, so it gives the driver's class, `"Redis::CannotConnectError"` and the like. Alert on the event, not on a particular class name. diff --git a/docs/sql-cache.md b/docs/sql-cache.md index 50e7ffb..bde5beb 100644 --- a/docs/sql-cache.md +++ b/docs/sql-cache.md @@ -3,9 +3,9 @@ ## What it's for If you already run Postgres or MySQL and do not want to stand up Redis for -one cache, `TranslationDiff::ActiveRecordCacheStore` caches translations in +one cache, `TranslationDiff::Stores::ActiveRecord` caches translations in the application's own database instead, and -`TranslationDiff::ActiveRecordRateLimiter` throttles requests there too. +`TranslationDiff::RateLimiters::ActiveRecord` throttles requests there too. Supported means exercised in CI: the suite runs against Postgres, MySQL and SQLite on every push. @@ -58,7 +58,7 @@ exception cannot carry the row into an error tracker either, see [`write_multi`](#write_multi) -- but a successful one is not; nothing here redacts your debug-level query log. If your application logs SQL at `debug` and what it translates is confidential, keep that log above `debug` around this store, or use -`RedisCacheStore` instead. +`Stores::Redis` instead. That scrubbing covers every `ActiveRecord::ActiveRecordError` the write path can raise, not just a syntax or constraint failure -- see @@ -171,7 +171,7 @@ Postgres and SQLite users have nothing to do here. ## `cache_ttl` becomes `expires_at` -`cache_ttl` (in seconds, same option `RedisCacheStore` reads) is written +`cache_ttl` (in seconds, same option `Stores::Redis` reads) is written into each row's `expires_at` at write time. A row past `expires_at` is never read, whether or not anything has deleted it yet -- expiry and deletion are two different questions here, unlike Redis, where a `SETEX` @@ -211,7 +211,7 @@ and there is no single right answer to "when," so none is forced on you: silently against the wrong (or unconfigured) configuration is worse than no pruning at all. - **`config.cache_prune_probability`** (default `0.0`, off). A fraction - between 0 and 1: on a write, `ActiveRecordCacheStore` rolls under it and + between 0 and 1: on a write, `Stores::ActiveRecord` rolls under it and prunes if it wins, in a savepoint of its own so a failed prune cannot abort a transaction the caller opened. A value outside `0.0..1.0`, or one that is not a number, is refused at `configure` time. Off by default, @@ -232,7 +232,7 @@ namespace if every tenant is to be pruned. ## `active_record_base`: a second database `config.active_record_base` (default `::ActiveRecord::Base`) is the class -`ActiveRecordCacheStore` and `ActiveRecordRateLimiter` build their model +`Stores::ActiveRecord` and `RateLimiters::ActiveRecord` build their model from. Point it at a class connected to a second database and this store's traffic follows that connection instead of your application's primary one: @@ -281,7 +281,7 @@ store, for that request. **The rate limiter fails differently, because it runs earlier.** If `config.rate_limiter = :active_record` and the same request hits it, -`ActiveRecordRateLimiter#check` cannot record what it is about to allow, so +`RateLimiters::ActiveRecord#check` cannot record what it is about to allow, so it raises `TranslationDiff::Error` naming the adapter's error class -- and because the check runs before the provider is ever called, the `translate` call fails outright rather than degrading. Nothing has been paid for at @@ -315,22 +315,22 @@ upsert_all takes unique_by and record_timestamps there. ``` `activerecord` is never a dependency of this gem -- neither in the gemspec -nor required at load time. `ActiveRecordCacheStore#model` and -`ActiveRecordRateLimiter#model` `require "active_record"` on first use, so +nor required at load time. `Stores::ActiveRecord#model` and +`RateLimiters::ActiveRecord#model` `require "active_record"` on first use, so an application that never configures `:active_record` never loads it, the -same way `RedisCacheStore` only reaches for `redis` when `redis_url` is +same way `Stores::Redis` only reaches for `redis` when `redis_url` is set. Add `gem "activerecord"` (and a database adapter) to your own Gemfile to use either. ## `write_multi` -Both `ActiveRecordCacheStore` and `RedisCacheStore` implement the cache +Both `Stores::ActiveRecord` and `Stores::Redis` implement the cache store contract's optional `write_multi(pairs)` -- see [`write_multi` is optional](caching.md#write_multi-is-optional) for what that means, and [The three write paths fail differently](caching.md#the-three-write-paths-fail-differently) for how a batch write fails differently from a per-key one. -`ActiveRecordCacheStore#write_multi` is a single `upsert_all` for the whole +`Stores::ActiveRecord#write_multi` is a single `upsert_all` for the whole batch: a forty-sentence paragraph is one statement, not forty. ## The rate limiter From 6306ee50cff67cf91624c3728473120191888ce0 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:20:59 +0400 Subject: [PATCH 6/7] refactor: keep the whole error tree in one file --- lib/translation_diff.rb | 1 - lib/translation_diff/error.rb | 2 -- lib/translation_diff/errors.rb | 3 +++ 3 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 lib/translation_diff/error.rb diff --git a/lib/translation_diff.rb b/lib/translation_diff.rb index 60a6d1f..3a76b8c 100644 --- a/lib/translation_diff.rb +++ b/lib/translation_diff.rb @@ -8,7 +8,6 @@ require "ox" require "translation_diff/version" -require "translation_diff/error" require "translation_diff/errors" require "translation_diff/redaction" require "translation_diff/capabilities" diff --git a/lib/translation_diff/error.rb b/lib/translation_diff/error.rb deleted file mode 100644 index f733c1e..0000000 --- a/lib/translation_diff/error.rb +++ /dev/null @@ -1,2 +0,0 @@ -# Common ancestor for every error this gem raises, so `rescue TranslationDiff::Error` is enough. -class TranslationDiff::Error < StandardError; end diff --git a/lib/translation_diff/errors.rb b/lib/translation_diff/errors.rb index 12854d1..9bddc8b 100644 --- a/lib/translation_diff/errors.rb +++ b/lib/translation_diff/errors.rb @@ -1,5 +1,8 @@ # No error carries the text being translated -- errors are logged, and this library handles other people's content. module TranslationDiff + # Common ancestor for every error this gem raises, so `rescue TranslationDiff::Error` is enough. + class Error < StandardError; end + class ConfigurationError < Error; end # Raised before any request: the pair is checked against data captured from the vendor, not by asking it. From b7e7abd874f564e7b065ee3de87ca79fe79e20d2 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 08:28:44 +0400 Subject: [PATCH 7/7] refactor: one RateLimitExceeded, whichever limiter noticed --- CHANGELOG.md | 10 ++++++++-- docs/contracts.md | 9 ++++----- docs/errors.md | 18 ++++++------------ lib/translation_diff/errors.rb | 3 +++ .../rate_limiters/active_record.rb | 4 +--- lib/translation_diff/rate_limiters/redis.rb | 4 +--- .../rate_limiters/active_record_test.rb | 2 +- .../rate_limiters/redis_test.rb | 6 +++--- 8 files changed, 27 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a00521..8c42e52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Breaking +- **One `TranslationDiff::RateLimitExceeded`, whichever limiter noticed it.** + The two limiters used to raise two same-named classes under their own + namespaces, so an application that rescued one and then switched + `rate_limiter` from `:redis` to `:active_record` quietly stopped catching + it. See [Errors](docs/errors.md). + - **Language validation is on by default.** `TranslationDiff.translate` now refuses, before making a request, any source/target pair the shipped data doesn't list for that provider -- raising @@ -191,8 +197,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `rate_limit` now falls back to the limiter's own default -- 8,000 characters per `rate_interval`, the same for both shipped limiters. See [Configuration options](docs/configuration.md#configuration-options). -- **A refused request now says what it hit.** Both `RateLimitExceeded` - classes raise with a message naming the namespace, the threshold and the +- **A refused request now says what it hit.** `RateLimitExceeded` + carries a message naming the namespace, the threshold and the interval (`"rate limit reached for translation-diff: 8000 characters per 60 seconds"`) -- never the content that tripped it. See [Errors](docs/errors.md). diff --git a/docs/contracts.md b/docs/contracts.md index 84ddf34..425a142 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -29,11 +29,10 @@ An object assigned to `rate_limiter` must implement: def check(size); end ``` -`TranslationDiff::RateLimiters::Redis` raises -`TranslationDiff::RateLimiters::Redis::RateLimitExceeded` when its threshold is -exceeded within its interval; -`TranslationDiff::RateLimiters::ActiveRecord` raises its own -`RateLimitExceeded`, a distinct class under the same name. Both raise with a +Both shipped limiters raise `TranslationDiff::RateLimitExceeded` when the +threshold is exceeded within the interval -- one class whichever limiter is +configured, so switching from `:redis` to `:active_record` does not quietly +stop a `rescue` from matching. They raise with a message naming the namespace, the threshold and the interval that were hit (`"rate limit reached for translation-diff: 8000 characters per 60 seconds"`) -- never the text that tripped it. Neither `redis` diff --git a/docs/errors.md b/docs/errors.md index b8bf111..8229026 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -41,20 +41,14 @@ TranslationDiff::Error │ # Pragmatic computed offsets that │ # violate its own postcondition -- │ # not raised by ordinary use -├── TranslationDiff::RateLimiters::Redis::RateLimitExceeded -│ # the configured rate_limit was exceeded, -│ # raised by the Redis-backed limiter -└── TranslationDiff::RateLimiters::ActiveRecord::RateLimitExceeded - # the same condition, raised by the SQL-backed - # limiter -- a distinct class under its own - # namespace, not the class above. Rescuing - # `RateLimiters::Redis::RateLimitExceeded` - # specifically and switching `rate_limiter` to - # `:active_record` stops catching it; rescue - # `TranslationDiff::Error` to catch both. +└── TranslationDiff::RateLimitExceeded # the configured rate_limit was exceeded -- + # one class whichever limiter noticed, so + # switching `rate_limiter` between `:redis` + # and `:active_record` cannot quietly stop a + # rescue from matching ``` -Both `RateLimitExceeded` classes raise with a message naming the namespace, +`RateLimitExceeded` carries a message naming the namespace, the threshold and the interval that were exceeded (`"rate limit reached for translation-diff: 8000 characters per 60 seconds"`) -- never the text that tripped it. diff --git a/lib/translation_diff/errors.rb b/lib/translation_diff/errors.rb index 9bddc8b..7ff4b40 100644 --- a/lib/translation_diff/errors.rb +++ b/lib/translation_diff/errors.rb @@ -5,6 +5,9 @@ class Error < StandardError; end class ConfigurationError < Error; end + # Raised by whichever limiter is configured, so a caller rescues one class rather than the one it happens to use. + class RateLimitExceeded < Error; end + # Raised before any request: the pair is checked against data captured from the vendor, not by asking it. class UnsupportedLanguageError < Error; end diff --git a/lib/translation_diff/rate_limiters/active_record.rb b/lib/translation_diff/rate_limiters/active_record.rb index e54f39c..f3b52cf 100644 --- a/lib/translation_diff/rate_limiters/active_record.rb +++ b/lib/translation_diff/rate_limiters/active_record.rb @@ -2,8 +2,6 @@ class TranslationDiff::RateLimiters::ActiveRecord include TranslationDiff::ActiveRecord::Support - class RateLimitExceeded < TranslationDiff::Error; end - DEFAULT_THRESHOLD = 8000 DEFAULT_INTERVAL = 60 @@ -32,7 +30,7 @@ def initialize(namespace:, table_name:, threshold: DEFAULT_THRESHOLD, interval: # A sliding window: every bucket covering the last `interval` seconds is summed, not just the current one. def check(size) - raise RateLimitExceeded, exceeded_message if current_total >= @threshold + raise TranslationDiff::RateLimitExceeded, exceeded_message if current_total >= @threshold add(size) rescue StandardError => e diff --git a/lib/translation_diff/rate_limiters/redis.rb b/lib/translation_diff/rate_limiters/redis.rb index c843eb4..9cd540a 100644 --- a/lib/translation_diff/rate_limiters/redis.rb +++ b/lib/translation_diff/rate_limiters/redis.rb @@ -1,6 +1,4 @@ class TranslationDiff::RateLimiters::Redis - class RateLimitExceeded < TranslationDiff::Error; end - DEFAULT_THRESHOLD = 8000 DEFAULT_INTERVAL = 60 DEFAULT_NAMESPACE = "translation-diff".freeze @@ -32,7 +30,7 @@ def check(size) connection_pool.with do |redis| rate_limit = limiter_class.new(namespace, redis: redis) exceeded = rate_limit.exceeded?(SUBJECT, threshold: threshold, interval: interval) - raise RateLimitExceeded, exceeded_message if exceeded + raise TranslationDiff::RateLimitExceeded, exceeded_message if exceeded rate_limit.add(SUBJECT, size) end diff --git a/test/translation_diff/rate_limiters/active_record_test.rb b/test/translation_diff/rate_limiters/active_record_test.rb index 987a61b..b953dbe 100644 --- a/test/translation_diff/rate_limiters/active_record_test.rb +++ b/test/translation_diff/rate_limiters/active_record_test.rb @@ -215,7 +215,7 @@ def test_add_quotes_the_table_name_in_the_on_duplicate_fragment private - def rate_limit_exceeded_error = TranslationDiff::RateLimiters::ActiveRecord::RateLimitExceeded + def rate_limit_exceeded_error = TranslationDiff::RateLimitExceeded # Held still, so a five-second bucket boundary cannot fall between two reads of the clock. def frozen_clock = MutableClock.new(Time.at(1_700_000_000)) diff --git a/test/translation_diff/rate_limiters/redis_test.rb b/test/translation_diff/rate_limiters/redis_test.rb index 5c86307..0cb946e 100644 --- a/test/translation_diff/rate_limiters/redis_test.rb +++ b/test/translation_diff/rate_limiters/redis_test.rb @@ -86,7 +86,7 @@ def test_check_raises_once_the_threshold_is_passed limiter(server, threshold: 100).check(100) - assert_raises(TranslationDiff::RateLimiters::Redis::RateLimitExceeded) do + assert_raises(TranslationDiff::RateLimitExceeded) do limiter(server, threshold: 100).check(1) end assert_equal({ "ratelimit:translation-diff:call" => 100 }, server.totals) @@ -97,7 +97,7 @@ def test_check_uses_the_default_threshold limiter(server).check(TranslationDiff::RateLimiters::Redis::DEFAULT_THRESHOLD) - assert_raises(TranslationDiff::RateLimiters::Redis::RateLimitExceeded) { limiter(server).check(1) } + assert_raises(TranslationDiff::RateLimitExceeded) { limiter(server).check(1) } end # Ratelimit buckets five seconds at a time, so buckets swept is the interval divided by five. @@ -170,7 +170,7 @@ def limiter(server, **) TranslationDiff::RateLimiters::Redis.new(FakeConnectionPool.new(server), **) end - def rate_limit_exceeded_error = TranslationDiff::RateLimiters::Redis::RateLimitExceeded + def rate_limit_exceeded_error = TranslationDiff::RateLimitExceeded def build_limiter(threshold:, interval:) @contract_server ||= FakeRedisServer.new