Skip to content

[7.0] Remove deprecated getDbo()/setDbo() from Table class#47440

Open
Hackwar wants to merge 8 commits into
joomla:7.0-devfrom
Hackwar:7.0-table-databaseawaretrait
Open

[7.0] Remove deprecated getDbo()/setDbo() from Table class#47440
Hackwar wants to merge 8 commits into
joomla:7.0-devfrom
Hackwar:7.0-table-databaseawaretrait

Conversation

@Hackwar
Copy link
Copy Markdown
Member

@Hackwar Hackwar commented Mar 20, 2026

  • I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.

Summary of Changes

As with everywhere in Joomla, we shouldn't be implementing custom database object setters and getters and instead use the DatabaseAwareTrait instead. The Table class has the methods getDbo(), setDbo(), getDatabase() and setDatabase() to retrieve the db object. The first 2 are supposed to be removed and the later 2 are only there to make the transition to the trait smoother. With the removal of the former 2, we can completely rely on the trait and remove the later 2 as well.

Testing Instructions

Codereview

Link to documentations

Please select:

Comment thread libraries/src/Helper/TagsHelper.php Outdated
@HLeithner HLeithner added Feature b/c break This item changes the behavior in an incompatible why. HEADS UP labels Apr 2, 2026
Copy link
Copy Markdown
Member

@HLeithner HLeithner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the right or best solution

@Hackwar
Copy link
Copy Markdown
Member Author

Hackwar commented Apr 2, 2026

What exactly do you mean?

@Hackwar
Copy link
Copy Markdown
Member Author

Hackwar commented Apr 3, 2026

Yes, replacing $table->getDbo() with Factory::getDbo() is bad, but since getDatabase() is not public and since the Tagshelper class already contains 13 other cases where it uses Factory::getDbo(), I'm willing to take the fall here and replace one deprecated method with another deprecated method. At least then we have one issue solved entirely.

@Hackwar Hackwar marked this pull request as ready for review April 3, 2026 18:51
@Hackwar Hackwar requested a review from HLeithner April 15, 2026 10:26
* Use setDatabase() and getDatabase() instead
* Example: $this->setDatabase($db);
*/
protected $_db;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not make a magic getter for another major?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've kept this around for a very long time, so I don't think extending the deprecation period further would be of any help.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public function addTagMapping($ucmId, TableInterface $table, $tags = [])
{
$db = $table->getDbo();
$db = Factory::getDbo();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load at least the database from the container

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole code actually needs to be refactored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

b/c break This item changes the behavior in an incompatible why. HEADS UP Feature PR-7.0-dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants