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

Support s/keys key-groups in openapi spec #619

Open
jimpil opened this issue May 3, 2023 · 3 comments
Open

Support s/keys key-groups in openapi spec #619

jimpil opened this issue May 3, 2023 · 3 comments

Comments

@jimpil
Copy link

jimpil commented May 3, 2023

Hello,

I was under the impression that specs like the one below, didn't show correctly on the swagger-v2 schema because it doesn't support this kind of one-of semantics. I believe that openapi-v3 does support it, but unfortunately, the same thing happens - all the keys show up as required (i.e. with a red asterisk).

(s/keys :req-un [(or ::foo ::bar ::baz)])

Any thoughts?

[EDIT]:
Sorry, I should have mentioned that I am using the recently released 0.7.0-alpha1.

@opqdonut
Copy link
Member

opqdonut commented May 8, 2023

spec-tools isn't sophisticated enough to know how to convert that into json-schema.

See here: https://github.com/metosin/spec-tools/blob/8a3284f1a1d43636648f28a7f32e6122c4a1912a/src/spec_tools/json_schema.cljc#L187

@jimpil
Copy link
Author

jimpil commented Nov 18, 2023

Hi again.

I may be able to help on this after all - here is some initial (somewhat encouraging) info:

The whole thing seemed impossible when I first looked at it (several months back), but I've had two significant realisations since...

  1. First of all, I realised that the whole key-groups thing really only applies for a single case - and that is the outer OR, with potentially nesting ANDs - i.e. (or ::foo (and ::bar ::baz)). In other words, it is meaningless to have an outer AND (as that is already implied). This makes finding these (at the top-level) super easy.
  2. Secondly, I randomly stumbled upon this last week, and I immediately thought of this issue! This is basically the other piece of the puzzle - instead of emitting a flat {:required [...]}, reitit could emit an :allOf with (potentially) nested oneOfs. That's actually very easy to do in accept-spec 'clojure.spec.alpha/keys, but it does mean that impl/parse-keys needs to be re-worked to return either {:required [x]} (for the base case of non-group), or {:oneOf [x y z]} (for any top-level or groups). I guess what I'm trying to say is that impl/parse-keys is the main thing that needs to touched (any changes in accept-spec 'clojure.spec.alpha/keys should be very straight -forward - e.g. replacing :required with :allOf).

In any case, as I said I'll try to have a stub at this of not this week, then the following after it... 👍

@jimpil
Copy link
Author

jimpil commented Nov 19, 2023

Ok, so here is an attempt at discovering key-groups:

(defn- parse-required-with [f x]
  (if (seq? x) ;; key-group
    (let [k (condp = (first x) 
              'or  :oneOf ;; or :anyOf ?
              'and :allOf 
              (throw 
               (IllegalArgumentException. "unsupported key-group expression")))] 
      {k (mapv (partial parse-required-with f) (next x))})
    {:required [(f x)]}))

(def parse-req    (partial parse-required-with impl/qualified-name))
(def parse-req-un (partial parse-required-with name))

(parse-req-un '(or :foo (and :bar :baz))) ;; => {:oneOf [{:required ["foo"]} {:allOf [{:required ["bar"]} {:required ["baz"]}]}]}

You can get a list of those maps by simply mapping over the req(-un) vectors, and wrapping everything in top level {:allOf [...]}.
Leaving this here for future reference - as I said, I'll try to find some time to prepare a real PR sometime this coming week or the next...

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