fix(update): add json-merge-patch guidance

[toumorokoshi]

After offline discussion, it was agreed upon that IETF RFC 7396 (JSON Merge Patch) is the more sensible choice for a partial update on an HTTP+JSON API. The rationale includes:

• can generally be expressed via protobuf field masks.
• aligns with the AEPs usage of resource-oriented guidance.
• aligns with an existing IETF standard, which as already been widely adopted.

Other minor changes:

• removes duplicative guidance around LROs to make the AEP succinct.
• populates oas guidance, sans etags which may require some more design (in HTTP etags are not added to the resource body).

Fixes #109.

🍱 Types of changes

What types of changes does your code introduce to AEP? Put an x in the boxes
that apply

• Enhancement
• New proposal
• Migrated from google.aip.dev
• Chore / Quick Fix

📋 Your checklist for this pull request

Please review the AEP Style and Guidance for
contributing to this repository.

General

• Basic Guidance is met.
• Ensure that your PR
  references AEPs
  correctly.
• My code has been formatted
  (usually prettier -w .)

Additional checklist for a new AEP

💝 Thank you!

[toumorokoshi]

FYI @dv-stewarts if you'd like to take a look and give your thoughts.

[dv-stewarts]

AEP includes guidance on HTTP and gRPC transcoding. Will supporting update_mask for gRPC and json-merge-patch for OAS cause the transcoded gRPC APIs to be invalid wrt the OAS spec? Is that a common occurrence and is it viewed as a problem?

We have been using gRPC transcoding as a way to get people into gRPC on the server side without having to fight so many battles with API consumers. Having "native" OAS APIs working differently than transcoded APIs while both claiming to be AEP compliant is likely to sow confusion.

[toumorokoshi]

@dv-stewarts Thanks for taking a look!

AEP includes guidance on HTTP and gRPC transcoding. Will supporting update_mask for gRPC and json-merge-patch for OAS cause the transcoded gRPC APIs to be invalid wrt the OAS spec? Is that a common occurrence and is it viewed as a problem?

We have been using gRPC transcoding as a way to get people into gRPC on the server side without having to fight so many battles with API consumers. Having "native" OAS APIs working differently than transcoded APIs while both claiming to be AEP compliant is likely to sow confusion.

It isn't a very common occurrence - and ideally we wouldn't deviate. There are a lot of tensions w.r.t. this problem overall, including:

• different features in protocols (e.g. protobuf 2 will default non-nullable fields, while in json all fields are nullable).
• HTTP+JSON already has a rich ecosystem of IETF RFCs, many with significant adoption and also not looking like AEP guidance with existing protobufs.

I don't think there's a lot of conflicts generally between the protobuf and HTTP+JSON APIs in the context of the AEPs. and when there are, we try to reconcile them. I don't think we've agreed 100% on how this should be addressed. The conclusion we have reached so far is:

• json-merge-patch is probably the best solution so far (sans yet another standard) for PATCH in JSON.
• field masks are not a great solution for protobuf, but is already widely adopted.

Ideally, I think we'd replace existing protobuf guidance with something that aligns better with json-merge-patch. However, json-merge-patch uses the resource as the schema, while in protobuf that's not really an option given that the message schema may include non-nullable fields, which mean that you can't really send the intent that "you don't care about the field" without some sort of sidecar (in this case, the field mask).

At Google IIUC json-merge-patch is supported (e.g. https://cloud.google.com/workflows/docs/reference/googleapis/compute/v1/backendBuckets/patch), and there is a transcoding layer that handles the conversion of the patch to the protobuf field mask.

If you have a specific suggestion, it's definitely up for discussion! I'd encourage you to share any ideas you may have.

[toumorokoshi]

@mkistler adding you as an official reviewer, thank you!

[mkistler]

Left one suggestion for a minor wording change.

I'm also unclear if we should allow "application/json" content type.

Otherwise this looks good.

[fsaintjacques]

I find it confusing that this would be a JSON first solution where-as this is a protobuf driven spec. I'd much prefer to stick with FieldMask than having to deal with this required infrastructure (transcoding from json-patch to grpc-fieldmask). That almost makes me ignore aep as a potential user.

[toumorokoshi]

I find it confusing that this would be a JSON first solution where-as this is a protobuf driven spec. I'd much prefer to stick with FieldMask than having to deal with this required infrastructure (transcoding from json-patch to grpc-fieldmask). That almost makes me ignore aep as a potential user.

Thanks for the feedback! It's worth clarifying - aep is not a proto-driven spec. It is intended to be protocol agnostic principles, with a precise specification in multiple protocols (grpc and HTTP+JSON are the two in scope today). We're working on better docs, but there's a video where the project goals are explained: https://www.youtube.com/watch?v=we9Qp2kHsvY&t=1s.

That all said I do acknowledge the difficulty in building a proper transcoding layer when the interface is so different. I think we'd like to introduce tooling (possibly building adapter services similar to grpc-gateway, or extend / contribute to it). But in the weekly meeting discussions we've agreed that we should be provide idiomatic guidance for each protcol we support, and diverge only when there's complete incompatibility between the two. In this case, we discussed that it's better to choose the widely supported json-merge-patch specification. (But of course, a separate maintainer has to approve this guidance to ratify it).

If there is an alternate idiomatic interface that works well for both, definitely interested to hear of one!

[toumorokoshi]

I've also added an explanation of json-merge-patch: As an FYI, grpc-HTTP bindings such grpc-gateway support this patch-style translation, so I think ultimately this guidance is actual interoperable with existing proto and proto-http-binding best practices.

[toumorokoshi]

@gibson042 @dv-stewarts @mkistler sorry for the long delay on this one - I've rebased the content, removed the guidance to suggest application/json, and added a blurb explaining why this actually works out ok for both HTTP and proto. PTAL, and I appreciate the dialog!