Fix broken tests by refershing materalized views #2699
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
780b38c to
1527c2c
Compare
522ac8d to
22b9428
Compare
cac7a84 to
563d472
Compare
DANGER: the after_create hooks will trigger materialized view refreshes potentially en masse, which could have a potentially profound impact on the database as opposed to refreshing materialized views from the test angle.
563d472 to
57e78d7
Compare
|
Is this still relevant @codihuston? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Desired Outcome
Further resolves broken tests after the addition of materialized views. This PR contains two commits that contain two different approaches to resolving the issues.
The materialized views introduces a form of caching for some compute expensive operations for authorizing a role to a resource. As a result, this cache needs to be rehydrated / invalidated and repopulated on a per-policy load basis. This is fine, and should function out-of-box in production.
However, our testing infrastructure has an amalgamation of integration and end-to-end tests across both rspec and cucumber that leverage the models and/or repository classes directly to persist records into the database outside of a policy load. Thus, we need to
Commit 1: Attempts to refresh the materialized views from the tests, outside of the application code.
Commit 2: Attempts to refresh the materialized views from the application code using the sequel gem lifecycle hooks.
Both of these have tradeoffs. The former is a bit of a code smell that will sour the developer experience, as they have to remember to refresh these views explicitly. The latter may incur application performance implications, as loading policy that will create multiple resources may trigger multiple refreshes per policy load, which could have a profound impact on the database. We would also need to account for the update/destroy hooks as well.
Ideally, we would clean up our testing infrastructure such these resource creations are only occurring through policy load as opposed to using the model / repository classes.
Implemented Changes
Describe how the desired outcome above has been achieved with this PR. In
particular, consider:
Connected Issue/Story
Resolves #[relevant GitHub issue(s), e.g. 76]
CyberArk internal issue ID: [ONYX-30396
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
READMEs) were updated in this PRBehavior
Security