[8.x] Accept enums for insert update and where

[themsaid]

This PR allows using Enums for insert(), update(), and select().

[driesvints]

@themsaid can you rebase with 8.x so the new database builds are also running?

[mikerockett]

@taylorotwell

This has resulted in a breaking change in my codebase. We have our own home-grown enum class that is castable on models. The following code is used to cast enums:

public static function castUsing(array $arguments)
{
  return new class(static::class) implements CastsAttributes
  {
    public function __construct(private string $enumClass)
    {}

    public function get($model, $key, $value, $attributes)
    {
      return $this->enumClass::coerce($value);
    }

    public function set($model, $key, $value, $attributes)
    {
      if (is_array($value)) {
        $value = $this->enumClass::coerce($value['value']);
      }

      if ($value === null || is_string($value) || is_numeric($value)) {
        return $value;
      }

      return match (get_class($value) === $this->enumClass) {
        true => $value->jsonSerialize(),
        false => $value
      };
    }
  };
}

Our enum instances are structured as follows:

Application\Enums\Operations\SomeEnum^ {#5570
  +key: "someName"
  +value: 1
  +meta: array:1 [
    "display_as" => "Some Enumeration Value"
  ]
}

They are casted in models as follows:

protected $casts = [
  'enum' => SomeEnum::class
]

Whilst the casting continues to work correctly (1 is stored in the database, and the enum instance is hydrated on retrieval), however the addBinding method of the query builder is now using a different method to determine the binding value, which results in it injecting the incorrect value into the query. In the above example, it would correctly use 1 as the binding, but now it is using someName instead. When running ->dd() on the builder instance, I now get the enum class instance shown above.

To confirm this, I reverted the change from this PR manually as follows:

    public function addBinding($value, $type = 'where')
    {
        if (! array_key_exists($type, $this->bindings)) {
            throw new InvalidArgumentException("Invalid binding type: {$type}.");
        }

        if (is_array($value)) {
            $this->bindings[$type] = array_values(array_merge($this->bindings[$type], $value));
            // $this->bindings[$type] = collect($this->bindings[$type])
            //                 ->merge($value)
            //                 ->map([$this, 'castBinding'])
            //                 ->values()
            //                 ->toArray();
        } else {
            $this->bindings[$type][] = $this->castBinding($value);
        }

        return $this;
    }

This corrected the issue.

I think the problem lies in the call to merge(), which not only uses array_merge, but also uses getArrayableItems() - my suspicion is that this is the culprit.

Is there anything I can do on my end to correct this, or can the PR changes be adjusted to accomodate? From my perspective, I naturally see this as a breaking change, but am happy to work around it if it cannot be adjusted/reverted.

[themsaid]

@mikerockett can you provide a test case that fails so I can see exactly what the issue is?

[mikerockett]

@themsaid I'll try find the time to build a test case for you. It's unfortunately a tad time consuming, but in the meantime, I have confirmed that it is the getArrayableItems method that is causing a change in behaviour here.

Our enums implement Arrayable so that they may return the structure mentioned above as an array. What's happening is that the query builder eventually builds out a whereIn, and it is now passing an array of arrays instead of an array of enum class instances to the where key of the bindings. For example, when I dump the bindings after line 975 of the Query\Builder, I get the following after this change:

array:9 [
  "select" => []
  "from" => []
  "join" => []
  "where" => array:1 [ // <----- array
    0 => array:3 [
      "key" => "someName"
      "value" => 1
      "meta" => array:1 [
        "display_as" => "Some Enumeration Value"
      ]
    ]
  ]
  "groupBy" => []
  "having" => []
  "order" => []
  "union" => []
  "unionOrder" => []
]

If I either revert the change in the PR or make my enum class not implement Arrayable (which causes other side effects, unfortunately), the bindings become:

array:9 [
  "select" => []
  "from" => []
  "join" => []
  "where" => array:1 [
    0 => Application\Enums\Operations\SomeEnum^ {#5429  // <----- original instance
      +key: "someName"
      +value: 1
      +meta: array:1 [
        "display_as" => "Some Enumeration Value"
      ]
    }
  ]
  "groupBy" => []
  "having" => []
  "order" => []
  "union" => []
  "unionOrder" => []
]

Not sure what happens beyind this point, but I assume the builder then casts the bindings further so that they are strings. Our enum class does have string casting, and it simply casts to the value, which results in the correct query. However, with the changes in this PR, the enum class instance is cast to an array first, due to getArrayableItems, which causes a change in expected behaviour. As it is an array on the binding, it has no way of know that it should be cast to 1.

[mikerockett]

Using a PHP-land approch to this, which prevents the use of getArrayableItems, solves the issue as well and I'm pretty sure that would also allow 8.1 enum support without issue:

// ...
            $this->bindings[$type] = array_values(
                array_map(
                    [$this, 'castBinding'],
                    array_merge($this->bindings[$type], $value),
                )
            );
// ...

[mikerockett]

It would also be nice if we could have a custom approach to doing our own bindings for classes that are passed into the builder. For example, I'm imaginging having a CastsToBinding interface that allows us to declare a method called castToBinding, giving us full control on how our objects are converted to bindings. In my case, it would be as simple as implementing as follows:

public function castToBinding()
{
    return $this->value;
}

[themsaid]

@mikerockett handled in da7aa38

[mikerockett]

Great, thanks @themsaid!