swaggest / php-json-schema

High definition PHP structures with JSON-schema based validation
MIT License
438 stars 50 forks source link

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

Closed mmelvin0 closed 3 years ago

mmelvin0 commented 3 years ago

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: https://github.com/swaggest/php-json-schema/blob/d0126bd830c92a498691d278f01d3ce13a2c9c0a/src/Schema.php#L556

vearutop commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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!