From 216b0c9621f896fa16553ed9eb1687619e7c9555 Mon Sep 17 00:00:00 2001 From: David Spencer Date: Tue, 4 Nov 2025 15:32:01 -0500 Subject: [PATCH] Make deployment manager stop on first found app instance This way we don't always run the tenant creation fallback --- .../deployment_manager/deployment_manager.rb | 97 +++++++++---------- .../deployment_manager_spec.rb | 34 +++---- 2 files changed, 59 insertions(+), 72 deletions(-) diff --git a/lib/atomic_tenant/deployment_manager/deployment_manager.rb b/lib/atomic_tenant/deployment_manager/deployment_manager.rb index b7ff5da..2c2ae53 100644 --- a/lib/atomic_tenant/deployment_manager/deployment_manager.rb +++ b/lib/atomic_tenant/deployment_manager/deployment_manager.rb @@ -1,72 +1,69 @@ module AtomicTenant module DeploymentManager - class DeploymentStrategyResult - attr_accessor :application_instance_id - attr_accessor :details - - def initialize(application_instance_id: nil, details: nil) - @application_instance_id = application_instance_id - @details = details - end + class DeploymentStrategyResult + attr_accessor :application_instance_id, :details + def initialize(application_instance_id: nil, details: nil) + @application_instance_id = application_instance_id + @details = details end - class DeploymentManagerStrategy - def name; end - def call(decoded_id_token:); end - end - + end + class DeploymentManagerStrategy + def name; end + def call(decoded_id_token:); end + end # Associate deployment class DeploymentManager - def initialize(strageties) - @strageties = strageties || [] - end + def initialize(strageties) + @strageties = strageties || [] + end - def link_deployment_id(decoded_id_token:) - deployment_id = decoded_id_token[AtomicLti::Definitions::DEPLOYMENT_ID] - iss = decoded_id_token["iss"] + def link_deployment_id(decoded_id_token:) + deployment_id = decoded_id_token[AtomicLti::Definitions::DEPLOYMENT_ID] + iss = decoded_id_token["iss"] - results = @strageties.flat_map do |strategy| - begin - [{name: strategy.name, result: strategy.call(decoded_id_token: decoded_id_token)}] - rescue StandardError => e - Rails.logger.error("Error in lti deployment linking strategy: #{strategy.name}, #{e}") - [] - end - end + to_link = nil + strategy_name = nil - Rails.logger.debug("Linking Results: #{results}") - - matched = results.filter { |r| r[:result].application_instance_id.present? } + @strageties.each do |strategy| + result = strategy.call(decoded_id_token: decoded_id_token) + if result.application_instance_id.present? + to_link = result + strategy_name = strategy.name + break + end + rescue StandardError => e + Rails.logger.error("Error in lti deployment linking strategy: #{strategy.name}, #{e}") + end - to_link = if matched.size > 0 - matched.first[:result] - else - raise AtomicTenant::Exceptions::UnableToLinkDeploymentError - end + raise AtomicTenant::Exceptions::UnableToLinkDeploymentError if to_link.nil? - if matched.size > 1 - Rails.logger.info("Colliding strategies, Linking iss / deployment id: #{iss} / #{deployment_id} to application instance: #{to_link.application_instance_id}, all results: #{results}") - else - Rails.logger.info("Linking iss / deployment id: #{iss} / #{deployment_id} to application instance: #{to_link.application_instance_id}") - end + Rails.logger.info( + "Linking iss / deployment id: #{iss} / #{deployment_id} to application instance: " \ + "#{to_link.application_instance_id} using strategy: #{strategy_name}" + ) - associate_deployment(iss: iss, deployment_id: deployment_id,application_instance_id: to_link.application_instance_id) - end + associate_deployment( + iss: iss, + deployment_id: deployment_id, + application_instance_id: to_link.application_instance_id + ) + end - private + private - def associate_deployment(iss:, deployment_id:, application_instance_id:) - AtomicTenant::LtiDeployment.create!( - iss: iss, - deployment_id: deployment_id, - application_instance_id: application_instance_id - ) - end + def associate_deployment(iss:, deployment_id:, application_instance_id:) + AtomicTenant::LtiDeployment.create!( + iss: iss, + deployment_id: deployment_id, + application_instance_id: application_instance_id + ) + end end end end diff --git a/spec/lib/atomic_tenant/deployment_manager/deployment_manager_spec.rb b/spec/lib/atomic_tenant/deployment_manager/deployment_manager_spec.rb index b75145e..ad3f7f3 100644 --- a/spec/lib/atomic_tenant/deployment_manager/deployment_manager_spec.rb +++ b/spec/lib/atomic_tenant/deployment_manager/deployment_manager_spec.rb @@ -32,7 +32,7 @@ it "logs standard linking message" do expect(Rails.logger).to receive(:info).with( - "Linking iss / deployment id: #{iss} / #{deployment_id} to application instance: #{application_instance.id}" + "Linking iss / deployment id: #{iss} / #{deployment_id} to application instance: #{application_instance.id} using strategy: test_strategy" ) manager.link_deployment_id(decoded_id_token: decoded_id_token) @@ -64,21 +64,13 @@ expect(deployment.application_instance_id).to eq(application_instance.id) end - it "logs colliding strategies message with to_link defined" do - allow(Rails.logger).to receive(:debug).and_call_original - allow(Rails.logger).to receive(:info).and_call_original + it "uses first strategy and does not evaluate second strategy" do + expect(strategy1).to receive(:call).and_call_original + expect(strategy2).not_to receive(:call) - expect(Rails.logger).to receive(:info).with( - a_string_including( - "Colliding strategies", - iss, - deployment_id, - "application instance: #{application_instance.id}", - "all results:" - ) - ).and_call_original + deployment = manager.link_deployment_id(decoded_id_token: decoded_id_token) - manager.link_deployment_id(decoded_id_token: decoded_id_token) + expect(deployment.application_instance_id).to eq(application_instance.id) end end @@ -145,7 +137,7 @@ end end - context "debug logging" do + context "strategy evaluation" do let(:strategy1) do MockStrategy.new( name: "strategy_1", @@ -161,15 +153,13 @@ end let(:manager) { described_class.new([strategy1, strategy2]) } - it "logs debug information about all results" do - allow(Rails.logger).to receive(:debug).and_call_original - allow(Rails.logger).to receive(:info).and_call_original + it "stops evaluation after first match" do + expect(strategy1).to receive(:call).and_call_original + expect(strategy2).not_to receive(:call) - expect(Rails.logger).to receive(:debug).with( - a_string_including("Linking Results:") - ).and_call_original + deployment = manager.link_deployment_id(decoded_id_token: decoded_id_token) - manager.link_deployment_id(decoded_id_token: decoded_id_token) + expect(deployment.application_instance_id).to eq(application_instance.id) end end end