openactive / open-booking-api

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

Refactor requests to use compact JSON-LD @id references #191

Open nickevansuk opened 3 years ago

nickevansuk commented 3 years ago

As outlined in openactive/modelling-opportunity-data#264 the requests and Orders Feed within the Open Booking API should be updated to use compact @id references, in line with the existing open data feeds.

Although a compromise form of "orderedItem": { "@id": "https://example.com/events/452/subEvents/132" } is also possible, the most compact string representation for @id is likely much easier to implement in terms of tooling support.

This makes it easier to reliably distinguish between when the full object data is provided in the feed, or when just a reference is provided - as the property is either a Url (reference) or an object (data), rather than the current object-as-a-reference setup.

This will also reduce the size and complexity of the requests and Orders feed responses that need to be constructed.

So instead of this:

  "seller": {
    "@type": "Organization",
    "@id": "https://example.com/api/organisations/123"
  },
  "orderedItem": [
    {
      "@type": "OrderItem",
      "acceptedOffer": {
        "@type": "Offer",
        "@id": "https://example.com/events/452#/offers/878"
      },
      "orderedItem": {
        "@type": "ScheduledSession",
        "@id": "https://example.com/events/452/subEvents/132"
      }
    }
  ]

We would have this:

  "seller": "https://example.com/api/organisations/123",
  "orderedItem": [
    {
      "@type": "OrderItem",
      "acceptedOffer": "https://example.com/events/452#/offers/878",
      "orderedItem": "https://example.com/events/452/subEvents/132"
    }
  ]
nathansalter commented 3 years ago

This is good, but should be optional. In some cases it can simplify the code for generating responses as you don't have to check the whole object to see if it has any other properties than the @id

nickevansuk commented 3 years ago

you don't have to check the whole object to see if it has any other properties than the @id

Yes exactly this! It also turns out the previous approach was inconsistent from a modelling perspective - much better to just use @id references as URLs rather than empty objects. In terms of making it optional: suggest that creates more complexity for all implementations? Would be save time/cost/complexity for the ecosystem overall to switch wholesale so that everything is consistent?

nathansalter commented 3 years ago

Although it must be frustrating, we're using this in production between three different systems. So any backwards incompatible change has to be made in all three places at once, which just isn't possible. Although it does add complexity, we need some kind of migration path between the two. This should be fairly easy to handle in the model libraries however, just update the object validator to create empty objects if it's a URI

nickevansuk commented 3 years ago

Yes agreed - another option is a booking system whitelist for those that only support expanded representation? So e.g. Bookteq and Schools Plus could have a flag set to use the expanded representation until they can migrate to the compated one?

nathansalter commented 3 years ago

Yes, some kind of feature flag system does make sense here, but I'm fairly sure we don't have this in the models libraries yet, and as all communication between systems goes through these libraries, the update will need to be there.

nickevansuk commented 3 years ago

The models libraries should all be updated to support @id referencing across all relevant attributes in a non-breaking way (this is something that's valid JSON-LD and was just missing from the libraries before), though there's not been any specific support for mapping between empty objects and an @id reference...

If the models library would help smooth the migration path perhaps that could be an option? Given this is very focussed on acceptedOffer and orderedItem properties might it be the case that this only touches a few lines of code in the affected systems (and therefore is simpler to update the systems that write a shim in the model library)?

nathansalter commented 3 years ago

If the models library would help smooth the migration path perhaps that could be an option?

100% on this. I'll put a task in for next sprint to handle this case. The main issue is that because communication is two-way all these cases need to be handled, we'll need some kind of configuration allowed to change what's serialized before sending. As if the opposite library can't handle the @id strings instead of objects it will fail to parse what you're sending it. Also if a Seller updates their library the seller integration will break, so it requires communication between both parties.