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: Entity casts do not cast the original data #2441

Closed
swearl opened this issue Dec 6, 2019 · 5 comments
Closed

Bug: Entity casts do not cast the original data #2441

swearl opened this issue Dec 6, 2019 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@swearl
Copy link

swearl commented Dec 6, 2019

Describe the bug

namespace App\Entities;

use CodeIgniter\Entity;

class Item extends Entity {
	protected $attributes = [
		"id" => null,
		"type" => null,
	];

	protected $casts = [
		"type" => "int"
	];
}

eg: I load a data from model, type value is 1, if only use this, no problem

var_dump($item->type); // int(1)

but if i set the value again

$item->type = 1;
var_dump($item->hasChanged()); // true

because the original type value is "1"(string)

CodeIgniter 4 version
rc3

Affected module(s)
CodeIgniter\Entity

Context

  • OS: Windows 10 Pro
  • Web server: Apache/2.4.41 (Win64)
  • PHP version 7.3.12
@swearl swearl added the bug Verified issues on the current code behavior or pull requests that will fix them label Dec 6, 2019
@MGatner
Copy link
Member

MGatner commented Dec 6, 2019

Yeah that's an interesting one. Probably need to update hasChanged() to check each attribute, casting as it goes, instead of this bulk check:

return $this->original !== $this->attributes;

@lonnieezell
Copy link
Member

Casting is only designed to be on read, not on "write" to the Entity. Doing it on write could cause weird data issues when you try to save it. It is only meant as a convenience feature for working with the data, not as something that defines the data structure that gets saved to the database. If you need to convert to a particular type, you can use a setter method which will be automatically called when you set the value on the class.

public function setType($value) { 
    ... 
}

In this particular example, what is the issue? It all gets saved correctly. I have an inkling that in more complex situations casting when you're checking if something has changed could be problematic but haven't thought through all of that just yet.

@swearl
Copy link
Author

swearl commented Dec 6, 2019

Casting is only designed to be on read, not on "write" to the Entity. Doing it on write could cause weird data issues when you try to save it. It is only meant as a convenience feature for working with the data, not as something that defines the data structure that gets saved to the database. If you need to convert to a particular type, you can use a setter method which will be automatically called when you set the value on the class.

public function setType($value) { 
    ... 
}

In this particular example, what is the issue? It all gets saved correctly. I have an inkling that in more complex situations casting when you're checking if something has changed could be problematic but haven't thought through all of that just yet.

Thanks for reply, but i don't think this is what i want...
actually, my "type" is int, but in the entity original "type" is string...
why i need to use a setter method to cast my correct int type into a wrong string type???

@lonnieezell
Copy link
Member

lonnieezell commented Dec 6, 2019

Remember - PHP is a loosely typed language. It's not Java or something. The example case here should work perfectly fine for you since numbers in strings cast to ints automatically.

And, I get that it seems a little silly in this simplistic example. But I have a feeling more complex use-cases (like arrays/json/dates) will cause issues if it's cast on the way into the entity.

If you're really concerned that the type is correctly an int within the Entity, you'll just cast it on the way in:

$entity->type = (int)$type;

@rmilecki
Copy link

rmilecki commented Apr 12, 2020

Hi guys, I'm new to CodeIgniter but I also stumbled on this issue. I'd like to ask you about it again as I'm not sure if there was a full understanding of the possible problem.

I believe I have the same problem but:

  1. I don't expect CodeIgniter to do any casting on write:
    I set int and I expect CodeIgniter to store int.
  2. I already use proper type when setting an attribute
    I do $item->type = 1; which is the same as $item->type = (int)1 suggested by @lonnieezell above.

I have MySQL table created as:

CREATE TABLE items (
  id int(11) NOT NULL,
  type int(11) NOT NULL,
)

and trivial entity that looks like this:

class Item extends Entity
{
    protected $casts = [
        'id' => 'integer',
        'type' => 'integer',
    ];
} 

Now my PHP code is as follows:

$item = $model->find($id);
var_dump($item->type, $item->hasChanged('type'));

$item->type = 1;
var_dump($item->type, $item->hasChanged('type'));

if ($item->hasChanged('type') {
    $this->sendTypeNotifications($item->id);
}

executing it results in:

int(1)
bool(false)
int(1)
bool(true)

and sending notifications to my users.

This is not what I expect. That type hasn't changed at all: it was int(1) originally and int(1) after that no-change.

@lonnieezell: I think that maybe there was a misunderstanding between you and @swearl. Could you have another look at this report, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

4 participants