Prestashop: function "existsInDatabase" in "ObjectModel" discrepancy with "orders" table

[roncarino]

Describe the bug

Function existsInDatabase inside ObjectModel.php use $id_entity and $table as parameters.
In case of orders this call will throw a PrestaShopDatabaseException because the query won't find any fields called id_orders.
In Prestashop current design it has been chosen orders (plural version) instead of order (singular version) which means the query will become something like this

SELECT `id_orders` as id
FROM `prefix_orders` e
WHERE e.`id_orders` = $id LIMIT 1

Expected behavior

As you can understand from above explanation this might be either a database design error or a missed update on the function existsInDatabase. So there are 2 potential fixes:

The first one will necessarily face changing the name table from orders to order in order to have a match with its auto incremental id.

The second case will see an update of the function itself from:

public static function existsInDatabase($id_entity, $table)
    {
        $row = Db::getInstance()->getRow('
            SELECT `id_' . bqSQL($table) . '` as id
            FROM `' . _DB_PREFIX_ . bqSQL($table) . '` e
            WHERE e.`id_' . bqSQL($table) . '` = ' . (int) $id_entity, false
        );

        return isset($row['id']);
    }

to something like

public static function existsInDatabase($id_entity, $table, $field)
    {
        $row = Db::getInstance()->getRow('
            SELECT `id_' . bqSQL($field) . '` as id
            FROM `' . _DB_PREFIX_ . bqSQL($table) . '` e
            WHERE e.`id_' . bqSQL($field) . '` = ' . (int) $id_entity, false
        );

        return isset($row['id']);
    }

This means an extra parameter where you can define the field of the table of interest. I feel like it is more a database design error so the first solution would be the best one, looking at the current database design it looks like it has been chosen the _singular version naming rule_ for the majority of the tables.

Note

I didn't check other broken case scenarios, with other kind of entities.

Steps to Reproduce

Working example:
Product::existsInDatabase(1, 'product');

Broken example:
Order::existsInDatabase(1, 'orders');

Additional information

• PrestaShop version: 1.7.5.2
• PHP version: 7.2.22
• MySql: 5.7

[prestashop-issue-bot[bot]]

Thanks for opening this issue! We will help you to keep its state consistent

[khouloudbelguith]

Hi @roncarino,

Thanks for your report.
You are using an old version.
Have you tried to reproduce the issue with PS1770beta2?

Thanks!

[roncarino]

Hi @khouloudbelguith, thanks for your answer.
the function existsInDatabase hasn't been touched in PS1770beta2 so I assume the issue is still there. Cheers.

Loris

[khouloudbelguith]

Ping @PrestaShop/prestashop-core-developers can you check this issue?

Thanks!

[matks]

Hi !

I dont know why (the people who did it are long gone) but I can confirm the usage of orders is intentional

See https://github.com/PrestaShop/PrestaShop/blob/develop/classes/controller/AdminController.php#L3290
=> this statement clearly handles the same issue you mention by replacing order by orders.

Although we can only use guesses, I think indeed the database design is flawed. But it'd be very hard to fix it because so much logic has been built around it. So I suggest the 2nd solution.

Did you notice Order does extend ObjectModel ? Since it inherits its functions we might be able to simply override the function at Order level

[PululuK]

To avoid this we can use the table name and primary in the object definition, something like :

public static function existsInDatabase($id_entity, $table = null)
{
      $primary = 'id_' . bqSQL($table);

      if ($table === null) {
            $table = static::$definition['table'];
            $primary = static::$definition['primary'];
      }

      $row = Db::getInstance()->getRow('
            SELECT `' . $primary . '` as id
            FROM `' . _DB_PREFIX_ . bqSQL($table) . '` e
            WHERE e.`'.$primary.'` = ' . (int) $id_entity, false
        );

        return isset($row['id']);
 }

[PierreRambaud]

@PululuK I was writing something like you :sweat_smile:
It's a possible "hack" and will be easier using an Object class instead of ObjectModel class

[roncarino]

Hi !

I dont know why (the people who did it are long gone) but I can confirm the usage of orders is intentional

See https://github.com/PrestaShop/PrestaShop/blob/develop/classes/controller/AdminController.php#L3290
=> this statement clearly handles the same issue you mention by replacing order by orders.

Although we can only use guesses, I think indeed the database design is flawed. But it'd be very hard to fix it because so much logic has been built around it. So I suggest the 2nd solution.

Did you notice Order does extend ObjectModel ? Since it inherits its functions we might be able to simply override the function at Order level

Hi @matks , thanks for your answer. Yes, in this way there is not consistency of _table naming rule_ which is quite odd definitely. But, I do understand that a change like this would impact deeply.
So yes, I guess we need an alternative fix with existsInDatabase

To avoid this we can use the table name and primary in the object definition, something like :

public static function existsInDatabase($id_entity, $table = null)
{
      $primary = 'id_' . bqSQL($table);

      if ($table === null) {
            $table = self::$definition['table'];
            $primary = self::$definition['primary'];
      }

      $row = Db::getInstance()->getRow('
          SELECT `' . $primary . '` as id
          FROM `' . _DB_PREFIX_ . bqSQL($table) . '` e
          WHERE e.`'.$primary.'` = ' . (int) $id_entity, false
        );

        return isset($row['id']);
 }

Hi @PululuK, maybe I didn't get your code but I cannot see how this would solve the issue honestly. If I understood well your code, that should produce exactly the same wrong query I think..... 🤔

[PierreRambaud]

Hi !
I dont know why (the people who did it are long gone) but I can confirm the usage of orders is intentional
See https://github.com/PrestaShop/PrestaShop/blob/develop/classes/controller/AdminController.php#L3290
=> this statement clearly handles the same issue you mention by replacing order by orders.
Although we can only use guesses, I think indeed the database design is flawed. But it'd be very hard to fix it because so much logic has been built around it. So I suggest the 2nd solution.
Did you notice Order does extend ObjectModel ? Since it inherits its functions we might be able to simply override the function at Order level

Hi @matks , thanks for your answer. Yes, in this way there is not consistency of _table naming rule_ which is quite odd definitely. But, I do understand that a change like this would impact deeply.
So yes, I guess we need an alternative fix with existsInDatabase

To avoid this we can use the table name and primary in the object definition, something like :

public static function existsInDatabase($id_entity, $table = null)
{
      $primary = 'id_' . bqSQL($table);

      if ($table === null) {
            $table = self::$definition['table'];
            $primary = self::$definition['primary'];
      }

      $row = Db::getInstance()->getRow('
            SELECT `' . $primary . '` as id
            FROM `' . _DB_PREFIX_ . bqSQL($table) . '` e
            WHERE e.`'.$primary.'` = ' . (int) $id_entity, false
        );

        return isset($row['id']);
 }

Hi @PululuK, maybe I didn't get your code but I cannot see how this would solve the issue honestly. If I understood well your code, that should produce exactly the same wrong query I think.....

You will only have to do Order::existsInDatabase(1); the database table name will be automatically retrieve from the ObjectModel definition.

[roncarino]

Hi @PululuK, maybe I didn't get your code but I cannot see how this would solve the issue honestly. If I understood well your code, that should produce exactly the same wrong query I think..... thinking

Try

 Order::existsInDatabase(1, null);

Or

 Order::existsInDatabase(1);

I didn't try honestly but I expect an undefined index: table probably... 🤔 I'll try later maybe. Thanks 😃

[roncarino]

Hi @PululuK, maybe I didn't get your code but I cannot see how this would solve the issue honestly. If I understood well your code, that should produce exactly the same wrong query I think..... thinking

Try

 Order::existsInDatabase(1, null);

Or

 Order::existsInDatabase(1);

I didn't try honestly but I expect an undefined index: table probably... 🤔 I'll try later maybe. Thanks 😃

Yep..

[PululuK]

Yep..

Hi @roncarino
Updated see https://github.com/PrestaShop/PrestaShop/issues/21210#issuecomment-701227278 ... sorry !

[roncarino]

Yep..

Hi @roncarino
Updated see #21210 (comment) ... sorry !

Yep, thanks! It does make sense. I haven't tried yet but it definitely makes more sense. It looks like also a "not breaking" fix because it will work for previous cases as well. No need for more parameters as my "potential solution"...

[roncarino]

Hi @PululuK
yes, I can confirm the solution should work properly. I saw you have already done the PR. Well done. Cheers