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

Export API for a system level export returning one resource per resource type #2005

Merged
merged 14 commits into from
May 11, 2023

Conversation

tonylee80
Copy link
Contributor

@tonylee80 tonylee80 commented May 9, 2023

fixes export returning one resource per resource type and fixes infinite loop when there are greater than 1000 resources for a resource type

🤖 Generated by Copilot at 5c37862

Summary

🆕🐛📄

Enhanced the bulk export operation to support pagination and resource limits, and added tests to verify the functionality. Fixed a bug in exportResourceType that affected the output files.

We're exportResourceTypeing with a mighty heave-ho
We're paginating and limiting as we go
We're fixing bugs and writing tests to make the code run right
We're exportResourceTypeing with a mighty heave-ho

Walkthrough

  • Changed the exportResourceType function to accept a maxResources parameter and an offset variable, and to iterate through paginated search results by following the next link in the search bundle (link, link)
  • Fixed a bug in the exportResourceType function that caused some resources to be skipped or written to the wrong file by calling the getWriter function on the exporter before writing each resource (link)
  • Removed the unused project parameter from the exportResourceType function call in the bulkExportHandler function (link)
  • Added two test cases to the export.test.ts file to test the functionality of exporting multiple resources by resource type and iterating through paginated search results, using the request, waitFor, and expect libraries, and the initTestAuth, createTestProject, and createReference functions (link)
  • Imported the Observation and Patient types from @medplum/fhirtypes, and the systemRepo, exportResourceType, BulkExporter, and createReference functions from their respective modules, to be used in the new test cases (link, link)
  • Reordered the imports in the export.ts file to match the order of the imports in the export.test.ts file, for consistency and readability (link)

@vercel
Copy link

vercel bot commented May 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
medplum-storybook ⬜️ Ignored (Inspect) May 10, 2023 11:05pm
medplum-www ⬜️ Ignored (Inspect) May 10, 2023 11:05pm

@tonylee80 tonylee80 marked this pull request as ready for review May 9, 2023 18:36
@tonylee80 tonylee80 requested a review from a team as a code owner May 9, 2023 18:36
@coveralls
Copy link

coveralls commented May 9, 2023

Coverage Status

Coverage: 94.283% (+0.009%) from 94.274% when pulling e8cf78a on tony-system-level-export-returning-one-resource into 8c4f294 on main.

@tonylee80 tonylee80 self-assigned this May 9, 2023
@tonylee80 tonylee80 added the fhir-datastore Related to the FHIR datastore, includes API and FHIR operations label May 9, 2023
@tonylee80 tonylee80 added this to the May 15, 2023 milestone May 9, 2023
while (hasMore) {
const bundle = await repo.search({
resourceType,
count: 1000,
offset,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

addresses infinite loop when there are more than 1000 resources for a resource type

for (const entry of bundle.entry) {
if (entry.resource?.id) {
writes.push(exporter.writeResource(entry.resource));
await exporter.writeResource(entry.resource);
Copy link
Contributor Author

@tonylee80 tonylee80 May 9, 2023

Choose a reason for hiding this comment

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

removing using 'Promise.all'. Each time export.writeResource was called this.writers was always {}. A new writer for each resourceType was always being created at

const binary = await this.repo.createResource<Binary>({

@@ -34,7 +34,7 @@ export async function bulkExportHandler(req: Request, res: Response): Promise<vo
if (!canBeExported(resourceType) || (types && !types.includes(resourceType))) {
continue;
}
await exportResourceType(exporter, project, resourceType as ResourceType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

project was unused

@@ -44,30 +44,40 @@ export async function bulkExportHandler(req: Request, res: Response): Promise<vo
res.set('Content-Location', `${baseUrl}fhir/R4/bulkdata/export/${bulkDataExport.id}`).status(202).json(accepted);
}

async function exportResourceType(exporter: BulkExporter, project: Project, resourceType: ResourceType): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

project was unused

@@ -44,30 +44,40 @@ export async function bulkExportHandler(req: Request, res: Response): Promise<vo
res.set('Content-Location', `${baseUrl}fhir/R4/bulkdata/export/${bulkDataExport.id}`).status(202).json(accepted);
}

async function exportResourceType(exporter: BulkExporter, project: Project, resourceType: ResourceType): Promise<void> {
export async function exportResourceType(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding export and param maxResources to unit test pagination.

@tonylee80 tonylee80 requested a review from mattwiller May 10, 2023 20:51
mattwiller
mattwiller previously approved these changes May 10, 2023
packages/server/src/fhir/operations/export.ts Outdated Show resolved Hide resolved
packages/server/src/fhir/operations/export.ts Outdated Show resolved Hide resolved
packages/server/src/fhir/operations/export.ts Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented May 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.9% 90.9% Coverage
0.0% 0.0% Duplication

@codyebberson codyebberson merged commit d5a00a6 into main May 11, 2023
@codyebberson codyebberson deleted the tony-system-level-export-returning-one-resource branch May 11, 2023 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fhir-datastore Related to the FHIR datastore, includes API and FHIR operations
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants