jeremyharris / cakephp-lazyload

A lazy loader for CakePHP entities.
MIT License
61 stars 10 forks source link

Composite foreign keys #32

Open TheLevti opened 2 years ago

TheLevti commented 2 years ago

The issue

This trait is currently missing support for foreign keys that consist out of multiple columns.

The cause

The suspected line of code is at line 133:

$isMissingBelongsToFK = $association instanceof BelongsTo && !isset($this->_fields[$association->getForeignKey()]);

The code fails to check the return type of the getForeignKey() call, which is defined as string|string[]. If the foreign key is defined as an array of columns, the trait will fail with:

Response: {\n
    "message": "Illegal offset type in isset or empty",\n
    "exception": "TypeError",\n
    "file": "/var/www/html/vendor/jeremyharris/cakephp-lazyload/src/ORM/LazyLoadEntityTrait.php",\n
    "line": 133,\n
    "trace": [\n
        {\n
            "file": "/var/www/html/vendor/jeremyharris/cakephp-lazyload/src/ORM/LazyLoadEntityTrait.php",\n
            "line": 39,\n
            "function": "_lazyLoad",\n
            "class": "App\\Model\\Entity\\MyModel",\n
            "type": "->"\n
        },\n
        {\n
            "file": "/var/www/html/vendor/cakephp/cakephp/src/Datasource/EntityTrait.php",\n
            "line": 129,\n
            "function": "get",\n
            "class": "App\\Model\\Entity\\MyModel",\n
            "type": "->"\n
        },\n
...

What happens here is that you try to access an array index of type array, which can't work.

The solution

$isMissingBelongsToFK = false;
if ($association instanceof BelongsTo) {
    $foreignKeys = $association->getForeignKey();
    if (is_array($foreignKeys)) {
        foreach ($foreignKeys as $foreignKey) {
            if (!isset($this->_fields[$foreignKey])) {
                $isMissingBelongsToFK = true;
                break;
            }
        }
    } else {
        $isMissingBelongsToFK = !isset($this->_fields[$foreignKeys]);
    }
}
iNviNho commented 2 years ago

+1

jeremyharris commented 2 years ago

Thanks for the detailed report! When I get a chance I'll create a test case and PR a fix :)

jeremyharris commented 2 years ago

Fixed in https://github.com/jeremyharris/cakephp-lazyload/pull/31

TheLevti commented 2 years ago

How is it fixed if the exception causing code is this referenced fix?

Was this closed by mistake?

!isset($this->_fields[$association->getForeignKey()] can not work when getForeignKey() returns an array.

jeremyharris commented 2 years ago

Yes I closed it by mistake. My apologies! I was going through older issues and thought this was fixed by the other PK fix.

jeremyharris commented 2 years ago

I took a look and can't create a test case around your scenario. I'm not sure how really.

Can you give me 2 simple table examples where you end up with a BelongsTo that has a composite key that is stored on the parent table?