lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Correctly handle leaf inputs that were cast to PHP native enum instances

Open eslym opened this issue 3 years ago • 6 comments

Describe the bug

Enum passed to validator resulting some dependant rules not working, for example: required_if

enum ItemType: string
{
    case SKU = "sku";
    case VOUCHER = "voucher";
}
enum ItemType {
  SKU
  VOUCHER
}
input OrderItem {
  type: ItemType!
  uuid: String @rules(apply: ["required_if:type,sku"])
  code: String @rules(apply: ["required_if:type,voucher"])
}

at the end, the ItemType enum will passed to the Validator::validateRequiredIf function and it will compare it with a string, which is always false

see Laravel's ValidatesAttributes.php line 1589

Expected behavior/Solution

Probably convert all enum into its value/name before passing it to validator, but not sure any efficient way to do it or not.

Steps to reproduce

See the explanation above

Lighthouse Version v5.61.0

-- Update --

Some workaround solution
// Make validator class to replace the value before validation

class MyValidator extends \Illuminate\Validation\Validator
{
    public function parseDependentRuleParameters($parameters): array
    {
        [$values, $other] = parent::parseDependentRuleParameters($parameters);
        if ($other instanceof UnitEnum) {
            $other = $other->name;
        }
        return [$values, $other];
    }

    public function getDisplayableValue($attribute, $value): string
    {
        if ($value instanceof UnitEnum) $value = $value->name;
        return parent::getDisplayableValue($attribute, $value);
    }
}
// replace the original validator somewhere in service provider

$this->app->afterResolving('validator', function (ValidatorFactory $factory){
    $factory->resolver(fn(...$params)=>new MyValidator(...$params));
});

eslym avatar Sep 29 '22 06:09 eslym

How did you define your enum in the schema - did you implement a conversion to native enums akin to https://lighthouse-php.com/master/the-basics/types.html#enum? If not, the values would just be the - in your case uppercase - string equivalents of the keys (e.g. "SKU").

In case the values are converted to enum instances, the default implementation of required_if will indeed not work. I think your workaround solves that nicely, maybe we can include it with Lighthouse?

spawnia avatar Oct 03 '22 10:10 spawnia

@spawnia my bad, I use the feature from php-graphql master branch causing this issue.

the feature is not in release yet, but I think its good for you guys to acknowledge first

https://webonyx.github.io/graphql-php/type-definitions/enums/

what I put in composer.json

"webonyx": "dev-master as 14.11"

eslym avatar Oct 03 '22 10:10 eslym

I would like to keep this issue open, as it is a problem that is generally present with leaf values that are cast into non-primitive values.

spawnia avatar Oct 03 '22 10:10 spawnia

i found out the @cache directive will face the similar problem

enum BlogPostType {
  ARTICLE
  KNOWLEDGE_BASE
}
query {
  posts(type: BlogPostType! @eq): [Post!]! @paginate @cache(maxAge: 300)
}
[2022-10-05 01:51:54] local.ERROR: Object of class App\Enums\BlogPostType could not be converted to string {"exception":"[object] (Error(code: 0): Object of class App\\Enums\\BlogPostType could not be converted to string at /root/project/vendor/nuwave/lighthouse/src/Cache/CacheKeyAndTagsGenerator.php:43)
[stacktrace]
#0 /root/project/vendor/nuwave/lighthouse/src/Cache/CacheKeyAndTagsGenerator.php(43): implode()
#1 /root/project/vendor/nuwave/lighthouse/src/Cache/CacheDirective.php(84): Nuwave\\Lighthouse\\Cache\\CacheKeyAndTagsGenerator->key()
#2 /root/project/vendor/nuwave/lighthouse/src/Schema/Factories/FieldFactory.php(97): Nuwave\\Lighthouse\\Cache\\CacheDirective->Nuwave\\Lighthouse\\Cache\\{closure}()
#3 /root/project/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(711): Nuwave\\Lighthouse\\Schema\\Factories\\FieldFactory->Nuwave\\Lighthouse\\Schema\\Factories\\{closure}()
#4 /root/project/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(634): GraphQL\\Executor\\ReferenceExecutor->resolveFieldValueOrError()
#5 /root/project/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(1293): GraphQL\\Executor\\ReferenceExecutor->resolveField()
#6 /root/project/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(1239): GraphQL\\Executor\\ReferenceExecutor->executeFields()

My workaround solution is simple also

<?php

namespace App\GraphQL\Cache;

use Illuminate\Contracts\Auth\Authenticatable;
use Nuwave\Lighthouse\Cache\CacheKeyAndTagsGenerator as BaseGenerator;
use UnitEnum;

class CacheKeyAndTagsGenerator extends BaseGenerator
{
    public function key(?Authenticatable $user, bool $isPrivate, string $parentName, $id, string $fieldName, array $args): string
    {
        $args = array_map(fn($v) => $v instanceof UnitEnum ? $v->name : $v, $args);
        return parent::key($user, $isPrivate, $parentName, $id, $fieldName, $args);
    }
}
// AppServiceProvider
$this->app->bind(CacheKeyAndTags::class, fn() => new \App\GraphQL\Cache\CacheKeyAndTagsGenerator());

p/s: if the parameter is an object, the person who wrote the code just need to implement __toString by their own

eslym avatar Oct 05 '22 02:10 eslym

Could implementing __toString() in the enum class solve both issues at once?

spawnia avatar Oct 05 '22 08:10 spawnia

no, php enum does not allow __toString(), but it can implement an interface, so a custom contract might help in this case

eslym avatar Oct 05 '22 08:10 eslym