Make deployment manager stop on first found app instance

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

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