Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119662
MIME-Version: 1.0
References: <CANMwmbMObQJD=bBncxXcHOgs1yoBfHXsTE1KZDYcSKMxMUUrRQ@mail.gmail.com>
 <CALKiJKpuDUJ6cyh8DdCFV2o740A3ua+H0nc3_qYHwCrrSdLkGg@mail.gmail.com>
 <CAEKnhAGsAgNxfhGBLRU69HibaFYaeef4NsefxHYOQXyF2ZNSkw@mail.gmail.com>
 <CALKiJKr2nGAZpBEteUMX2EO_8HmX4s_TvVh1FeM3NJdeg=+6Kg@mail.gmail.com>
 <CAEKnhAGjn54aKAd-GQM7e=OScZJK02+060zRJO-N7r4jAbFHzA@mail.gmail.com> <7e08b0f9-9d30-4ead-8194-2494ecc78e2e@app.fastmail.com>
In-Reply-To: <7e08b0f9-9d30-4ead-8194-2494ecc78e2e@app.fastmail.com>
Date: Fri, 3 Mar 2023 12:19:19 +0000
Message-ID: <CAEKnhAFL_H19o3WbHYgT-Xi0KwwYVSabLOakCgycYBhRf+2XiA@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000001276fc05f5fdf548"
Subject: Re: [PHP-DEV] RFC Idea - json_validate() validate schema
From: bukka@php.net (Jakub Zelenka)

--0000000000001276fc05f5fdf548
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Mar 2, 2023 at 8:30=E2=80=AFPM Larry Garfield <larry@garfieldtech.c=
om>
wrote:

> On Thu, Mar 2, 2023, at 1:31 PM, Jakub Zelenka wrote:
> > On Thu, Mar 2, 2023 at 5:53=E2=80=AFPM Rowan Tommins <rowan.collins@gma=
il.com>
> > wrote:
> >
> >> On Thu, 2 Mar 2023 at 12:34, Jakub Zelenka <bukka@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 tha=
t
> as
> >> > something that happens often. But essentially you are right that the=
re
> >> 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 t=
he
> > different classes make less sense if $schema is specified as I would
> think
> > that the $schema should have a priority. So having just default argumen=
t
> > 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 =3D 1;
  const VERSION_DRAFT_06 =3D 2;
  const VERSION_DRAFT_07 =3D 3;
  const VERSION_LATEST =3D 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 =3D JsonSchema::VERSION_LATEST): JsonSchema {}

  public static function createFromClasl(string $className, int|string
$version =3D JsonSchema::VERSION_LATEST): JsonSchema {}

  public static function createFromArray(array $schemaData, int|string
$version =3D 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

--0000000000001276fc05f5fdf548--