frictionlessdata / datapackage-js

A JavaScript library for working with Data Package.
http://frictionlessdata.io/
MIT License
43 stars 15 forks source link

Improve validation for data package descriptor properties #96

Closed ghost closed 6 years ago

ghost commented 6 years ago

Hi Not sure if I missed it in the library, but when adding non-required properties, http://frictionlessdata.io/specs/data-package/#required-properties, such as:

it did not seem that the package descriptor validated these (e.g., version). I can see these fields (and any others I create) added to the datapackage descriptor (if I'm putting these in the right place?), but they do not seem to have any validation against them. Please let me know what I might have missed, or could validation of these properties be considered in a future release? @Stephen-Gates, thoughts?

Stephen-Gates commented 6 years ago

Thanks @mattRedBox

Are you saying that only the existence of at least one resource is tested and other items aren't?

Without looking at the code, I'd like to think that checks like the following should be done:

If that's the case, then I agree with your feature request.

ghost commented 6 years ago

Hi @Stephen-Gates Sorry a little overzealous in vomiting up properties. From what I can see, and again it may just be that I'm not engaging the library correctly, the descriptor validation for package follows similar/same for descriptor validation for resource. In both:

But again some of these I may have missed or there may be others that I haven't included- so just seeking clarification first, but then if there are fields that don't have validation present, just asking if perhaps it is in the pipeline of work for datapackage.js or could be considered for future work

Hats off to you guys, though - really tidy work - I really like the consistency of this and tableschema.js. Both libraries have been straightforward to engage with and I really appreciate the doco. 👍

Stephen-Gates commented 6 years ago

I guess I'd be looking for validation on the MUST statements in the specification regardless of the metadata property being required, recommended or optional.

Specifically for version,

a version string identifying the version of the package. It should conform to the Semantic Versioning requirements.

There's a pattern recommending an approach for versioning but given the spec only requires a string, I'd expect the validation to check that if the optional metadata properties version exists, it must be a string.

It may be useful to identify the MUST statements in the spec that don't have validation so it is clear what needs to be done.

Here's my attempt:

Again, I didn't look at the code to check if some of these are implemented.

roll commented 6 years ago

@mattRedBox @Stephen-Gates The package uses general validation based on JSON Schema profiles e.g. http://frictionlessdata.io/schemas/data-package.json. So even it's not in the code many of checklist above points is covered by the JSON Schema validation. But some properties still requires special case validation like semver validation etc.

roll commented 6 years ago

CLOSING it for now because almost every property having the specs level constraints are validated using JSON Schema - https://frictionlessdata.io/schemas/data-package.json

I suggest to us to open individual issue if we find discrepancies between implemented validation and the specs text (with a data example for quick fixing).