Replace use of eval with safer solution

Instead of evaluating the YAML contents directly, we can specify a method in
the YAML file and call in the `Client`. This also has the benefit of being able
to more easily test the possible `id` options.

app/services/migration/parity_check/client.rb

@@ -1,5 +1,7 @@
 module Migration
   class ParityCheck::Client
+    class UnsupportedIdOption < RuntimeError; end
+
     attr_reader :lead_provider, :method, :path, :options, :page
 
     PAGINATION_PER_PAGE = 10
@@ -88,8 +90,40 @@ def formatted_path
       @formatted_path ||= begin
         return path unless options[:id] && path.include?(":id")
 
-        path.sub(":id", eval(options[:id]).to_s) # rubocop:disable Security/Eval
+        raise UnsupportedIdOption, "Unsupported id option: #{options[:id]}" unless respond_to?(options[:id], true)
+
+        path.sub(":id", send(options[:id]).to_s)
       end
     end
+
+    def application_ecf_id
+      lead_provider.applications.order("RANDOM()").limit(1).pick(:ecf_id)
+    end
+
+    def declaration_ecf_id
+      Declaration.where(lead_provider:).order("RANDOM()").limit(1).pick(:ecf_id)
+    end
+
+    def participant_outcome_ecf_id
+      ParticipantOutcome
+        .includes(declaration: { application: :user })
+        .where(declaration: { lead_provider: })
+        .order("RANDOM()")
+        .limit(1)
+        .pick("users.ecf_id")
+    end
+
+    def participant_ecf_id
+      User
+        .includes(:applications)
+        .where(applications: { lead_provider:, lead_provider_approval_status: :accepted })
+        .order("RANDOM()")
+        .limit(1)
+        .pick(:ecf_id)
+    end
+
+    def statement_ecf_id
+      lead_provider.statements.order("RANDOM()").limit(1).pick(:ecf_id)
+    end
   end
 end

config/parity_check_endpoints.yml

@@ -7,11 +7,11 @@ get:
     paginate: true
 
   "/api/v1/npq-applications/:id":
-    id: 'lead_provider.applications.order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: application_ecf_id
   "/api/v2/npq-applications/:id":
-    id: 'lead_provider.applications.order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: application_ecf_id
   "/api/v3/npq-applications/:id":
-    id: 'lead_provider.applications.order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: application_ecf_id
 
   "/api/v1/participant-declarations":
     paginate: true
@@ -21,11 +21,11 @@ get:
     paginate: true
 
   "/api/v1/participant-declarations/:id":
-    id: 'Declaration.where(lead_provider:).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: declaration_ecf_id
   "/api/v2/participant-declarations/:id":
-    id: 'Declaration.where(lead_provider:).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: declaration_ecf_id
   "/api/v3/participant-declarations/:id":
-    id: 'Declaration.where(lead_provider:).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: declaration_ecf_id
 
   "/api/v1/participants/npq/outcomes":
     paginate: true
@@ -35,11 +35,11 @@ get:
     paginate: true
 
   "/api/v1/participants/npq/:id/outcomes":
-    id: 'ParticipantOutcome.includes(declaration: { application: :user }).where(declaration: { lead_provider: }).order("RANDOM()").limit(1).pick("users.ecf_id")'
+    id: participant_outcome_ecf_id
   "/api/v2/participants/npq/:id/outcomes":
-    id: 'ParticipantOutcome.includes(declaration: { application: :user }).where(declaration: { lead_provider: }).order("RANDOM()").limit(1).pick("users.ecf_id")'
+    id: participant_outcome_ecf_id
   "/api/v3/participants/npq/:id/outcomes":
-    id: 'ParticipantOutcome.includes(declaration: { application: :user }).where(declaration: { lead_provider: }).order("RANDOM()").limit(1).pick("users.ecf_id")'
+    id: participant_outcome_ecf_id
 
   "/api/v1/participants/npq":
     paginate: true
@@ -49,11 +49,11 @@ get:
     paginate: true
 
   "/api/v1/participants/npq/:id":
-    id: 'User.includes(:applications).where(applications: { lead_provider:, lead_provider_approval_status: :accepted }).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: participant_ecf_id
   "/api/v2/participants/npq/:id":
-    id: 'User.includes(:applications).where(applications: { lead_provider:, lead_provider_approval_status: :accepted }).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: participant_ecf_id
   "/api/v3/participants/npq/:id":
-    id: 'User.includes(:applications).where(applications: { lead_provider:, lead_provider_approval_status: :accepted }).order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: participant_ecf_id
 
   "/api/v3/statements":
     paginate: true
@@ -63,4 +63,4 @@ get:
     paginate: true
 
   "/api/v3/statements/:id":
-    id: 'lead_provider.statements.order("RANDOM()").limit(1).pick(:ecf_id)'
+    id: statement_ecf_id

spec/services/migration/parity_check/client_spec.rb

@@ -101,20 +101,85 @@
       end
 
       context "when using id substitution" do
-        let(:options) { { id: "lead_provider.statements.pluck(:ecf_id).sample" } }
-        let(:path) { "/api/v3/statements/:id" }
+        let(:options) { { id: } }
+        let(:path) { "/path/:id" }
 
-        it "evaluates the id option and substitutes it into the path" do
-          statement = create(:statement, lead_provider:)
+        before do
+          stub_request(:get, "#{ecf_url}/path/#{resource.ecf_id}")
+          stub_request(:get, "#{npq_url}/path/#{resource.ecf_id}")
+        end
+
+        context "when using an unsupported id" do
+          let(:id) { "not_recognised" }
+          let(:resource) { OpenStruct.new(ecf_id: "123") }
+
+          it { expect { instance.make_requests {} }.to raise_error described_class::UnsupportedIdOption, "Unsupported id option: not_recognised" }
+        end
 
-          stub_request(:get, "#{ecf_url}/api/v3/statements/#{statement.ecf_id}")
-          stub_request(:get, "#{npq_url}/api/v3/statements/#{statement.ecf_id}")
+        context "when using statement_ecf_id" do
+          let(:id) { "statement_ecf_id" }
+          let(:resource) { create(:statement, lead_provider:) }
 
-          instance.make_requests do |_, _, formatted_path|
-            expect(formatted_path).to eq("/api/v3/statements/#{statement.ecf_id}")
+          it "evaluates the id option and substitutes it into the path" do
+            instance.make_requests do |_, _, formatted_path|
+              expect(formatted_path).to eq("/path/#{resource.ecf_id}")
+            end
+
+            expect(requests.count).to eq(2)
           end
+        end
+
+        context "when using application_ecf_id" do
+          let(:id) { "application_ecf_id" }
+          let(:resource) { create(:application, lead_provider:) }
 
-          expect(requests.count).to eq(2)
+          it "evaluates the id option and substitutes it into the path" do
+            instance.make_requests do |_, _, formatted_path|
+              expect(formatted_path).to eq("/path/#{resource.ecf_id}")
+            end
+
+            expect(requests.count).to eq(2)
+          end
+        end
+
+        context "when using declaration_ecf_id" do
+          let(:id) { "declaration_ecf_id" }
+          let(:resource) { create(:declaration, lead_provider:) }
+
+          it "evaluates the id option and substitutes it into the path" do
+            instance.make_requests do |_, _, formatted_path|
+              expect(formatted_path).to eq("/path/#{resource.ecf_id}")
+            end
+
+            expect(requests.count).to eq(2)
+          end
+        end
+
+        context "when using participant_outcome_ecf_id" do
+          let(:id) { "participant_outcome_ecf_id" }
+          let(:declaration) { create(:declaration, lead_provider:) }
+          let(:resource) { create(:participant_outcome, declaration:).declaration.application.user }
+
+          it "evaluates the id option and substitutes it into the path" do
+            instance.make_requests do |_, _, formatted_path|
+              expect(formatted_path).to eq("/path/#{resource.ecf_id}")
+            end
+
+            expect(requests.count).to eq(2)
+          end
+        end
+
+        context "when using participant_ecf_id" do
+          let(:id) { "participant_ecf_id" }
+          let(:resource) { create(:application, :accepted, lead_provider:).user }
+
+          it "evaluates the id option and substitutes it into the path" do
+            instance.make_requests do |_, _, formatted_path|
+              expect(formatted_path).to eq("/path/#{resource.ecf_id}")
+            end
+
+            expect(requests.count).to eq(2)
+          end
         end
       end