@Fedik fof is an external project not owned / controled by Joomla ;) It is owned by Nicholas K. Dionysopoulos / Akeeba Ltd. Please report any issues to the original repo / owner: https://github.com/akeeba/fof/tree/fof-2.x

Thanks.

ok, thanks

@nikosdion say it Joomla bug .. @zero-24 can you please reopen this one?

I am cross-posting here from my reply on the issue opened on FOF

That's a bug in Joomla! and I believe @mbabker has already told you. The updateObject's signature says that the method returns a boolean. See https://github.com/joomla/joomla-cms/blob/5c0f405d5f4f9a26e275bec375cefe641cb209b5/libraries/joomla/database/driver.php#L1838-L1838

Please fix Joomla!. FYI the problem is that JDatabaseDriverPdo::execute() returns an object (instead of boolean) when a non-SELECT statement is executed. As a result you should overload JDatabaseDriverPdo::updateObject (and insertObject) with something like return (parent::updateObject(.....) !== false).

I mean, guys, it's trivial. The method signature promises to return a boolean. We rely on that published signature in our code, checking if the result is boolean true. I have no idea why you insist on sending people to us when the bug in Joomla!'s code is so obvious...

In Tobias' defense the initial report, without any deep digging into the issue or awareness of the misbehavior (I'm pretty sure I've seen it a few times, moaned about it in closed channels, and moved on without thinking about a fix), the opening post does lean heavily toward sending the report to FOF.

similar issue #8322 .. but there was fixed locally, by cast the result to the boolean

That issue (#8332) is an architecturally incorrect approach. It assumes that the method signature is not to be trusted (wtf?!!!) and instead tries to monkey a solution around the actual bug.

And that's how every Joomla! developer worth their salt have resorted to building their own frameworks.

I'm very sorry. Thanks for correcting me.

@zero-24 @Fedik This issue should be retitled
something like JDatabaseDriverPdo::updateObject PDO Object not a boolean

Done. Thanks

This probably needs to be tossed into the 4.0 pile because it technically constitutes a B/C break (even though the current return contradicts the documented type). One of those existing behavior versus documented standard judgement calls.

Actually, the documentation on JDatabaseDriver::execute() (and its subclasses' implementations) is wrong too as the method no longer returns a boolean on an error condition but throws exceptions; it'll only return a database resource.

In general, anything returning the result of $this->execute() in the database layer right now will return the database resource object (it's not unique to PDO either). In JDatabaseDriver there are 3 methods doing this; updateObject() (documented to return a boolean), alterDbCharacterSet, and createDatabase (both of those documented to return strings).

So there's a whole heap of code in the database layer that's inconsistent with the documentation and it needs to be decided whether the documentation should be changed to reflect the existing behavior or if it should be noted that a B/C break will occur at 4.0 and the returns will change to what's documented.

@nikosdion @mbabker - just ran in to this one (using j2store with PDO).

You both have valid points. But unless I'm missing something obvious, could this not be "fixed" for now by simply changing line 1298 in the FOF table.php store() from "if ($result !== true)" to "if ($result === false)"?

https://github.com/joomla/joomla-cms/blob/staging/libraries/fof/table/table.php#L1298

For 3.x support only you'd need a try/catch around $db->execute() calls (or anything inherently calling it). The execute methods won't return a boolean false.

So at least with ext/mysql and ext/mysqli, the return values from mysql(i)_query() are either a resource object for queries returning a result set, boolean true for other statement types, or boolean false on fail. A check is made in the driver for a false return and throws an exception otherwise returns the query result (either the resource object or boolean true). It looks the same for ext/pgsql and ext/sqlsrv. In the PDO stack, a check is made against whether the stored PDOStatement object was executed and said object returns on success.

So long and short we still have a documentation issue, but since I've been messing with database code the last couple of weeks I can say it's not as bad as I implied a couple of months ago; just hugely inconsistent. In either case, a check against a boolean false return isn't suitable. $db->insertObject() triggers $db->execute() without a try/catch so it'll never reach the return false condition and $db->updateObject() just returns $db->execute() so you hit the wall of text above for the possible return conditions.

And then it gets tricky. IIRC FOF 2 supports Joomla 2.5, which doesn't have exception handling but boolean returns. So in that case you need a try/catch for the 3.x code and the checks for boolean returns for 2.5 code.

Right now FOF 2 isn't supported anymore. So any change you make into the copy in J! 3 won't be a problem.

Ah, OK. I submitted a PR on the 2.0x branch just now ...

akeeba/fof#610

Should I submit that on the cms instead?

@mbabker @nikosdion - any opinion on which repo changes to FOF 2 should be PR'ed against?

@mbabker - ping ...

-- hugh

Just make them here for the reason commented above.

@mbabker If this issue still relevant, or should it be closed?

Honestly, I don't remember exactly what the problem is/was.

If this PR get no Response, it will be closed at 23th July 2017.

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/8525

closed as stated above.