-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Making sure that we are not accidentally overwriting an existing reso… #6291
base: master
Are you sure you want to change the base?
Making sure that we are not accidentally overwriting an existing reso… #6291
Conversation
…urce with the same ID from another IG by comparing identifier, url or whatever constitutes uniqueness.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6291 +/- ##
============================================
+ Coverage 83.54% 83.58% +0.04%
- Complexity 27432 27562 +130
============================================
Files 1707 1715 +8
Lines 106185 106675 +490
Branches 13397 13434 +37
============================================
+ Hits 88710 89169 +459
+ Misses 11750 11746 -4
- Partials 5725 5760 +35 ☔ View full report in Codecov by Sentry. |
@codeforgreen can I persuade you to have a look at this? |
Ping @codeforgreen ? |
HI @jkiddo what is this related to? The title of this issue does not ring a bell to me. Can you provide more details and link any related issue or pull request? Thanks. |
An issue I discovered in the starter project using the IG installer functionality. If you install multiple IGs with overlapping resources then the order of installing suddenly starts to play a part which it shouldnt. This PR makes sure that all resources are installed using conditional logic and stops that non deterministic behaviour. So yes, please review/comment/merge. |
@codeforgreen does the above provide the needed clarity? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would update(T theResource, String theMatchUrl, RequestDetails theRequestDetails);
actually throw the exceptions that you're queueing in the mock?
theMatchUrl
is different for oldSpec
and newSpec
. Where is the logic that would throw this exception?
@dotasek the error will be thrown in hapi-fhir/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaResourceDaoCodeSystem.java Line 252 in 8676cf2
I've added this test here https://github.com/hapifhir/hapi-fhir/pull/6291/files#diff-aa258240972be5bb70e69c9bb6ff0a8cd9ea9a93815c0d1e79e4cf446986f789R273 that might better explain and reproduce it. |
…urce with the same ID from another IG by comparing identifier, url or whatever constitutes uniqueness.