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.

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

XWIKI-22665: Required rights should be cached #3695

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

michitux
Copy link
Contributor

@michitux michitux commented Nov 26, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-22665

Changes

Description

  • Introduce a new SimpleDocumentCache for caching values associated with document references.
  • Use the cache in the DocumentRequiredRightsManager.
  • Return static instances in DocumentRequiredRightsReader to reduce the memory usage of the cache.
  • Remove DocumentRequiredRightsManager from the page test component list and instead add a mock in MockitoOldcore.
  • Move DocumentRequiredRightsManager to the authorization bridge as it fits better there and with the mock implementation in MockitoOldcore there is no need to have it available for tests.

Clarifications

  • I decided not to use the existing DocumentCache as the internal logic for having several values per key vastly exceeds the memory requirements of the required rights cache.

Screenshots & Video

No UI changes.

Executed Tests

LANG=C.UTF-8 mvn clean install -Pdocker,legacy,integration-tests,snapshotModules,quality -pl $(git diff --name-only  --merge-base origin/master | awk -F'/' '{for(i=NF-1; i>0; i--){if($i ~ /src$/){printf(":%s\n", $(i-1)); break;}}}' | sort -u | paste -sd "," -),:xwiki-platform-legacy-oldcore

Manual test of the changed modules in a 17.0.0-SNAPSHOT demo instance.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • stable-16.10.x

* Introduce a new SimpleDocumentCache for caching values associated with
  document references.
* Use the cache in the DocumentRequiredRightsManager.
* Return static instances in DocumentRequiredRightsReader to reduce the
  memory usage of the cache.
* In the mock implementation, handle the null case similar to the real
  implementation.
@michitux michitux merged commit d9830e7 into xwiki:master Nov 29, 2024
1 check passed
@michitux michitux deleted the XWIKI-22665 branch November 29, 2024 11:16
github-actions bot pushed a commit that referenced this pull request Nov 29, 2024
* Introduce a new `SimpleDocumentCache` for caching values associated with document references.
* Use the cache in the `DocumentRequiredRightsManager`.
* Return static instances in `DocumentRequiredRightsReader` to reduce the memory usage of the cache.
* Remove `DocumentRequiredRightsManager` from the page test component list and instead add a mock in `MockitoOldcore`.
* Move `DocumentRequiredRightsManager` to the authorization bridge as it fits better there and with the mock implementation in `MockitoOldcore` there is no need to have it available for tests.

(cherry picked from commit d9830e7)
michitux added a commit that referenced this pull request Nov 29, 2024
* Introduce a new `SimpleDocumentCache` for caching values associated with document references.
* Use the cache in the `DocumentRequiredRightsManager`.
* Return static instances in `DocumentRequiredRightsReader` to reduce the memory usage of the cache.
* Remove `DocumentRequiredRightsManager` from the page test component list and instead add a mock in `MockitoOldcore`.
* Move `DocumentRequiredRightsManager` to the authorization bridge as it fits better there and with the mock implementation in `MockitoOldcore` there is no need to have it available for tests.

(cherry picked from commit d9830e7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants