[#38405] - [5.0] Remove the last code artefacts for Microsoft SQL Server / SQL Azure databases
- Fixed in Code Base
- 23 Oct 2022
- Medium
- Build: 5.0-dev
- # 38405
- Diff
- richard67:5.0-dev-remove-sqlsrv
User tests: Successful: 0 Unsuccessful: 0
Pull Request for Issue # .
Summary of Changes
This pull request (PR) removes the last code artefacts for using Joomla with Microsoft SQL Server / SQL Azure databases.
These database types are not supported anymore since Joomla 4.0, but there is the "SqlsrvChangeItem" class left in the "Joomla\CMS\Schema\ChangeItem" namespace, which is of no use in J4 and should be removed with 5.0.0.
For deprecation see PR #38428 .
This PR removes that class and corresponding code:
- Remove file "SqlsrvChangeItem.php".
- Remove alias for that file in "classmap.php".
- Remove obsolete check for server type "mssql" in file "ChangeItem.php".
In addition, this PR removes an obsolete special handling for server type "mssql" in file "ChangeSet.php". The "sqlazure" subfolder doesn't exist anymore for core update SQL scripts since Joomla 4.0, and 3rd party extensions (which the database checker also checks since 4.0) are not expected to have any update SQL scripts for Microsoft SQL Server / SQL Azure.
All changes affect only the database checker ("System -> Maintenance -> Database", i.e. administrator/index.php?option=com_installer&view=database).
Additional information
The unit tests for the "Joomla\CMS\Schema\ChangeItem" namespace have never been migrated from Joomla 3 to Joomla 4, so in J4 we don't have any unit tests for the subject ot this PR.
I am preparing a PR to add unit tests for that namespace for the 4.2-dev branch (or maybe 4.3-dev), but that will still take some time until completed.
Testing Instructions
Code review, or make a new installation with the package built for this PR by drone and verify that the database checker works.
Actual result BEFORE applying this Pull Request
Obsolete code for Microsoft SQL Server / SQL Azure databases in the "Joomla\CMS\Schema\ChangeItem" namespace.
Expected result AFTER applying this Pull Request
No obsolete code for Microsoft SQL Server / SQL Azure databases in the "Joomla\CMS\Schema\ChangeItem" namespace.
The database checker works as well as without this PR.
Documentation Changes Required
None.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
Just clarified, will do it in 4.3-dev.
There are a few other places that refer to sqlserv that would be good to remove at the same time even if they are only comments
There are a few other places that refer to sqlserv that would be good to remove at the same time even if they are only comments
@brianteeman Where? Could you point me to these? But if they are outside the schema namespace, I would do that with another PR.
Note that the database drivers still support to connect to MS SQL Server databases as external database. Only Joomla running on it is not supported anymore.
/administrator/components/com_joomlaupdate/src/Model/UpdateModel.php#L1173
/administrator/components/com_joomlaupdate/src/Model/UpdateModel.php#L1199
/administrator/components/com_menus/src/Model/ItemModel.php#L788
@brianteeman /administrator/components/com_joomlaupdate/src/Model/UpdateModel.php#L1199 should be kept, otherwise the complete function would not make sense and should be removed. But we might keep the function for later use when we again remove support for some database type and need that to be checked before updating, and if we keep it we should keep the complete code so later readers can understand it.
I have tested this item
I used the package built for this PR to test. DB checker works.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38405.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38405.
@richard67 can you please solve the conflicts please?
And maybe write the deprecation and removal documentation in manual? If not no problem I try to write it tomorrow.
| Labels |
Added:
?
PR-5.0-dev
|
||
@HLeithner I've updated the branch and resolved the conflict. The failing appveyor is not related to this branch, it fails also in the 5.0-dev branch with the same error:
Error parsing appveyor.yml: (Line: 52, Col: 1, Idx: 1809) - (Line: 52, Col: 9, Idx: 1817): Duplicate key
Regarding deprecation docs: I don't know how to do that and won't learn that in the next days or weeks. So feel free to do that. But I do not see any need for any deprecations docs for the following reasons:
- Before J4 the database schema checker only worked for the CMS core but not for 3rd party extensions.
- Beginning with J4, including all pre-releases from 4.0.0-alpha1 on, the core does not support anymore to run on an MS SQL Server or MS Azure database.
- Therefore it is impossible that the database schema checker ever has been used with such a database in J4, neither by the core nor by any 3rd party extension.
So there is no need for any deprecation message. It absolutely makes no sense.
P.S.: It seems also drone is broken in the 5.0-dev branch. It fails with some error related to certificates.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-10-23 07:59:43 |
| Closed_By | ⇒ | HLeithner |
Thanks
I have tested this item✅ successfully on 96b73ca
Database checker works after patch applied.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38405.