Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composite foreign keys #32

Open
TheLevti opened this issue Feb 11, 2022 · 6 comments
Open

Composite foreign keys #32

TheLevti opened this issue Feb 11, 2022 · 6 comments

Comments

@TheLevti
Copy link

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
Copy link

iNviNho commented Feb 11, 2022

+1

@jeremyharris
Copy link
Owner

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

@jeremyharris
Copy link
Owner

Fixed in #31

@TheLevti
Copy link
Author

TheLevti commented Apr 19, 2022

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
Copy link
Owner

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 jeremyharris reopened this Apr 19, 2022
@jeremyharris
Copy link
Owner

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants