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

BUG: navi does not support OpenApi operation anyOf #9

Closed
taxonomy-man opened this issue Jan 14, 2025 · 13 comments
Closed

BUG: navi does not support OpenApi operation anyOf #9

taxonomy-man opened this issue Jan 14, 2025 · 13 comments

Comments

@taxonomy-man
Copy link
Contributor

taxonomy-man commented Jan 14, 2025

hi!

Thanks for sharing this project, I have the same use case so Im trying it out right now! 👍

I think I have found a bug, and It would be super nice to get it sorted out. I have case where it is possible to send in either a number or two string enums. From my understanding of openApi spec this is valid yaml. https://swagger.io/docs/specification/v3_0/data-models/oneof-anyof-allof-not/#oneof

        - in: query
          name: version
          description: Taxonomy version, either number that indicates the version, "latest" for latest release, or "next" for unpublished changes (requires admin rights)
          schema:
            anyOf:
              - type: integer
                format: int64
              - type: string
                enum:
                  - latest
                  - next

but in my generated reitit routes the children (the schema options), and an exception is thrown if we try to start web server.

["/v1/taxonomy/main/concepts"
  {:get
   {:handler nil,
    :parameters
    {:query
     [:map
      [:offset {:optional true} #function[clojure.core/int?]]
      [:version {:optional true} [:or]]
      [:relation {:optional true} #function[clojure.core/string?--5494]]]},

As you can see, the :version key here does just contain [:or],
so when I start web server I get this error

       [{:type java.lang.RuntimeException,
    :message "could not start [#'dev/http-server] due to",
    :at [mount.core$up$fn__81798 invoke "core.cljc" 80]}
   {:type clojure.lang.ExceptionInfo,
    :message
    ":malli.core/child-error\n\n{:type :or, :properties nil, :children nil, :min 1, :max nil}",
    :data
    {:type :malli.core/child-error,
     :message :malli.core/child-error,
     :data
     {:type :or, :properties nil, :children nil, :min 1, :max nil},
     :reitit.exception/cause #error {
 :cause ":malli.core/child-error"
 :data {:type :malli.core/child-error, :message :malli.core/child-error, :data {:type :or, :properties nil, :children nil, :min 1, :max nil}}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message ":malli.core/child-error"
   :data {:type :malli.core/child-error, :message :malli.core/child-error, :data {:type :or, :properties nil, :children nil, :min 1, :max nil}}
   :at [malli.core$_exception invokeStatic "core.cljc" 157]}]
@lispyclouds
Copy link
Owner

Yeah navi as of now has support for basic types and their collections. The composite ones are a bit of a challenge to generate malli specs for, given the bit of complexity the openapi parser has. Will take a closer look soon, thanks for raising this! PRs are always welcome!

@taxonomy-man
Copy link
Contributor Author

thanks for instant response! Oh that would be super! I am also looking at it right now. Yes without this functionality navi is quite limited I suppose. Im looking at the paths right now

(let [api-spec (slurp "newopenapispec.yaml")
parse-options (doto (ParseOptions.)
(.setResolveFully true))
contents (.readContents (OpenAPIV3Parser.) api-spec nil parse-options)
paths (.getPaths (.getOpenAPI contents))]

            class Parameter {
                name: version
                in: null
                description: Taxonomy version, either number that indicates the version, \"latest\" for latest release, or \"next\" for unpublished changes (requires admin rights)
                required: false
                deprecated: null
                allowEmptyValue: null
                style: form
                explode: true
                allowReserved: null
                schema: class ComposedSchema {
                    class Schema {
                        type: null
                        format: null
                        $ref: null
                        description: null
                        title: null
                        multipleOf: null
                        maximum: null
                        exclusiveMaximum: null
                        minimum: null
                        exclusiveMinimum: null
                        maxLength: null
                        minLength: null
                        pattern: null
                        maxItems: null
                        minItems: null
                        uniqueItems: null
                        maxProperties: null
                        minProperties: null
                        required: null
                        not: null
                        properties: null
                        additionalProperties: null
                        nullable: null
                        readOnly: null
                        writeOnly: null
                        example: null
                        externalDocs: null
                        deprecated: null
                        discriminator: null
                        xml: null
                    }
                }
                examples: null
                example: null
                content: null
                $ref: null
            }
            in: query

and I dont see any metadata how to decide if its anyOf, oneOf or allOf, but perhaps I have not understood the problem correctly yet. Thanks again for reply!

@lispyclouds
Copy link
Owner

yeah, because there isn't a complete openapi parser for clojure, i need to rely on the java one and it has its non-trivial class structures to comprehend. i will try to see what would be a good way to solve this once i get a bit more time, struggling with work atm.

@lispyclouds
Copy link
Owner

lispyclouds commented Jan 15, 2025

Adding in some helpful context from my bit of digging:

@lispyclouds
Copy link
Owner

The getAllOf and getAnyOf returns a List which can be mapped over with the schema->spec fn

@lispyclouds
Copy link
Owner

@taxonomy-man just pushed a4da2a9 with some initial support while i think more about it. Could you test it out? I'm not finalised on the impl details yet.

lispyclouds added a commit that referenced this issue Jan 16, 2025
lispyclouds added a commit that referenced this issue Jan 16, 2025
lispyclouds added a commit that referenced this issue Jan 17, 2025
lispyclouds added a commit that referenced this issue Jan 17, 2025
@lispyclouds
Copy link
Owner

This is now on main.

@taxonomy-man
Copy link
Contributor Author

@lispyclouds oh super nice! Did not think that you would be able to look at it that quick! I will try it out

I have been working on this on my side also. I also found the getAllOf, etc and came up with this so far.
This was the code in main before your latest change:

(defn schema->spec [^Schema schema]
  (if schema
    (let [types (.getTypes schema)]
      (if (= 1 (count types))
        (spec schema)
        (try
          (->> (map (fn [type]
                      (.setTypes schema #{type})
                      (spec schema))
                    types)
               (into [:or]))
          (finally
            (.setTypes schema types)))))
    (throw (ex-info "Missing schema" {}))))

and I came up with this, which seems to be working so far from my tests

(defn schema->spec [^Schema schema]
  (cond
    (nil? schema)
    (throw (ex-info "Missing schema" {}))

    (instance? ComposedSchema schema)
    (let [cs (cast ComposedSchema schema)
          one-of (seq (.getOneOf cs))
          any-of (seq (.getAnyOf cs))
          all-of (seq (.getAllOf cs))]
      (cond
        one-of (into [:or] (mapv schema->spec one-of)) ; exactly one
        any-of (into [:set] (mapv schema->spec any-of)) ; one or more
        all-of (into [:and] (mapv schema->spec all-of)) ; all must match
        :else (spec cs)))

    (seq (.getTypes schema))
    (let [types (.getTypes schema)]
      (if (= 1 (count types))
        (spec schema)
        (try
          (->> (map (fn [type]
                      (.setTypes schema #{type})
                      (spec schema))
                    types)
               (into [:set])) ; Changed from :or to :set for multiple types
          (finally
            (.setTypes schema types)))))

    :else
    (spec schema)))

@lispyclouds
Copy link
Owner

Looks good! From what I think, anyOf corresponds to an [:or] and allOf to an [:and]. Not sure :or is a good thing for oneOf. As I understand, oneOf strictly checks just one match and more than one isn't valid. Or implies one or more. Also the [:set] schema doesn't work as needed here in Malli: its for homogeneous sets: [:set int?] etc

@lispyclouds
Copy link
Owner

lispyclouds commented Jan 17, 2025

I've left out the oneOf impl for now, lemme know if you have a need for it. From what I gather its rather an obscure one and bit weird to implement. Can do a release once you confirm it works for your cases.

@taxonomy-man
Copy link
Contributor Author

aha , yes you are right, I have not actually tried to run the generated reitit routes, just inspecting the output. Ok so then set Is not correct, as you say :) . Actually it was oneOf that I needed, but I suppose I can go with anyOf instead. Thanks again for your efforts!

@lispyclouds
Copy link
Owner

lispyclouds commented Jan 17, 2025

Yeah I'm not quite sure where one would use oneOf over anyOf. In the sense when implemented, anyOf will check only one of them to be true (the first one matching). For a proper oneOf impl, we need to check true for one and false for the rest. Regardless will think of a way to implement this which gives some helpful messages for a later release. Can do a release in the current state if you are happy with testing.

@lispyclouds
Copy link
Owner

Released in 0.1.0

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

No branches or pull requests

2 participants