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

Convert JSON schema to malli #915

Draft
wants to merge 90 commits into
base: master
Choose a base branch
from

Conversation

PavlosMelissinos
Copy link

@PavlosMelissinos PavlosMelissinos commented Jul 3, 2023

Still heavily WIP.

  • Fix refs (could consider topological sorting)
  • Identify missing features

Built on top of #211

hkupty and others added 30 commits January 6, 2021 14:42
This allows a very basic json-schema->malli conversion and will most likely need to be adequated to:
   - cleanup TODOS;
   - ensure function names are sane;
   - ensure all json-schema features are supported (at least for one specific version, i.e. draft-7).
Also, reorganize functions to simplify processing, reduce duplication and avoid special casing.

Lastly, make schema->malli the first entry point since anything can be a schema. It then delegates to types, aggregates or `$ref`s.
Also, adds top-level enums (and consts) and prepares for better range checks in int/number.
ikitommi and others added 25 commits April 7, 2023 14:38
This allows us to see the whole spec at once and fix a bug w/ custom registry definitions
...in json-schema. Also removes the leading `:` which the definitions keys also didn't have. This allows the refs and definitions keys to match and be standards-compliant.
Perhaps this really needs to be fixed at a lower level, but I noticed that at times I was getting definitions whose values where just refs back to the same definition. These seemed to come in addition to the correct result that had the actual definition in it. So this just filters out the circular ones.
...in json-schema-test/references-test. Fix was in 23488b2.
...including one commented-out failing test that should pass once issue metosin#464 is fixed.
Unless I'm missing something, present tense should be used here as it's currently true that you have to bring in your state atom in order to have a global registry
@timothypratley
Copy link

The simplest failing test case:

  {:$schema     "https://json-schema.org/draft/2020-12/schema",
   :$ref        "#/definitions/TopLevelSpec"
   :definitions {:A {:enum ["a" "A"]}}}
=> malli.core/invalid-schema

Some other nice small examples here:
https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/tests/draft7/ref.json

@timothypratley
Copy link

I've been playing around with this branch and how to test it.

Adding to deps.edn test alias:

io.github.json-schema-org/JSON-Schema-Test-Suite {:git/sha       "7950d9e0579382103031ef3cfcdc37e7c58b2d1f"
                                                  :deps/manifest :deps}

Then to use it:

(def json-schema-tests-dir
  ".gitlibs/libs/io.github.json-schema-org/JSON-Schema-Test-Suite/7950d9e0579382103031ef3cfcdc37e7c58b2d1f/tests")

(def test-files
  ["draft7/ref.json"])

(defn read-test-suite-file [path]
  (-> (slurp (io/file (System/getProperty "user.home") json-schema-tests-dir path))
      (tu/from-json)
      (walk/keywordize-keys)))

(deftest json-schema-test-suite-test
  (doseq [test-file test-files]
    (testing test-file
      (doseq [{:keys [schema tests description]} (read-test-suite-file test-file)]
        (testing description
          (let [malli (try (sut/json-schema-document->malli schema)
                           (catch Throwable ex
                             (is false
                                 (str "failed to create malli for schema: " \newline
                                      schema \newline
                                      ex))))
                malli-schema (try (when malli (m/schema malli))
                                  (catch Throwable ex
                                    (is false
                                        (str "failed to create malli-schema or schema: " \newline
                                             schema \newline
                                             "from malli:" \newline
                                             malli \newline
                                             ex))))]
            (when (and malli-schema (m/schema? malli-schema))
              (doseq [{:keys [valid data description]} tests]
                (testing description
                  (is (= valid (m/validate malli-schema data))
                      (str "schema: " schema \newline
                           "data: " data \newline
                           "malli: " malli \newline
                           (if valid
                             (str "valid, but got errors: " \newline
                                  (str/join \newline (:errors (m/explain malli-schema data))))
                             "invalid, but matched"))))))))))))

Lots of things don't pass even for this one file, which is probably unsurprising as there are lots of weird edge cases in the tests.

I've been making changes to try to fix problems as I encounter them

Here is the where I got to:

parse.cljc.txt

But it still doesn't work with vega-lite

@PavlosMelissinos
Copy link
Author

Thanks a lot for your contributions @timothypratley ! This is a weird period for me because I just moved (so I'm still settling down at home) and I haven't managed to spend time on open source at work. I'll get back on this as soon as I can though, starting with crystallizing the failing test cases you've provided into unit tests!

@ieugen
Copy link

ieugen commented Mar 15, 2024

I would have loved to have support for json-schema to use on https://github.com/compose-spec/compose-spec/blob/master/schema/compose-spec.json

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

Successfully merging this pull request may close these issues.

9 participants