Overlay-Specification icon indicating copy to clipboard operation
Overlay-Specification copied to clipboard

Switch from add to replace/update properties

Open lornajane opened this issue 1 year ago • 6 comments

I'm proposing this change as a response to the ongoing discussion in #30 . Using "add" can be confusing and leaves us without being able to support some use cases, such as setting an array to a particular set of elements. Looking around for similar-ish approaches, we found that JSONPath uses "update" for our existing "add" operation, and "replace" to achieve another sort of adding something. I've therefore proposed a specification update to adopt the new terminology.

It's a big change and while sooner is of course the best time to be making these changes, I'd appreciate as many eyes and as much feedback as possible. Comments and questions welcome.

lornajane avatar Mar 30 '24 16:03 lornajane

Typo in lorna's message there, she meant to say:

we found that JSON Patch

Basically nicking the relevant and useful concepts from JSON Patch RFC 6902 and their list of operations: https://www.rfc-editor.org/rfc/rfc6902.html#section-4

philsturgeon avatar Apr 02 '24 12:04 philsturgeon

Really appreciate having some parallels with JSON Patch!

kevinswiber avatar Apr 02 '24 19:04 kevinswiber

@kevinswiber Thanks for the approval, but do you have any thoughts on how to version this in a way that won't confuse existing users? That's what got me really stuck.

lornajane avatar Jun 20 '24 12:06 lornajane

@kevinswiber Thanks for the approval, but do you have any thoughts on how to version this in a way that won't confuse existing users? That's what got me really stuck.

@lornajane Seeing as how Overlays has been in a pre-release phase, I'm not compelled to allow for backwards-compatibility. Breaking backwards-compatibility has a negative impact on early users but a positive impact on future users. I look at an example of a project that made this hard decision, Node.js. Before their first major release, they'd break backwards compatibility all the time. This was becoming more difficult as adoption was growing. In retrospect, I believe it was the right move. It led to greater stability when even more users were being impacted by changes.

I believe @ThomasRooney has a different perspective worth including in the conversation, as they're dealing with existing users.

kevinswiber avatar Jun 20 '24 19:06 kevinswiber

I believe @ThomasRooney has a different perspective worth including in the conversation, as they're dealing with existing users.

Thanks for asking. My persective is that making a breaking change without a corresponding version change is always bad, the moment that specification has been adopted. I understand that this specification remains a "draft", but it is also:

  1. Over 3 years old.
  2. Already in use by a non-trivial number of organisations, given it solves a significant existing need. Whilst a few of these organisations (e.g. Speakeasy, Bump) work in public, through my work with Speakeasy I can definitely state that there are a number of organisations that have also adopted the overlay specification internally that would also be affected.

There's nothing wrong with breaking changes, but I strongly feel that if a breaking change is deemed important, it would be kinder to:

  1. Leave the current document as 1.0.0.
  2. Make any breaking changes in 2.0.0

Or, alternatively:

  1. Reduce the current specification version to 0.1.0.
  2. Make any breaking changes wanted in 0.2.0.

That way, any of the organisations that manage documents with overlay: 1.0.0 get an easy migration path: we'd just automatically alter the version of any overlay documents we interact with from 1.0.0 to 0.1.0. This pathway would remain viable for however many months (years?) it'll take to make a non-draft 1.0.0.


I personally think the change from update to add isn't actually a positive one anyway, because:

  1. An openapi overlay is data-format-agnostic. I.e. it doesn't just apply to JSON documents, but to whatever format an openapi document is kept in. This, whilst primarily YAML, also includes pkl, cue, as well as in-memory representations. Hence I have a bit of aversion to leaning on JSON Patch add nomenclature to respresent the "shallow merge + array append" that is actually happening.
  2. Probably the most common thing we see overlays used for is overriding something like description. This is primarily by a DX/technical writing team trying to produce a good documentation site or SDK type info that's produced downstream of the OAS. Whilst this is usually done by a shallow merge of the parent object, we've also seen target be applied to the scalar string value. add feels ambiguous when applied to scalars (strings, numbers), whereas update does not. E.g. consider the case where a description node is targetted to be overridden, add sounds to me like it should append to the existing string, rather than override it.

For the underlying problem:

  1. In my opinion, adding a replace action would be a nice bit of syntactic sugar to reduce ambiguity, especially for cases where an array item is being replaced.
  2. Would also love to have a move action for something like adding a oneOf (or converting a oneOf into an allOf/anyOf) in a JSON Schema. This is a relatively common action for us as we help represent different use-cases with more constrained json schemas to give better DX for our customers APIs, but we currently risk a higher degree of brittleness to do this with update overrides.

ThomasRooney avatar Jun 20 '24 21:06 ThomasRooney

@ThomasRooney

An openapi overlay is data-format-agnostic. I.e. it doesn't just apply to JSON documents, but to whatever format an openapi document is kept in. This, whilst primarily YAML, also includes pkl, cue, as well as in-memory representations.

This is a good point to support my recent proposal that OAS 4.0 "Moonwalk" should define an in-memory structure and how it can (and can't) be manipulated, so that we're less format-dependent for programmatic editing. Which doesn't change anything for this PR given how long it will be before 4.0 is a practical concern. But I feel like the Overlays project should impact that design.

handrews avatar Jun 20 '24 22:06 handrews