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

feat: add HAPI validator lambda fn #221

Merged
merged 3 commits into from
Mar 2, 2021
Merged

feat: add HAPI validator lambda fn #221

merged 3 commits into from
Mar 2, 2021

Conversation

carvantes
Copy link
Contributor

@carvantes carvantes commented Feb 23, 2021

Description of changes:

Add the java lambda function that runs the HAPI FHIR Validator.

It is expected that the main compiler script copies the IGs to javaHapiValidatorLambda/src/main/resources/implementationGuides so that they are bundled into the lambda deployment package an loaded to the HAPI validator.

Checklist:

  • Have you successfully deployed to an AWS account with your changes?
  • Have you written new tests for your core changes, as applicable?

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

@carvantes carvantes marked this pull request as ready for review February 23, 2021 15:48
Copy link
Contributor

@nguyen102 nguyen102 left a comment

Choose a reason for hiding this comment

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

Overall looks good. I have some formatting suggestions and a question about the try/catch.

Btw, what are your thoughts on moving this code out into it's own package? I know we initially designed this to be similar to the auditLogMover. However, it's a big mental shift to change from looking at TS/JS code in this package to Java. This code has very clear separations of concern from the rest of the deployment package and it seems like it could exist as it's own package.

For now, it makes sense if we're just including the code in this package to make development easier. I'm ok with keeping it here for now, with the plan to move it to its own package in the future.

