On Fri, Mar 3, 2023, at 6:19 AM, Jakub Zelenka wrote:
>> You mean using the version from the JSON string, and allowing an
>> override? Like this?
>>
>>
> It should never allow overriding the $schema as it would go against spec so
> this would be just default if $schema is not specified. Just the default
> could be overridden.
That sounds like it could be hard to explain why a parameter only sometimes
does something... Which suggests we should take a different approach.
>> new JsonSchema($schema_string, version: JsonSchema::DRAFT_4);
>>
> Would be probably called more JsonSchema::VERSION_DRAFT_04 but essentially
> yeah I was thinking either that or something like just global constant
> JSON_SCHEMA_VERSION_DRAFT_04 which is currently more convention in json
> extension. I wouldn't really mind using the class constant though.
Class constants FTW.
> As I said my main point was that custom defined schema should contain
> version in the schema - I realise that it might not be used in wild but
> this is defined as SHOULD in all drafts so we should follow that
> recommendation in our API design. When this is the case, there is no point
> for user to explicitly pick the schema class IMO.
>
> Another thing to note is that we might want to introduce some sort of a
> factory method or factory class instead of using constructor because as I
> said before we would probably like to introduce more sources for schema
> than just string in the future. It means it could be automatically
> generated schema from a class so only the class name would be passed or for
> convenience it could be just passed directly from the assoc array. It is
> basically pointless to always convert it to string because internally it
> will just decode the json string to object (stdClass) or more likely array
> and parse the schema internal representation from that. If we had this, we
> could maybe introduce a different schema classes as well but it would be
> more invisible for users and could be just subclasses of JsonSchema or
> JsonSchema would be just an interface.
I'm totally fine with factory methods. I'm less enamored with a factory class,
but open to discussing it.
>> I see two issues there.
>>
>> 1. If I want to see if DRAFT_6 is available, I have to use defined()[1]
>> with strings. This is fugly.
>>
>>
> This is a good point as for class with autoloader you don't need strings.
>
> Maybe we could also introduce JsonSchema::VERSION_LATEST which would have
> value of the last supported draft. Then you could check if draft-06 is
> supported by just doing something like
>
> if (JsonSchema::VERSION_LATEST > JsonSchema::VERSION_DRAFT_04) {
> // at lest draft 06
> } else if (JsonSchema::VERSION_LATEST > JsonSchema::VERSION_DRAFT_06) {
> // at lest draft 07
> } else ...
>
>
> We could also support string for the actual version and allowing passing
> the well defined values for $schema. The it would throw exception if it's
> not supported. We could even have static method to check whether version is
> supported and then for example doing something like
>
> if (JsonSchema::isVersionSupported("
> https://json-schema.org/draft/2020-12/schema";)) { ... }
thinking-face.gif
That could work. For simplicity someone in user space (FIG?) could release a
set of constants that can be rapidly updated, but the actual PHP API is just
looking at the URL. If we expect it to be a rarely-used feature, I'd be on
board with that.
>> 2. I don't know how to polyfill newer spec versions if I don't want to
>> wait for internals to get around to adding a new version.
>>
>
> I guess it could be possible to support custom user validators (e.g.
> instances implementing JsonSchema interface if above concept is used) in
> the future but it would be of course more limited. That's not something
> that would happen initially but it might be good design an API with that in
> mind.
>
> So to sum it up, maybe the rough structure could be something like
>
> interface JsonSchema {
> const VERSION_DRAFT_04 = 1;
> const VERSION_DRAFT_06 = 2;
> const VERSION_DRAFT_07 = 3;
> const VERSION_LATEST = 3;
>
> public function validate(array|stdClass $data): bool;
> }
>
> class JsonSchemaForVersionDraft04 implements JsonSchema {
> public function validate(array|stdClass $data): bool {}
> }
>
> class JsonSchemaForVersionDraft06 implements JsonSchema {
> public function validate(array|stdClass $data): bool {}
> }
>
> class JsonSchemaForVersionDraft07 implements JsonSchema {
> public function validate(array|stdClass $data): bool {}
> }
>
> class JsonSchemaFactory {
> public static function createFromJsonString(string $jsonString,
> int|string $version = JsonSchema::VERSION_LATEST): JsonSchema {}
>
> public static function createFromClasl(string $className, int|string
> $version = JsonSchema::VERSION_LATEST): JsonSchema {}
>
> public static function createFromArray(array $schemaData, int|string
> $version = JsonSchema::VERSION_LATEST): JsonSchema {}
>
> public static function isVersionSupported(int|string $version): bool {}
> }
>
> Just a draft of course so any ideas how to improve it are welcome.
>
> What do you think?
Hm. So you're idea is that you'd parse the JSON string with json_decode()
first, then pass that to the validator? Is that really the most
performant/convenient approach? (I don't know, but I question if it is.)
Thinking about it another way, I see three base primitives that are better done
in C than in PHP.
1) validate(SchemaDefinition $schema_definition, $json_value): bool
2) make_schema(string $jsonSchemaString): SchemaDefinition (and possibly
alternates with other typed parameters)
3) make_schema_from_class(string $className, string $version = latest
available): SchemaDefinition
Everything else could be done in user-space without any significant performance
impact. Even 3 could technically be done in user space if SchemaDefinition had
a robust API that could be populated from user space, rather than being opaque.
So a major question is what level of exposed API we want the schema object to
have. (I could probably argue both ways here.)
(If you want to continue this real-time, I'm available in most of the PHP chat
fora these days.)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
