diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f057c67..fc42838 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -18,17 +18,15 @@ jobs: # AttributeError: module 'collections' has no attribute 'Callable' # https://github.com/nose-devs/nose/issues/1099 django: - - 'Django~=2.2.0' - - 'Django~=3.0.0' - - 'Django~=3.1.0' - 'Django~=3.2.0' - 'Django~=4.0.0' - 'Django~=4.1.0' + - 'Django~=4.2.0' experimental: [false] - include: - - python: '3.9' - django: 'https://github.com/django/django/archive/refs/heads/main.zip#egg=Django' - experimental: true + # include: + # - python: '3.9' + # django: 'https://github.com/django/django/archive/refs/heads/main.zip#egg=Django' + # experimental: true # NOTE this job will appear to pass even when it fails because of # `continue-on-error: true`. Github Actions apparently does not # have this feature, similar to Travis' allow-failure, yet. @@ -38,6 +36,8 @@ jobs: django: 'Django~=4.0.0' - python: '3.7' django: 'Django~=4.1.0' + - python: '3.7' + django: 'Django~=4.2.0' services: postgres: image: postgres:latest @@ -53,8 +53,8 @@ jobs: --health-timeout 5s --health-retries 5 steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v3 + - uses: actions/setup-python@v3 with: python-version: ${{ matrix.python }} - name: Setup @@ -69,7 +69,7 @@ jobs: env: DB_SETTINGS: >- { - "ENGINE":"django.db.backends.postgresql_psycopg2", + "ENGINE":"django.db.backends.postgresql", "NAME":"querybuilder", "USER":"postgres", "PASSWORD":"postgres", @@ -78,7 +78,7 @@ jobs: } DB_SETTINGS2: >- { - "ENGINE":"django.db.backends.postgresql_psycopg2", + "ENGINE":"django.db.backends.postgresql", "NAME":"querybuilder2", "USER":"postgres", "PASSWORD":"postgres", diff --git a/.gitignore b/.gitignore index a170e49..0e78253 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,4 @@ docs/_build/ test_ambition_dev .tox/ +tmp/ diff --git a/README.rst b/README.rst index 64da66c..f26cfe9 100644 --- a/README.rst +++ b/README.rst @@ -34,9 +34,8 @@ where django querysets simplify the sql generation process for simple queries. Requirements ------------ -* Python 2.7 -* Python 3.3, 3.4 -* Django 1.7+ +* Python 3.7 - 3.9 +* Django 2.2 - 4.1 * Postgres 9.3+ Installation diff --git a/docs/release_notes.rst b/docs/release_notes.rst index ba0de72..89be726 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -1,9 +1,23 @@ Release Notes ============= +v3.1.0 +------ +* django 4.2 support + +v3.0.4 +------ +* Adjusted querybuilder select functionality to process json values as needed in the result set + rather than tweak the deep cursor settings. This was observed to interfere with complex query chains. + +v3.0.3 +------ +* Addressed bug in `json_cursor` if Django cursor has extra wrappers + v3.0.2 ------ -* Add `json_cursor` to handle django no longer automatically parsing json fields +* Add `json_cursor` context to handle Django3.1.1+ no longer automatically parsing json fields +* Adjusted query functionality also to handle jsonb columns correctly v3.0.1 ------ diff --git a/manage.py b/manage.py index e374933..b997ce1 100644 --- a/manage.py +++ b/manage.py @@ -1,6 +1,13 @@ #!/usr/bin/env python import sys +# Show warnings about django deprecations - uncomment for version upgrade testing +import warnings +from django.utils.deprecation import RemovedInNextVersionWarning +warnings.filterwarnings('always', category=DeprecationWarning) +warnings.filterwarnings('always', category=PendingDeprecationWarning) +warnings.filterwarnings('always', category=RemovedInNextVersionWarning) + from settings import configure_settings diff --git a/querybuilder/__init__.py b/querybuilder/__init__.py index 7ce0b4e..ed53594 100644 --- a/querybuilder/__init__.py +++ b/querybuilder/__init__.py @@ -1,4 +1,2 @@ # flake8: noqa from .version import __version__ - -default_app_config = 'querybuilder.apps.QueryBuilderConfig' diff --git a/querybuilder/cursor.py b/querybuilder/cursor.py index f5c1a39..69a56d2 100644 --- a/querybuilder/cursor.py +++ b/querybuilder/cursor.py @@ -1,6 +1,60 @@ import contextlib import json -import psycopg2 +from psycopg2.extras import register_default_jsonb +from psycopg2._json import JSONB_OID + + +def jsonify_cursor(django_cursor, enabled=True): + """ + Adjust an already existing cursor to ensure it will return structured types (list or dict) + from jsonb columns instead of strings. Django 3.1.1+ returns strings for raw queries. + https://code.djangoproject.com/ticket/31956 + https://code.djangoproject.com/ticket/31973 + https://www.psycopg.org/docs/extras.html#psycopg2.extras.register_default_jsonb + """ + + # The thing that is returned by connection.cursor() is (normally) a Django object + # of type CursorWrapper that itself has the "real" cursor as a property called cursor. + # However, it could be a CursorDebugWrapper instead, or it could be an outer wrapper + # wrapping one of those. For example django-debug-toolbar wraps CursorDebugWrapper in + # a NormalCursorWrapper. The django-db-readonly package wraps the Django CursorWrapper + # in a ReadOnlyCursorWrapper. I'm not sure if they ever nest multiple levels. I tried + # looping with `while isinstance(inner_cursor, CursorWrapper)`, but it seems that the + # outer wrapper is not necessarily a subclass of the Django wrapper. My next best option + # is to make the assumption that we need to get to the last property called `cursor`, + # basically assuming that any wrapper is going to have a property called `cursor` + # that is the real cursor or the next-level wrapper. + # Another option might be to check the class of inner_cursor to see if it is the real + # database cursor. That would require importing more django libraries, and probably + # having to handle some changes in those libraries over different versions. + + # This register_default_jsonb functionality in psycopg2 does not itself have a "deregister" + # capability. So to deregister, we pass in a different value for the loads method; in this + # case just the str() built-in, which just returns the value passed in. Note that passing + # None for loads does NOT do a deregister; it uses the default value, which as it turns out + # is json.loads anyway! + loads_func = json.loads if enabled else str + + # We expect that there is always at least one wrapper, but we might as well handle + # the possibility that we get passed the inner cursor. + inner_cursor = django_cursor + + while hasattr(inner_cursor, 'cursor'): + inner_cursor = inner_cursor.cursor + + # Hopefully we have the right thing now, but try/catch so we can get a little better info + # if it is not. Another option might be an isinstance, or another function that tests the cursor? + try: + register_default_jsonb(conn_or_curs=inner_cursor, loads=loads_func) + except TypeError as e: + raise Exception(f'jsonify_cursor: conn_or_curs was actually a {type(inner_cursor)}: {e}') + + +def dejsonify_cursor(django_cursor): + """ + Re-adjust a cursor that was "jsonified" so it no longer performs the json.loads(). + """ + jsonify_cursor(django_cursor, enabled=False) @contextlib.contextmanager @@ -12,5 +66,62 @@ def json_cursor(django_database_connection): https://www.psycopg.org/docs/extras.html#psycopg2.extras.register_default_jsonb """ with django_database_connection.cursor() as cursor: - psycopg2.extras.register_default_jsonb(conn_or_curs=cursor.cursor, loads=json.loads) + jsonify_cursor(cursor) yield cursor + # This should really not be necessary, because the cursor context manager will + # be closing the cursor on __exit__ anyway. But just in case. + dejsonify_cursor(cursor) + + +def json_fetch_all_as_dict(cursor): + """ + Iterates over a result set and converts each row to a dictionary. + The cursor passed in is assumed to have just executed a raw Postgresql query. + If the cursor's columns include any with the jsonb type, the process includes + examining every value from those columns. If the value is a string, a json.loads() + is attempted on the value, because in Django 3.1.1 and later, this is not + handled automatically for raw sql as it was before. There is no compatibility + issue running with older Django versions because if the value is not a string, + (e.g. it has already been converted to a list or dict), the loads() is skipped. + Note that JSON decoding errors are ignored (and the original result value is provided) + because it is possible that the query involved an actual json query, say on a single + string property of the underlying column data. In that case, the column type is + still jsonb, but the result value is a string as it should be. This ignoring of + errors is the same logic used in json handling in Django's from_db_value() method. + + :return: A list of dictionaries where each row is a dictionary + :rtype: list of dict + """ + + colnames = [col.name for col in cursor.description] + coltypes = [col.type_code for col in cursor.description] + # Identify any jsonb columns in the query, by column index + jsonbcols = [i for i, x in enumerate(coltypes) if x == JSONB_OID] + + # Optimize with a simple comprehension if we know there are no jsonb columns to handle. + if not jsonbcols: + return [ + dict(zip(colnames, row)) + for row in cursor.fetchall() + ] + + # If there are jsonb columns, intercept the result rows and run a json.loads() on any jsonb + # columns that are presenting as strings. + # In Django 3.1.0 they would already be a json type (e.g. dict or list) but in Django 3.1.1 it changes + # and raw sql queries return strings for jsonb columns. + # https://docs.djangoproject.com/en/4.0/releases/3.1.1/ + results = [] + + for row in cursor.fetchall(): + rowvals = list(row) + for colindex in jsonbcols: + if type(rowvals[colindex]) is str: # need to check type to avoid attempting to jsonify a None + try: + rowvals[colindex] = json.loads(rowvals[colindex]) + # It is possible that we are selecting a sub-value from the json in the column. I.e. + # we got here because it IS a jsonb column, but what we selected is not json and will + # fail to parse. In that case, we already have the value we want in place. + except json.JSONDecodeError: + pass + results.append(dict(zip(colnames, rowvals))) + return results diff --git a/querybuilder/query.py b/querybuilder/query.py index 7dfb681..216764a 100644 --- a/querybuilder/query.py +++ b/querybuilder/query.py @@ -9,10 +9,11 @@ get_model = apps.get_model import six - from querybuilder.fields import FieldFactory, CountField, MaxField, MinField, SumField, AvgField from querybuilder.helpers import set_value_for_keypath, copy_instance from querybuilder.tables import TableFactory, ModelTable, QueryTable +from querybuilder.cursor import json_fetch_all_as_dict + SERIAL_DTYPES = ['serial', 'bigserial'] @@ -640,7 +641,11 @@ def get_cursor(self): :rtype: :class:`CursorDebugWrapper ` :returns: A database cursor """ - return self.connection.cursor() + cursor = self.connection.cursor() + # Do not set up the cursor in psycopg2 to run json.loads on jsonb columns here. Do it + # right before we run a select, and then set it back after that. + # jsonify_cursor(cursor) + return cursor def from_table(self, table=None, fields='*', schema=None, **kwargs): """ @@ -1936,11 +1941,7 @@ def _fetch_all_as_dict(self, cursor): :return: A list of dictionaries where each row is a dictionary :rtype: list of dict """ - desc = cursor.description - return [ - dict(zip([col[0] for col in desc], row)) - for row in cursor.fetchall() - ] + return json_fetch_all_as_dict(cursor) class QueryWindow(Query): diff --git a/querybuilder/tables.py b/querybuilder/tables.py index 158fdd7..37e60f5 100644 --- a/querybuilder/tables.py +++ b/querybuilder/tables.py @@ -1,7 +1,6 @@ -import abc +from abc import ABCMeta from django.db.models.base import ModelBase -from six import string_types, with_metaclass import querybuilder from querybuilder.fields import FieldFactory @@ -31,7 +30,7 @@ def __new__(cls, table, *args, **kwargs): kwargs.update(alias=list(table.keys())[0]) table = list(table.values())[0] table_type = type(table) - if isinstance(table, string_types): + if isinstance(table, str): return SimpleTable(table, **kwargs) elif issubclass(table_type, ModelBase): return ModelTable(table, **kwargs) @@ -44,7 +43,7 @@ def __new__(cls, table, *args, **kwargs): return None -class Table(with_metaclass(abc.ABCMeta, object)): +class Table(object, metaclass=ABCMeta): """ Abstract table class that all table types extend. @@ -259,7 +258,7 @@ def add_fields(self, fields): or ``Field`` instance :type fields: str or tuple or list of str or list of Field or :class:`Field ` """ - if isinstance(fields, string_types): + if isinstance(fields, str): fields = [fields] elif type(fields) is tuple: fields = list(fields) diff --git a/querybuilder/tests/__init__.py b/querybuilder/tests/__init__.py index 9e04634..e69de29 100644 --- a/querybuilder/tests/__init__.py +++ b/querybuilder/tests/__init__.py @@ -1 +0,0 @@ -default_app_config = 'querybuilder.tests.apps.QuerybuilderTestConfig' diff --git a/querybuilder/tests/json_tests.py b/querybuilder/tests/json_tests.py index 1638665..60af073 100644 --- a/querybuilder/tests/json_tests.py +++ b/querybuilder/tests/json_tests.py @@ -1,5 +1,4 @@ import unittest -from django import VERSION from django.test.utils import override_settings from querybuilder.fields import JsonField from querybuilder.query import Query, JsonQueryset @@ -47,11 +46,7 @@ def test_one(self): ) ) - # Django 3.1 changes the raw queryset behavior so querybuilder isn't going to change that behavior - if self.is_31_or_above(): - self.assertEqual(query.select(), [{'my_two_alias': '"two"'}]) - else: - self.assertEqual(query.select(), [{'my_two_alias': 'two'}]) + self.assertEqual(query.select(), [{'my_two_alias': 'two'}]) query = Query().from_table(MetricRecord, fields=[one_field]).where(**{ one_field.get_where_key(): '1' @@ -64,11 +59,7 @@ def test_one(self): ) ) - # Django 3.1 changes the raw queryset behavior so querybuilder isn't going to change that behavior - if self.is_31_or_above(): - self.assertEqual(query.select(), [{'my_one_alias': '1'}]) - else: - self.assertEqual(query.select(), [{'my_one_alias': 1}]) + self.assertEqual(query.select(), [{'my_one_alias': 1}]) query = Query().from_table(MetricRecord, fields=[one_field]).where(**{ one_field.get_where_key(): '2' @@ -82,12 +73,14 @@ def test_one(self): ) self.assertEqual(query.select(), []) - def is_31_or_above(self): - if VERSION[0] == 3 and VERSION[1] >= 1: - return True - elif VERSION[0] > 3: - return True - return False + # Currently unused (but maybe again sometime) function to check django version for test results + # from django import VERSION + # def is_31_or_above(self): + # if VERSION[0] == 3 and VERSION[1] >= 1: + # return True + # elif VERSION[0] > 3: + # return True + # return False @override_settings(DEBUG=True) diff --git a/querybuilder/tests/migrations/0001_initial.py b/querybuilder/tests/migrations/0001_initial.py index 7fb1cee..96a0100 100644 --- a/querybuilder/tests/migrations/0001_initial.py +++ b/querybuilder/tests/migrations/0001_initial.py @@ -1,18 +1,9 @@ # -*- coding: utf-8 -*- # Generated by Django 1.9.1 on 2016-02-12 19:03 -from __future__ import unicode_literals from querybuilder.tests.utils import get_postgres_version # These migrations should only be run during tests and not in your installed app. -try: - if get_postgres_version() < (9, 4): - raise ImportError('Invalid Postgres version') - import django.contrib.postgres.fields.jsonb - json_field = django.contrib.postgres.fields.jsonb.JSONField() -except ImportError: - import jsonfield.fields - json_field = jsonfield.fields.JSONField() from django.db import migrations, models import django.db.models.deletion @@ -38,7 +29,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('other_value', models.FloatField(default=0)), - ('data', json_field), + ('data', models.JSONField()), ], ), migrations.CreateModel( @@ -63,7 +54,7 @@ class Migration(migrations.Migration): ('field5', models.CharField(default=None, max_length=16, null=True)), ('field6', models.CharField(max_length=16)), ('field7', models.CharField(max_length=16)), - ('field8', json_field), + ('field8', models.JSONField()), ('custom_field_name', models.CharField(max_length=16, null=True, default='foo', db_column='actual_db_column_name')), ], ), diff --git a/querybuilder/tests/models.py b/querybuilder/tests/models.py index cc87117..484127d 100644 --- a/querybuilder/tests/models.py +++ b/querybuilder/tests/models.py @@ -54,7 +54,7 @@ class Uniques(models.Model): field5 = models.CharField(max_length=16, null=True, default=None) field6 = models.CharField(max_length=16) field7 = models.CharField(max_length=16) - field8 = JSONField(default={}) + field8 = JSONField(default=dict) custom_field_name = models.CharField( max_length=16, null=True, default='foo', db_column='actual_db_column_name' ) diff --git a/querybuilder/urls.py b/querybuilder/urls.py index 04452e5..1a43da5 100644 --- a/querybuilder/urls.py +++ b/querybuilder/urls.py @@ -1,6 +1 @@ -from django.conf.urls import patterns # pragma: no cover - - -urlpatterns = patterns( - '', -) # pragma: no cover +urlpatterns = [] # pragma: no cover diff --git a/querybuilder/version.py b/querybuilder/version.py index da4039b..7f5601d 100644 --- a/querybuilder/version.py +++ b/querybuilder/version.py @@ -1 +1 @@ -__version__ = '3.0.2' +__version__ = '3.1.0' diff --git a/requirements/requirements-testing.txt b/requirements/requirements-testing.txt index 6d9a66d..1a13ee2 100644 --- a/requirements/requirements-testing.txt +++ b/requirements/requirements-testing.txt @@ -4,7 +4,5 @@ six psycopg2 django-nose>=1.4 django-dynamic-fixture -jsonfield==0.9.20 -mock coverage flake8 diff --git a/requirements/requirements.txt b/requirements/requirements.txt index a9cf3ca..6c29a1f 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -1,4 +1,3 @@ -Django>=2.2 +Django>=3.2 pytz>=2015.6 fleming>=0.6.0 -six diff --git a/settings.py b/settings.py index 35c08c7..d3400ec 100644 --- a/settings.py +++ b/settings.py @@ -51,7 +51,6 @@ def configure_settings(): settings.configure( TEST_RUNNER='django_nose.NoseTestSuiteRunner', NOSE_ARGS=['--nocapture', '--nologcapture', '--verbosity=1'], - MIDDLEWARE_CLASSES=(), DATABASES={ 'default': db_config, 'mock-second-database': db_config2, @@ -65,4 +64,5 @@ def configure_settings(): ROOT_URLCONF='querybuilder.urls', TIME_ZONE='UTC', USE_TZ=False, + DEFAULT_AUTO_FIELD='django.db.models.AutoField', ) diff --git a/setup.cfg b/setup.cfg index 60aaa3e..363b3fe 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [flake8] max-line-length = 120 -exclude = docs,venv,env,*.egg,migrations,south_migrations +exclude = docs,venv,env,*.egg,migrations,south_migrations,tmp max-complexity = 21 ignore = E402