Redocly / redocly-cli

⚒️ Redocly CLI makes OpenAPI easy. Lint/validate to any standard, generate beautiful docs, and more.
https://redocly.com/docs/cli/
MIT License
849 stars 129 forks source link

Two schemas are referenced with the same name but different content. Renamed Item to Item-2. #661

Open colinmollenhour opened 2 years ago

colinmollenhour commented 2 years ago

If using subdirectories to organize a large number of components and some of these components share the same file basename (but exist in different subdirectories) you get warnings like this:

[6] paths/beta/rating/merchantRates.yaml:59:7 at #/get/parameters/3

Two schemas are referenced with the same name but different content. Renamed fields to fields-5.

This also comes up with models like models/Order/Item.yaml and models/Shipment/Item.yaml because both files are named Item so they are merged in as Item and Item-2 and so-on.

Example spec

openapi.yaml

openapi: 3.0.3
info:
  title: test
paths:
  '/api/global/beta/rating/rateGroups':
    $ref: 'paths/beta/rating/rateGroups.yaml'
  '/api/global/beta/rating/merchantRates':
    $ref: 'paths/beta/rating/merchantRates.yaml'

paths/beta/rating/rateGroups.yaml

get:
  summary: List all Rate Groups
  description: foo
  parameters:
    - $ref: ../../../components/parameters/rate/group/fields.yaml

paths/beta/rating/merchantRates.yaml

get:
  summary: List all Merchant Rates
  description: foo
  parameters:
    - $ref: ../../../components/parameters/merchant/rate/fields.yaml

Expected behavior The refs are valid so ideally there should not be such warnings and ideally it should not require the refs to be declared explicitly in the top level file as a workaround as that is much more tedious than just directly referencing the files where they are needed.

openapi-cli Version(s) 1.0.0-beta.73

tatomyr commented 2 years ago

Hi @colinmollenhour, Thank you for reporting. I agree it might feel like the behaviour is not too obvious. We might consider implementing this in future. However, you can safely skip these warnings, I believe. Or alternatively you can also try using different file names.

adamaltman commented 2 years ago

These warnings are for your benefit (not because there is an issue with your definitions) -- they could be posed as "informational" instead of "warning". It is letting you know we cannot use the same file name when we move the reference from a remote reference to the components section in the bundled file. If you want it to be completely dereferenced, you could use the dereferenced flag.

However, I think there could be a better way for us to come up with schema names. For example, in the case of a conflict, we append -2, -3, -etc. In your example above, we could try something like using the folder name such as Shipment_Item.

colinmollenhour commented 2 years ago

I like them remaining as references as that keeps the spec concise and agree that the warnings can safely be ignored but then other warnings get masked quickly.. Using one parent folder name as a prefix would be a big improvement in my opinion and probably work in 95% of cases. As of now I've resorted to naming files like Order/OrderItem.yaml instead of Order/Item.yaml to avoid the warnings.

pvonr commented 2 years ago

We would like to have the revers behavior :) Instead of deduplicating models via adding a number automatically I'd like to error out if this is happening. Is there a way to achieve this?

Context: We have our specs distributed in different repos and bundle it into one big public spec. It's semantically incorrect for us to have different models with the same name. If this happens the names are too generic and should be revised (e.g. in the above example it should be OrderItem and ShippingItem). Having this we could enforces clear domain based naming schemes and fail fast if people introduce naming conflicts.

adamaltman commented 2 years ago

The reverse behavior could be strict mode.

djMax commented 9 months ago

I'm getting this error when the refs point to the SAME file but from different base paths - i.e. if I have $ref: something/file.yaml and in the something directory I have $ref: file.yaml it will complain.

troccoli commented 7 months ago

Is there a specific rule that can be turned off for this?

tatomyr commented 7 months ago

@troccoli do you mean turning off the warnings? No, currently there's no way to suppress them.

troccoli commented 7 months ago

Yes @tatomyr that's what I meant.

I'm fine not caring for those warnings but it would be nice if they can be excluded from the output so that I don't miss other warnings.

Thanks for answering anyway

kennethaasan commented 3 months ago

Why is not the title in the schema used instead of the file name?

domenico-renna commented 3 months ago

This behavior is really annoying, we have a spec-root.yaml file which is referenced in many other spec files for common answers or parameters also we use a folder related to every resource containg a spec.yaml (with the routes specification) and a schema.yaml file (with the entity obyect definition).

With this behaviour you full our console with caothic and unusefull log hiding real error if occurs

lornajane commented 3 months ago

@domenico-renna Could you open a new issue with some examples for that use case? It sounds a little different to the example above with individual schemas in same-named files. If you can link to this issue, that would help.

lornajane commented 3 months ago

One approach to solving this is to name the files more meaningfully so that those names get used when bundling. The simplest way to do this (that I use whenever I see this problem) is to bundle the API description, rename everything the way you want, then use the split command (see https://redocly.com/docs/cli/commands/split/) to turn it back into a structure of manageable-sized files.

If we were to implement changes, it could be to improve the naming policy so that the tool automatically picks probably-sensible names and doesn't warn about adding suffixes. Adam's suggestion further up the thread of adding directory prefixes would work for most of the use cases here I think. The first item encountered would still get its current name (e.g. fields in the original issue example), but the next one would get a path-related prefix (e.g. rate-fields) to distinguish them more meaningfully.

dchertousov commented 3 weeks ago

I have similar issue.

I have created directory structure for the params, so it will be easily understandable where they come from. And there is one param, that could be passed either in path or in query, depending on the call, so the have the same file name though in different dirs (path/application-id.yaml and query/application-id.yaml):

└── openapi
    ├── parameters
    │   ├── _index.yaml
    │   ├── header
    │   │   └── x-api-key.yaml
    │   ├── path
    │   │   ├── application-id.yaml
    │   │   ├── organization-id.yaml
    │   │   ├── payment-id.yaml
    │   │   └── subscription-id.yaml
    │   └── query
    │       ├── application-id.yaml
    │       ├── date.yaml
    │       └── return-expired-subscriptions.yaml

May be it is possible to take not only the file name, but the whole relative path to account, so they will be different?