update JSON Schema to handle optional fields

[hunterowens]

Running list of JSON validator fixes

• Currently, the parking_verification_field in trips is required, not optional, causing my validator to fail.
• Change 0.{version_number}.{anynumber} regex for version to {anynumber}.{anynumber}.{anynumber}-{anystring_or_null} to more close match semvar

[ian-r-rose]

On the second point: I agree that the last field should be {anynumber}-{anystring-or-null}. However, I think the first two should be kept the same: if an instance with major version 0.1.0 tries to validate against a schema of version 0.2.0, then it should fail based on the version number alone. My understanding of semver is that, pre-1.0, every minor version is effectively a major version.

[thekaveman]

I think the major concern @hunterowens raises with point two is, having to update that regex every time there is a new point release is one more thing to remember to do. And it would be a bad thing to forget.

Edit: hopefully the results of #129 will make this process a little less error-prone.

[hunterowens]

Additional thing to fix is that actual_cost and real_cost are optional, but current fail if null.

[ian-r-rose]

Should accuracy, trip_duration, and trip_distance be integers? My ontological sense thinks they should be floating point numbers, but there may be some context that I am not aware of.

[thekaveman]

@hunterowens:

Currently, the parking_verification_field in trips is required, not optional, causing my validator to fail.

That field is not in the list of required properties in the Trips schema:

 "trips": {
    "$id": "#/properties/data/properties/trips",
    "type": "array",
    "title": "The Trips Schema",
    "items": {
        "$id": "#/properties/data/properties/trips/items",
        "type": "object",
        "title": "The Trip Schema",
        "required": [
            "provider_name",
            "provider_id",
            "device_id",
            "vehicle_id",
            "vehicle_type",
            "propulsion_type",
            "trip_id",
            "trip_duration",
            "trip_distance",
            "route",
            "accuracy",
            "start_time",
            "end_time"
        ],
    }
}

However that property does have a validation regex pattern that forces HTTPS, is that maybe the issue?

"parking_verification_url": {
    "$id": "#/properties/data/properties/trips/items/properties/parking_verification_url",
    "type": "string",
    "description": "A URL to a photo (or other evidence) of proper vehicle parking",
    "default": "",
    "examples": [
        "https://data.provider.co/parking_verify/1234.jpg"
    ],
    "pattern": "^(https://.*)$"
}

e.g. are you validating data that looks like:

{
    "etc": "other data",
    "parking_verification_url": null
}

or

{
    "etc": "other data",
    "parking_verification_url": ""
}

This might fail, since null and "" both don't pass that regex. vs. data that just doesn't include that field at all (which is what the Schema says is allowed).

@ian-r-rose any insight here?

[hunterowens]

The data I'm trying to validate is

        "parking_verification_url": null,

[thekaveman]

Ok, that makes sense then. I think this SO post has a way to update the schema to handle null. I'll get that in a branch and we can try.

This should also work for the cost fields.

[ian-r-rose]

@thekaveman I think your proposed solution looks good to me. Unless you want the parking_verification_url to be non-nullable :) Some people have very strong opinions about null values.

[thekaveman]

It sounds like we're already seeing it in the wild right @hunterowens? So do we allow data like you pasted above, or should the spec say "optional fields must not be included when they do not have a value"?

[thekaveman]

I submitted a PR with the fix to allow null values. I can easily back that out if we don't want to, and re-word the README language to be clearer per my suggestion above.

There is another fix in that PR related to the Schema, and I targeted a new branch 0.2.x I just cut from dev for non-breaking changes for the next patch release.

[thekaveman]

@hunterowens was this fixed by #142?

[hunterowens]

Yep.

[thekaveman]

@hunterowens Question about the links property for API paging: I'm seeing data like

{
    "links":
    {
        "next": "https://....",
        "prev": null,
        "first": null,
        "last": null
    }
}

For the purposes of paging, this is really all we need... but it doesn't pass schema validation. I think we should probably allow an additional exception like in #142 for these links properties (only next would be required). Thoughts?