JTable::store changes proposal

[84.le0n]

Hi all,
during some tests for PostgreSQL integration inside CMS I've found
some problems that lead to JTable.
Less important thing is that this class contain call to deprecated
JException class.

Now the big problem.

Inside these three files, called when installing component inside CMS
libraries/joomla/installer/adapters/component.php
libraries/joomla/installer/adapters/file.php
libraries/joomla/installer/adapters/module.php
there are calls to "$db->insertid()" after JTable::store().

The problem is represented by PostgreSQL's insertid that needs to know
last "INSERT INTO" query to get its table, so, as I've added inside
example block of insertid's call, insertid MUST be run always after
"INSERT INTO", else it fails.

So the correct way is when JTable::store runs
"$this->_db->insertObject" (line 609) without doing other queries.
Problems occur when JTable::store goes to "Asset tracking" part (line
631) where is present another store call (line 669) and, worse case
for postgresql, a new UPDATE JDatabaseQuery (line 680) will overwrite
last "INSERT INTO", so insertid will fail.

Other than this, if instead JTable::store runs
"$this->_db->updateObject" (line 605), insertid will always fail
because there wasn't any "INSERT INTO" query and, logic question, why
do we run "insertid" in these three files after an update ?

Managing that part from line 613 to 631 (between insert/update and
"Asset tracking"), my proposal is to change JTable::store behavior,
returning an integer coming from insertid call if there was an
insertion, true if was an update or throwing exception if there was an
error.
Doing so a caller could know if there was an error or, if there
wasn't, wich is last ID inserted.

Hoping to have explained this situation, I'm waiting your thoughts.

Eng. Gabriele Pongelli.

[Carlo Pires]

Gabriele,

I think the use of insertid outside JDatabase::insertObject is dangerous and can lead to problems like this.

The right solution is to remove all insertid calls outside JDatabase::insertObject and fix the other modules to extract the KEY from the row inserted. To set/verify the KEY in row will be a responsibility of JDatabase::insertObject. The others drivers should be revisited to make JDatabase::insertObject to set the KEY in row inserted.

[Johnathan Hair]

Thank you so much. I have been banging my head wondering if I coded incorrectly. I'm just overloading the method store on JTable to remove asset tracking until we get a fix for a standalone application.