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

[BUG]: Magic getters for relationships throws exception when there is no relation in 4.0 #14460

Closed
wurst-hans opened this issue Oct 11, 2019 · 6 comments
Labels
bug A bug report duplicate Duplicate issue. The duplicate issue is referenced in the comments status: medium Medium

Comments

@wurst-hans
Copy link

wurst-hans commented Oct 11, 2019

My projects are running on Phalcon 3.4. There I'm using many model relationships like

class Requests extends Model {
    public function initialize() {
        $this->belongsTo('customerId', 'App\Models\Customers', 'id', [ 'alias' => 'Customer' ]);
	}
}

which works fine in Phalcon 3.4, so I can use it like

$request = Requests::findFirst();
$customer = $request->getCustomer();

At the moment, I'm trying to migrate all projects to Phalcon 4.0.0.-rc.1-783 but when executing above code, I get an error

Phalcon\Mvc\Model\Exception: The method 'getCustomer' doesn't exist on model 'App\Models\Requests' in ...

I found out that getCustomer() fails and throws exception if customerId is NULL in model Requests. In Phalcon 3.4 I get a false as result, but 4.0 throws an exception.
Getting false allows to initialize a new object, when there is no other already. For this I can modify the code to

$request = Requests::findFirst();
$customer = $request->getCustomer();
$customer = $customer ?: new Customers();

There are hundreds of lines of code in my projects which have to be altered then to prevent this exception in Phalcon 4.0. Is there a workaround or is it planned to add 3.4 behavior again?

IMHO: It's not correct to not add magic getters when column used for relationship is NULL. The expected behavior (like when calling findFirst() without result) is getting false or an object.

Forgot to mention, that I have tried it with lowercase alias already without success. Even with Vokuro this happens.

@Jurigag
Copy link
Contributor

Jurigag commented Oct 12, 2019

Can you check what happens if you add allowNulls => true ?

@wurst-hans
Copy link
Author

wurst-hans commented Oct 12, 2019

Can you check what happens if you add allowNulls => true ?

You mean by adding

'foreignKeys' => [
    'allowNulls' => true
],

to model Requests? Nothing has changed, exception is still thrown.

BTW: I'm logging the database requests and I can see, that Phalcon tries to query database for customer SELECT ... FROM customers WHERE customers.id=:APR0 having :APR0=NULL which is senseless IMO. Apart of fixing this issue, there is no need to query related table when key (i.e. customerId) is NULL.

@zsilbi
Copy link
Member

zsilbi commented Oct 14, 2019

@wurst-hans, this has just been fixed in #14444
Please test the current 4.0.x branch if you can.

@ruudboon ruudboon added duplicate Duplicate issue. The duplicate issue is referenced in the comments Bug - Medium and removed Bug - Unverified labels Oct 14, 2019
@wurst-hans
Copy link
Author

@wurst-hans, this has just been fixed in #14444
Please test the current 4.0.x branch if you can.

Looks good. No exception is thrown and I get NULL for non existing relationship. Nevertheless, one could prevent unnecessary database request when relationship key is NULL.
Anyway, thanks for your help. When will this be available in mainline repo?

@zsilbi
Copy link
Member

zsilbi commented Oct 14, 2019

Nevertheless, one could prevent unnecessary database request when relationship key is NULL.

I'll take a look at that soon.

Anyway, thanks for your help. When will this be available in mainline repo?

It will be included in the following 4.0.0-rc.2 release in the next few weeks.

@ruudboon
Copy link
Member

@wurst-hans Can you open a new issue for the suggested improvement (good catch!)? Don't want to log this in the same issue. Closing this one as duplicate.

@niden niden added bug A bug report status: medium Medium and removed Bug - Medium labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report duplicate Duplicate issue. The duplicate issue is referenced in the comments status: medium Medium
Projects
None yet
Development

No branches or pull requests

5 participants