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

DX: Remove db-diff dependency #288

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
Unreleased
Remove db-diff dependency

2023-10-30
Add support for Python 3.12
Add support for Django 5.0
Expand Down
4 changes: 4 additions & 0 deletions src/cities_light/apps.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
from django.apps import AppConfig
from django.core.serializers import register_serializer


class CitiesLightConfig(AppConfig):
default_auto_field = 'django.db.models.AutoField'
name = 'cities_light'

def ready(self):
register_serializer('sorted_json', 'cities_light.serializers.json')
Comment on lines +9 to +10
Copy link
Author

@pfouque pfouque Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Register an additional serializer where the dbdiff package was overriding the default json serializer.

1 change: 1 addition & 0 deletions src/cities_light/serializers/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Serializers with predictible (ordered) output."""
83 changes: 83 additions & 0 deletions src/cities_light/serializers/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
"""Shared code for serializers."""

import collections
import datetime
import decimal


class BaseSerializerMixin(object):
"""Serializer mixin for predictible and cross-db dumps."""

@classmethod
def recursive_dict_sort(cls, data):
"""
Return a recursive OrderedDict for a dict.

Django's default model-to-dict logic - implemented in
django.core.serializers.python.Serializer.get_dump_object() - returns a
dict, this app registers a slightly modified version of the default
json serializer which returns OrderedDicts instead.
"""
ordered_data = collections.OrderedDict(sorted(data.items()))

for key, value in ordered_data.items():
if isinstance(value, dict):
ordered_data[key] = cls.recursive_dict_sort(value)

return ordered_data

@classmethod
def remove_microseconds(cls, data):
"""
Strip microseconds from datetimes for mysql.

MySQL doesn't have microseconds in datetimes, so dbdiff's serializer
removes microseconds from datetimes so that fixtures are cross-database
compatible which make them usable for cross-database testing.
"""
for key, value in data['fields'].items():
if not isinstance(value, datetime.datetime):
continue

data['fields'][key] = datetime.datetime(
year=value.year,
month=value.month,
day=value.day,
hour=value.hour,
minute=value.minute,
second=value.second,
tzinfo=value.tzinfo
)

@classmethod
def normalize_decimals(cls, data):
"""
Strip trailing zeros for constitency.

In addition, dbdiff serialization forces Decimal normalization, because
trailing zeros could happen in inconsistent ways.
"""
for key, value in data['fields'].items():
if not isinstance(value, decimal.Decimal):
continue

if value % 1 == 0:
data['fields'][key] = int(value)
else:
print(type(value), " => ", type(value.normalize()))
print(value, " => ", value.normalize())
# data['fields'][key] = value
data['fields'][key] = value.normalize()

def get_dump_object(self, obj):
"""
Actual method used by Django serializers to dump dicts.

By overridding this method, we're able to run our various
data dump predictability methods.
"""
data = super(BaseSerializerMixin, self).get_dump_object(obj)
self.remove_microseconds(data)
self.normalize_decimals(data)
data = self.recursive_dict_sort(data)
return data
15 changes: 15 additions & 0 deletions src/cities_light/serializers/json.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"""Django JSON Serializer override."""

from django.core.serializers import json as upstream

from .base import BaseSerializerMixin


__all__ = ('Serializer', 'Deserializer')


class Serializer(BaseSerializerMixin, upstream.Serializer):
"""Sorted dict JSON serializer."""


Deserializer = upstream.Deserializer
22 changes: 22 additions & 0 deletions src/cities_light/tests/base.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
"""."""
import json
import os
from unittest import mock

from django import test
from django.core import management
from django.conf import settings

from io import StringIO

class FixtureDir:
"""Helper class to construct fixture paths."""
Expand Down Expand Up @@ -80,3 +82,23 @@ def _patch(setting, *values):
management.call_command('cities_light', progress=True,
force_import_all=True,
**options)

def export_data(self, app_label=None) -> bytes:
out = StringIO()
management.call_command(
"dumpdata",
app_label or "cities_light",
format="sorted_json",
natural_foreign=True,
indent=4,
stdout=out
)
return out.getvalue()

def assertNoDiff(self, fixture_path, app_label=None):
"""Assert that dumped data matches fixture."""

with open(fixture_path) as f:
self.assertListEqual(
json.loads(f.read()), json.loads(self.export_data(app_label))
)
15 changes: 15 additions & 0 deletions src/cities_light/tests/fixtures/update/noinsert.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@
"model": "cities_light.region",
"pk": 1
},
{
"fields": {
"alternate_names": "Юргинский район",
"country": [2017370],
"display_name": "Yurginskiy Rayon, Russia",
"geoname_code": "1485714",
"geoname_id": 1485714,
"name": "Yurginskiy Rayon",
"name_ascii": "Yurginskiy Rayon",
"region": [1503900],
"slug": "yurginskiy-rayon"
},
"model": "cities_light.subregion",
"pk": 1
},
Comment on lines +32 to +46
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this fixture wasn't up to date :/

{
"fields": {
"alternate_names": "\u041a\u0435\u043c\u0435\u0440\u043e\u0432\u043e",
Expand Down
39 changes: 33 additions & 6 deletions src/cities_light/tests/test_fixtures.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
"""Test for cities_light_fixtures management command."""
import bz2
import json
import os
from unittest import mock

from django import test
from django.core.management import call_command
from django.core.management.base import CommandError

from dbdiff.fixture import Fixture
from cities_light.settings import DATA_DIR, FIXTURES_BASE_URL
from cities_light.management.commands.cities_light_fixtures import Command
from cities_light.downloader import Downloader
from cities_light.models import City
from .base import FixtureDir
from .base import TestImportBase, FixtureDir


class TestCitiesLigthFixtures(test.TransactionTestCase):
class TestCitiesLigthFixtures(TestImportBase):
"""Tests for cities_light_fixtures management command."""

def test_dump_fixtures(self):
Expand All @@ -36,13 +36,34 @@ def test_dump_fixtures(self):
mock_func.assert_any_call('cities_light.SubRegion', cmd.subregion_path)
mock_func.assert_any_call('cities_light.City', cmd.city_path)

# def export_data(self, app_label=None) -> bytes:
# out = StringIO()
# management.call_command(
# "dumpdata",
# app_label or "cities_light",
# format="sorted_json",
# natural_foreign=True,
# indent=4,
# stdout=out
# )
# return out.getvalue()

def assertNoDiff(self, fixture_path, app_label=None):
"""Assert that dumped data matches fixture."""

with open(fixture_path) as f:
self.assertListEqual(
json.loads(f.read()), json.loads(self.export_data(app_label))
)

def test_dump_fixture(self):
"""
Test dump_fixture calls dumpdata management command
and tries to save it to file."""
# Load test data
destination = FixtureDir('import').get_file_path('angouleme.json')
call_command('loaddata', destination)

# Dump
try:
fixture_path = os.path.join(os.path.dirname(__file__), "fixtures", "test_dump_fixture.json")
Expand All @@ -52,7 +73,13 @@ def test_dump_fixture(self):
data = bzfile.read()
with open(fixture_path, mode='wb') as file:
file.write(data)
Fixture(fixture_path, models=[City]).assertNoDiff()

# with open(destination) as f2, open(fixture_path) as f:
# assert f.read() == f2.read()
# self.assertListEqual(json.loads(f.read()), json.loads(f2.read()))
# assert destination == fixture_path.read()
self.assertNoDiff(fixture_path, 'cities_light.City')

finally:
if os.path.exists(fixture_path):
os.remove(fixture_path)
Expand Down Expand Up @@ -86,15 +113,15 @@ def test_load_fixtures(self):
mock_func.assert_any_call(
cmd.city_url, cmd.city_path, force=True)

def test_load_fixture(self):
def test_load_fixture_result(self):
"""Test loaded fixture matches database content."""
destination = FixtureDir('import').get_file_path('angouleme.json')
with mock.patch.object(Downloader, 'download') as mock_func:
cmd = Command()
cmd.load_fixture(source='/abcdefg.json',
destination=destination,
force=True)
Fixture(destination).assertNoDiff()
self.assertNoDiff(destination)
mock_func.assert_called_with(source='/abcdefg.json',
destination=destination,
force=True)
Expand Down
12 changes: 8 additions & 4 deletions src/cities_light/tests/test_import.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import glob
import os

from dbdiff.fixture import Fixture
from django.core import management
from django.core.management.commands import dumpdata

from .base import TestImportBase, FixtureDir
from ..settings import DATA_DIR

Expand All @@ -20,7 +22,8 @@ def test_single_city(self):
'angouleme_city',
'angouleme_translations'
)
Fixture(fixture_dir.get_file_path('angouleme.json')).assertNoDiff()

self.assertNoDiff(fixture_dir.get_file_path("angouleme.json"))

def test_single_city_zip(self):
"""Load single city."""
Expand All @@ -38,7 +41,7 @@ def test_single_city_zip(self):
'angouleme_translations',
file_type="zip"
)
Fixture(FixtureDir('import').get_file_path('angouleme.json')).assertNoDiff()
self.assertNoDiff(FixtureDir('import').get_file_path("angouleme.json"))

def test_city_wrong_timezone(self):
"""Load single city with wrong timezone."""
Expand All @@ -51,7 +54,8 @@ def test_city_wrong_timezone(self):
'angouleme_city_wtz',
'angouleme_translations'
)
Fixture(fixture_dir.get_file_path('angouleme_wtz.json')).assertNoDiff()

self.assertNoDiff(FixtureDir('import').get_file_path("angouleme_wtz.json"))

from ..loading import get_cities_model
city_model = get_cities_model('City')
Expand Down
44 changes: 20 additions & 24 deletions src/cities_light/tests/test_migrations.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
import unittest
from io import StringIO

from django import test
from django.apps import apps
from django.db.migrations.autodetector import MigrationAutodetector
from django.db.migrations.loader import MigrationLoader
from django.db.migrations.questioner import (
InteractiveMigrationQuestioner, )
from django.db.migrations.state import ProjectState
from django.core.management import call_command
from django.test.utils import override_settings


@override_settings(
MIGRATION_MODULES={
"cities_light": "cities_light.migrations",
},
)
class TestNoMigrationLeft(test.TestCase):
@unittest.skip("TODO: make the test pass")
def test_no_migration_left(self):
loader = MigrationLoader(None, ignore_no_migrations=True)
conflicts = loader.detect_conflicts()
app_labels = ['cities_light']

autodetector = MigrationAutodetector(
loader.project_state(),
ProjectState.from_apps(apps),
InteractiveMigrationQuestioner(specified_apps=app_labels, dry_run=True),
)

changes = autodetector.changes(
graph=loader.graph,
trim_to_apps=app_labels or None,
convert_apps=app_labels or None,
)

assert 'cities_light' not in changes
out = StringIO()
try:
call_command(
"makemigrations",
"cities_light",
"--dry-run",
"--check",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call the real makemigrations command instead of the MigrationLoader.
As it's kind of a public interface it should be more stable (it's also shorter and easier to test out of the tests)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, and some other modifications.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I will try to move forward and get a stable state.
That would make this package more independent and easier to maintain

stdout=out,
stderr=StringIO(),
)
except SystemExit: # pragma: no cover
raise AssertionError("Pending migrations:\n" + out.getvalue()) from None
Loading
Loading