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.

Sign up for GitHub

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

Jump to bottom

equivalency check when X-FHIR-UPDATE-IF-MODIFIED to true is not detecting that a field changed #2313

Closed
kmbarton423 opened this issue May 3, 2021 · 1 comment
Closed

equivalency check when X-FHIR-UPDATE-IF-MODIFIED to true is not detecting that a field changed #2313

kmbarton423 opened this issue May 3, 2021 · 1 comment
Assignees
@lmsurpre
Labels
bug Something isn't working P1 Priority 1 - Must Have
Milestone
Sprint 2021-06

Comments

@kmbarton423
Copy link
Contributor

kmbarton423 commented May 3, 2021 •
edited
Loading

Working with conditional update functionality (issues 2263 and 2284). See Performance Guide section 5.2:
https://ibm.github.io/FHIR/guides/FHIRPerformanceGuide/#5-ingestion-scenarios

Using FHIR main branch.

To reproduce:
-Create a Patient (PUT with predefined resource.id)
-Set header X-FHIR-UPDATE-IF-MODIFIED to true
-Set header Prefer to return=OperationOutcome
-PUT the same patient again
-Confirm history stays at "1" and OperationOutcome returns the following:
"Update resource matches the existing resource; skipping the update"
-Modify a field within "identifier" in the Patient resource and PUT
-Again received history at "1" and OperationOutcome returns the following:
"Update resource matches the existing resource; skipping the update"

Expected the resource to be saved as version "2"
@kmbarton423 kmbarton423 added the bug Something isn't working label May 3, 2021
lmsurpre added a commit that referenced this issue May 4, 2021
@lmsurpre
issue #2313 - refactor FingerprintVisitor to avoid startsWith checks
8fc33d2 
Previously, we constructed the excludePaths on each leaf node, then
checked whether the current path started with any of those values.

This was faulty because the exclude path Resource.id accidentally
resulted in us ignoring fields like Resource.identifier as well.

Now we construct the excludePaths as a set during the initial
doVisitStart and we don't need "startsWith" because we the visitor
supports returning a boolean to control whether child paths should be
visited or not.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue May 4, 2021
@lmsurpre
issue #2313 - refactor FingerprintVisitor to avoid startsWith checks
fffe4b0 
Previously, we constructed the excludePaths on each leaf node, then
checked whether the current path started with any of those values.

This was faulty because the exclude path Resource.id accidentally
resulted in us ignoring fields like Resource.identifier as well.

Now we construct the excludePaths as a set during the initial
doVisitStart and we don't need "startsWith" because we the visitor
supports returning a boolean to control whether child paths should be
visited or not.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue May 4, 2021
@lmsurpre
issue #2313 - refactor FingerprintVisitor to avoid startsWith checks
fc0f238 
Previously, we constructed the excludePaths on each leaf node, then
checked whether the current path started with any of those values.

This was faulty because the exclude path Resource.id accidentally
resulted in us ignoring fields like Resource.identifier as well.

Now we construct the excludePaths as a set during the initial
doVisitStart and we don't need "startsWith" because we the visitor
supports returning a boolean to control whether child paths should be
visited or not.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre added the P1 Priority 1 - Must Have label May 4, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-06 milestone May 4, 2021
lmsurpre added a commit that referenced this issue May 4, 2021
@lmsurpre
issue #2313 - refactor FingerprintVisitor to avoid startsWith checks
7e80d93 
Previously, we constructed the excludePaths on each leaf node, then
checked whether the current path started with any of those values.

This was faulty because the exclude path Resource.id accidentally
resulted in us ignoring fields like Resource.identifier as well.

Now we construct the excludePaths as a set during the initial
doVisitStart and we don't need "startsWith" because we the visitor
supports returning a boolean to control whether child paths should be
visited or not.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre self-assigned this May 10, 2021
lmsurpre added a commit that referenced this issue May 10, 2021
@lmsurpre
issue #2313 - refactor FingerprintVisitor to avoid startsWith checks (#…
3bfacdf 
…2314)

* issue #2313 - refactor FingerprintVisitor to avoid startsWith checks

Previously, we constructed the excludePaths on each leaf node, then
checked whether the current path started with any of those values.

This was faulty because the exclude path Resource.id accidentally
resulted in us ignoring fields like Resource.identifier as well.

Now we construct the excludePaths as a set during the initial
doVisitStart and we don't need "startsWith" because we the visitor
supports returning a boolean to control whether child paths should be
visited or not.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@kmbarton423
Copy link
Contributor Author

Fix looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmsurpre lmsurpre

Labels
bug Something isn't working P1 Priority 1 - Must Have
Projects
None yet
Milestone
Sprint 2021-06
Development

No branches or pull requests
2 participants
@lmsurpre @kmbarton423