-
Notifications
You must be signed in to change notification settings - Fork 299
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
Resource Creation using POST http verb (SingleResourcePost) #2464
Resource Creation using POST http verb (SingleResourcePost) #2464
Conversation
engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt
Outdated
Show resolved
Hide resolved
engine/src/androidTest/java/com/google/android/fhir/sync/upload/ResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
engine/src/androidTest/java/com/google/android/fhir/sync/upload/ResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
@santosh-pingle How is this going to work? Are we making it the default behaviour for all CREATEs. Right now if you POST to a HAPI server, the server assigns the resource its own e.g posting the following resource {
"resourceType": "List",
"id": "af17fe86-561a-44b0-84d3-5e75c753f6f8",
"meta": {
"versionId" : "1" ,
"lastUpdated": "2024-03-13T15:41:42.808+00:00",
"source" : "#fd2dc30247cf61b8"
},
"identifier": [
{"use": "official", "value": "f39c5f68-ab0f-4ae5-a9e2-47b0beb73d8e"}
],
"status": "current",
"code": {
"coding": [
{
"system" : "http://smartregister.org/",
"code" : "22138876" ,
"display": "Supply Inventory List"
}
],
"text": "Supply Inventory List"
}
} will result in {
"resourceType": "List",
"id": "1759",
"meta": {
"versionId" : "1" ,
"lastUpdated": "2024-04-04T09:09:05.431+00:00",
"source" : "#fd2dc30247cf61b8"
},
"identifier": [
{"use": "official", "value": "f39c5f68-ab0f-4ae5-a9e2-47b0beb73d8e"}
],
"status": "current",
"code": {
"coding": [
{
"system" : "http://smartregister.org/",
"code" : "22138876" ,
"display": "Supply Inventory List"
}
],
"text": "Supply Inventory List"
}
} The above behaviour is going to distort the resource references for any resources created on the mobile application |
engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt
Show resolved
Hide resolved
This was clarified on the SDK dev call. I think this is a good addition |
engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt
Show resolved
Hide resolved
...src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
...src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
...src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
...src/androidTest/java/com/google/android/fhir/sync/upload/HttpPostResourceConsolidatorTest.kt
Outdated
Show resolved
Hide resolved
We have kept existing DefaultConsolidator as it is. |
engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
Show resolved
Hide resolved
Checkout this PR which added bunch of stuffs for POST upload strategy. |
Yes, I am using the core logic as it is. Only delta changes are added on top of it to achieve the end-to-end use case for resource consolidation. |
engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt
Outdated
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/sync/upload/ResourceConsolidator.kt
Show resolved
Hide resolved
engine/src/main/java/com/google/android/fhir/sync/upload/UploadStrategy.kt
Outdated
Show resolved
Hide resolved
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.
Thanks this looks good now!
Can we add few integration tests as well ? Maybe in FhirEngineImplTest ?
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.
LGTM. One nit.
engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt
Outdated
Show resolved
Hide resolved
Addressed review comments, reviewed and approved by code owner @MJ1998, hence dismissing review.
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2464
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2440
Description
The resource is created locally, and the HTTP POST verb is used to synchronize the resource with the FHIR server. In return, the server sends a response containing the resource with the newly assigned resource ID. Subsequently, the local database is updated with the provided resource.
Upload strategy : SingleResourcePost
Update the resource with the received resourceId in the local database
Update the resource with the received meta data in the local database
Update the resource with the received resource json string in the local database
Update the resource with the received reference id in the local database.
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.