From f7ca46a382aba7103d157bb72a69a501dad87fda Mon Sep 17 00:00:00 2001 From: Vanessa Fotso <46642178+vanessuniq@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:04:50 -0400 Subject: [PATCH] FI-3358: Check for Duplicate Ids in In-Memory Repositories (#551) * Add duplicate id exception Signed-off-by: Vanessa Fotso * Check for duplicate ids and add spec to check for duplicate entity id Signed-off-by: Vanessa Fotso * Fix failing spec due to duplicate ids Signed-off-by: Vanessa Fotso * Remove invalid G10 validator url env Signed-off-by: Vanessa Fotso * refactored in memorry repo Signed-off-by: Vanessa Fotso --------- Signed-off-by: Vanessa Fotso --- .env | 1 - lib/inferno/exceptions.rb | 6 ++++++ lib/inferno/repositories/in_memory_repository.rb | 13 +++++-------- spec/inferno/entities/test_suite_spec.rb | 1 + spec/inferno/repositories/presets_spec.rb | 2 +- spec/inferno/repositories/test_suites_spec.rb | 14 ++++++++++++++ spec/inferno/web/serializers/test_group_spec.rb | 2 +- 7 files changed, 28 insertions(+), 11 deletions(-) diff --git a/.env b/.env index 05b2dd3ec..6fed38a7f 100644 --- a/.env +++ b/.env @@ -1,7 +1,6 @@ # URL for FHIR validator service VALIDATOR_URL=http://localhost/validatorapi REDIS_URL=redis://localhost:6379/0 -G10_VALIDATOR_URL=http://localhost/validatorapi FHIR_RESOURCE_VALIDATOR_URL=http://localhost/hl7validatorapi FHIRPATH_URL=http://localhost/fhirpath diff --git a/lib/inferno/exceptions.rb b/lib/inferno/exceptions.rb index be2cf498d..bd56d09b6 100644 --- a/lib/inferno/exceptions.rb +++ b/lib/inferno/exceptions.rb @@ -119,5 +119,11 @@ def initialize(id) super("ID '#{id}' exceeds the maximum id length of 255 characters") end end + + class DuplicateEntityIdException < StandardError + def initialize(id) + super("ID '#{id}' already exists. Ensure the uniqueness of the IDs.") + end + end end end diff --git a/lib/inferno/repositories/in_memory_repository.rb b/lib/inferno/repositories/in_memory_repository.rb index b6afdc3ad..8c5e72441 100644 --- a/lib/inferno/repositories/in_memory_repository.rb +++ b/lib/inferno/repositories/in_memory_repository.rb @@ -1,4 +1,5 @@ require 'forwardable' +require_relative '../exceptions' module Inferno module Repositories @@ -8,7 +9,10 @@ class InMemoryRepository def_delegators 'self.class', :all, :all_by_id def insert(entity) + raise Exceptions::DuplicateEntityIdException, entity.id if exists?(entity.id) + all << entity + all_by_id[entity.id.to_s] = entity entity end @@ -17,7 +21,7 @@ def find(id) end def exists?(id) - all_by_id.include? id + all_by_id.key?(id.to_s) end class << self @@ -28,13 +32,6 @@ def all # @private def all_by_id @all_by_id ||= {} - @all_by_id.length == all.length ? @all_by_id : index_by_id - end - - def index_by_id - @all_by_id = {} - all.each { |klass| @all_by_id[klass.id] = klass } - @all_by_id end end end diff --git a/spec/inferno/entities/test_suite_spec.rb b/spec/inferno/entities/test_suite_spec.rb index a7fe5948c..56c138a71 100644 --- a/spec/inferno/entities/test_suite_spec.rb +++ b/spec/inferno/entities/test_suite_spec.rb @@ -7,6 +7,7 @@ let(:id) { 'GROUP_ID' } before do + suite_class.id(SecureRandom.uuid) allow(Class).to receive(:new).with(Inferno::Entities::TestGroup).and_return(group_class) end diff --git a/spec/inferno/repositories/presets_spec.rb b/spec/inferno/repositories/presets_spec.rb index 97d3c3b4a..6dd65e1f3 100644 --- a/spec/inferno/repositories/presets_spec.rb +++ b/spec/inferno/repositories/presets_spec.rb @@ -13,7 +13,7 @@ File.realpath(File.join(Dir.pwd, 'spec/fixtures/simple_preset.json.erb')) end - let(:uuid) { 'very-random-uuid' } + let(:uuid) { SecureRandom.uuid } before { allow(SecureRandom).to receive(:uuid).and_return(uuid) } diff --git a/spec/inferno/repositories/test_suites_spec.rb b/spec/inferno/repositories/test_suites_spec.rb index 9d91d7ad6..325013698 100644 --- a/spec/inferno/repositories/test_suites_spec.rb +++ b/spec/inferno/repositories/test_suites_spec.rb @@ -24,4 +24,18 @@ expect(record).to be_nil end end + + describe '#insert' do + it 'inserts a new suite into the repository' do + records = repo.all + new_suite = Class.new(Inferno::TestSuite) + new_suite.id('suite_id') + expect { repo.insert(new_suite) }.to change { records.size }.by(1) + expect(records).to include(new_suite) + end + + it 'raises an error if the id already exist' do + expect { repo.insert(suite) }.to raise_error(Inferno::Exceptions::DuplicateEntityIdException, /already exists/) + end + end end diff --git a/spec/inferno/web/serializers/test_group_spec.rb b/spec/inferno/web/serializers/test_group_spec.rb index a71e490f7..9d574b4d3 100644 --- a/spec/inferno/web/serializers/test_group_spec.rb +++ b/spec/inferno/web/serializers/test_group_spec.rb @@ -2,10 +2,10 @@ RSpec.describe Inferno::Web::Serializers::TestGroup do let(:group) { InfrastructureTest::SerializerGroup } - let(:test) { group.tests.first } before do options_multi_group = Class.new(OptionsSuite::AllVersionsGroup) do + id SecureRandom.uuid group from: :v1_group, required_suite_options: { ig_version: '1' }