javaHapiValidatorLambda/serverless.yml Outdated Show resolved Hide resolved
javaHapiValidatorLambda/serverless.yml Outdated Show resolved Hide resolved
}
}
}
} catch (IOException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we can't read one of the files, should we error out here, and stop execution or should we only report an error happened?

What happens if the validator expects to load in a file, but fails to do it. Will the validator not validate against that IG?

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 changed it to just rethrow the exception.

Since we are using provisioned concurrency, failing to load an IG will be an uncaught exception that will make the deployment fail. This is an ok behavior since that means that the IG files are wrong and you need to fix them and redeploy.

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

class ValidatorTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a test to validate that we can correctly load an IG and validate a resource with that IG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will be covered by integ tests.

The thing is that the IGs are added to src/main/resources/implementationGuides as if they were part of the lambda source code and it is very tricky to mock those files in a way that makes sense.

@carvantes carvantes requested a review from nguyen102 February 26, 2021 13:33
@carvantes
Copy link
Contributor Author

Btw, what are your thoughts on moving this code out into it's own package? I know we initially designed this to be similar to the auditLogMover. However, it's a big mental shift to change from looking at TS/JS code in this package to Java. This code has very clear separations of concern from the rest of the deployment package and it seems like it could exist as it's own package.

For now, it makes sense if we're just including the code in this package to make development easier. I'm ok with keeping it here for now, with the plan to move it to its own package in the future.

yeah, we can think about it. I'm not quite sure what's best approach. There's no straightforward way to "import" a package that has CFN templates + code (and java code only makes it harder).

IMO it's ok to keep it here in the short term.

@carvantes carvantes requested a review from rsmayda February 26, 2021 22:43
@carvantes carvantes changed the base branch from feat-search-igs to feat-igs March 2, 2021 06:13
@carvantes carvantes merged commit ffb34ce into feat-igs Mar 2, 2021
@carvantes carvantes deleted the dev-hapi-lambda branch March 2, 2021 06:14
carvantes added a commit that referenced this pull request Mar 29, 2021
* feat: add Implementation Guides compiler script (#192)
* feat: load compiled implementation guides (#199)
* feat: add Implementation Guides integ tests (#205)
* chore: add compile-igs yarn script
* feat: add HAPI validator lambda fn (#221)
* feat: Compile IG StructureDefinitions (#235)
* feat: add useHapiValidator CFN parameter (#236)
* test: IGs: Validation during create/update and supportedProfile in CapStatement (#237)
* feat: specify paths to IG resources (#241)
* fix: copy IG files to webpack deployment package (#247)
* chore: add third party attributions file (#251)
* feat: add support for DSTU3 on Java Hapi validator (#254)
* docs: add IGs documentation (#256)
* chore: update gh actions tu build and deploy hapi validator (#257)
* feat: support Implementation Guides
* docs: fix changelog headers
* chore: update version in package.json

Co-authored-by: justinusmenzel <69522360+justinusmenzel@users.noreply.github.com>
Co-authored-by: Justinus Menzel <justinus.menzel@abacusinsights.com>
Co-authored-by: Emil Diaz <emilhdiaz@users.noreply.github.com>
Co-authored-by: Tim Nguyen <nguyen102@users.noreply.github.com>
Co-authored-by: shyogesh-sw <79225266+shyogesh-sw@users.noreply.github.com>
rsmayda added a commit that referenced this pull request Jun 25, 2021
BREAKING CHANGE: Interact with ES via aliases. Aliases need to be added to existing index prior to deployment (feat: Use alias for all ES operations #349)
- Run the addAlias script prior to deploying. This script will create aliases for all existing indices


To highlight some of the features being merged in:
- Add logging framework (feat: deployment change for adding logging framework  #310)
- Add $docref implementation (feat: add $docref implementation #332)
- Add better search integration tests (test: add search tests for exact token matching #306, feat: Add post search and integ tests #296)
- Add DLQ for ddbToEs sync failures (feat: add DLQ for ddbToEs sync failures #295)

Full list of changes merged in: 

* feat: Update dependencies to support SearchFilters better (#197)

* fix: Update persistence and interface dependencies (#201)

* fix(search): bump search version to pick up bug fixes (#202)

* chore: add PR size labeler (#198)

* chore: add PR size labeler

* chore: update labeler yml syntax

* empty commit fix mainline protection

* feat: bump routing and interface version to use validators (#208)

* docs: Update docs, Third party file & diagram (#207)

* Fixes a link in DEVELOPMENT.md (#215)

* docs: add a troubleshooting case for windows (#220)

* chore: change log retention and add DDB tables as output (#222)

* test: Add tests for RBAC Auth (#229)

* test: update cognito usernames (#230)

* test: Default fhirClient should use the role of practitioner (#231)

* chore: dependency update (#232)

* chore: Update serverless dependencies and use serverless-bundle to optimize package size (#240)

* fix: http-errors from routing dependency (#243)

* chore: update deploy script for working with local packages (#245)

* chore: update log settings (#252)

* chore: add encryption, https-only, access logging to all buckets (#253)

* chore: Update routing dependency. Pass in user agent value (#263)

* chore: fix gh merge workflow to do ff-only (#264)

* chore: use our own script instead of merge action (#265)

* chore: fix yaml syntax

* chore: fix merge script

* chore: use merge token in merge script

* ci: add workaround for merge with branch protection

* feat: support Implementation Guides (#266)

* feat: add Implementation Guides compiler script (#192)
* feat: load compiled implementation guides (#199)
* feat: add Implementation Guides integ tests (#205)
* chore: add compile-igs yarn script
* feat: add HAPI validator lambda fn (#221)
* feat: Compile IG StructureDefinitions (#235)
* feat: add useHapiValidator CFN parameter (#236)
* test: IGs: Validation during create/update and supportedProfile in CapStatement (#237)
* feat: specify paths to IG resources (#241)
* fix: copy IG files to webpack deployment package (#247)
* chore: add third party attributions file (#251)
* feat: add support for DSTU3 on Java Hapi validator (#254)
* docs: add IGs documentation (#256)
* chore: update gh actions tu build and deploy hapi validator (#257)
* feat: support Implementation Guides
* docs: fix changelog headers
* chore: update version in package.json

* ci: use HAPI validator on integ tests (#269)

* chore: Update routing package dependency. Update custom agent env variable with correct package version (#271)

* docs: removing team email from feedback (#275)

* fix: fix unable to locate compiledImplementationGuides/* glob (#277)

* fix: exit from installation script if serverless deployment fails (#280)

* docs: Clarify IG documentation (#279)

* docs: Updated FHIR Works readme (#283)

* feat: enhance date and token search. Add integ tests (#284)

* test: add token search integ tests (#285)

* chore(release): 2.6.0 (#286)

* docs: Update CUSTOM_USER_AGENT (#287)

* fix: update persistence dependency; to fix meta bug (#288)

* feat: enhance numeric and quantity search (#291)

* fix: Suppress deprecation warning when writing to Info_Output.yml during installation (#294)

* feat: add DLQ for ddbToEs sync failures (#295)

* chore: lock down versions of external deployment dependencies (#298)

* fix: increment persistence package (#300)

* feat: Add post search and integ tests (#296)

* feat: Add post search and integ tests

* chore: add security scanning (#302)

* feat(search): support Period type fields for date params (#299)

* test: add search tests for exact token matching (#306)

* chore(release): 2.7.0  (#307)

* chore: add cfn-nag to all yaml changes (#308)

* chore: dependency updates (#321)

* chore: dependency updates

* fix: resolved registry url updated

* chore: upgrade dependencies (#322)

* chore: bump dependency version (#328)

* chore: bump dependency version

* chore: adding GitHub issue templates (#325)

* chore: update README for another Auth url

* feat: add logging framework  (#310)

* chore: workflow only run if yaml/yml is updated (#333)

* feat: add $docref implementation (#332)

* chore(deps): bump browserslist from 4.16.3 to 4.16.6 (#335)

* chore(deps): bump browserslist from 4.16.3 to 4.16.6

* trigger mainline protection check

* chore(deps): bump browserslist from 4.16.3 to 4.16.6 in /auditLogMover (#334)

* chore(deps): bump browserslist from 4.16.3 to 4.16.6 in /auditLogMover

* trigger mainline protection check

* chore: dependency fix (#339)

* chore: release 2.8.0 (#338)

* fix: Allow running sls offline with Hapi Validator (#343)

* fix: typo for passing in custom log level (#345)

* chore: script for add alias to existing index (#346)

* feat: remove unneeded scope checks in authorizer (#347)

* Revert "feat: remove unneeded scope checks in authorizer (#347)"

* feat!: Use alias for all ES operations (#349)

* BREAKING CHANGE: Aliases need to be added to existing index
* Run the addAlias [script](https://github.com/awslabs/fhir-works-on-aws-deployment/blob/94a3187a6fb7a673946a215869c154048603389b/scripts/elasticsearch-operations.js) created in this [PR](#346) will create aliases for all existing indices 
* Update or create resource in a specific type will automatically create alias for the corresponding index

* chore(release): 3.0.0 (#353)

* chore: remove routing package from bundle.externals (#354)

* chore(deps): bump ssri from 6.0.1 to 6.0.2 in /auditLogMover (#351)

* chore(deps): bump ssri from 6.0.1 to 6.0.2 in /auditLogMover

* chore(deps): bump lodash from 4.17.20 to 4.17.21 in /auditLogMover (#350)

* chore(deps): bump lodash from 4.17.20 to 4.17.21 in /auditLogMover

* chore(deps): bump ssri from 6.0.1 to 6.0.2 (#352)

* chore(deps): bump ssri from 6.0.1 to 6.0.2

* chore(deps-dev): bump lodash from 4.17.20 to 4.17.21 (#355)

* chore(deps-dev): bump lodash from 4.17.20 to 4.17.21

* chore: update dependencies (#356)

* fix: type definition issues

Co-authored-by: Tim Nguyen <nguyen102@users.noreply.github.com>
Co-authored-by: Nestor Carvantes <carvantes@gmail.com>
Co-authored-by: Yanyu Zheng <yz2690@columbia.edu>
Co-authored-by: zheyanyu <zheyanyu@amazon.com>
Co-authored-by: J Kendal <13680617+joekendal@users.noreply.github.com>
Co-authored-by: Tim Nguyen <thingut@amazon.com>
Co-authored-by: justinusmenzel <69522360+justinusmenzel@users.noreply.github.com>
Co-authored-by: Justinus Menzel <justinus.menzel@abacusinsights.com>
Co-authored-by: Emil Diaz <emilhdiaz@users.noreply.github.com>
Co-authored-by: shyogesh-sw <79225266+shyogesh-sw@users.noreply.github.com>
Co-authored-by: Sanket Dharwadkar <sdharwad@amazon.com>
Co-authored-by: Bakha Nurzhanov <74073037+awsbakha@users.noreply.github.com>
Co-authored-by: Ranjan Bhandari <ranjanbhandari@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants