-
Notifications
You must be signed in to change notification settings - Fork 237
Add support for JSON Schema If-Then-Else via :if #1288
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: master
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 |
|---|---|---|
|
|
@@ -3023,6 +3023,102 @@ | |
| :re-transformer (fn [_ children] (apply re/alt-transformer children)) | ||
| :re-min-max (fn [_ children] (reduce -re-alt-min-max {:max 0} (-vmap last children)))})}) | ||
|
|
||
| (defn -if-conditional-schema | ||
| "Malli schema for JSON Schema's If-Then-Else conditional support. | ||
|
|
||
| Usage: | ||
| ``` | ||
| [::jse/if [:map [:x [:= true]]] ; IF | ||
| [:map [:y int?]] ; THEN | ||
| [:map [:y string?]]] ; ELSE | ||
| ``` | ||
| Validates the same as: | ||
| ``` | ||
| [:or [:and [:map [:x [:= true]]] ; IF | ||
| [:map [:y int?]]] ; THEN | ||
| [:and [:not [:map [:x [:= true]]]] ; NOT IF | ||
| [:map [:y string?]]]] ; ELSE | ||
| ``` | ||
|
|
||
| The `ELSE` branch can be omitted for `when`-like behavior." | ||
| [] | ||
| ^{:type ::into-schema} | ||
| (reify IntoSchema | ||
| (-type [_] :if) | ||
| (-type-properties [_]) | ||
| (-properties-schema [_ _]) | ||
| (-children-schema [_ _]) | ||
| (-into-schema [parent {:keys [tags] :as properties} children options] | ||
| (-check-children! ::if properties children 2 3) | ||
| (let [condition' (schema (nth children 0) options) | ||
| then' (schema (nth children 1) options) | ||
| else' (when (= 3 (count children)) | ||
| (schema (nth children 2) options)) | ||
| condition-v (validator condition') | ||
| equivalent-schema' (schema (if else' | ||
| [:or | ||
| [:and condition' then'] | ||
| [:and [:not condition'] else']] | ||
| [:and condition' then']) | ||
| options)] | ||
| ^{:type ::schema} | ||
| (reify | ||
| Schema | ||
| (-validator [this] | ||
| (let [then-v (validator then') | ||
| else-v (when else' (validator else'))] | ||
| (fn [value] | ||
| (if (condition-v value) | ||
| (then-v value) | ||
| (if else-v (else-v value) true))))) | ||
| (-explainer [this path] | ||
| (let [then-explainer (-explainer then' (conj path 'then)) | ||
|
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. The |
||
| else-explainer (when else' (-explainer else' (conj path 'else)))] | ||
| (fn explain [x in acc] | ||
| (if (condition-v x) | ||
| (let [before (count acc) | ||
| acc' (then-explainer x in acc)] | ||
| (if (> (count acc') before) | ||
| (into (subvec (vec acc') 0 before) | ||
| (map #(assoc % :branch 'then)) | ||
| (subvec (vec acc') before)) | ||
| acc')) | ||
| (if else-explainer | ||
| (let [before (count acc) | ||
| acc' (else-explainer x in acc)] | ||
| (if (> (count acc') before) | ||
| (into (subvec (vec acc') 0 before) | ||
| (map #(assoc % :branch 'else)) | ||
| (subvec (vec acc') before)) | ||
| acc')) | ||
| acc))))) | ||
| (-parser [_] | ||
| (let [then-p (-parser then') | ||
| else-p (when else' (-parser else'))] | ||
| (fn [x] | ||
| (if (condition-v x) | ||
| (then-p x) | ||
| (if else-p (else-p x) x))))) | ||
| (-unparser [_] | ||
| (let [then-u (-unparser then') | ||
| else-u (when else' (-unparser else'))] | ||
| (fn [x] | ||
| (if (condition-v x) | ||
|
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 don't think this is right. I think there's a few options to choose here:
|
||
| (then-u x) | ||
| (if else-u (else-u x) x))))) | ||
| (-transformer [this transformer method options] | ||
| (-transformer equivalent-schema' transformer method options)) | ||
| (-walk [this walker path options] | ||
| (-walk-leaf this walker path options)) | ||
|
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.
|
||
| (-properties [this] properties) | ||
| (-options [this] options) | ||
| (-children [this] children) | ||
| (-parent [this] parent) | ||
| (-form [this] form)))))) | ||
|
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. Here's the correct implementation here for ParserInfo. ParserInfo
(-parser-info [_ opts]
{:simple-parser (and (:simple-parser (-parser-info then' opts))
(if else' (:simple-parser (-parser-info else' opts)) true))})
There are some other useful methods missed that are implemented in the surrounding schemas. EDIT: actually this depends on which implementation of |
||
|
|
||
| (defn conditional-schemas [] | ||
| {:if (-if-conditional-schema)}) | ||
|
|
||
| (defn base-schemas [] | ||
| {:and (-and-schema) | ||
| :andn (-andn-schema) | ||
|
|
@@ -3050,7 +3146,7 @@ | |
| ::schema (-schema-schema {:raw true})}) | ||
|
|
||
| (defn default-schemas [] | ||
| (merge (predicate-schemas) (class-schemas) (comparator-schemas) (type-schemas) (sequence-schemas) (base-schemas))) | ||
| (merge (predicate-schemas) (class-schemas) (comparator-schemas) (type-schemas) (sequence-schemas) (conditional-schemas) (base-schemas))) | ||
|
|
||
| (def default-registry | ||
| (let [strict #?(:cljs (identical? mr/mode "strict") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3697,3 +3697,32 @@ | |
| (is (= @count-into-schemas 3)) ;; was 6 | ||
| (is (m/coerce ConsCell [1 [2 [3 [4 [1 [2 [3 [4 nil]]]]]]]])) | ||
| (is (= @count-into-schemas 3)))) ;; was 10 | ||
|
|
||
| (deftest if-conditional-schema-test | ||
|
Member
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. good test, though it doesn't exercise the 2-arity form at all! |
||
| (testing "validation equivalence" | ||
| (let [condition [:map [:x [:= true]]] | ||
| then [:map [:y int?]] | ||
| else [:map [:y string?]] | ||
| jse-schema [:if condition | ||
| then | ||
| else] | ||
| equivalent-schema [:or [:and condition then] | ||
| [:and [:not condition] else]] | ||
| valid-when-true {:x true :y 42} | ||
| valid-when-false {:x false :y "hello"} | ||
| invalid-when-true {:x true :y "oops"} | ||
| invalid-when-false {:x false :y 99}] | ||
| (testing ":if validates with matching JSON Schema If-Then-Else semantics" | ||
| (is (true? (m/validate jse-schema valid-when-true))) | ||
| (is (true? (m/validate jse-schema valid-when-false))) | ||
| (is (false? (m/validate jse-schema invalid-when-true))) | ||
| (is (false? (m/validate jse-schema invalid-when-false)))) | ||
| (testing "matches the equivalent :or/:and malli schema" | ||
| (is (= (m/validate equivalent-schema valid-when-true) | ||
| (m/validate jse-schema valid-when-true))) | ||
| (is (= (m/validate equivalent-schema valid-when-false) | ||
| (m/validate jse-schema valid-when-false))) | ||
| (is (= (m/validate equivalent-schema invalid-when-true) | ||
| (m/validate jse-schema invalid-when-true))) | ||
| (is (= (m/validate equivalent-schema invalid-when-false) | ||
| (m/validate jse-schema invalid-when-false))))))) | ||
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.
It's more like logical implication as written. This is the opposite of what I expected. For example
[:or [:if S T] [:if S' T']]would never hit the second schema.Additionally, the equivalent schema to
[:if S T]is actually[:or [:and S T] :any], so there's some internal inconsistency here.This might need more time to bake, I propose we support 2 args later and start with 3. I understand JSON Schema maps missing branches to true but this doesn't map nicely to Clojure's semantics.