Skip to content
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

Is there something missing in supporting OAS3 servers? #3143

Closed
fulviodenza opened this issue Sep 14, 2023 · 9 comments
Closed

Is there something missing in supporting OAS3 servers? #3143

fulviodenza opened this issue Sep 14, 2023 · 9 comments

Comments

@fulviodenza
Copy link

fulviodenza commented Sep 14, 2023

From this line we deduce that is still something missing in the OAS3 servers support:

// TODO: OAS3: support servers here

Although it looks it already has been added support to it:
https://github.com/swagger-api/swagger-js/tree/96d261987700297a472db5587c0a8c769095d73e/src/execute

I found it a bit confusing, is it to delete?

@char0n
Copy link
Member

char0n commented Nov 1, 2023

Hi @fulviodenza,

Good question. Investigating....


Maybe slightly related: #2967

@fulviodenza
Copy link
Author

Hi @fulviodenza,

Good question. Investigating....

Maybe slightly related: #2967

We are already using the feature with no issues, opening a PR to remove the stale comment.

@char0n
Copy link
Member

char0n commented Nov 2, 2023

@fulviodenza yes I can confirm as well. Probably the intention was to make server URLs absolute using the spec url when they are represented as relative URI Reference, but that is already handled in baseUrl function.

@char0n
Copy link
Member

char0n commented Nov 2, 2023

On the other hand, the following code is fully OpenAPI 2.0 specific:

  if (specUrl && specUrl.startsWith('http')) {
    const parsed = new URL(specUrl);
    if (!spec.host) {
      spec.host = parsed.host;
    }
    if (!spec.schemes) {
      spec.schemes = [parsed.protocol.replace(':', '')];
    }
    if (!spec.basePath) {
      spec.basePath = '/';
    }
  }

I think what the // TODO: OAS3: support servers here comment is trying to relay to us that we should do the same for OpenAPI 3.x.y Server Object. Interpolating here probable intention for OpenAPI 3.x.y:

if (specUrl && specUrl.startsWith('http') && spec is OpenAPI 3.x.y) {
  if (!spec.servers) {
    spec.servers = [{ url: specUrl }];
  }
}

@fulviodenza what do you think?

char0n added a commit that referenced this issue Nov 2, 2023
This change is specific to OpenAPI 3.x.y definitions.

Refs #3143
@char0n
Copy link
Member

char0n commented Nov 2, 2023

I've issued #3219 to align the code with #3143 (comment)

char0n added a commit that referenced this issue Nov 2, 2023
This change is specific to OpenAPI 3.x.y definitions.

Refs #3143
@char0n
Copy link
Member

char0n commented Nov 2, 2023

Closed by #3219. @fulviodenza thanks for pointing this TODO comment out. It lead to adding missing piece of functionality!

@char0n char0n closed this as completed Nov 2, 2023
swagger-bot pushed a commit that referenced this issue Nov 2, 2023
## [3.24.1](v3.24.0...v3.24.1) (2023-11-02)

### Bug Fixes

* create default OpenAPI.servers fields if omitted ([#3219](#3219)) ([781077a](781077a)), closes [#3143](#3143)
@char0n
Copy link
Member

char0n commented Nov 2, 2023

OpenAPI 3.x.y spec actually tells us what to do when, the OpenAPI.servers field is missing or is defined as empty array:

image

I'll have to amend the implementation to reflect this.

@char0n char0n reopened this Nov 2, 2023
char0n added a commit that referenced this issue Nov 2, 2023
This change is specific to OpenAPI 3.x.y definitions.

Refs #3143
@char0n
Copy link
Member

char0n commented Nov 2, 2023

Closed by #3220 which introduces spec compliant default for OpenAPI.servers field in OpenAPI 3.x.y definitions.

@char0n char0n closed this as completed Nov 2, 2023
char0n added a commit that referenced this issue Nov 2, 2023
…3220)

This change is specific to OpenAPI 3.x.y definitions.

Refs #3143
swagger-bot pushed a commit that referenced this issue Nov 2, 2023
## [3.24.2](v3.24.1...v3.24.2) (2023-11-02)

### Bug Fixes

* create spec compliant default OpenAPI.servers fields if omitted ([#3220](#3220)) ([becdf3c](becdf3c)), closes [#3143](#3143)
@fulviodenza
Copy link
Author

Sorry for not answering before didn't read, I'm happy this led to a fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants