Problem/Motivation

We use
\Drupal\Core\TypedData\Plugin\DataType\IntegerData
and
\Drupal\Core\Field\Plugin\Field\FieldType\IntegerItem
for integer id entity fields like user.uid.

However, when we load an entity from the database, we get the string version of these values, instead of the integer version. E.g. "123" instead of 123.

The 'integer' field type (IntegerItem) declares a property 'value' with data type 'integer' (IntegerData).
But:
- The property class is never instantiated, so any validation that the property could do does not happen.
- Even if the property class would be instantiated, it does nothing to validate the value.
- The field type class (IntegerItem) also does no validation.

Steps to reproduce

Create an entity (node or user).
Load the entity (make sure we don't get a cached value).
Call $entity->id().

Expected: Integer id like 123.
Actual: String id like "123".

Proposed resolution

The clean solution would be to instantiate the property class (IntegerData) in IntegerItem (or a base class), and let that property class do the casting.

If we care about performance, it would be simpler to let IntegerItem do the job by itself.

A proper cast + validation might look like this:

      $value = match (TRUE) {
        $value === NULL => NULL,
        is_int($value) => $value,
        is_string($value) && (string) (int) $value === $value => (int) $value,
        is_string($value),
        is_float($value) => throw new \InvalidArgumentException(sprintf(
          'Expected an integer value or NULL, found %s.',
          var_export($value, TRUE),
        )),
        default => throw new \InvalidArgumentException(sprintf(
          'Unexpected value of type %s found as entity id.',
          get_debug_type($value),
        )),
      };

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3590321

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

donquixote created an issue.

donquixote’s picture

Interesting observation:

In SqlContentEntityStorage we assign string to `$entity->$idKey`:

        $entity->{$this->idKey} = (string) $record->{$this->idKey};

This dates back to DatabaseStorageControllerNG.
#2057401-50: Make the node entity database schema sensible (@berdir)

Why the string cast here?

and in comment #54 (@plach):

I don't remember honestly: it was needed to fix some failing tests. I attached a patch version that reverts that to see which ones.

Anyway that value is still sent through the field item object where we could cast to integer if we wanted to.

donquixote’s picture

Intermediate conclusions:
The cast I propose here seems quite disruptive, even without the validation part.

There are a lot of places that do strict comparison, which would fail if one of the values is represented as string, the other as integer.
On the other hand, weak comparison with "==" or "!=" or simple cast with "(int)" or "(string)" is not a good alternative, it has undesired effects with values like "", TRUE, FALSE, NULL.

The IntegerLike* classes provide specialized casts and comparisons.
We might end up introducing these safe comparisons in 11.x, while we might have to postpone the actual casting to 12.x. (and even then it could be problematic.)