Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composite Identifiers with type integer and string both cast to int #5396

Closed
starred-gijs opened this issue Feb 8, 2023 · 1 comment
Closed
Labels

Comments

@starred-gijs
Copy link

API Platform version(s) affected: 2.7.9 and 3.0+

Description

I have a ApiResource with 2 identifiers:

class Response
{
    #[ApiProperty(identifier: true)]
    private ?int $id;

    #[ApiProperty(identifier: true)]
    private ?string $verificationKey;
}

Which produces a uri eg: /responses/id=22133;verificationKey=7d75af772e637e45c36d041696e1128d

During the UriVariablesConverter::convert method, both values are cast to integer, which is incorrect because verificationKey in above case is cast to 7, which creates the wrong query in Provider.

Possible Solution
I have created a similar transformer, but with one addition

final class IntegerUriVariableTransformer implements UriVariableTransformerInterface
{
    public function transform($value, array $types, array $context = [])
    {
        return is_numeric($value) ? (int) $value : $value; // is_numeric check before cast
    }

    public function supportsTransformation($value, array $types, array $context = []): bool
    {
        return Type::BUILTIN_TYPE_INT === $types[0] && \is_string($value);
    }
}

I not sure this is the right way to address the issue, and if even my use-case is supported.

@soyuka
Copy link
Member

soyuka commented Feb 8, 2023

Indeed its hard to handle all cases on that matter. Prefer using string everywhere for identifiers and avoid multiple identifier properties if possible.

Your solution is good there's probably something we can improve there I'll transfer this to core

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants