braid-org / braid-spec

Working area for Braid extensions to HTTP
https://braid.org
233 stars 16 forks source link

Feature detection #89

Open josephg opened 3 years ago

josephg commented 3 years ago

There's a few cases where clients will need to behave differently based on the features of the server:

We should add a way for the client to discover the behaviour of a server. We could either add a accept-patches / accept-ranges headers (and ?? not sure for versions). Or we could add some fields to the OPTIONS response for a braid object.

Thoughts?

mitar commented 3 years ago

Related: https://github.com/braid-work/braid-spec/issues/49

And I completely agree.

josephg commented 3 years ago

This came up at our fortnightly meeting today - we spent some time discussing how braid should behave if a client does not support a specified merge type.

My preferred approach here is an optional header in subscribe requests:

accept-patch-types: braid-content-ranges json-patch
accept-merge-types: automerge yjs

If a client does not support a document's patch type, the server should replace patches in the stream with complete document snapshots.

If a client does not support a document's merge type, the server should explicitly add merges into the stream. (Normally the merged document state is implicit, but the server could send explicit merge states for clients which need them.

Eg given this situation:

  A
 / \
B   C
 \ /
  BC (implicit merged version after the merge)

If a client supports the merge type, the subscription stream would contain:

Snapshot A (parents: ROOT)
Patch B (parents: A)
Patch C (parents: A)

Version BC does not need to be sent because the client can compute its state based on patches B and C.

However, if a client does not understand the merge type, the stream may be rewritten to:

Snapshot A (parents: ROOT)
Patch B (parents: A)
Placeholder for patch C (parents: A) (?? Does this need to be sent? It exists in history, but it cannot be merged by the client)
Snapshot for version "B" "C" (parents: "B", "C") (Merged document explicit because it cannot be inferred by the client)

This s a bit of a departure from the existing model because currently sent versions contain exactly one value, and merge states are implicit. Thoughts?

toomim commented 1 year ago

I've realized that we need feature detection to work across caches/proxies as well.

As was brought up in issue #74, if you issue a PUT with a content-range, you need to ensure that the endpoint understands content-range, or it will obliterate the entire resource and replace it with just the new range of content.

This needs to be true both for the origin server, and for any proxy in the middle. Otherwise, the proxy might interpret the PUT range as replacing the whole resource, and start serving that range to its clients instead of the entire resource, even if the origin server understands content-range.

Conclusion: We need to make sure that feature detection works for all proxies in the path between the client and origin server, as well as the origin server itself.

toomim commented 1 year ago

Seph notes in issue https://github.com/braid-org/braid-spec/issues/67#issuecomment-804434215 that we'll want feature detection for both requests and responses:

  • Does the server allow patches from the client? If so, what type?
  • Are patches going to be sent in subscriptions? If so, what type?