Merge pull request #2575 from Kinto/adjust-ujson-3.X

Cleanup usage of ultrajson vs. native json

CHANGELOG.rst

@@ -6,7 +6,9 @@ This document describes changes between each past release.
 13.7.0 (unreleased)
 -------------------
 
-- Nothing changed yet.
+**Breaking changes**
+
+- Drop the ``strict_json`` option, and use ``ultrajson`` everywhere
 
 
 13.6.6 (2020-06-26)

kinto/core/__init__.py

@@ -67,6 +67,7 @@
         "kinto.core.events.setup_transaction_hook",
     ),
     "event_listeners": "",
+    "json_renderer": "ultrajson",
     "heartbeat_timeout_seconds": 10,
     "newrelic_config": None,
     "newrelic_env": "dev",
@@ -117,6 +118,7 @@ def error_handler(self, request):
 
     @classmethod
     def init_from_settings(cls, settings):
+        cls.renderer = settings["json_renderer"]
         cls.cors_origins = tuple(aslist(settings["cors_origins"]))
         cors_max_age = settings["cors_max_age_seconds"]
         cls.cors_max_age = int(cors_max_age) if cors_max_age else None

kinto/core/initialization.py

@@ -53,7 +53,7 @@ def setup_json_serializer(config):
 
     # Override json renderer using ujson
     renderer = JSONRenderer(serializer=utils.json_serializer)
-    config.add_renderer("json", renderer)
+    config.add_renderer("ultrajson", renderer)  # See `kinto.core.Service`
 
 
 def setup_csp_headers(config):

kinto/core/storage/__init__.py

@@ -1,10 +1,8 @@
-import json
 import logging
 import random
 import warnings
 from collections import namedtuple
 
-import ujson
 from pyramid.settings import asbool
 
 from kinto.core.decorators import deprecate_kwargs
@@ -59,13 +57,6 @@ class StorageBase:
     id_generator = generators.UUID4()
     """Id generator used when no one is provided for create."""
 
-    def __init__(self, strict_json=True):
-        """initialize json (de)serializer to be the strict, slow json or ujson"""
-        if strict_json:
-            self.json = json
-        else:
-            self.json = ujson
-
     def initialize_schema(self, dry_run=False):
         """Create every necessary objects (like tables or indices) in the
         backend.

kinto/core/storage/memory.py

@@ -1,11 +1,8 @@
-import json
 import numbers
 import operator
 import re
 from collections import abc, defaultdict
 
-import ujson
-
 from kinto.core import utils
 from kinto.core.decorators import deprecate_kwargs, synchronized
 from kinto.core.storage import (
@@ -16,7 +13,7 @@
     StorageBase,
     exceptions,
 )
-from kinto.core.utils import COMPARISON, find_nested_value
+from kinto.core.utils import COMPARISON, find_nested_value, json
 
 
 def tree():
@@ -28,9 +25,6 @@ class MemoryBasedStorage(StorageBase):
     methods for in-memory implementations of sorting and filtering.
     """
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
     def initialize_schema(self, dry_run=False):
         # Nothing to do.
         pass
@@ -136,11 +130,6 @@ class Storage(MemoryBasedStorage):
     Enable in configuration::
 
         kinto.storage_backend = kinto.core.storage.memory
