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.

Sign up for GitHub

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

Jump to bottom

feature(erase): Implement Erase Operation #2334

Merged
merged 45 commits into from
May 14, 2021
Merged

feature(erase): Implement Erase Operation #2334

merged 45 commits into from
May 14, 2021

Conversation

prb112
Copy link
Contributor

@prb112 prb112 commented May 10, 2021 •
edited
Loading

  • Issues resolved are:

  • Remove the timeout and count parameters in lieu of the version-specific erase

  • Update the README with clarity on the parameters

    • logicalId, version, patient, reason
    • clarified the HTTP Method, Api Paths, Response Codes, and Responses
    • Add Acceptance Criteria to explain VERSION delete

  • Update the test harnesses for Db2 and Postgres to use the simple Erase approach

  • Clean up the DAO, JdbcEraseTest to move away from Timeout

  • Removed patient/reason so we don't reflect illegal content.

  • Change function/stored proc to use resource_type, logical_id only

  • Refactor erase.json to support only specific values.

  • Revert test.properties to logging false

  • Improve IT/UT test coverage

  • Added Code Coverage and Mock Implementations

  • db2: Add a stored procedure to Erase multiple versions in a controlled fashion. (must have a standard logfilsz so you don't blow out the transaction log)

  • Resolve the FHIROperationContext inconsistently available to Batch.java FHIROperationContext inconsistently available to Batch.java #2297

  • Convert the setting of Method Type to a string (instead of casting downstream)

  • Relax FHIROperationUtil getOutputParameters to not throw Exception

  • Add Postgres Function to Erase Data

  • Moved around the extreme version history test to last.

  • Updated the large version history test to create the versions with a bundle (up to 1100 versions)

  • Updated the create-database.sh logfilsiz 60000 to increase the transaction log size to support larger transaction where we delete lots of resources

  • Add Integration Test for $erase that works with the forbidden case (tenant1)

  • Added support at the Type level and include id,version in the Parameters object

  • Add fhir-persistence tests for Erase

  • Added concurrent-1.0 to the server.xml and reference/use in the EraseResourceDAO

  • ManagedExecutorService is used to dispatch a callable which can be terminated if it exceeds a timeout.

  • Changed eraseBean references for eraseDto

  • Update the formatting for the Postgres Function

  • Updated the README.md to add Description Column

  • Update the EraseOperationTest to use a ThreadPool when testing across all resources

  • Reduce processing time

  • Update the fhir-server-config.json to support default, tenant1, and tenant2

@prb112
feat(fhir-operation-erase): Introduce the operation
d015f72 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Merge branch 'main' into feat-erase
6978fa0
@prb112
Split the DAO/REST activities and fhir-operation-erase
924cff2 
- Added Code Coverage and Mock Implementations
- db2: Add a stored procedure to Erase multiple versions in a controlled
fashion. (must have a standard logfilsz so you don't blow out the
transaction log)
- Resolve the FHIROperationContext inconsistently available to
Batch.java #2297
- Convert the setting of Method Type to a string (instead of casting
downstream)
- Relax FHIROperationUtil getOutputParameters to not throw Exception
- Add Postgres Function to Erase Data
- Moved around the extreme version history test to last.
- Updated the large version history test to create the versions with a
bundle (up to 1100 versions)
- Updated the create-database.sh logfilsiz 60000 to increase the
transaction log size to support larger transaction where we delete lots
of resources
- Add Integration Test for $erase that works with the forbidden case
(tenant1)
- Added support at the Type level and include id,version in the
Parameters object
- Add fhir-persistence tests for Erase
- Added concurrent-1.0 to the server.xml and reference/use in the
EraseResourceDAO
- ManagedExecutorService is used to dispatch a callable which can be
terminated if it exceeds a timeout.

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Erase Operation
004ed5e 
- Issues resolved are:
	- Add GDPR FHIR Operation Support - $erase #850
	- FHIROperationContext inconsistently available to Batch.java #2297
	- Support $erase (hard delete) for a specific historical version of a
resources #2304

- Remove the timeout and count parameters in lieu of the
version-specific erase
- Update the README with clarity on the parameters
	- logicalId, version, patient, reason
	- clarified the HTTP Method, Api Paths, Response Codes, and Responses
	- Add Acceptance Criteria to explain VERSION delete
- Update the test harnesses for Db2 and Postgres to use the simple Erase
approach
- Clean up the DAO, JdbcEraseTest to move away from Timeout
- Removed patient/reason so we don't reflect illegal content.
- Change function/stored proc to use resource_type, logical_id only
- Refactor erase.json to support only specific values.
- Revert test.properties to logging false
- Improve IT/UT test coverage

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Merge branch 'main' into feat-erase
debeaa8
@prb112
Repair for a bad merge
6597e4f 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
update fhir-parent pom
fbd8ddd 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update for copyright year
07d03bc 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 changed the title Feature(erase): Implement Erase Operation feature(erase): Implement Erase Operation May 11, 2021
@prb112
Update EraseOperation IT Test and Support
af42a43 
- Update the EraseOperationTest to use a ThreadPool when testing across
all resources
- Reduce processing time
- Update the fhir-server-config.json to support default, tenant1, and
tenant2

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update EraseOperation IT Test and Support
cda8dfb 
- Cleanup on unneeded examples.

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Tweaked per Code Review
bbdc08a 
- Changed eraseBean references for eraseDto
- Update the formatting for the Postgres Function
- Updated the README.md to add Description Column

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update fhir-server-config for Integration Tests
acbf28b 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Reset test.properties
33736c2 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Fix for bad run in CICD
0355efb 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update for fhir-operation-erase review
f905919 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Update the create-database.sh and fork boot in surefire plugin
c37188c 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Updates per Code Review
5fc1d48 
- Important change is the fhir-server-config.json is necessary to use
the cache
- Update EraseResourceDAO to use the cache to get the ResourceType long
id
- Update the EraseTestMain to use CommonUtil
- Update JDBCEraseTest to inject the cache
- Update the pom.xml in fhir-server-test to use less memory

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Updates per Code Review
702627a 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Refactored per Showcase
442f544 
- Added an Integration test update to check Bundle Size
- Refactored the NOT_FOUND handler
- Review the README.md

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Refactored per Showcase
846a7a3 
- Updated the resource_change_log erase

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@lmsurpre lmsurpre self-requested a review May 13, 2021 19:53
lmsurpre
lmsurpre approved these changes May 13, 2021
View reviewed changes
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

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

LGTM

@prb112
Updated Resource Change Log Deletion
ff44a90 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Updated Resource Change Log Deletion for build failure
7611784 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112
Updated Resource Change Log Deletion for build failure
2f64d23 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
punktilious
punktilious approved these changes May 14, 2021
View reviewed changes
Copy link
Collaborator

@punktilious punktilious left a comment

Choose a reason for hiding this comment

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

LGTM

@prb112
Removing one query in the fun/sp per Code Review
61439c3 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 merged commit 9075c3b into main May 14, 2021
@prb112 prb112 deleted the feat-erase branch May 14, 2021 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbieste tbieste tbieste left review comments

@michaelwschroeder michaelwschroeder michaelwschroeder approved these changes

@punktilious punktilious punktilious approved these changes

@lmsurpre lmsurpre lmsurpre approved these changes

@JohnTimm JohnTimm Awaiting requested review from JohnTimm

@d0roppe d0roppe Awaiting requested review from d0roppe

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@prb112 @lmsurpre @punktilious @tbieste @michaelwschroeder