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

Patch types via Patch-Type instead of Content-Type #126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

toomim
Copy link
Member

@toomim toomim commented Sep 18, 2023

The spec currently specifies custom patch types in the same way that the HTTP PATCH method does: using the Content-Type header.

But this sucks, because it clobbers other uses of Content-Type that we care about.

  • We can't allow an actual Content-Type to exist in the same message as a patch type expressed as Content-Type
  • We can't use Content-Type to specify the Patch Type in a place where Content-Type already means something (like a GET response)

This rules out these use-cases:

  • A client creating a resource with a patch can't specify the content-type of the resource it wants to create
  • A client editing a resource with multiple representations (e.g. text/markdown that gets rendered into application/html), can't specify which representation it wants its patch to apply to
  • A server can't return a custom patch type in a GET response, without wrapping it in a Patches: N, where there is not existing definition of content-type.

So I propose that we ignore how PATCH does things, and introduce an actual Patch-Type header for patch types. This PR makes the change. We only need to change 6 lines.

(More context in https://braid.org/meeting-69/http-patch-design)

@toomim toomim changed the title Specify patch types with Patch-Type instead of Content-Type Patch types via Patch-Type instead of Content-Type Sep 18, 2023
@toomim
Copy link
Member Author

toomim commented Sep 18, 2023

Note: this is a counter-proposal to #120

@toomim toomim added this to the Braid-HTTP 03 milestone Sep 18, 2023
@toomim
Copy link
Member Author

toomim commented Oct 18, 2023

Rahul (@CxRes) pointed out a problem with my thinking here: Content-Type describes the content of the HTTP message, not the resource. Specifically:

HTTP/1.1 200 OK
Header1: value1
Header2: value2

this is content         <--- "content" is the stuff after the newline

That means that if we put a patch in the content section, we need the Content-Type: header to describe that patch.

But we still need a way to communicate the mime-type that the patch is applying to. So perhaps we just need two headers, like:

Content-Type: application/range-patch
Representation-Type: application/json

Or a hybrid parameterized patch type, like:

Content-Type: application/range-patch(on: application/json)

@toomim
Copy link
Member Author

toomim commented Oct 18, 2023

(Chair:) Since this has gotten more complicated, I don't think we can resolve it in time for draft 03, and I'm removing it from that milestone.

@toomim toomim removed this from the Braid-HTTP 03 milestone Oct 18, 2023
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

Successfully merging this pull request may close these issues.

1 participant