feat: add support for Update as Create #57
Conversation
|
|
||
| private async createResourceWithId(resourceType: string, resource: any, resourceId: string) { |
There was a problem hiding this comment.
Could we add a regex validation for resourceId?
I believe this regex will do the job: ^[a-zA-Z0-9\-\.]{1,64}$
FHIR spec: "Ids can be up to 64 characters long, and contain any combination of upper and lowercase ASCII letters, numerals, "-" and ".""
There was a problem hiding this comment.
Yes good idea.
We could end up with a case where resources that are created have a different id format to those that are updateCreated, do we care about this?
There was a problem hiding this comment.
From a technical stand-point I don't see any issues with a Dynamo key (id) being different or ES param (id) being different.
One issue that may come up is create a 'hot' shard meaning if all the keys started to look the same that shard of Dynamo may suffer in performance. I don't think a regex would solve this and I think anything but random can. So with us allowing updateCreate there will be some additional risk if users are malicious in crafting their ids
There was a problem hiding this comment.
Agree. An application could use solely updateCreate operations to create resources with a different style of id. If these are not random and performance is affected then that is down to poor application design rather than a server implementation issue.
| @@ -103,6 +103,11 @@ export class DynamoDbDataService implements Persistence, BulkDataAccess { | |||
| } | |||
|
|
|||
| private async createResourceWithId(resourceType: string, resource: any, resourceId: string) { | |||
| const regex = new RegExp('^[a-zA-Z0-9-.]{1,64}$'); | |||
| if (!regex.test(resourceId)) { | |||
| throw new Error(`Resource creation failed, id ${resourceId} is not valid`); | |||
There was a problem hiding this comment.
In this case -- it would be more accurate to throw a InvalidResourceError; since this will only fail if folks are using updateCreate: https://github.com/awslabs/fhir-works-on-aws-interface/blob/mainline/src/errors/InvalidResourceError.ts
FYI this will result in a 400 bad request being returned: https://github.com/awslabs/fhir-works-on-aws-routing/blob/mainline/src/router/routes/errorHandling.ts#L31
Issue #, if available:
#55
Description of changes:
Refactored createResource & updateResource functions to support update create (https://www.hl7.org/fhir/http.html#upsert).
This is can be enabled when the Persistence component is created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.