Stop using getService for lexicon, registry, error, smarty, mail, and hashing - #16985
Open
Ibochkarev wants to merge 6 commits into
Open
Stop using getService for lexicon, registry, error, smarty, mail, and hashing#16985Ibochkarev wants to merge 6 commits into
Ibochkarev wants to merge 6 commits into
Conversation
Register modError during initialize next to registry (lexicon already comes from _initCulture). Replace core/manager getService call sites for those three services with the DI container or synced properties. Part of the series planned on modxcms#15986 (tier 1).
Register hashing eagerly and mail/smarty as lazy shared DI services during initialize, then replace remaining core/manager call sites so tiers 1–2 of modxcms#15986 ship in one PR.
Ibochkarev
changed the title
Extract _registerCoreServices, add getSmarty/getMail/getManagerTemplatePath, drop duplicated smarty bootstraps and dead widget ensures, and simplify Reload registry handling.
Do not pre-register mail/smarty (keeps getService param+property sync), ensure widgets call getSmarty(), sync properties in modX::getService, and resolve hashing on plain xPDO without touching a missing property.
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
1 participant
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.
What changed and why
getService()is deprecated, but core and manager still used it as a lazy factory for built-in services. This PR finishes tiers 1–2 from the plan on #15986: stop calling it for six names incore/srcandmanager.That work sits on the MAB-03 stack (xPDO 3 via Composer, autoloading, fewer map-era habits). Here we push the same direction for runtime services: register them on the DI container / accessors instead of the old xPDO service locator.
Tier 1 —
lexicon,registry,errorregisterCoreServices()registersregistry,error, andhashingduringinitialize()(lexiconalready comes from_initCulture)$modx->services->get(…)or the synced$modx->registry/$modx->lexicon/$modx->errorpropertiesTier 2 —
smarty,mail,hashinghashingis eager (password paths need it)getSmarty()/getMail()/getManagerTemplatePath()are the canonical accessors; smarty/mail register on first usegetSmarty($path)modX::getService()still works for extras/tests and syncs$modx->$namewhen the service was already in the containerGrep gate for those six names under
core/srcandmanageris empty.How to test
rg "getService\s*\(\s*['\"](lexicon|registry|error|smarty|mail|hashing)" core/src manager→ emptyRelated issue(s)/PR(s)
fileHandler/phpthumb/archive/translit, extension packages, then stronger pressure ongetServiceitselfCompatibility notes
Extras that still call
getService()for these names keep working. Core prefers accessors and container/$modx->*properties after a normalinitialize().Breaking change assessment
No public signature removals. New helpers:
getSmarty(),getMail(),getManagerTemplatePath(),registerCoreServices().Calling
getService('lexicon')beforeinitialize()/_initCulture()was already unsupported.Test coverage
Controller test harnesses now use
getSmarty(). No new dedicated unit tests. Grep gate + PHPUnit + the manual smoke list above.Contributors
Thanks @BobRay for filing #15986 and pointing out the container did not already hold these services.
AI tool use
Cursor helped inventory call sites, apply the changes, and draft this PR. Human review still required.