-
Notifications
You must be signed in to change notification settings - Fork 2
Validates OpenAPI Specification on upload and redesigns spec entry #45
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,9 +18,11 @@ defmodule Mimicry.OpenAPI.Parser do | |||||
| def json(str), do: parse(str, :json) | ||||||
|
|
||||||
| @doc """ | ||||||
| parses a given string depending on its extension | ||||||
| parses a given string based on the extension to a Mimicry.Specifaction | ||||||
|
|
||||||
| If you need a map instead of the specification, see parse_to_map/2 | ||||||
| """ | ||||||
| @spec parse(String.t(), :yaml | :json) :: Specification.t() | ||||||
| @spec parse(term(), :yaml | :json) :: Specification.t() | ||||||
| def parse(str, atom) do | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this function could now be shortened to something like this: def parse(str, atom) do
str
|> parse_to_map(atom)
|> case do
{:ok, decoded} ->
decoded |> build_specification()
{:error, _err} ->
Specification.unsupported()
end
end |
||||||
| str | ||||||
| |> decoder(atom).() | ||||||
|
|
@@ -35,10 +37,28 @@ defmodule Mimicry.OpenAPI.Parser do | |||||
| end | ||||||
| end | ||||||
|
|
||||||
| @doc """ | ||||||
| parses a given string to a map | ||||||
| """ | ||||||
| @spec parse_to_map(any(), :yaml | :json) :: :error | any() | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the doc says string, shouldn't it be also in the spec?
Suggested change
|
||||||
| def parse_to_map(str, atom) do | ||||||
| str | ||||||
| |> decoder(atom).() | ||||||
| |> case do | ||||||
| {:ok, decoded} -> | ||||||
| decoded | ||||||
|
|
||||||
| {:error, err} -> | ||||||
| Logger.warn("Could not decode #{atom |> to_string() |> String.upcase()} specification") | ||||||
| Logger.error(err) | ||||||
| :error | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| @doc """ | ||||||
| builds a new Specification from inputs given | ||||||
| """ | ||||||
| @spec build_specification(map()) :: Specification.t() | ||||||
| @spec build_specification(any()) :: Specification.t() | ||||||
|
Comment on lines
-41
to
+61
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as above about spec changes. |
||||||
| def build_specification( | ||||||
| parsed = %{ | ||||||
| "info" => info, | ||||||
|
|
@@ -65,6 +85,7 @@ defmodule Mimicry.OpenAPI.Parser do | |||||
| # NOTE: This cannot read multiple specifications in a YAML file as of yet | ||||||
| :yaml -> &YamlElixir.read_from_string/1 | ||||||
| :json -> &Jason.decode/1 | ||||||
| _ -> raise RuntimeError, "Unknown extension: #{atom}" | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| defmodule MimicryApi.Errors do | ||
| @moduledoc """ | ||
| Provides functions for transforming errors from libs into something usable to the API | ||
| """ | ||
| def make_errors_from_openapi_validation(errors) do | ||
| errors |> Enum.map(fn {msg, path} -> %{path => msg} end) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| openapi: 3.0.3 | ||
| info: | ||
| title: 'Test YAML' | ||
| version: '1.0' | ||
| license: | ||
| name: MIT | ||
| description: 'A simple API' | ||
| servers: | ||
| - url: https://simple-api.testing.com | ||
| description: production | ||
| paths: | ||
| '/': | ||
| get: | ||
| responses: | ||
| default: | ||
| description: 'The default response for this endpoint' | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/SimpleMessage' | ||
| examples: | ||
| simple-reference: | ||
| $ref: '#/components/examples/simple-message-example' | ||
| simple-embedded: | ||
| summary: 'An embedded example' | ||
| value: | ||
| message: 'embedded message example' | ||
| simple-combined-example: | ||
| $ref: '#/components/examples/simple-message-example' | ||
| summary: 'simple example with overriden summary' | ||
| 404: # <- this is the problem, while technically correct YAML, this will break the parser, since it need compat with JSON | ||
| description: The response for when something is not found | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/SimpleError' | ||
| examples: | ||
| simple-error: | ||
| $ref: '#/components/examples/simple-error' | ||
|
|
||
| components: | ||
| schemas: | ||
| SimpleMessage: | ||
| title: 'A simple message' | ||
| description: 'an example of a simple message' | ||
| type: object | ||
| properties: | ||
| message: | ||
| type: string | ||
| SimpleError: | ||
| title: 'A simple Error' | ||
| description: 'A simple error model' | ||
| type: object | ||
| properties: | ||
| code: | ||
| type: integer | ||
| examples: | ||
| simple-message-example: | ||
| summary: 'a simple message' | ||
| value: | ||
| message: 'foobar' | ||
| simple-error: | ||
| summary: 'a simple error' | ||
| value: | ||
| code: 42 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for this spec change?
Also shouldn't parse/2 and parse_to_map/2 have the same definition for the inputs?