This is incredibly frustrating. In attempting to maintain compliance with the standards laid out by PHPUnit’s testing suite, I find myself constantly having to create preposterous, duplicative checks for object types such as the following:
$result = $this->Authentication->getResult();
if (!is_null($result) && $result->isValid()) {
$user = $this->Authentication->getIdentity();
if (
is_object($user) &&
is_a($user, 'Authentication\Identity') &&
is_a($user, 'Visualize\Model\Entity\User')
) {
$user = $this->Users->get($user->id);
$cart = $this->CartManager->getUserCart($user);
// Just once on login, we scan the database for abandoned carts and prune them:
if (is_object($cart) && is_a($cart, 'Cake\Datasource\EntityInterface')) {
$this->CartManager->pruneCarts($user->id, (int)$cart->id);
}
// redirect to user dashboard after login success
$redirect = $this->request->getQuery('redirect');
if (!empty($redirect) && $redirect != '/') {
$this->redirect($redirect);
} else {
$this->redirect(Router::url([
'controller' => 'Users',
'action' => 'dashboard',
$user->id,
]));
}
} else {
Log::error('Unable to find authenticated user.');
}
}
This seems like a pretty standard case of authentication. But in order to make sure that the code does not error out, I need to guarantee that the object that comes out of getIdentity() is simultaneously an object, an object that implements Identity and also an Entity of my User type. This is pretty sad, considering that I thought the idea was DRY code.
I remember reading something that said CakePHP had planned on ironing out the documentation and function return problems so this would not be an issue. But despite being at the latest version of CakePHP, 4.7, and staying on top of updates to my vendor files, I’ve not seen any fixes yet?
Is there something I ought to be doing differently to prevent these constant object type checks?
$user is always either null, or an instance of \Authentication\IdentityInterface, why exactly does your code need to know anything else about it? And how is PHPUnit involved in this?
btw. $user instanceof \Visualize\Model\Entity\User will basically (identity interface vs implementation) cover your three checks. Generally if you actually really need additional checks, and in multiple different places that is, then you could look into putting them for example in a custom/extended authentication component, or any other service.
I’m curious here as well. Do you have linting running such as phpstan at a high level and its complaining about strict types? That’s the only thing that would make sense to me here.
How is PHPUnit involved? It’s PHPUnit that refuses to allow code without these checks because it “Cannot access property $id on Authentication\IdentityInterface|null”. Basically, PHPUnit does not allow code unless you can be certain that a given object is in fact the correct object. So if you’d like to get your code to validate, you need to wrap every function in these checks.
I’m using the CakePHP cakedc/cakephp-phpstan library which ostensibly is supposed to help with this, but it’s not. Am I really the only person you know using PHPUnit? Or the only one having these issues? Because its rampant?
I see, so it’s the static analysis. Well, the instanceof check mentioned in my first post should get rid of that specific violation.
Another option could be to use the identity interface’s getIdentifier() or getOriginalData() methods to obtain the ID, and to make your cart manager accept an ID to obtain the user’s cart, like getUserCartByUserId(), that way you’re not violating any contracts and you don’t need any additional checks.
I’d go out on a limb here and say there’s nothing wrong as far as the framework is concerned, it uses an abstract interface for the identity to stay flexible, any concrete implementations would be rather limiting.
Either drop your phpstan level or add exclusions is my advice.
The cakedc lib you are including is cranked to the max: cakephp-phpstan/phpstan.neon at master · CakeDC/cakephp-phpstan · GitHub which is nice for new projects, but for existing projects with lots of code I recommend slowly turning up levels by a single increment over time until you hit diminishing returns.
Yeah, I mean: this probably is less of a Cake issue than a Stan issue. But I don’t get how any code is going to run tightly enough to meet this standard. At the same time, I don’t like the idea of “lowering my standards,” since obviously I’m trying to stay current with my code. Harumph.
You’ll encounter problems running cakephp at a high stan level, because it must stay compatible with older versions of php which don’t have the ability to do union types and things of that nature. Even still, cakephp core runs at phpstan level 6 which is fairly high.