Hi, On Thu, Mar 2, 2023 at 8:30 PM Larry Garfield <la...@garfieldtech.com> wrote:
> On Thu, Mar 2, 2023, at 1:31 PM, Jakub Zelenka wrote: > > On Thu, Mar 2, 2023 at 5:53 PM Rowan Tommins <rowan.coll...@gmail.com> > > wrote: > > > >> On Thu, 2 Mar 2023 at 12:34, Jakub Zelenka <bu...@php.net> wrote: > >> > >> > It's possible that we might decide to stop supporting some drafts if > the > >> > maintenance burden is too big and usage small but I wouldn't see that > as > >> > something that happens often. But essentially you are right that there > >> will > >> > be minimum (draft-04 initially) and maximum (latest implemented > draft). > >> > > >> > >> > >> Thinking about this, particularly if there is the ability to install a > PECL > >> extension which supports different versions from the core PHP version, > >> perhaps it would be useful to expose a function or constant that lists > the > >> supported versions, so that code needing a particular version could > check > >> for support directly, rather than having to attempt and catch an > exception? > >> > >> > > That's a good point. I think a constant should be sufficient. I think the > > different classes make less sense if $schema is specified as I would > think > > that the $schema should have a priority. So having just default argument > > using the constant for the cases where it's not specified should be > > sufficient IMHO. > > > > Cheers > > > > Jakub > > 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. > 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. 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 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")) { ... } > 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? Cheers Jakub