Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nickevansuk opened this issue Mar 8, 2021 · 7 comments
Open

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

nickevansuk opened this issue Mar 8, 2021 · 7 comments
Labels
CR3 Issues relating to CR3

Comments

@nickevansuk
Copy link
Contributor

nickevansuk commented Mar 8, 2021

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
Copy link

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
Copy link
Contributor Author

nickevansuk commented Jun 17, 2021

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR3 Issues relating to CR3
Projects
None yet
Development

No branches or pull requests

2 participants