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

mu/closed-schema (and other m/schema-walker uses) produce extra wrappers for reference schemas #884

Closed
opqdonut opened this issue Mar 28, 2023 · 1 comment · Fixed by #883

Comments

@opqdonut
Copy link
Member

The result of mu/closed-schema needs one more m/deref to get to the concrete schema. The same happens with a trivial m/schema-walker use.

Demonstration:

user=> (require '[malli.core :as m])
nil
user=> (require '[malli.util :as mu])
nil
user=> (def s1 (m/schema [:schema {:registry {"Foo" :int}} "Foo"]))
#'user/s1
user=> (def s2 (mu/closed-schema s1))
#'user/s2
user=> (m/form s1)
[:schema {:registry {"Foo" :int}} "Foo"]
user=> (m/form s2)
[:schema {:registry {"Foo" :int}} "Foo"]
user=> (-> s1 m/deref m/deref)
:int
user=> (-> s2 m/deref m/deref)
"Foo"
user=> (-> s2 m/deref m/deref m/deref)     ;; one more deref needed!
:int
user=> (defn noop-walk [schema] (m/walk schema (m/schema-walker identity)))
#'user/noop-walk
user=> (def s3 (noop-walk s1))
#'user/s3
user=> (-> s3 m/deref m/deref)
"Foo"
user=> (-> s3 m/deref m/deref m/deref)     ;; one more deref needed!
:int

The root cause is the -walk implementation for -schema-schema:

            (-walk [this walker path options]
              (when (-accept walker this path options)
                (if (or (not id) ((-boolean-fn (::walk-schema-refs options false)) id))
                  (-outer walker this path (-inner-indexed walker path children options) options)
                  (-outer walker this path [id] options))))

This combined with m/schema-walker causes (m/-set-children schema [id]), which overrides the child from :int to "Foo". Luckily this doesn't cause a loop in later schema walking, just the need for one more deref.

@opqdonut
Copy link
Member Author

I tried to set ::walk-schema-refs for m/closed-schema (in #883), but it had some really funky consequences:

user> (def closed (mu/closed-schema [:schema {:registry {"Foo" [:map [:a :int]]}} "Foo"]))
#'user/closed
user> (m/form closed)
[:schema {:registry {"Foo" [:map [:a :int]]}} "Foo"]
user> (-> closed m/deref m/deref m/form)
[:map {:closed true} [:a :int]]

Even though the schema prints out as "Foo", it's not equal to the "Foo" in the registry any more.

opqdonut added a commit that referenced this issue Apr 12, 2023
remove the special case from the -schema-schema -walk method instead
of having a workaround on the schema-walker level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant