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

Hyphenless followup #1192

Merged
merged 43 commits into from
May 26, 2020
Merged

Hyphenless followup #1192

merged 43 commits into from
May 26, 2020

Conversation

XVincentX
Copy link
Contributor

@XVincentX XVincentX commented May 22, 2020

Update the library and make it great again with dashes

P.S: Damn OpenAPI 3.

Closes #1191

chohmann and others added 30 commits May 19, 2020 13:55
…prism into fix/hyphenated_query_params

# Conflicts:
#	test-harness/specs/content-negotiation/E2E-Accept-Content-Negotiation-AC-14.oas2.txt
@XVincentX XVincentX marked this pull request as ready for review May 23, 2020 17:36
@XVincentX XVincentX requested a review from chohmann May 23, 2020 17:36
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

Aren't we loosing data with current approach?

Consider this spec:

openapi: 3.0.2
paths:
  /pet:
    get:
      parameters:
        - in: query
          name: pet-id
          required: true
          schema:
            type: number
        - in: query
          name: petid
          required: true
          schema:
            type: number
      responses:
        200:
          description: ok
          content:
            application/json:
              example: "ok"

This turns into:

[5:32:33 PM] › [CLI] ℹ  info      GET        http://127.0.0.1:4010/pet?petid=834.7748617345209

We have lost pet-id param because after removing hyphens it looked the same as petid.


Constructive part:

I guess we should have two paths. One for hyphen-full params and second for hyphen-less ones. Last step will merge both results into single URL.

packages/cli/src/util/paths.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@karol-maciaszek karol-maciaszek left a comment

Choose a reason for hiding this comment

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

Minor thing and ship it!

packages/cli/src/util/paths.ts Outdated Show resolved Hide resolved
@XVincentX XVincentX merged commit 14a5615 into master May 26, 2020
@XVincentX XVincentX deleted the feat/hypheless-map branch May 26, 2020 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the getHttpOperationsFromResource
3 participants