Support partial paths in config:show (#39610) - #41170
Open
lbajsarowicz wants to merge 5 commits into
Open
Conversation
…erage Rebased rogerdz's fix onto current 2.4-develop, dropping the header-rewrite hunk that conflicted with the locale-emulator wrapper added upstream since. Suppressed PHPMD's now-legitimate UnusedPrivateField warning on $pathValidatorFactory (kept for constructor BC) and marked it @deprecated. Tightened three unit tests to assert PathValidator is never invoked anymore and that a path resolving to nothing still throws ValidatorException deterministically, instead of relying on implicit mock defaults.
The revived fix replaced the path validator with an empty() guard, which reports every disabled flag (web/seo/use_rewrites and the rest of the tree, all stored as "0") as a non-existent path. Check explicitly for the null/''/[] that ConfigSourceInterface::get() returns for a missing path.
Contributor
Author
|
@magento run all tests |
|
Hi @lbajsarowicz. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
5 tasks
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.
Description
bin/magento config:showrejects partial configuration paths.config:show catalog/frontendfails even though the path resolves to a perfectly valid subtree, becauseConfigShowCommand::execute()runs the path throughMagento\Config\Model\Config\PathValidatorfirst, and that validator only accepts complete field paths registered in thesystem.xmlstructure.This removes the pre-validation and instead reports an unknown path from the resolved value, so partial paths print their subtree while a genuinely unknown path still fails with
ValidatorException.This PR continues #39711 by @rogerdz, rebased onto current
2.4-develop. Their commits and authorship are preserved. The PR was approved by @Den4ik in March 2025 and then never picked up by triage — it never received aProgress:label.Changes made on top of the original
A stored
"0"is a value, not a missing path. The original replaced the validator with anempty($configValue)guard. Every disabled flag in the config tree is stored as"0", so that would have madeconfig:show web/seo/use_rewrites— and everything like it — fail with "The path doesn't exist".ConfigSourceInterface::get()is documented as returningstring|array, and returns''or[]for a missing path, so the check is now explicitly againstnull/''/[]. Added a regression test for it.PathValidatorFactoryis retained. It is now unused, but the constructor parameter stays so the signature remains backward compatible; the class-level@SuppressWarnings(PHPMD.UnusedPrivateField)documents why.Test coverage tightened. The shared mock now asserts
PathValidatoris never invoked, so a future change cannot silently reintroduce the pre-validation, andtestConfigPathNotExistmocks the resolution explicitly instead of relying on PHPUnit's implicit null return.Fixed Issues
Fixes #39610
Manual testing scenarios
bin/magento config:show catalog/frontendBefore: The "catalog/frontend" path doesn't exist. Verify and try again. After: the subtree under that path is printed.
bin/magento config:show web/seo/use_rewriteson a store where rewrites are disabled — confirm it prints0rather than reporting a missing path.bin/magento config:show some/made/up/path— confirm it still fails with The path doesn't exist.bin/magento config:show web/secure/base_url --scope=websites --scope-code=base— confirm scoped lookups still work.Questions or comments
One deliberate behaviour change for reviewers to weigh:
config:showno longer validates the path against thesystem.xmlstructure at all, only that it resolves to something. A path that is not a declared field but happens to have a stored value incore_config_datawill now be printed where it previously errored. That is the direct consequence of supporting partial paths, and matches what the issue asks for, but it is a real widening of what the command accepts.Gates run locally on
2.4-develop(Warden, PHP 8.3):Config/Test/Unit/Console/Command/ConfigShowCommandTest.php— 8 tests pass.Magento2: clean. PHPStan level 1: no errors.ConfigShowCommandis@api, but only method bodies change — no signature is touched.Contribution checklist