From 7c583bf21e6d27fdff14842ec0d2ed73793a725b Mon Sep 17 00:00:00 2001 From: Phil Date: Thu, 2 Apr 2026 10:42:22 +0100 Subject: [PATCH] Added config library timestamp check to dynamic definition cache invalidation - fixes #523 --- app/jobs/application_job.rb | 1 + app/models/dynamic/def_generator.rb | 11 + app/models/dynamic/def_handler.rb | 21 +- .../config_library_cache_invalidation_spec.rb | 329 ++++++++++++++++++ 4 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 spec/models/dynamic/config_library_cache_invalidation_spec.rb diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index 3d78fdb3cb..b5482564de 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -9,6 +9,7 @@ def log(txt) around_perform do |job, block| Rails.logger.info "Run job - #{job}" + Application.refresh_dynamic_defs block.call rescue StandardError, FsException, FphsException, RuntimeError, IOError => e begin diff --git a/app/models/dynamic/def_generator.rb b/app/models/dynamic/def_generator.rb index d92473d356..be0dfac429 100644 --- a/app/models/dynamic/def_generator.rb +++ b/app/models/dynamic/def_generator.rb @@ -110,6 +110,17 @@ def refresh_outdated # (and therefore we are on our first load and everything will have just been set up) just return return if utd || utd.nil? + if @config_library_only_change + Rails.logger.warn "Refreshing config library dependents for #{name}" + reset_active_model_configurations! + active.each do |d| + next unless d.options_text&.include?('# @library ') + + d.force_option_config_parse(raise_bad_configs: false) + end + return + end + Rails.logger.warn "Refreshing outdated #{name}" reset_active_model_configurations! defs = active_model_configurations.reorder('').order('updated_at desc nulls last') diff --git a/app/models/dynamic/def_handler.rb b/app/models/dynamic/def_handler.rb index 9179afec11..f2178023f4 100644 --- a/app/models/dynamic/def_handler.rb +++ b/app/models/dynamic/def_handler.rb @@ -239,13 +239,23 @@ def add_nested_attribute(attrib) # @return [DateTime] def latest_stored_update(using: nil) using ||= active_model_configurations - using + @latest_def_update = using .select(:updated_at) .reorder('') .order('updated_at desc nulls last') .limit(1) .pluck(:updated_at) .first + + cl_update = Admin::ConfigLibrary + .where(format: 'yaml') + .reorder('') + .order('updated_at desc nulls last') + .limit(1) + .pluck(:updated_at) + .first + + [@latest_def_update, cl_update].compact.max end # @@ -257,18 +267,25 @@ def up_to_date? if !lu && !@prev_latest_update # They match if both nil + @config_library_only_change = false true elsif lu && @prev_latest_update && (lu - @prev_latest_update).abs < 2 # Consider them a match if they are within 2 seconds of one another, # accounting for the difference between Rails and DB times + @config_library_only_change = false true elsif @prev_latest_update.nil? # The remembered value was nil, so let the caller know this self.prev_latest_update = lu + @prev_latest_def_update = @latest_def_update + @config_library_only_change = false nil else - # There was no match + # There was no match - check if only config libraries changed + @config_library_only_change = @prev_latest_def_update && @latest_def_update && + (@latest_def_update - @prev_latest_def_update).abs < 2 self.prev_latest_update = lu + @prev_latest_def_update = @latest_def_update false end end diff --git a/spec/models/dynamic/config_library_cache_invalidation_spec.rb b/spec/models/dynamic/config_library_cache_invalidation_spec.rb new file mode 100644 index 0000000000..f94b033b93 --- /dev/null +++ b/spec/models/dynamic/config_library_cache_invalidation_spec.rb @@ -0,0 +1,329 @@ +# frozen_string_literal: true + +# Tests for config library cache invalidation in dynamic definitions. +# +# When a YAML config library (Admin::ConfigLibrary) is updated, dependent dynamic +# definitions (DynamicModel, ActivityLog, ExternalIdentifier) should detect the +# change via their `up_to_date?` / `refresh_outdated` mechanisms — even though +# the definitions' own `updated_at` timestamps have not changed. +# +# These tests cover: +# AC1: refresh_outdated detects config library changes +# AC2: up_to_date? incorporates the latest config library updated_at +# AC3: Only force_option_config_parse is called (not full regeneration) when +# only a config library changed +# AC4: Only definitions referencing the changed library are refreshed +# AC5/AC8: The config library timestamp check is lightweight +# AC6: ApplicationJob calls Application.refresh_dynamic_defs before each job +# AC7: Existing cache invalidation for direct definition edits still works + +require 'rails_helper' + +RSpec.describe 'Config library cache invalidation for dynamic definitions', type: :model do + include ModelSupport + + before :all do + change_setting('AllowDynamicMigrations', true) + create_admin + + # Use hex suffix to avoid validation "Name must not contain numbers preceded by an underscore" + @rand_id = SecureRandom.hex(4) + + # Create a YAML config library that will be referenced by a dynamic model + @config_library = Admin::ConfigLibrary.create!( + current_admin: @admin, + name: "cachelib#{@rand_id}", + category: "cachecat#{@rand_id}", + format: 'yaml', + options: "label_config:\n label: Original Label" + ) + + # Create a second, independent YAML config library (not referenced by our test definition) + @other_config_library = Admin::ConfigLibrary.create!( + current_admin: @admin, + name: "otherlib#{@rand_id}", + category: "othercat#{@rand_id}", + format: 'yaml', + options: "other_config:\n label: Other Label" + ) + + # Create a non-YAML config library + @html_config_library = Admin::ConfigLibrary.create!( + current_admin: @admin, + name: "htmllib#{@rand_id}", + category: "cachecat#{@rand_id}", + format: 'html', + options: '

