w3c / wpub

W3C Web Publications
https://w3c.github.io/wpub/
Other
79 stars 19 forks source link

Minor comments on the JSON schema #316

Closed iherman closed 6 years ago

iherman commented 6 years ago

I have some minor comments on the JSON schema. Caveat: I have never created any serious schema myself, so I may have misunderstood some of the syntax...

Localizable string object

I believe that the following structure should be allowed:

{
    "@value": "some text
}

It is certainly valid JSON-LD, just indicates that no language has been set at all. This is the way canonicalization works (for now). On the other hand, it says (at two places) "required": ["@value","@language"] in localizable.schema.json. It should probably refer to @value only.

Arrays of localizable strings

The current version is such that once I have an array, I can only use full blown objects (unless I misread it). This means that "editor": ["Matt Garrish", "Ivan Herman"] would not be acceptable. I do not think we should have this restriction.

Contributor objects

The current schema restricts the value of "@type" values to Person or Organization. This means that a type of the sort "@type" : ["Person","SomeOtherType"] would not be valid. At this moment there is no extension to, say, "Person" for publishing (there is one: Patient) which is not relevant to us) but that may come later. Maybe it is better to be a bit more liberal and require either Person or Organization to be present in the "@type" string, but otherwise allow everything else.

Type

The "main" schema (ie, publication.schema.json) does not include "@type" explicitly. We do require the presence of types in the draft, maybe it is better to reflect this in the schema, too...

iherman commented 6 years ago

One more entry: I think it would be worth checking "@context", too; that is a requirement in the spec.

HadrienGardeur commented 6 years ago

It is certainly valid JSON-LD, just indicates that no language has been set at all.

It is valid JSON-LD, but is it something that we want to allow in our manifest? I can relax that requirement, but it makes very little sense to use @value when you could simply use a string.

The current version is such that once I have an array, I can only use full blown objects (unless I misread it).

That's not the case, an array of strings is allowed: https://github.com/w3c/wpub/blob/master/schema/contributor.schema.json#L14

Maybe it is better to be a bit more liberal and require either Person or Organization to be present in the @type string, but otherwise allow everything else.

I'll tweak the schema for @type accordingly.

The "main" schema (ie, publication.schema.json) does not include @type explicitly. We do require the presence of types in the draft, maybe it is better to reflect this in the schema, too...

I'll require a string or an array of string with unique items. I don't think we can require any specific value though.

One more entry: I think it would be worth checking @context, too; that is a requirement in the spec.

I'll add it and validate the presence of both context documents.

iherman commented 6 years ago

It is certainly valid JSON-LD, just indicates that no language has been set at all.

It is valid JSON-LD, but is it something that we want to allow in our manifest? I can relax that requirement, but it makes very little sense to use @value when you could simply use a string.

I think so. The canonicalization turns all localizable strings into an object with value and, possibly, a language. The reason is, like elsewhere in canonicalization, to provide a fully uniform structure everywhere. "Possibly", because if there is no language set, than that member is missing. (A canonical manifest must be a valid manifest...).

We could require the language in all cases and use the "language"="und" value, which is a valid tag for 'undefined'. But I find that even worse...

The current version is such that once I have an array, I can only use full blown objects (unless I misread it).

That's not the case, an array of strings is allowed: > https://github.com/w3c/wpub/blob/master/schema/contributor.schema.json#L14

So I did misread it. Sorry.

...

The "main" schema (ie, publication.schema.json) does not include @type explicitly. We do require the presence of types in the draft, maybe it is better to reflect this in the schema, too...

I'll require a string or an array of string with unique items. I don't think we can require any specific value though.

Agreed.

Thanks!