refactor(encryption): Migrate appconfig keys to typed bool IAppConfig with repair step#60002
Open
cuppett wants to merge 6 commits intonextcloud:masterfrom
Open
refactor(encryption): Migrate appconfig keys to typed bool IAppConfig with repair step#60002cuppett wants to merge 6 commits intonextcloud:masterfrom
cuppett wants to merge 6 commits intonextcloud:masterfrom
Conversation
cuppett
requested review from
Altahrim,
ArtificialOwl,
leftybournes and
salmart-dev
and removed request for
a team
cuppett
added
3. to review
6 tasks
cuppett
commented
Apr 30, 2026
Contributor
Author
cuppett
left a comment
cuppett
left a comment
There was a problem hiding this comment.
I don't know the convention when trying to refactor the legacy values out. Happy to simplify to only the minimal set or wait until next version for another PR. Just LMK.
CarlSchwan
reviewed
Apr 30, 2026
CarlSchwan
reviewed
Apr 30, 2026
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
864ad6a to
f3679b4
Compare
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
f3679b4 to
39b6d31
Compare
artonge
reviewed
Apr 30, 2026
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
a6fff8f to
e4f6ece
Compare
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
4 times, most recently
from
e5291af to
07461ee
Compare
Contributor
Author
|
The psalm things (DAV) seem to be coming from the master branch (not related to this PR). Do I need to fix that or can it be overridden to merge? |
artonge
approved these changes
May 1, 2026
Collaborator
artonge
left a comment
artonge
left a comment
There was a problem hiding this comment.
Looks good, thanks for splitting :)
The psalm error is indeed unrelated.
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
6e52b67 to
c4baa76
Compare
artonge
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
c4baa76 to
ef7b789
Compare
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
ef7b789 to
9f0c402
Compare
… with repair step Switch all encryption config reads/writes from deprecated string-typed IConfig to bool-typed IAppConfig (getValueBool/setValueBool). Adds RetypeEncryptionConfigKeys repair step to retype existing string values to bool on upgrade. Includes lazy IAppConfig resolution in Manager and AppConfigTypeConflictException fallbacks throughout for safety during the upgrade window. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
Signed-off-by: Stephen Cuppett <steve@cuppett.com>
Apply suggestion from @artonge Co-authored-by: Louis <louis@chmn.me> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
Co-authored-by: Louis <louis@chmn.me> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
The IAppConfig API converts stored values to bool on read (getValueBool) and re-stamps the type on write (setValueBool), so legacy string-typed encryption config keys migrate lazily without an explicit repair step. Per PR review feedback, drop the repair step, its test, and the related AppConfigTypeConflictException fallback in Encryption\Manager::isEnabled that only existed to bridge the now-unneeded migration window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
… to match strict 'yes'-only validation The verifyConfigKey check on core.encryption_enabled was reverted to master's strict $value !== 'yes' in 626fadd per review feedback, but the test data providers still asserted the broader truthy set (1/true/YES/on). Drop those entries so the tests match the controller. This is validation, not storage — IAppConfig::setValueBool's broader input handling is unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
cuppett
force-pushed
the
cuppett/migrate-appconfig-keys-to-bool
branch
from
9f0c402 to
1db5b05
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Switch all encryption config reads/writes from deprecated string-typed IConfig to bool-typed IAppConfig (getValueBool/setValueBool). Adds RetypeEncryptionConfigKeys repair step to retype existing string values to bool on upgrade. Includes lazy IAppConfig resolution in Manager and AppConfigTypeConflictException fallbacks throughout for safety during the upgrade window.
conflict between new type (mixed) and old type (boolean)#58778Checklist
3. to review, feature component)stable32)AI (if applicable)