Skip to content

Fixes examples for compliance with RFC-9535 #118

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: v1.0-dev
Choose a base branch
from
Open

Fixes examples for compliance with RFC-9535 #118

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc4e075
Fixes examples for compliance with RFC-9535
mikekistler Jan 29, 2025
96cf4c2
Address PR review comments
mikekistler Feb 11, 2025
24391ca
Revert change to x-oai-traits
mikekistler Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/overlay.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ info:
actions:
- target: $.paths.*.get.parameters
update:
name: newParam
in: query
- name: newParam
in: query
Comment on lines +215 to +216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says

If the target selects an array, the value of this field MUST be an entry to append to the array.

The original example appended a Parameter Object to an array of Parameter Objects, which feels correct.

The modified example appends a one-element array to an array, so I would expect the result be an array whose last element is the one-element array.

If we want "append to an array" to mean "concatenate two arrays", we should make that really clear in the text.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralfhandl I don't think we do want that. I think the behavior you described is what I would expect. We had a whole issue and PR discussing arrays. I think it's #30?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralfhandl can you take another look at this? Do we need to make the changes you requested?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that depends 😎

The Overlay specification says for action field update

If the target selects an array, the value of this field MUST be an entry to append to the array.

I would interpret this as

  • If I want to add a Tag Object to the tags array, the value of update must be single the Tag Object I want to add.
  • If I want to add two entries, I need two actions, each of which appends one entry to the array.

This interpretation is supported by the first Array Modification Example in the 1.0.0 spec.

However, both Bump.sh and Speakeasy require update to be an array with one or more entries, and this array is concatenated with the target.

If we want to keep the spec as it is now, the array modification example must not be changed, and we live with the fact that the two first implementations of the spec got array modification wrong.

If we want to make the example compatible with the two first implementations, we also must change the specification and say something like

If the target selects an array, the value of this field MUST be an array of entriesentry to append to the array.

or

If the target selects an array, the value of this field MUST be an arrayentry to concatenate withappend to the target array.

This however would be a breaking change and should be done in a new major version of the spec.

```

```yaml
Expand Down Expand Up @@ -252,7 +252,7 @@ info:
title: Apply Traits
version: 1.0.0
actions:
- target: $.paths.*.get[[email protected].paged]
- target: $.paths[?@.get['x-oai-traits'][?@ == 'paged']].get
update:
parameters:
- name: top
Expand Down