fix(nextcloud_occ_app_config): compare array values as JSON (idempotency)#272
Merged
Merged
Conversation
…s JSON Nextcloud stores an array config value and returns it as a parsed JSON array (verified against Nextcloud 33: config:list yields ["alpha", "beta"]). The module stringified that list with str() (Python repr, single quotes) and compared it against the user's array literal, which never matched - so the module reported a change and re-ran config:app:set on every run. Compare array values as parsed JSON instead (values_match()), and store the cached current value as canonical JSON. Add unit tests for the helper and for the cached (installed_config_json) idempotency path. The occ output formats were verified empirically in a Nextcloud podman container.
951b0db to
3831de9
Compare
ebuerki-lf
pushed a commit
that referenced
this pull request
May 26, 2026
…s JSON (#272) Nextcloud stores an array config value and returns it as a parsed JSON array (verified against Nextcloud 33: config:list yields ["alpha", "beta"]). The module stringified that list with str() (Python repr, single quotes) and compared it against the user's array literal, which never matched - so the module reported a change and re-ran config:app:set on every run. Compare array values as parsed JSON instead (values_match()), and store the cached current value as canonical JSON. Add unit tests for the helper and for the cached (installed_config_json) idempotency path. The occ output formats were verified empirically in a Nextcloud podman container.
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
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.
Last of the deferred behavior-changing fixes.
Bug
For an
arrayconfig value the module comparedstr(<python list>)(e.g."['alpha', 'beta']", Python repr with single quotes) against the user's array literal ('["alpha","beta"]'). They never matched, so the module reportedchanged=trueand re-ranocc config:app:seton every run - non-idempotent.Empirical verification (podman)
Verified against a Nextcloud 33 container:
config:app:set core k --value='["alpha","beta"]' --type=arraythenconfig:list core --output=json --privatereturns the value as a parsed JSON array:["alpha", "beta"](a Python list).config:app:get core k --details --output=jsonreturnsvalueas the JSON string"[\"alpha\",\"beta\"]"(whitespace-sensitive).Fix
values_match()comparesarrayvalues as parsed JSON (both the cached/list path and the live JSON-string path), so equal arrays match regardless of whitespace or Python-vs-JSON quoting. The cached current value is now stored as canonical JSON.Tests
values_match()unit tests (equal-ignoring-whitespace, different, invalid-JSON, non-array).main()via theinstalled_config_jsoncache path (no occ needed): an already-set array is idempotent; a differing array reports a change.Validation
Full tox matrix green (103 tests across 13 envs);
ansible-docrenders; pre-commit green.This completes the deferred behavior-changing fixes (the
get_item_by_idcontract was dropped: the current by-id "fail if missing" behavior is correct and documented, only the error message is raw - changing it risks the by-name create-on-not-found idempotency).