redocly-cli icon indicating copy to clipboard operation
redocly-cli copied to clipboard

Properties from merged references are ignored when using `bundle`

Open Gidgidonihah opened this issue 9 months ago • 6 comments

When merging references, redocly-cli bundle will ignore existing properties.

For example with 2 files: id.yaml:

type: integer
format: int32
description: A generic ID used everywhere
example: 42

id_ro.yaml:

$ref: ./id.yaml
readOnly: true
description: A generic ID but that is specifically read only

The result will never include readOnly: true or the updated description.

To Reproduce

I've created a gist with a minimal working example and reproduction steps.

  1. Given default settings
  2. And the example.yaml (and referenced files) in the gist
  3. Run the bundle command(s) found in the gist (e.g. docker run --rm -v $PWD:/spec redocly/cli bundle example.yaml -o out-redocly-cli.yaml)
  4. See ignored merged properties

Expected behavior

The duplicate properties defined after reference should appear in the resulting yaml file.

Logs

N/A

OpenAPI description

See the files supplied in the linked gist. Only default configuration was used with the supplied yaml files.

Redocly Version(s)

1.4.0 as built in the docker image

Node.js Version(s)

v21.1.0 as built in the docker image

Additional context

I used the latest tagged docker image at the time I ran it. Image hash 3bcfba9186cf Additional details and output can be found in the gist.

Gidgidonihah avatar Nov 14 '23 01:11 Gidgidonihah

Hi @Gidgidonihah, Could you check if using the allOf trick solves your issue?

allOf: 
- $ref: ./id.yaml
readOnly: true
description: A generic ID but that is specifically read only

tatomyr avatar Nov 14 '23 15:11 tatomyr

It does result in the readOnly: true being present. That's a functional workaround that I hadn't thought about. That can get me moving forward easier than completely moving things around. So that's good.

But I don't think that should be a long-term solution. It feels like silently dropping them is the wrong thing to do.

Gidgidonihah avatar Nov 14 '23 19:11 Gidgidonihah

But I don't think that should be a long-term solution.

I agree. The behaviour isn't obvious. I believe it stems from the difference in JSON Schema drafts:

Draft-specific info In Draft 4-7, $ref behaves a little differently. When an object contains a $ref property, the object is considered a reference, not a schema. Therefore, any other properties you put in that object will not be treated as JSON Schema keywords and will be ignored by the validator. $ref can only be used where a schema is expected.

tatomyr avatar Nov 16 '23 10:11 tatomyr

Both ideas are not valid though:

The spec (3.1.0) allows only for summary or description to be sibling to the $ref and allOf doesn't make sense with readOnly in the example (I understand it may impact the Redoc rendering but it doesn't change that it is illogical (I could update that article to add readOnly as an example of an illogical allOf -- because it's not an extension, it's a logical and.)

Separately though which should be checked is overriding the description within a reference that points to a reference with a sibling. We may have a bug with nested $ref where the inner one has a sibling description and it is not used.

adamaltman avatar Dec 02 '23 20:12 adamaltman

@adamaltman the issue is still there though, as we don't properly resolve description either.

Regarding other keys, maybe it's worth introducing a rule that will warn if someone uses a property that will be ignored?

Moving further, the spec says that this restriction (only description and summary keys) only refers to the Reference Object and not to a Schema Object that contains $ref (leaving this link here for more context). So I'd assume that a resolved (using the allOf) part of a schema could be extended with any other allowed keys for composition.

tatomyr avatar Dec 04 '23 13:12 tatomyr

Related issue: https://github.com/Redocly/redocly-cli/issues/1510

tatomyr avatar Apr 02 '24 15:04 tatomyr