-
-    Enable strict json validation before saving (instead of the more lenient
-    ujson, see #1238)::
-
-        kinto.storage_strict_json = true
     """
 
     def __init__(self, *args, readonly=False, **kwargs):
@@ -189,7 +178,13 @@ def create(
         modified_field=DEFAULT_MODIFIED_FIELD,
     ):
         id_generator = id_generator or self.id_generator
-        obj = {**obj}
+
+        # This is very inefficient, but memory storage is not used in production.
+        # The serialization provides the necessary consistency with other
+        # backends implementation, and the deserialization creates a deep
+        # copy of the passed object.
+        obj = json.loads(json.dumps(obj))
+
         if id_field in obj:
             # Raise unicity error if object with same id already exists.
             try:
@@ -202,7 +197,6 @@ def create(
 
         self.set_object_timestamp(resource_name, parent_id, obj, modified_field=modified_field)
         _id = obj[id_field]
-        obj = ujson.loads(self.json.dumps(obj))
         self._store[parent_id][resource_name][_id] = obj
         self._cemetery[parent_id][resource_name].pop(_id, None)
         return obj
@@ -233,9 +227,13 @@ def update(
         id_field=DEFAULT_ID_FIELD,
         modified_field=DEFAULT_MODIFIED_FIELD,
     ):
-        obj = {**obj}
+        # This is very inefficient, but memory storage is not used in production.
+        # The serialization provides the necessary consistency with other
+        # backends implementation, and the deserialization creates a deep
+        # copy of the passed object.
+        obj = json.loads(json.dumps(obj))
+
         obj[id_field] = object_id
-        obj = ujson.loads(self.json.dumps(obj))
 
         self.set_object_timestamp(resource_name, parent_id, obj, modified_field=modified_field)
         self._store[parent_id][resource_name][object_id] = obj
@@ -442,7 +440,8 @@ def extract_object_set(
 
 
 def canonical_json(obj):
-    return json.dumps(obj, sort_keys=True, separators=(",", ":"))
+    # We just a predictable serialization so that we just compare strings.
+    return json.dumps(obj, sort_keys=True)
 
 
 def apply_filters(objects, filters):
@@ -586,6 +585,4 @@ def _get_objects_by_parent_id(store, parent_id, resource_name, with_meta=False):
 
 
 def load_from_config(config):
-    settings = {**config.get_settings()}
-    strict = settings.get("storage_strict_json", False)
-    return Storage(strict_json=strict)
+    return Storage()

kinto/core/storage/postgresql/__init__.py

@@ -14,7 +14,7 @@
 )
 from kinto.core.storage.postgresql.client import create_from_config
 from kinto.core.storage.postgresql.migrator import MigratorMixin
-from kinto.core.utils import COMPARISON
+from kinto.core.utils import COMPARISON, json
 
 logger = logging.getLogger(__name__)
 HERE = os.path.dirname(__file__)
@@ -307,7 +307,7 @@ def create(
             parent_id=parent_id,
             resource_name=resource_name,
             last_modified=obj.get(modified_field),
-            data=self.json.dumps(query_object),
+            data=json.dumps(query_object),
         )
         with self.client.connect() as conn:
             result = conn.execute(query % safe_holders, placeholders)
@@ -383,7 +383,7 @@ def update(
             parent_id=parent_id,
             resource_name=resource_name,
             last_modified=obj.get(modified_field),
-            data=self.json.dumps(query_object),
+            data=json.dumps(query_object),
         )
 
         with self.client.connect() as conn:
@@ -427,7 +427,7 @@ def delete(
                AND NOT deleted
             RETURNING as_epoch(last_modified) AS last_modified;
             """
-        deleted_data = self.json.dumps(dict([(deleted_field, True)]))
+        deleted_data = json.dumps(dict([(deleted_field, True)]))
         placeholders = dict(
             object_id=object_id,
             parent_id=parent_id,
@@ -510,7 +510,7 @@ def delete_all(
 
         id_field = id_field or self.id_field
         modified_field = modified_field or self.modified_field
-        deleted_data = self.json.dumps(dict([(deleted_field, True)]))
+        deleted_data = json.dumps(dict([(deleted_field, True)]))
         placeholders = dict(
             parent_id=parent_id, resource_name=resource_name, deleted_data=deleted_data
         )
@@ -804,9 +804,9 @@ def _format_conditions(self, filters, id_field, modified_field, prefix="filters"
                     COMPARISON.EXCLUDE,
                     COMPARISON.CONTAINS_ANY,
                 ):
-                    value = self.json.dumps(value)
+                    value = json.dumps(value)
                 else:
-                    value = [self.json.dumps(v) for v in value]
+                    value = [json.dumps(v) for v in value]
 
             if filtr.operator in (COMPARISON.IN, COMPARISON.EXCLUDE):
                 value = tuple(value)
@@ -996,12 +996,9 @@ def _format_sorting(self, sorting, id_field, modified_field):
 def load_from_config(config):
     settings = config.get_settings()
     max_fetch_size = int(settings["storage_max_fetch_size"])
-    strict = settings.get("storage_strict_json", False)
     readonly = settings.get("readonly", False)
     client = create_from_config(config, prefix="storage_")
-    return Storage(
-        client=client, max_fetch_size=max_fetch_size, strict_json=strict, readonly=readonly
-    )
+    return Storage(client=client, max_fetch_size=max_fetch_size, readonly=readonly)
 
 
 UNKNOWN_SCHEMA_VERSION_MESSAGE = """

kinto/core/storage/postgresql/client.py

@@ -15,7 +15,6 @@
     "max_fetch_size",
     "max_size_bytes",
     "prefix",
-    "strict_json",
     "hosts",
 ]
 

kinto/core/testing.py

@@ -148,7 +148,6 @@ def get_app_settings(cls, extras=None):
         settings = {**DEFAULT_SETTINGS}
 
         settings["storage_backend"] = "kinto.core.storage.memory"
-        settings["storage_strict_json"] = True
         settings["cache_backend"] = "kinto.core.cache.memory"
         settings["permission_backend"] = "kinto.core.permission.memory"
 

kinto/core/views/version.py

@@ -1,10 +1,10 @@
-import json
 import os
 
 import colander
 from pyramid.security import NO_PERMISSION_REQUIRED
 
 from kinto.core import Service
+from kinto.core.utils import json
 
 HERE = os.path.dirname(__file__)
 ORIGIN = os.path.dirname(HERE)

kinto/plugins/admin/__init__.py

@@ -1,9 +1,10 @@
-import json
 import os
 
 from pyramid.httpexceptions import HTTPTemporaryRedirect
 from pyramid.static import static_view
 
+from kinto.core.utils import json
+
 from .views import admin_home_view
 
 HERE = os.path.dirname(__file__)

kinto/plugins/quotas/utils.py

@@ -2,5 +2,6 @@
 
 
 def record_size(record):
+    # We cannot use ultrajson here, since the `separator` option is not available.
     canonical_json = json.dumps(record, sort_keys=True, separators=(",", ":"))
     return len(canonical_json)

tests/core/test_storage.py

@@ -95,7 +95,6 @@ def test_backend_raise_not_implemented_error(self):
 
 class MemoryStorageTest(StorageTest, unittest.TestCase):
     backend = memory
-    settings = {"storage_strict_json": True}
 
     def setUp(self):
         super().setUp()
@@ -120,10 +119,6 @@ def test_backenderror_message_default_to_original_exception_message(self):
     def test_ping_logs_error_if_unavailable(self):
         pass
 
-
-class LenientMemoryStorageTest(MemoryStorageTest):
-    settings = {"storage_strict_json": False}
-
     def test_create_bytes_raises(self):
         data = {"steak": "haché".encode(encoding="utf-8")}
         self.assertIsInstance(data["steak"], bytes)
@@ -148,7 +143,6 @@ class PostgreSQLStorageTest(StorageTest, unittest.TestCase):
         "storage_backend": "kinto.core.storage.postgresql",
         "storage_poolclass": "sqlalchemy.pool.StaticPool",
         "storage_url": "postgresql://postgres:postgres@localhost:5432/testdb",
-        "storage_strict_json": True,
     }
 
     def setUp(self):

tests/openapi/support.py

@@ -1,11 +1,12 @@
-import json
 import unittest
 
 from bravado_core.request import IncomingRequest, unmarshal_request
 from bravado_core.resource import build_resources
 from bravado_core.response import OutgoingResponse, validate_response
 from bravado_core.spec import Spec
 
+from kinto.core.utils import json
+
 from ..support import (
     MINIMALIST_BUCKET,
     MINIMALIST_COLLECTION,

tests/plugins/test_history.py

@@ -19,7 +19,6 @@ def test_a_statsd_timer_is_used_for_history_if_configured(self):
         settings = {
             "statsd_url": "udp://127.0.0.1:8125",
             "includes": "kinto.plugins.history",
-            "storage_strict_json": True,
         }
         config = testing.setUp(settings=settings)
         with mock.patch("kinto.core.statsd.Client.timer") as mocked:

tests/plugins/test_quotas.py

@@ -27,7 +27,6 @@ def test_a_statsd_timer_is_used_for_quotas_if_configured(self):
         settings = {
             "statsd_url": "udp://127.0.0.1:8125",
             "includes": "kinto.plugins.quotas",
-            "storage_strict_json": True,
         }
         config = testing.setUp(settings=settings)
         with mock.patch("kinto.core.statsd.Client.timer") as mocked: