Alternative approach to compatibility

.github/workflows/R-CMD-check.yaml

@@ -9,6 +9,7 @@ on:
     branches:
       - main
       - master
+      - mrc-2476
 
 name: R-CMD-check
 

.github/workflows/test-coverage.yaml

@@ -7,6 +7,7 @@ on:
     branches:
       - main
       - master
+      - mrc-2476
 
 name: test-coverage
 

DESCRIPTION

@@ -3,6 +3,7 @@ Title: Validate 'JSON' Schema
 Version: 1.3.0
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
     email = "rich.fitzjohn@gmail.com"),
+    person("Rob", "Ashton", role = "aut"),
     person("Alicia", "Schep", role = "ctb"),
     person("Ian", "Lyttle", role = "ctb"),
     person("Kara", "Woo", role = "ctb"),
@@ -23,7 +24,8 @@ Suggests:
     knitr,
     jsonlite,
     rmarkdown,
-    testthat
+    testthat,
+    withr
 RoxygenNote: 7.1.1
 VignetteBuilder: knitr
 Encoding: UTF-8

NEWS.md

@@ -2,7 +2,6 @@
 
 * Upgrade to ajv version 8.5.0
 * Add arg `strict` to `json_validate` and `json_validator` to allow evaluating schema in strict mode for ajv only. This is off (`FALSE`) by default to use permissive behaviour detailed in JSON schema
-* Use ajv as default validation engine if schema version explicitly set to > draft-04
 
 ## jsonvalidate 1.2.3
 

R/read.R

@@ -114,12 +114,6 @@ read_meta_schema_version <- function(schema, v8) {
                   "(draft-\\d{2}|draft/\\d{4}-\\d{2})/schema#*$")
   version <- gsub(regex, "\\1", meta_schema)
 
-  versions_legal <- c("draft-04", "draft-06", "draft-07", "draft/2019-09",
-                      "draft/2020-12")
-  if (!(version %in% versions_legal)) {
-    stop(sprintf("Unknown meta schema version '%s'", version))
-  }
-
   version
 }
 

R/util.R

@@ -60,3 +60,10 @@ set_names <- function(x, nms) {
   names(x) <- nms
   x
 }
+
+
+note_imjv <- function(msg) {
+  if (!isTRUE(getOption("jsonvalidate.no_note_imjv", FALSE))) {
+    message(msg)
+  }
+}

R/validate.R

@@ -1,5 +1,46 @@
 ##' Create a validator that can validate multiple json files.
 ##'
+##' @section Validation Engines:
+##'
+##' We support two different json validation engines, \code{imjv}
+##'   ("is-my-json-valid") and \code{ajv} ("Another JSON
+##'   Validator"). \code{imjv} was the original validator included in
+##'   the package and remains the default for reasons of backward
+##'   compatibility. However, users are encouraged to migrate to
+##'   \code{ajv} as with it we support many more features, including
+##'   nested schemas that span multiple files, meta schema versions
+##'   later than draft-04, validating using a subschema, and
+##'   validating a subset of an input data object.
+##'
+##' If your schema uses these features we will print a message to
+##'   screen indicating that you should update. We do not use a
+##'   warning here as this will be disruptive to users. You can
+##'   disable the message by setting the option
+##'   \code{jsonvalidate.no_note_imjv} to \code{TRUE}. Consider
+##'   using \code{withr::with_options} (or simply
+##'   \code{suppressMessages}) to scope this option if you want to
+##'   quieten it within code you do not control.
+##'
+##' Updating the engine should be simply a case of adding \code{engine
+##'   = "ajv"} to your \code{json_validator} or \code{json_validate}
+##'   calls, but you may see some issues when doing so.
+##'
+##' \itemize{
+##' \item Your json now fails validation: We've seen this where
+##'   schemas spanned several files and are silently ignored. By
+##'   including these, your data may now fail validation and you will
+##'   need to either fix the data or the schema.
+##'
+##' \item Your code depended on the exact payload returned by
+##'   \code{imjv}: If you are inspecting the error result and checking
+##'   numbers of errors, or even the columns used to describe the
+##'   errors, you will likely need to update your code to accommodate
+##'   the slightly different format of \code{ajv}
+##'
+##' \item Your schema is simply invalid: If you reference an invalid
+##'   metaschema for example, jsonvalidate will fail
+##' }
+##'
 ##' @title Create a json validator
 ##'
 ##' @param schema Contents of the json schema, or a filename
@@ -44,14 +85,7 @@ json_validator <- function(schema, engine = "imjv", reference = NULL,
                            strict = FALSE) {
   v8 <- env$ct
   schema <- read_schema(schema, v8)
-  not_04 <- !is.null(schema$meta_schema_version) &&
-    !identical(schema$meta_schema_version, "draft-04")
-  if (not_04 && identical(engine, "imjv")) {
-    message(sprintf(paste0("Trying to use schema %s, imjv only supports ",
-                           "draft-04. Falling back to ajv engine."),
-                    schema$meta_schema_version))
-    engine <- "ajv"
-  }
+
   switch(engine,
          imjv = json_validator_imjv(schema, v8, reference),
          ajv = json_validator_ajv(schema, v8, reference, strict),
@@ -102,17 +136,25 @@ json_validator_imjv <- function(schema, v8, reference) {
   meta_schema_version <- schema$meta_schema_version %||% "draft-04"
 
   if (!is.null(reference)) {
+    ## This one has to be an error; it has never worked and makes no
+    ## sense.
     stop("subschema validation only supported with engine 'ajv'")
   }
 
   if (meta_schema_version != "draft-04") {
-    stop(sprintf(
-      "meta schema version '%s' is only supported with engine 'ajv'",
-      meta_schema_version))
+    ## We detect the version, so let the user know they are not really
+    ## getting what they're asking for
+    note_imjv(paste(
+      "meta schema version other than 'draft-04' is only supported with",
+      sprintf("engine 'ajv' (requested: '%s')", meta_schema_version),
+      "- falling back to use 'draft-04'"))
+    meta_schema_version <- "draft-04"
   }
 
   if (length(schema$dependencies) > 0L) {
-    stop("Schema references are only supported with engine 'ajv'")
+    ## We've found references, but can't support them. Let the user
+    ## know.
+    note_imjv("Schema references are only supported with engine 'ajv'")
   }
 
   v8$call("imjv_create", name, meta_schema_version, V8::JS(schema$schema))
@@ -140,6 +182,12 @@ json_validator_ajv <- function(schema, v8, reference, strict) {
   name <- random_id()
   meta_schema_version <- schema$meta_schema_version %||% "draft-07"
 
+  versions_legal <- c("draft-04", "draft-06", "draft-07", "draft/2019-09",
+                      "draft/2020-12")
+  if (!(meta_schema_version %in% versions_legal)) {
+    stop(sprintf("Unknown meta schema version '%s'", meta_schema_version))
+  }
+
   if (is.null(reference)) {
     reference <- V8::JS("null")
   }

tests/testthat/test-read.R

@@ -77,22 +77,6 @@ test_that("can't read external schemas", {
 })
 
 
-test_that("invalid schema version", {
-  schema <- "{
-    '$schema': 'http://json-schema.org/draft-99/schema#',
-    'type': 'object',
-    'properties': {
-      'a': {
-        'const': 'foo'
-      }
-    }
-  }"
-  expect_error(
-    read_schema(schema, env$ct),
-    "Unknown meta schema version 'draft-99'")
-})
-
-
 test_that("Conflicting schema versions", {
   a <- c(
     '{',

tests/testthat/test-util.R

@@ -28,3 +28,17 @@ test_that("get_string passes along strings", {
   expect_equal(get_string("file_that_does_not_exist.json"),
                "file_that_does_not_exist.json")
 })
+
+
+test_that("control printing imjv notice", {
+  testthat::skip_if_not_installed("withr")
+  withr::with_options(
+    list(jsonvalidate.no_note_imjv = NULL),
+    expect_message(note_imjv("note"), "note"))
+  withr::with_options(
+    list(jsonvalidate.no_note_imjv = FALSE),
+    expect_message(note_imjv("note"), "note"))
+  withr::with_options(
+    list(jsonvalidate.no_note_imjv = TRUE),
+    expect_silent(note_imjv("note")))
+})

tests/testthat/test-validator.R

@@ -174,6 +174,7 @@ test_that("can't use subschema reference with imjv", {
 })
 
 test_that("can't use nested schemas with imjv", {
+  testthat::skip_if_not_installed("withr")
   parent <- c(
     '{',
     '    "type": "object",',
@@ -194,9 +195,14 @@ test_that("can't use nested schemas with imjv", {
   writeLines(parent, file.path(path, "parent.json"))
   writeLines(child, file.path(path, "child.json"))
 
-  expect_error(
-    json_validator(file.path(path, "parent.json"), engine = "imjv"),
-    "Schema references are only supported with engine 'ajv'")
+  withr::with_options(
+    list(jsonvalidate.no_note_imjv = FALSE),
+    expect_message(
+      v <- json_validator(file.path(path, "parent.json"), engine = "imjv"),
+      "Schema references are only supported with engine 'ajv'"))
+  ## We incorrectly don't find this invalid, because we never read the
+  ## child schema; the user should have used ajv!
+  expect_true(v('{"hello": 1}'))
 })
 
 
@@ -207,6 +213,7 @@ test_that("can't use invalid engines", {
 
 
 test_that("can't use new schema versions with imjv", {
+  testthat::skip_if_not_installed("withr")
   schema <- "{
     '$schema': 'http://json-schema.org/draft-07/schema#',
     'type': 'object',
@@ -217,9 +224,14 @@ test_that("can't use new schema versions with imjv", {
     }
   }"
   schema <- read_schema(schema, env$ct)
-  expect_error(
-    json_validator_imjv(schema, env$ct, NULL),
-    "meta schema version 'draft-07' is only supported with engine 'ajv'")
+  withr::with_options(
+    list(jsonvalidate.no_note_imjv = FALSE),
+    expect_message(
+      v <- json_validator_imjv(schema, env$ct, NULL),
+      "meta schema version other than 'draft-04' is only supported with"))
+  ## We incorrectly don't find this invalid, because imjv does not
+  ## understand the const keyword.
+  expect_true(v('{"a": "bar"}'))
 })
 
 
@@ -529,8 +541,11 @@ test_that("unknown format type throws an error if in strict mode", {
                paste0('Error: unknown format "test" ignored in schema at ',
                       'path "#/properties/date"'))
 
-  ## Warnings printed in non-strict mode
-  msg <- capture_warnings(v <- json_validator(str, "ajv", strict = FALSE))
+  ## Warnings printed in non-strict mode; these include some annoying
+  ## newlines from the V8 engine, so using capture.output to stop
+  ## these messing up testthat output
+  capture.output(
+    msg <- capture_warnings(v <- json_validator(str, "ajv", strict = FALSE)))
   expect_equal(msg[1], paste0('unknown format "test" ignored in ',
                               'schema at path "#/properties/date"'))
   expect_true(v("{'date': '123'}"))
@@ -555,21 +570,6 @@ test_that("json_validate can be run in strict mode", {
     'Error: strict mode: unknown keyword: "reference"')
 })
 
-test_that("json_validator falls back to ajv if version > draft-04", {
-  schema <- "{
-    '$schema': 'http://json-schema.org/draft-07/schema#',
-    'type': 'object',
-    'properties': {
-      'a': {
-        'const': 'foo'
-      }
-    }
-  }"
-  msg <- capture_messages(v <- json_validator(schema))
-  expect_true(v("{'a': 'foo'}"))
-  expect_equal(msg, paste0("Trying to use schema draft-07, imjv only supports ",
-                           "draft-04. Falling back to ajv engine.\n"))
-})
 
 test_that("validation works with 2019-09 schema version", {
   schema <- "{
@@ -643,3 +643,20 @@ test_that("validation works with 2020-12 schema version", {
   expect_true(json_validate("{'enabled': true}", schema, engine = "ajv"))
   expect_false(json_validate("{'enabled': 'test'}", schema, engine = "ajv"))
 })
+
+
+test_that("ajv requires a valid meta schema version", {
+  schema <- "{
+    '$schema': 'http://json-schema.org/draft-99/schema#',
+    'type': 'object',
+    'properties': {
+      'a': {
+        'const': 'foo'
+      }
+    }
+  }"
+
+  expect_error(
+    json_validator(schema, engine = "ajv"),
+    "Unknown meta schema version 'draft-99'")
+})