search

openactive / open-booking-api

Repository for the Open Booking API specification
Other
2 stars 3 forks source link

Add InvalidUuidError #186

Open nickevansuk opened 3 years ago

nickevansuk commented 3 years ago

Add InvalidUuidError for the case when a UUID is supplied in the path of an Open Booking endpoint that is not in a valid GUID format

Add to spec that UUID outputted and inputted by Open Booking API implementations be must lowercase. This is a more constrained version of RFC 4122 Section 3, which requires that the characters be generated in lower case, while being case-insensitive on input. Forcing lowercase input reduces the number of error conditions internally to the implementation.

(Validation: add a "valueConstraint": "UUID" to the data model with matching rule in the validator and model libraries)

(The test for this should use a new randomly generated string for each test. Lowercase UUIDs should be accepted, whereas random strings and UUIDs with any uppercase characters should not be.)

nathansalter commented 3 years ago

I don't think this is clear that this only applies to places where UUIDs are required (which is only the Orders as far as I know). Especially for the RPDE feeds:

Although the IDs shown here are GUIDs, and other examples are numeric, this specification does not prescribe any specific format.

nickevansuk commented 3 years ago

Great point yes - should be clear this only applies within the model (to the identifier property of the Orders Feed), and the UUID used within the URL of the C1, C2, P and B.

Additionally, perhaps the Orders Feed / Order Proposals Feed should also use this format for their id for testability (otherwise it is much more complex to test the deleted item scenarios) - unless there's a compelling use case not to use the UUID as the RPDE id for these feeds?

nathansalter commented 3 years ago

unless there's a compelling use case not to use the UUID as the RPDE id for these feeds?

It definitely makes sense to me for the id of the feed item to be the same as the UUID of the Order. Worth noting that requiring this would be a breaking change however.

nickevansuk commented 3 years ago

All implementations I'm aware of currently use the UUID as the RPDE id for the Orders Feed, is that the same for the ones you're aware of @nathansalter ?

nathansalter commented 3 years ago

Schools Plus currently use numeric id in their Orders and Slots feeds