Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

feat: add Batch support #162

Merged
merged 11 commits into from
Apr 6, 2022
Merged

feat: add Batch support #162

merged 11 commits into from
Apr 6, 2022

Conversation

ssvegaraju
Copy link
Contributor

@ssvegaraju ssvegaraju commented Mar 25, 2022

Issue #, if available:

Description of changes:
Added logic to parse requests made to the root URL with a Batch request. Contains logic to validate the bundle according to https://www.hl7.org/fhir/http.html#brules, and adds unit tests for batch. This PR is in draft until the persistence changes are released and updated on the interface package.

We use a limit of 750 requests per bundle for the following reason:
Lambda has a payload limit of 6MB. Taking an average request size of 4KB, this gives us that 6MB/4KB = 1500 requests. To give some leeway, we divide by half to get 750.

TESTING:
tested through deployment and querying for various different bundle configurations such as:

  • batches with references to resource within the bundle
  • all different CRUD operations
  • failing CRUD operations should not affect bundle success

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

src/router/bundle/bundleGenerator.ts Outdated Show resolved Hide resolved
src/router/bundle/bundleGenerator.ts Outdated Show resolved Hide resolved
src/router/bundle/bundleGenerator.ts Outdated Show resolved Hide resolved
src/router/bundle/bundleHandler.ts Outdated Show resolved Hide resolved
src/router/bundle/bundleHandler.ts Show resolved Hide resolved
@github-actions github-actions bot added the size/m label Apr 4, 2022
src/constants.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2022

Codecov Report

Merging #162 (61c83f2) into mainline (1108326) will increase coverage by 0.17%.
The diff coverage is 92.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##           mainline     #162      +/-   ##
============================================
+ Coverage     75.76%   75.93%   +0.17%     
============================================
  Files            47       47              
  Lines          1366     1384      +18     
  Branches        284      296      +12     
============================================
+ Hits           1035     1051      +16     
- Misses          331      333       +2     
Impacted Files Coverage Δ
src/router/bundle/bundleParser.ts 98.37% <80.00%> (-0.53%) ⬇️
src/router/bundle/bundleGenerator.ts 94.11% <87.50%> (-1.44%) ⬇️
src/router/bundle/bundleHandler.ts 91.66% <100.00%> (+1.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1108326...61c83f2. Read the comment docs.

Copy link
Contributor

@Bingjiling Bingjiling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Some last changes for generateBatchBundle and we're good to go.

src/router/bundle/bundleGenerator.ts Outdated Show resolved Hide resolved
},
};
// remove unnecessary fields from response
if (bundleEntryResponse.error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this logic in Persistence instead of here? If etag and lastModified should not exist, it is Persistence's responsibility to remove them instead of depending on another package to handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's possible to remove the vid/lastmodified fields from the persistence response as they are a required property of the type. However, I removed the logic to explicitly remove them, and instead combined the logic with the transaction bundle generation.

src/router/bundle/bundleHandler.ts Show resolved Hide resolved
@ssvegaraju ssvegaraju marked this pull request as ready for review April 6, 2022 15:08
@ssvegaraju ssvegaraju requested a review from a team as a code owner April 6, 2022 15:08
@ssvegaraju ssvegaraju merged commit 837aff6 into mainline Apr 6, 2022
@ssvegaraju ssvegaraju deleted the feat-addBundleSupport branch April 6, 2022 15:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants