[#36511] - [4] Bye bye getDBO() (and array() in the same files)
- Closed
- 6 Mar 2022
- Medium
- Build: 4.0-dev
- # 36511
- Diff
- PhilETaylor:getdbo
User tests: Successful: Unsuccessful:
Replacement for #35888 and #36502
Summary of Changes
-
The manual check of every "getDBO" string in the file base, replacing those calls where appropriate to use
Factory::getContainer()->get('DatabaseDriver'), some could already use the local class property$this->_dbso those have been refactored, and some were calling getDBO multiple times where not needed, so those have been cleaned up too -
In the files modified when completing 1 above, Some return types in docblocks have been updated to reflect the true state of the current code, as they were highlighted in red in my IDE - ONLY in the files modified in 1 above.
-
some minor rearranging and refactoring of code around the getDBO calls to make them flow better, and be "nicer"
-
Manual change of
array()to[]syntax for ALL arrays - ONLY in the files modified in 1 above.
This initial PR is 3.0 hours work.
Testing Instructions
Code Review
Test everything multiple times :-)
Actual result BEFORE applying this Pull Request
Technical Debt
Expected result AFTER applying this Pull Request
Less Technical Debt
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin com_associations com_banners com_categories com_config com_contact com_content com_contenthistory com_fields |
| Labels |
Added:
?
|
||
ok the fringe case is the installation app.
When in the installation app the databasedriver in the container is not connected to the db, and the first step of the installation uses its own local database connection var.
Its impossible to replace the container resource with a connected db, so, as this is a total exception, I have passed through the $db from the local var, to the LanguageHelper::getInstalledLanguages call and heavily documented why.
Drone should be happier now.
Phew - build passing again :)
So this one's a bit tricky for me. Whilst Factory::getContainer()->get('DatabaseDriver') is the direct replacement. the intention is obviously where possible to also use the container as well... as container :) and where possible start to inject the database object into the classes (obviously that only works for the non-static methods/classes - in the rest we probably substitute the container retrieval)
This replaces the direct deprecated code with fully supported future code. Another PR can be made in the future to use dependency injection instead. This PR removes the ton of deprecated code which would end up otherwise just getting logged I’m creating huge deprecated log files
Conflicts resolved.
Too complex to merge it seems, and now have conflicts for the second time.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-06 21:06:39 |
| Closed_By | ⇒ | PhilETaylor | |
| Labels |
Added:
Conflicting Files
?
Removed: ? |
||
Something broke with installation...