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

Feature detection #89

Open
josephg opened this issue Feb 16, 2021 · 4 comments
Open

Feature detection #89

josephg opened this issue Feb 16, 2021 · 4 comments

Comments

@josephg
Copy link
Contributor

josephg commented Feb 16, 2021

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

  • Does this object support subscribe at all? (Though you could just try and see what happens)
  • Patch format support
  • Support for different merge types
  • Expected version behaviour - should the client send a version with patches or not?

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

mitar commented Feb 16, 2021

Related: #49

And I completely agree.

@josephg
Copy link
Contributor Author

josephg commented Mar 9, 2021

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

toomim commented Aug 19, 2023

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

toomim commented Aug 20, 2023

Seph notes in issue #67 (comment) 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?

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

No branches or pull requests

3 participants