search

openactive / openactive-test-suite

Test suite for OpenActive implementations
MIT License
2 stars 9 forks source link

How to test GoneError? #323

Open lukehesluke opened 3 years ago

lukehesluke commented 3 years ago

In the Open Booking API, Order deletion can be either a hard delete or soft delete

When an Order is deleted the Booking System should hard delete it as though it had not been created in the first place (with only a stub deleted record remaining, for the Orders feed, if necessary), but may soft delete it providing all related personal data is purged

It suggests using a GoneError if accessing an Order which has been soft-deleted (https://openactive.io/open-booking-api/EditorsDraft/1.0CR3/#generic-errors).

Some Booking Systems may implement soft deletes and some hard deletes. How then can we write a test that tests the GoneError? Some options:

  1. Add a soft-deletes feature in which the GoneError tests are placed. A Booking System could then opt in to being tested in this way.

    • Con: Soft deletes are not greatly elaborated on in the spec. A Booking System could soft delete for a short while and then (at some later time) do a hard delete. Therefore, it's possible that a BS "supports" soft deletes, but may be set up in such a way that a GoneError test would fail (e.g. imagine a Booking System which does hard deletes on a cron job - say, every 24 hours. If the GoneError test runs on the margin of that 24 hour divide, an Order Deletion request may almost immediately end up in a hard delete, causing a test failure. It's a weird system, but is acceptable in the spec).

      Similarly, a system could support hard deletes in some cases and hard deletes in others. And then they would not have a clear setting for the feature.

  2. Create a generic test which checks that either new Orders can be created with the same UUID or a GoneError is returned. This way, regardless of which (hard deletes vs soft deletes) are supported, the test ensures that the system reacted appropriately

I'm leaning towards option 2

nickevansuk commented 3 years ago

For the case when the Order has already been cancelled, it should always appear in the feed, so perhaps we add a test Order delete following cancellation (as part of the cancellation tests?). This would guarantee that GoneError is returned. However that would require the cancellation feature to be enabled. Suggest this is therefore included in the seller-requested-cancellation feature.

To test as much as we can for booking systems without cancellation, option 2 sounds great.

lukehesluke commented 3 years ago

perhaps we add a test Order delete following cancellation (as part of the cancellation tests?)

@nickevansuk sounds like the orders-updated-then-deleted test, which is included as part of the order-deletion feature. (This has been written but awaits https://github.com/openactive/OpenActive.Server.NET/issues/87 to be finished).

(Aside: You raise a good point that it should only run if the seller-requested-cancellation AND order-deletion features are enabled. Currently, it's in order-deletion, but it is possible for a BS to support deletion but not cancellation. We have no other tests that require multiple features to be enabled, so perhaps we need to add something to support that. Not super-urgent, but it would become a problem if/when such a BS comes to use test suite)

Why does this guarantee that GoneError is returned? As I read the spec (https://openactive.io/open-booking-api/EditorsDraft/1.0CR3/#order-deletion), it looks like it could still be hard-deleted in the Booking System's orders database, say, but just persisted as a stub deleted item in their Orders RPDE database