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

[IAppConfig] returns non lazy value while searching for lazy one #43247

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Jan 31, 2024

There is this possible situation where your part of code is using already existing config value. This config value needs to be switched to lazy to improve performance, but the only way to store a value as lazy is:

  • set a value using function like setValueString() with the argument lazy: true and the config value itself.
  • using updateLazy() on the app/key pair without the need of the content of the config value.

the updateLazy() could be run at one point of the migration, or later on, but there is room for a use case where an app is requesting a lazy config value still stored as non lazy in the database

@ArtificialOwl ArtificialOwl requested review from nickvergessen, artonge, a team, icewind1991, Altahrim and come-nc and removed request for a team January 31, 2024 23:12
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Sounds like this can be easily added as a unit test? :)

@nickvergessen nickvergessen added bug 3. to review Waiting for reviews labels Feb 1, 2024
@nickvergessen nickvergessen added this to the Nextcloud 29 milestone Feb 1, 2024
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/load-non-lazy-config-value branch from a855e7d to 8ba2cd4 Compare February 6, 2024 01:01
@ArtificialOwl ArtificialOwl merged commit 5238b1c into master Feb 6, 2024
141 checks passed
@ArtificialOwl ArtificialOwl deleted the enh/noid/load-non-lazy-config-value branch February 6, 2024 01:25
@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Feb 6, 2024

Sounds like this can be easily added as a unit test? :)

public function testGetNonLazyValueStringAsLazy(): void {
$config = $this->generateAppConfig();
$this->assertSame('value', $config->getValueString('non-sensitive-app', 'non-lazy-key', 'default', lazy: true));
}

Added in #43370

@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants