-
Notifications
You must be signed in to change notification settings - Fork 17
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
Switch from add to replace/update properties #32
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ TBD | |
## Definitions | ||
|
||
##### <a name="overlayDocument"></a>Overlay Document | ||
An overlay document contains an ordered list of [Action Objects](#overlayActions) that are to be applied to the target document. Each [Action Object](#actionObject) has a `target` property and a modifier type (`update` or `remove`). The `target` property is a [JSONPath](https://datatracker.ietf.org/wg/jsonpath/documents/) query expression that identifies the elements of the target document to be updated and the modifier determines the change. | ||
An overlay document contains an ordered list of [Action Objects](#overlayActions) that are to be applied to the target document. Each [Action Object](#actionObject) has a `target` property and a modifier type (`add`, `replace`, or `remove`). The `target` property is a [JSONPath](https://datatracker.ietf.org/wg/jsonpath/documents/) query expression that identifies the elements of the target document to be updated and the modifier determines the change. | ||
|
||
## Specification | ||
|
||
|
@@ -80,7 +80,7 @@ Field Name | Type | Description | |
<a name="overlayVersion"></a>overlay | `string` | **REQUIRED**. This string MUST be the [version number](#versions) of the Overlay Specification that the Overlay document uses. The `overlay` field SHOULD be used by tooling to interpret the Overlay document. | ||
<a name="overlayInfo"></a>info | [Info Object](#infoObject) | **REQUIRED**. Provides metadata about the Overlay. The metadata MAY be used by tooling as required. | ||
<a name="overlayExtends"></a> extends | `string` | URL to the target document (such as an OpenAPI document) this overlay applies to. This MUST be in the form of a URL. | ||
<a name="overlayActions"></a>actions | [[Action Object](#actionObject)] | **REQUIRED** An ordered list of actions to be applied to the target document. The array MUST contain at least one value. | ||
<a name="overlayActions"></a> actions | [[Action Object](#actionObject)] | **REQUIRED** An ordered list of actions to be applied to the target document. The array MUST contain at least one value. | ||
|
||
This object MAY be extended with [Specification Extensions](#specificationExtensions). | ||
|
||
|
@@ -111,10 +111,11 @@ This object represents one or more changes to be applied to the target document | |
|
||
Field Name | Type | Description | ||
---|:---:|--- | ||
<a name="actionTarget"></a>target | `string` | **REQUIRED** A JSONPath query expression referencing the target objects in the target document. | ||
<a name="actionDescription"></a>description | `string` | A description of the action. [CommonMark syntax](https://spec.commonmark.org/) MAY be used for rich text representation. | ||
<a name="actionUpdate"></a>update | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. This property has no impact if `remove` property is `true`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth keeping update aroud for a minute as deprecated or is this early and experimental enough that whatever? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is always a hard decision to make. At this point in time, I'd vote to just kill it. We won't always have this luxury, but we should make use of it while we can. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll always have the commit history, but this does make me wonder if we should propose a 0.1.0 version and then start following a more formal release process. We're already seeing usage .... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .... Alternatively, how about moving forward from our current situation by making this a 1.1.0 and still a draft? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question retracted, you folks came up with a god solution on the call and I forget what it was, but I'm not standing in the way of anything 🫡 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This proposal will definitely be 2.0 in recognition of how breaking it is. |
||
<a name="actionRemove"></a>remove | `boolean` | A boolean value that indicates that the target object is to be removed from the the map or array it is contained in. The default value is `false`. | ||
target | `string` | **REQUIRED** A JSONPath query expression referencing the target objects in the target document. | ||
description | `string` | A description of the action. [CommonMark syntax](https://spec.commonmark.org/) MAY be used for rich text representation. | ||
add | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. If an object property exists, it is overwritten with the new value. Array elements are appended to any existing entries already in the array. This property MUST NOT be used with the `replace` property in the same action object. This property has no impact if `remove` property is `true`. | ||
replace | Any | An object with the properties and values to be merged with the object(s) referenced by the `target`. The entire contents of the target object or array is replaced by the new value(s) supplied. This property MUST NOT be used with the `add` property in the same action object. This property has no impact if `remove` property is `true`. | ||
lornajane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
remove | `boolean` | A boolean value that indicates that the target object is to be removed from the the map or array it is contained in. The default value is `false`. | ||
|
||
The result of the `target` JSONPath query expression must be zero or more objects or arrays (not primitive types or `null` values). If you wish to update a primitive value such as a string, the `target` expression should select the *containing* object in the target document. | ||
|
||
|
@@ -135,7 +136,7 @@ info: | |
version: 1.0.0 | ||
actions: | ||
- target: "$" # Root of document | ||
update: | ||
add: | ||
info: | ||
x-overlay-applied: structured-overlay | ||
paths: | ||
|
@@ -163,13 +164,13 @@ info: | |
version: 1.0.0 | ||
actions: | ||
- target: $.paths['/foo'].get | ||
update: | ||
add: | ||
description: This is the new description | ||
- target: $.paths['/bar'].get | ||
update: | ||
add: | ||
description: This is the updated description | ||
- target: $.paths['/bar'] | ||
update: | ||
add: | ||
post: | ||
description: This is an updated description of a child object | ||
x-safe: false | ||
|
@@ -196,7 +197,7 @@ actions: | |
|
||
#### Array Modification Examples | ||
|
||
Array elements MAY be deleted using the `remove` property. Use of array indexes to remove array items should be avoided where possible as indexes will change when items are removed. | ||
Array elements MAY be appended to existing, empty or non-existent array fields using the `update` property. | ||
lornajane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```yaml | ||
overlay: 1.0.0 | ||
|
@@ -210,6 +211,20 @@ actions: | |
in: query | ||
``` | ||
|
||
Arrays MAY be set to a set list of entries using the `replace` property. In this operation, the array will contain only the specified element(s), regardless of what the previous value of the target was. | ||
|
||
```yaml | ||
overlay: 1.0.0 | ||
info: | ||
title: Replace a whole array | ||
version: 1.0.0 | ||
- target: '$.servers' | ||
replace: | ||
- description: Test server | ||
url: https://testing.example.com/ | ||
|
||
Array elements MAY be deleted using the `remove` property. Use of array indexes to remove array items should be avoided where possible as indexes will change when items are removed. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the recommended way of removing certain array elements? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of them are really great, honestly. We should put some examples in the learn site. |
||
|
||
```yaml | ||
overlay: 1.0.0 | ||
info: | ||
|
@@ -247,7 +262,7 @@ info: | |
version: 1.0.0 | ||
actions: | ||
- target: $.paths.*.get[[email protected]] | ||
update: | ||
add: | ||
parameters: | ||
- name: top | ||
in: query | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that anchors are removed. I think they don't quite work well with GitHub's formatting, anyway, but it's worth pointing out. Not sure if this is used with other tooling. Not a blocker.