Repository: python-jsonschema/check-jsonschema

Issue:
New validator instances are built for each instancefile, discarding any internal caching
_First reported via https://github.com/python-jsonschema/check-jsonschema/pull/451_

For each file, we get a validator from the "schema loader" objects. This interface is correct for the top-level checker. It allows, for example, metaschema checking to proceed with different schemas for different files.

However, this also is being applied to the remote and local class -- `SchemaLoader`.
As a result, a new validator is built for each file, and any internal caching done under the validator is discarded.

To resolve, one of two approaches should be used:
- create exactly one validator per remote schema (or `(schema, settings)` where there might not be any current settings)
- create a new validator per call, but reuse the components from any matching schemas

Discussion:

Patch:
diff --git a/.flake8 b/.flake8
index e929422..c56dba3 100644
--- a/.flake8
+++ b/.flake8
@@ -1,11 +1,5 @@
 [flake8]
-exclude = .git,.tox,__pycache__,.eggs,dist,.venv*,docs,build,_build
-# we enforce 80 char width with `black` "loosely", so flake8 should be set to
-# not fail on up to 90 chars of width
+exclude = .git,.tox,__pycache__,dist,.venv*,docs,build
 max-line-length = 90
-
-# based on the flake8 conf for `black` itself:
-#   https://github.com/ambv/black/blob/master/.flake8
-#
-# W503/W504 conflict, black causes E203
-ignore = W503,W504,E203,
+# black related: W503/W504 conflict, black causes E203
+ignore = W503,W504,E203,B019
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index fb52d11..80e266c 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -26,6 +26,7 @@ repos:
   rev: 7.1.0
   hooks:
     - id: flake8
+      args: ['--config', '.flake8']
       additional_dependencies:
         - 'flake8-bugbear==24.1.17'
         - 'flake8-typing-as-t==0.0.3'
diff --git a/src/check_jsonschema/schema_loader/main.py b/src/check_jsonschema/schema_loader/main.py
index 4ce95c9..88ff6a0 100644
--- a/src/check_jsonschema/schema_loader/main.py
+++ b/src/check_jsonschema/schema_loader/main.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import functools
 import pathlib
 import typing as t
 import urllib.error
@@ -130,11 +131,21 @@ class SchemaLoader(SchemaLoaderBase):
         instance_doc: dict[str, t.Any],
         format_opts: FormatOptions,
         fill_defaults: bool,
+    ) -> jsonschema.protocols.Validator:
+        return self._get_validator(format_opts, fill_defaults)
+
+    @functools.lru_cache
+    def _get_validator(
+        self,
+        format_opts: FormatOptions,
+        fill_defaults: bool,
     ) -> jsonschema.protocols.Validator:
         retrieval_uri = self.get_schema_retrieval_uri()
         schema = self.get_schema()
 
         schema_dialect = schema.get("$schema")
+        if schema_dialect is not None and not isinstance(schema_dialect, str):
+            schema_dialect = None
 
         # format checker (which may be None)
         format_checker = make_format_checker(format_opts, schema_dialect)
diff --git a/src/check_jsonschema/schema_loader/resolver.py b/src/check_jsonschema/schema_loader/resolver.py
index c63b7bb..5084328 100644
--- a/src/check_jsonschema/schema_loader/resolver.py
+++ b/src/check_jsonschema/schema_loader/resolver.py
@@ -79,8 +79,8 @@ def create_retrieve_callable(
         else:
             full_uri = uri
 
-        if full_uri in cache._cache:
-            return cache[uri]
+        if full_uri in cache:
+            return cache[full_uri]
 
         full_uri_scheme = urllib.parse.urlsplit(full_uri).scheme
         if full_uri_scheme in ("http", "https"):
@@ -100,8 +100,8 @@ def create_retrieve_callable(
         else:
             parsed_object = get_local_file(full_uri)
 
-        cache[uri] = parsed_object
-        return cache[uri]
+        cache[full_uri] = parsed_object
+        return cache[full_uri]
 
     return retrieve_reference
 


Test patch:
diff --git a/tests/acceptance/test_remote_ref_resolution.py b/tests/acceptance/test_remote_ref_resolution.py
index d95fba5..3dafc4c 100644
--- a/tests/acceptance/test_remote_ref_resolution.py
+++ b/tests/acceptance/test_remote_ref_resolution.py
@@ -244,3 +244,60 @@ def test_ref_resolution_with_custom_base_uri(run_line, tmp_path, check_passes):
         assert result.exit_code == 0, output
     else:
         assert result.exit_code == 1, output
+
+
+@pytest.mark.parametrize("num_instances", (1, 2, 10))
+@pytest.mark.parametrize("check_passes", (True, False))
+def test_remote_ref_resolution_callout_count_is_scale_free_in_instancefiles(
+    run_line, tmp_path, num_instances, check_passes
+):
+    """
+    Test that for any N > 1, validation of a schema with a ref against N instance files
+    has exactly the same number of callouts as validation when N=1
+
+    This proves that the validator and caching are working correctly, and we aren't
+    repeating callouts to rebuild state.
+    """
+    schema_uri = "https://example.org/schemas/main.json"
+    ref_uri = "https://example.org/schemas/title_schema.json"
+
+    main_schema = {
+        "$id": schema_uri,
+        "$schema": "http://json-schema.org/draft-07/schema",
+        "properties": {
+            "title": {"$ref": "./title_schema.json"},
+        },
+        "additionalProperties": False,
+    }
+    title_schema = {"type": "string"}
+    responses.add("GET", schema_uri, json=main_schema)
+    responses.add("GET", ref_uri, json=title_schema)
+
+    # write N documents
+    instance_doc = {"title": "doc one" if check_passes else 2}
+    instance_paths = []
+    for i in range(num_instances):
+        instance_path = tmp_path / f"instance{i}.json"
+        instance_path.write_text(json.dumps(instance_doc))
+        instance_paths.append(str(instance_path))
+
+    result = run_line(
+        [
+            "check-jsonschema",
+            "--schemafile",
+            schema_uri,
+        ]
+        + instance_paths
+    )
+    output = f"\nstdout:\n{result.stdout}\n\nstderr:\n{result.stderr}"
+    if check_passes:
+        assert result.exit_code == 0, output
+    else:
+        assert result.exit_code == 1, output
+
+    # this is the moment of the "real" test run here:
+    # no matter how many instances there were, there should only have been two calls
+    # (one for the schema and one for the $ref)
+    assert len(responses.calls) == 2
+    assert len([c for c in responses.calls if c.request.url == schema_uri]) == 1
+    assert len([c for c in responses.calls if c.request.url == ref_uri]) == 1