Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FI-3358: Check for Duplicate Ids in In-Memory Repositories #551

Merged
merged 7 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .env
Original file line number Diff line number Diff line change
@@ -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

Expand Down
6 changes: 6 additions & 0 deletions lib/inferno/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 5 additions & 8 deletions lib/inferno/repositories/in_memory_repository.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'forwardable'
require_relative '../exceptions'

module Inferno
module Repositories
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/inferno/entities/test_suite_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/inferno/repositories/presets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down
14 changes: 14 additions & 0 deletions spec/inferno/repositories/test_suites_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion spec/inferno/web/serializers/test_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' }

Expand Down
Loading