Skip to content

Conversation

@prabhask5
Copy link
Contributor

Description

Finishing previous PR here: #4314

Currently, our system only allows for the invalidation of the entire user authentication cache, which can lead to numerous cache misses and inefficiencies. This change is required to allow for more precise cache management, specifically targeting stale cache entries at the individual user level without disrupting the cache state of other users

Category: New feature
Why these changes are required?
Currently, our system only allows for the invalidation of the entire user authentication cache. This change is required to allow for more precise cache management, specifically targeting stale cache entries at the individual user level without disrupting the cache state of other users.
What is the old behavior before changes and new behavior after changes?
Previously, invalidating a user's cache required clearing the entire cache, affecting all users. The new behavior introduces an endpoint that allows for the invalidation of cache entries on a per-user basis, thereby maintaining cache integrity for other users and reducing unnecessary cache misses.

Issues Resolved

#2829

Is this a backport? If so, please add backport PR # and/or commits #
no

Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here

Testing

Unit testing was written in FlushCacheApiTest.java

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

dlin2028 and others added 5 commits July 16, 2024 07:38
Signed-off-by: David Lin <dlin2028@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Prabhas Kurapati <prabhask@berkeley.edu>
@prabhask5
Copy link
Contributor Author

@cwperks This should be a better PR, I'm not sure why there's integration test errors I don't change them, can you help me out with that? It should be the same test errors as in #4568

@cwperks
Copy link
Member

cwperks commented Jul 16, 2024

Thank you @prabhask5 ! Looking at the integ test failures now.

10000-ki and others added 2 commits July 16, 2024 14:02
@cwperks
Copy link
Member

cwperks commented Jul 17, 2024

@prabhask5 Can you rebase this PR with the latest from main that has a fix for the compilation error?

dlin2028 and others added 6 commits July 17, 2024 11:58
Signed-off-by: David Lin <dlin2028@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Prabhas Kurapati <prabhask@berkeley.edu>
Signed-off-by: Prabhas Kurapati <prabhask@berkeley.edu>
@cwperks
Copy link
Member

cwperks commented Jul 25, 2024

@prabhask5 For the issue with FlushCacheApiIntegrationTest.testFlushCache there are 2 problems:

  1. In FlushCacheApiAction you need to modify the response message if the param username is non-null. i.e.
if (username != null) {
    LOGGER.debug("Cache invalidated for user: " + username);
    ok(channel, "Cache invalidated for user: " + username);
} else {
    LOGGER.debug("cache flushed successfully");
    ok(channel, "Cache flushed successfully.");
}

You would also need to move the extraction of this param up one level in scope.

  1. This line is using the wrong response object. It should be referencing deleteUserCacheResponse instead of deleteAllCacheResponse.

@DarshitChanpura
Copy link
Member

@prabhask5 Closing this PR since there hasn't been activity since 1 month. Please feel free to open a new PR if you intend to continue working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants