feat: add DataConverter to convert types

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!

[kenjis]

Description
Supersedes #7995

• add DataConverter class to convert column values
• add DataCaster class to cast values

Entity casting works at (1)(4), but DataConverter casting works at (2)(3).

[App Code] --- (1) --> [Entity] --- (2) --> [Database]
[App Code] <-- (4) --- [Entity] <-- (3) --- [Database]

Example:

$types = [
'id' => 'int',
'date' => 'datetime',
];
$converter = newDataConverter($types);
$dbData = [
'id' => '1',
'date' => '2023-11-18 14:18:18',
];
$data = $converter->fromDataSource($dbData);
dd($data);

array (2) [
'id' => integer 1
'date' => CodeIgniter\I18n\Time#44 (6) (
protected 'timezone' -> DateTimeZone#45 (2) (
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
protected 'locale' -> string (2) "en"
protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
public 'date' -> string (26) "2023-11-18 14:18:18.000000"
public 'timezone_type' -> integer 3
public 'timezone' -> string (3) "UTC"
)
]

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

Did not review this completely, but couple of thoughts:

1. How is this different from Entity casts? By looking at the code, these seem to duplicate the code there.
2. Would these qualify for generic typing, like T -> U for the fromDatabase method and U -> T for toDatabase method. That way we can have static analysers come into play? What do you think?

[kenjis]

1. The codes are almost identical. However, this PR is not a feature in Entity, and can be used in Models (or others)

The current Entity has problems.

• An Entity is a PHP object, but it holds data retrieved from a DB (except for dates) and does not have the correct type of values in PHP. So there is a bug Bug: Entity::hasChanged() is unreliable for casts #5905 where hasChange() returns the wrong value.
• Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

[kenjis]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

[paulbalandan]

Entity has too much knowledge about DB; since Entity is a PHP representation of a DB record, knowledge of DB is essentially unnecessary.

I agree. Entities could have been value objects only.

[paulbalandan]

2. Each handler has a specific type(s) to handle. So, I think I can do it, but as far as I tried a little, I could not specify the types well.

I tried something like these:

diff --git a/system/Database/DataConverter/Cast/CastInterface.php b/system/Database/DataConverter/Cast/CastInterface.php
index 540623ea33..b793b6ea20 100644
--- a/system/Database/DataConverter/Cast/CastInterface.php+++ b/system/Database/DataConverter/Cast/CastInterface.php@@ -12,27 +12,28 @@
namespace CodeIgniter\Database\DataConverter\Cast;
/**
- * Interface CastInterface+ * @template TDbColumn+ * @template TPhpValue
*/
interface CastInterface
{
/**
* Takes value from database, returns its value for PHP.
*
- * @param bool|float|int|string|null $value Data- * @param array $params Additional param+ * @param TDbColumn $value Data+ * @param list<string> $params Additional param
*
- * @return array|bool|float|int|object|string|null+ * @return TPhpValue
*/
- public static function fromDatabase($value, array $params = []);+ public static function fromDatabase(mixed $value, array $params = []): mixed;
/**
* Takes the PHP value, returns its value for database.
*
- * @param array|bool|float|int|object|string|null $value Data- * @param array $params Additional param+ * @param TPhpValue $value Data+ * @param list<string> $params Additional param
*
- * @return bool|float|int|string|null+ * @return TDbColumn
*/
- public static function toDatabase($value, array $params = []);+ public static function toDatabase(mixed $value, array $params = []): mixed;
}

diff --git a/system/Database/DataConverter/Cast/BaseCast.php b/system/Database/DataConverter/Cast/BaseCast.php
index d35b761622..e8ef0e13ba 100644
--- a/system/Database/DataConverter/Cast/BaseCast.php+++ b/system/Database/DataConverter/Cast/BaseCast.php@@ -14,28 +14,25 @@ namespace CodeIgniter\Database\DataConverter\Cast;
use TypeError;
/**
- * Class BaseCast+ * @template TDbColumn+ * @template TPhpValue+ *+ * @implements CastInterface<TDbColumn, TPhpValue>
*/
abstract class BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = [])+ public static function fromDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = [])+ public static function toDatabase(mixed $value, array $params = []): mixed
{
return $value;
}
/**
- * Throws TypeError+ * @throws TypeError
*/
protected static function invalidTypeValueError(mixed $value): never
{

diff --git a/system/Database/DataConverter/Cast/ArrayCast.php b/system/Database/DataConverter/Cast/ArrayCast.php
index 4b41d9367d..3f0e517289 100644
--- a/system/Database/DataConverter/Cast/ArrayCast.php+++ b/system/Database/DataConverter/Cast/ArrayCast.php@@ -15,13 +15,12 @@ namespace CodeIgniter\Database\DataConverter\Cast;
* Class ArrayCast
*
* DB column: string <--> PHP: array
+ *+ * @extends BaseCast<string, mixed[]>
*/
class ArrayCast extends BaseCast implements CastInterface
{
- /**- * {@inheritDoc}- */- public static function fromDatabase($value, array $params = []): array+ public static function fromDatabase(mixed $value, array $params = []): array
{
if (! is_string($value)) {
self::invalidTypeValueError($value);
@@ -34,10 +33,7 @@ class ArrayCast extends BaseCast implements CastInterface
return (array) $value;
}
- /**- * {@inheritDoc}- */- public static function toDatabase($value, array $params = []): string+ public static function toDatabase(mixed $value, array $params = []): string
{
return serialize($value);
}

and tested with this script:

<?phpuseCodeIgniter\Database\DataConverter\Cast\ArrayCast;
ArrayCast::fromDatabase([]);

resulted in:

$ vendor/bin/phpstan analyse -v test.phpNote: Using configuration file C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpstan.neon. 1/1 [============================] 100% 2 secs ------ ------------------------------------------------------------------------------------------------------------------------------------- Line test.php ------ ------------------------------------------------------------------------------------------------------------------------------------- :5 Parameter #1 $value of static method CodeIgniter\Database\DataConverter\Cast\ArrayCast::fromDatabase() expects string, array given. ✏️ test.php:5 ------ ------------------------------------------------------------------------------------------------------------------------------------- [ERROR] Found 1 error Used memory: 114 MB

[michalsn]

Hmm... Looks like a truncated Entity class with a cast option.

What is the plan for this? A part of the Entity class rewrite or an entirely separate thing?

As it stands, it is moderately useful. Sure - it works, but if I have to manually declare types every time, I'll start pulling my hair out. I would like to declare types one time and then reuse them for the data I work with. Just like in the Entity class.

I'm really curious about a plan for this class.

[kenjis]

@michalsn This class is now complete.

The ultimate goal is to fix bug #5905 in Entity, but it is not easy.

The next step is to make this class available from Model so that the model can return data with the correct type of values when Entity is not used.

[michalsn]

@kenjis IMO the solution for Entity is one and simple, as long as everyone will agree on it. The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

From what you're saying DataConverter class looks like an additional option that we will be able to use instead of the Entity class. Will the final Entity class deprecate the cast option and use the input values provided by the DataConverter class? Because if not, I don't see how this will help resolve the issues with an Entity class.

I'd like to know a little more concrete plan for what changes you want to make before we add another layer to convert the data. Right now I can't decide if I like the potential level of complexity the new layer adds.

[lonnieezell]

I realize I'm out of touch here, but this does seem like an overly complex solution to the original issue. And definitely doesn't seem to follow the old CodeIgniter way of attempting to keep things simple. This problem has been solved before by other frameworks. Have we looked at them to see how they've solved it as inspiration?

Sorry, without diving into the code deeper right now I don't have any advice on solutions but watching the email threads got me a little concerned we're slapping a complex bandaid on an issue that can be solved in a simpler fashion somewhere else.

[kenjis]

The values provided to the Entity class should be casted right away, when creating an instance and then treated as an original data.

That is what I'm trying to do.

[kenjis]

The big picture at this point looks like this.

1. This DataConverter provides some of the functionality of Entity's property casting. It only converts data to and from the database. (This PR)
2. Using DataConverter, Model will have the functionality of casting. (See feat: add Model field casting #8243)
3. Entity will use the casting in Model, and we don't need to use Entity's property casting for data conversion to and from the database. (See feat: add Model field casting #8243)

[kenjis]

@lonnieezell Thank you for your comment!

I don't think this is overly complex. The DataConverter just converts data, and I just move a part of functionality from Entity's property casting. Entity is already too complex.

I have no idea what the solution in other frameworks means. I would appreciate it if you could give me some concrete examples. At the very least, in Laravel the model has casting functionality.

[altrusl]

Thank you @kenjis for this enhancement. I asked for this feature some time ago.
My two cents:

1. This is different from using Entity, because using Entity is an ORM pattern, which could be an overcomplicattion for users over the Query Builder. I mean - it forces the user to create and work with new classes, which he might not want to do.
2. For me casting to Number type was fine, but nowadays many tables (almost all of mine) have JSON fields and casting from/to them every time working with DB is very inconvenient and error prone. So this feature is really in demand.
3. My workaround was to have a PHP file with table field type definitions and use Model hooks ($beforeUpdate etc) for type conversions, so it was transparent for users. But native solution would be much better
   This is the example of that file:

class ResourcesMeta
{
privatestatic$resources = [
"user_activity" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId"],
"json" => ["data"],
],
],
"user_balance_history" => [
"exposedFields" => ["*"],
"types" => [
"number" => ["id", "userId", "balanceAfter", "balanceBefore", "walletId"],
"json" => ["data"],
],
],

4. Entity has problems with updating inner properties of JSON fields - you need to create and reassign new object to that field

[kenjis]

In my opinion, this PR is ready to merge. This PR alone does not change anything in behavior, but the next PR #8243 will allow casting in Model.

The cast classes are in Entity and DataCaster, but both are needed because they are used in different purposes (timing) and Entity cast classes are loose, but DataCaster cast classes are strict.

Review when you have time.

[michalsn]

LGTM!