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

Correct errors in the API spec #95

Merged
merged 8 commits into from
Apr 7, 2021
Merged

Correct errors in the API spec #95

merged 8 commits into from
Apr 7, 2021

Conversation

Pamplemousse
Copy link
Contributor

@Pamplemousse Pamplemousse commented Apr 1, 2021

Fixes #11!

openapi-generator-cli validate -i doc/specs/lichess-api.yaml gives the remaining spec violations.

Signed-off-by: Pamplemousse xav.maso@gmail.com

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
@Pamplemousse Pamplemousse changed the title Add some missing description in the API spec Correct errors in the API spec Apr 2, 2021
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
  * `type` is already given for the property
  * `minimum` is a direct attribute of the property
  (see https://swagger.io/specification/#schema-object)

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
Signed-off-by: Pamplemousse <xav.maso@gmail.com>
  * remove `SameOrigin` security
  * add CORS to all responses not currently marked `SameOrigin`
  (sadly, not doable without duplication, see
  OAI/OpenAPI-Specification#690)

Signed-off-by: Pamplemousse <xav.maso@gmail.com>
@Pamplemousse
Copy link
Contributor Author

This is a best effort: I never worked with OpenAPI before, and it's my first contribution to Lichess...
I tried to fix the errors in a clear and idiomatic way, while doing my best to keep the original intent of the spec.
My contribution is split into several commits to separate issues that were "trivial" from the ones that might raise discussions / rejection.

cc @ornicar @niklasf (and @markv for the original issue)

@sjamesr
Copy link

sjamesr commented Apr 4, 2021

Thank you so much, this is great! If I'm not mistaken, the fen parameter in tablebaseStandard is missing a type around line 5010. It should read:

  /standard:
    get:
      operationId: tablebaseStandard
      summary: Tablebase lookup
      description: |
        **Endpoint: https://tablebase.lichess.ovh**

        Runs https://github.com/niklasf/lila-tablebase with `--sloppy-real-wdl`.

        Example: `curl http://tablebase.lichess.ovh/standard?fen=4k3/6KP/8/8/8/8/7p/8_w_-_-_0_1`
      tags:
        - Tablebase
      security: []
      parameters:
        - in: query
          name: fen
          description: FEN of the position. Underscores allowed. The halfmove clock is taken into account for WDL values.
          schema:          # <-----
            type: string   # <----- these two lines are missing
      responses:
        200:
          description: The tablebase information for the position in standard chess.
          headers:
            Access-Control-Allow-Origin:
              schema:
                type: string
                default: "'*'"
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TablebaseJson'

@Pamplemousse
Copy link
Contributor Author

@sjamesr Hey! How do you know this type is missing?

I don't see any more errors when running openapi-generator-cli validate -i doc/specs/lichess-api.yaml...
And it's not amongst the warnings either.

@sjamesr
Copy link

sjamesr commented Apr 5, 2021

Without it, using the default Java generator, the fen parameter would not be emitted correctly. I'll try reproduce the problem for you.

@Pamplemousse
Copy link
Contributor Author

Interesting...
I would say that there might be a bug in either openapi-generator-cli validate or "the default Java generator" (are you referring to this: https://github.com/OpenAPITools/openapi-generator ?).

@ornicar
Copy link
Contributor

ornicar commented Apr 7, 2021

Nice. I really wish we had a way to get rid of the duplication, such as

          headers:
            Access-Control-Allow-Origin:
              schema:
                type: string
                default: "'*'"

There is so much duplicated stuff in there, and I think it's the fault of the OpenAPI format.

Maybe we should be generating it with an actual programming language instead?

@ornicar ornicar merged commit 1a619d6 into lichess-org:master Apr 7, 2021
@Pamplemousse
Copy link
Contributor Author

@ornicar
Yeah, I looked into that, the OpenAPI format does not allow such modularity: see OAI/OpenAPI-Specification#690 .

Even if it had a default value, or was declared in a unique place, each response would need to declare a reference to this unique location... The duplication would just be "shifted".

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.

Make the API specs useful for API client generation with OpenAPI Generator
4 participants