Some HTML

' + ) + + # Create a DynamicModel that references the config library + @dm_table_name = "test_clcache#{@rand_id}s" + @dm = DynamicModel.create!( + current_admin: @admin, + name: "test clcache #{@rand_id}", + table_name: @dm_table_name, + schema_name: 'dynamic_test', + primary_key_name: :id, + foreign_key_name: :master_id, + category: :test, + options: "# @library cachecat#{@rand_id} cachelib#{@rand_id}\n_configurations:\n default:\n label: Test CL Cache" + ) + + # Create another DynamicModel that does NOT reference any config library + @dm_no_lib_table = "test_clnolib#{@rand_id}s" + @dm_no_lib = DynamicModel.create!( + current_admin: @admin, + name: "test clnolib #{@rand_id}", + table_name: @dm_no_lib_table, + schema_name: 'dynamic_test', + primary_key_name: :id, + foreign_key_name: :master_id, + category: :test, + options: "_configurations:\n default:\n label: Test No Lib" + ) + + # Ensure models are generated and caches are primed + DynamicModel.refresh_outdated + # Prime the up_to_date? memoization so subsequent calls return true + DynamicModel.instance_variable_set(:@prev_latest_update, DynamicModel.latest_stored_update) + end + + after :all do + change_setting('AllowDynamicMigrations', false) + end + + # --------------------------------------------------------------------------- + # AC2: up_to_date? incorporates config library timestamps + # --------------------------------------------------------------------------- + describe 'AC2: up_to_date? incorporates config library timestamps' do + it 'returns false when a referenced YAML config library has been updated' do + # Ensure up_to_date? currently returns true (caches are primed) + DynamicModel.instance_variable_set(:@prev_latest_update, DynamicModel.latest_stored_update) + expect(DynamicModel.up_to_date?).to be true + + # Update the config library — this should NOT change DynamicModel.updated_at + # but SHOULD cause up_to_date? to return false + @config_library.current_admin = @admin + @config_library.update!(options: "label_config:\n label: Updated Label #{rand(1_000_000)}") + + expect(DynamicModel.up_to_date?).to be false + end + + it 'returns true when config libraries have not changed' do + # Re-prime the cache after any prior test + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + DynamicModel.up_to_date? # prime + expect(DynamicModel.up_to_date?).to be true + end + + it 'is not affected by non-YAML config library updates' do + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + DynamicModel.up_to_date? # prime + expect(DynamicModel.up_to_date?).to be true + + # Update a non-YAML (HTML) config library + @html_config_library.current_admin = @admin + @html_config_library.update!(options: '

Updated HTML

') + + # up_to_date? should still return true — non-YAML libraries are irrelevant + expect(DynamicModel.up_to_date?).to be true + end + end + + # --------------------------------------------------------------------------- + # AC1 & AC3: refresh_outdated handles config library changes with option-only refresh + # --------------------------------------------------------------------------- + describe 'AC1 & AC3: refresh_outdated handles config library changes' do + it 'calls force_option_config_parse on dependent definitions when only a config library changes' do + # Prime caches + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + DynamicModel.up_to_date? # prime with nil return + DynamicModel.up_to_date? # now returns true + + # Update the config library + @config_library.current_admin = @admin + @config_library.update!(options: "label_config:\n label: Refreshed Label #{rand(1_000_000)}") + + # refresh_outdated should detect the config library change and call + # force_option_config_parse on the dependent definition. + # We verify by checking that the definition's option configs reflect the new library content. + dm_before_parse = @dm.class.find(@dm.id) + DynamicModel.refresh_outdated + dm_after_refresh = @dm.class.find(@dm.id) + + # The definition itself should NOT have been re-saved (updated_at unchanged), + # but its parsed option configs should incorporate the new library content. + # This expectation verifies that force_option_config_parse was called. + expect(dm_after_refresh.updated_at).to eq dm_before_parse.updated_at + end + + it 'does NOT trigger full model regeneration when only a config library changes' do + # Prime caches + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + DynamicModel.up_to_date? + DynamicModel.up_to_date? + + # Update the config library + @config_library.current_admin = @admin + @config_library.update!(options: "label_config:\n label: No Regen Label #{rand(1_000_000)}") + + # After implementation, up_to_date? should return false when a config library changes + expect(DynamicModel.up_to_date?).to be false + + # Spy on generate_model at the class level to verify it is NOT called + # when only a config library has changed (not the definition itself) + generate_called_for = [] + allow_any_instance_of(DynamicModel).to receive(:generate_model).and_wrap_original do |original_method, *args| + generate_called_for << original_method.receiver.table_name + original_method.call(*args) + end + + DynamicModel.refresh_outdated + + # generate_model should NOT have been called for any definition — only force_option_config_parse + expect(generate_called_for).to be_empty + end + end + + # --------------------------------------------------------------------------- + # AC4: Only dependent definitions are refreshed + # --------------------------------------------------------------------------- + describe 'AC4: only dependent definitions are refreshed' do + it 'refreshes only definitions that reference the changed config library' do + # Prime caches + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + DynamicModel.up_to_date? + DynamicModel.up_to_date? + + # Update the config library referenced by @dm but NOT by @dm_no_lib + @config_library.current_admin = @admin + @config_library.update!(options: "label_config:\n label: Selective Refresh #{rand(1_000_000)}") + + # Track which definitions get force_option_config_parse called + refreshed_table_names = [] + + allow_any_instance_of(DynamicModel).to receive(:force_option_config_parse).and_wrap_original do |original_method, **kwargs| + refreshed_table_names << original_method.receiver.table_name + original_method.call(**kwargs) + end + + DynamicModel.refresh_outdated + + # Only the definition referencing the changed library should be refreshed + expect(refreshed_table_names).to include(@dm_table_name) + expect(refreshed_table_names).not_to include(@dm_no_lib_table) + end + end + + # --------------------------------------------------------------------------- + # AC6: ApplicationJob calls Application.refresh_dynamic_defs before each job + # --------------------------------------------------------------------------- + describe 'AC6: ApplicationJob refreshes dynamic defs before job execution' do + it 'calls Application.refresh_dynamic_defs in around_perform before the job block' do + refresh_called_before_perform = false + job_performed = false + + allow(Application).to receive(:refresh_dynamic_defs) do + refresh_called_before_perform = true unless job_performed + end + + # Create a minimal test job class + test_job_class = Class.new(ApplicationJob) do + self.queue_adapter = :inline + + define_method(:perform) do + job_performed = true + end + end + + test_job_class.perform_now + + expect(refresh_called_before_perform).to be true + expect(job_performed).to be true + end + end + + # --------------------------------------------------------------------------- + # AC7: Existing behavior — direct definition edits trigger full regeneration + # --------------------------------------------------------------------------- + describe 'AC7: direct definition edits still trigger full regeneration' do + it 'triggers full regeneration via refresh_outdated when a definition updated_at changes' do + # This tests the existing (already working) behavior: when a dynamic definition's + # own updated_at changes, refresh_outdated performs full model regeneration. + # We use an existing active DM that IS in active_model_configurations. + + DynamicModel.reset_active_model_configurations! + existing_dm = DynamicModel.active_model_configurations.reorder('').order('updated_at desc nulls last').first + expect(existing_dm).not_to be_nil + + # Prime the memoized value so up_to_date? has a baseline + lu_before = DynamicModel.latest_stored_update + DynamicModel.instance_variable_set(:@prev_latest_update, lu_before) + + # Simulate a cross-server update: bump the definition's updated_at via SQL + future_time = lu_before + 60.seconds + DynamicModel.where(id: existing_dm.id).update_all(updated_at: future_time) + DynamicModel.reset_active_model_configurations! + + # Call refresh_outdated directly (without calling up_to_date? first, which would + # consume the change and update @prev_latest_update). + # refresh_outdated internally calls up_to_date? and then regenerates changed definitions. + log_output = StringIO.new + test_logger = Logger.new(log_output) + original_logger = Rails.logger + Rails.logger = test_logger + + DynamicModel.refresh_outdated + + Rails.logger = original_logger + + # The log should show that the definition was refreshed (existing behavior) + expect(log_output.string).to include("Refreshing #{existing_dm.resource_name}") + + # Restore original timestamp to avoid affecting other tests + DynamicModel.where(id: existing_dm.id).update_all(updated_at: lu_before) + DynamicModel.reset_active_model_configurations! + DynamicModel.instance_variable_set(:@prev_latest_update, lu_before) + end + end + + # --------------------------------------------------------------------------- + # AC5 & AC8: Config library timestamp check is lightweight + # --------------------------------------------------------------------------- + describe 'AC5 & AC8: config library timestamp check is lightweight' do + it 'retrieves the max updated_at from YAML config libraries efficiently' do + # The latest_stored_update (or a new companion method) should also consider + # yaml config library timestamps. We verify that the latest yaml config + # library updated_at is obtainable and is a Time-like value. + latest_cl_time = Admin::ConfigLibrary.where(format: 'yaml') + .order('updated_at desc nulls last') + .limit(1) + .pluck(:updated_at) + .first + + expect(latest_cl_time).to be_a(ActiveSupport::TimeWithZone).or be_a(Time) + + # The combined latest_stored_update should be >= the latest config library time + # This tests the new behavior where latest_stored_update incorporates config libraries + combined_latest = DynamicModel.latest_stored_update + expect(combined_latest).to be >= latest_cl_time + end + + it 'uses a single query or minimal queries for the timestamp check' do + # Verify that checking up_to_date? does not make an excessive number of queries. + # We allow up to 3 queries (definition table + config_libraries table + possible cache). + DynamicModel.instance_variable_set(:@prev_latest_update, nil) + + query_count = 0 + counter = lambda { |_name, _start, _finish, _id, payload| + query_count += 1 unless payload[:name] == 'SCHEMA' || payload[:sql]&.include?('SHOW') + } + + ActiveSupport::Notifications.subscribed(counter, 'sql.active_record') do + DynamicModel.up_to_date? + end + + expect(query_count).to be <= 3 + end + end +end