Skip to content

Schema->processObjectRequired() does not handle property mappings #124

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

Closed
mmelvin0 opened this issue Jul 13, 2021 · 3 comments · Fixed by #125
Closed

Schema->processObjectRequired() does not handle property mappings #124

mmelvin0 opened this issue Jul 13, 2021 · 3 comments · Fixed by #125

Comments

@mmelvin0
Copy link

If I have a property name mediaType and a property mapping defining its data name as media_type, I get an error that mediaType is missing but it should be looking for media_type.

Example from my entity's setUpProperties():

$ownerSchema->required = [static::names()->mediaType];
$ownerSchema->addPropertyMapping('media_type', static::names()->mediaType);

Am I missing something or should this work the way I'm expecting? Seems like the property mapping needs to be applied here:

if (!array_key_exists($item, $array)) {

@vearutop
Copy link
Member

Please check v0.12.36 that now allows using mapped reflected names:

// Define default mapping.
$ownerSchema->addPropertyMapping('media_type', static::names()->mediaType);

// Use mapped name references after the default mapping was configured.
$names = self::names($ownerSchema->properties);
$ownerSchema->required = [$names->mediaType];

@vearutop
Copy link
Member

Seems like the property mapping needs to be applied here

Property mapping is mainly introduced to allow certain code style in PHP classes, but at the same time schema is portable and operates with "raw" data. To keep this portability (and interoperability with other languages) schema needs to be defined in terms of raw property names, and not as a runtime logic.

@mmelvin0
Copy link
Author

mmelvin0 commented Jul 13, 2021

Makes total sense and I'd probably ultimately lead a happier life if I just named my property media_type and avoiding property mapping entirely.

That said, v0.12.36 now works perfectly for my use case. Rapid response much appreciated!

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

Successfully merging a pull request may close this issue.

2 participants