From 9aa6e46ae3979ee77f4b79ccf37febc517c0acaa Mon Sep 17 00:00:00 2001 From: Daniel Townsend Date: Tue, 28 May 2024 23:49:47 +0100 Subject: [PATCH] 1001 Fix `load_json` on joined tables (#1002) * fix `load_json` on joined tables * add tests * reduce repetition in tests * update tests * add migration * format migration file * update more tests --- piccolo/query/base.py | 6 +- .../commands/test_forwards_backwards.py | 3 + tests/apps/shell/commands/test_run.py | 1 + tests/conf/test_apps.py | 6 + tests/conftest.py | 1 + .../music_2024_05_28t23_15_41_018844.py | 84 ++++++++++++ tests/example_apps/music/tables.py | 10 ++ tests/table/test_output.py | 124 +++++++++++++----- 8 files changed, 194 insertions(+), 41 deletions(-) create mode 100644 tests/example_apps/music/piccolo_migrations/music_2024_05_28t23_15_41_018844.py diff --git a/piccolo/query/base.py b/piccolo/query/base.py index b10d42ee0..c169fde0e 100644 --- a/piccolo/query/base.py +++ b/piccolo/query/base.py @@ -87,13 +87,9 @@ async def _process_results(self, results) -> QueryResponseType: for column in json_columns: if column._alias is not None: json_column_names.append(column._alias) - elif column.json_operator is not None: - json_column_names.append(column._meta.name) elif len(column._meta.call_chain) > 0: json_column_names.append( - column.get_select_string( - engine_type=column._meta.engine_type - ) + column._meta.get_default_alias().replace("$", ".") ) else: json_column_names.append(column._meta.name) diff --git a/tests/apps/migrations/commands/test_forwards_backwards.py b/tests/apps/migrations/commands/test_forwards_backwards.py index d70d7d506..c3c2c6592 100644 --- a/tests/apps/migrations/commands/test_forwards_backwards.py +++ b/tests/apps/migrations/commands/test_forwards_backwards.py @@ -13,6 +13,7 @@ from tests.example_apps.music.tables import ( Band, Concert, + Instrument, Manager, Poster, RecordingStudio, @@ -33,6 +34,7 @@ Poster, Shirt, RecordingStudio, + Instrument, ] @@ -211,6 +213,7 @@ def test_forwards_fake(self): "2021-07-25T22:38:48:009306", "2021-09-06T13:58:23:024723", "2021-11-13T14:01:46:114725", + "2024-05-28T23:15:41:018844", ], ) diff --git a/tests/apps/shell/commands/test_run.py b/tests/apps/shell/commands/test_run.py index 8594f7101..dd5122b41 100644 --- a/tests/apps/shell/commands/test_run.py +++ b/tests/apps/shell/commands/test_run.py @@ -20,6 +20,7 @@ def test_run(self, print_: MagicMock, start_ipython_shell: MagicMock): call("Importing music tables:"), call("- Band"), call("- Concert"), + call("- Instrument"), call("- Manager"), call("- Poster"), call("- RecordingStudio"), diff --git a/tests/conf/test_apps.py b/tests/conf/test_apps.py index 907064e1f..44b2d4a4a 100644 --- a/tests/conf/test_apps.py +++ b/tests/conf/test_apps.py @@ -9,6 +9,7 @@ from tests.example_apps.music.tables import ( Band, Concert, + Instrument, Manager, Poster, RecordingStudio, @@ -113,6 +114,7 @@ def test_table_finder(self): [ "Band", "Concert", + "Instrument", "Manager", "Poster", "RecordingStudio", @@ -139,6 +141,7 @@ def test_table_finder_coercion(self): [ "Band", "Concert", + "Instrument", "Manager", "Poster", "RecordingStudio", @@ -182,6 +185,7 @@ def test_exclude_tags(self): [ "Band", "Concert", + "Instrument", "Manager", "RecordingStudio", "Shirt", @@ -228,6 +232,7 @@ def test_get_table_classes(self): [ Band, Concert, + Instrument, Manager, MegaTable, Poster, @@ -247,6 +252,7 @@ def test_get_table_classes(self): [ Band, Concert, + Instrument, Manager, Poster, RecordingStudio, diff --git a/tests/conftest.py b/tests/conftest.py index 457c4dae7..8411ebc38 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,7 @@ async def drop_tables(): "musician", "my_table", "recording_studio", + "instrument", "shirt", "instrument", "mega_table", diff --git a/tests/example_apps/music/piccolo_migrations/music_2024_05_28t23_15_41_018844.py b/tests/example_apps/music/piccolo_migrations/music_2024_05_28t23_15_41_018844.py new file mode 100644 index 000000000..a1428eead --- /dev/null +++ b/tests/example_apps/music/piccolo_migrations/music_2024_05_28t23_15_41_018844.py @@ -0,0 +1,84 @@ +from piccolo.apps.migrations.auto.migration_manager import MigrationManager +from piccolo.columns.base import OnDelete, OnUpdate +from piccolo.columns.column_types import ForeignKey, Serial, Varchar +from piccolo.columns.indexes import IndexMethod +from piccolo.table import Table + + +class RecordingStudio(Table, tablename="recording_studio", schema=None): + id = Serial( + null=False, + primary_key=True, + unique=False, + index=False, + index_method=IndexMethod.btree, + choices=None, + db_column_name="id", + secret=False, + ) + + +ID = "2024-05-28T23:15:41:018844" +VERSION = "1.5.1" +DESCRIPTION = "" + + +async def forwards(): + manager = MigrationManager( + migration_id=ID, app_name="music", description=DESCRIPTION + ) + + manager.add_table( + class_name="Instrument", + tablename="instrument", + schema=None, + columns=None, + ) + + manager.add_column( + table_class_name="Instrument", + tablename="instrument", + column_name="name", + db_column_name="name", + column_class_name="Varchar", + column_class=Varchar, + params={ + "length": 255, + "default": "", + "null": False, + "primary_key": False, + "unique": False, + "index": False, + "index_method": IndexMethod.btree, + "choices": None, + "db_column_name": None, + "secret": False, + }, + schema=None, + ) + + manager.add_column( + table_class_name="Instrument", + tablename="instrument", + column_name="recording_studio", + db_column_name="recording_studio", + column_class_name="ForeignKey", + column_class=ForeignKey, + params={ + "references": RecordingStudio, + "on_delete": OnDelete.cascade, + "on_update": OnUpdate.cascade, + "target_column": None, + "null": True, + "primary_key": False, + "unique": False, + "index": False, + "index_method": IndexMethod.btree, + "choices": None, + "db_column_name": None, + "secret": False, + }, + schema=None, + ) + + return manager diff --git a/tests/example_apps/music/tables.py b/tests/example_apps/music/tables.py index dff416c2f..9e0cdbb39 100644 --- a/tests/example_apps/music/tables.py +++ b/tests/example_apps/music/tables.py @@ -115,3 +115,13 @@ class RecordingStudio(Table): id: Serial facilities = JSON() facilities_b = JSONB() + + +class Instrument(Table): + """ + Used for testing foreign keys to a table with a JSON column. + """ + + id: Serial + name = Varchar() + recording_studio = ForeignKey(RecordingStudio) diff --git a/tests/table/test_output.py b/tests/table/test_output.py index 8298396fd..ecfc997bc 100644 --- a/tests/table/test_output.py +++ b/tests/table/test_output.py @@ -1,8 +1,9 @@ import json from unittest import TestCase -from tests.base import DBTestCase, engine_is -from tests.example_apps.music.tables import Band, RecordingStudio +from piccolo.table import create_db_tables_sync, drop_db_tables_sync +from tests.base import DBTestCase +from tests.example_apps.music.tables import Band, Instrument, RecordingStudio class TestOutputList(DBTestCase): @@ -32,51 +33,102 @@ def test_output_as_json(self): class TestOutputLoadJSON(TestCase): + tables = [RecordingStudio, Instrument] + json = {"a": 123} + def setUp(self): - RecordingStudio.create_table().run_sync() + create_db_tables_sync(*self.tables) + + recording_studio = RecordingStudio( + { + RecordingStudio.facilities: self.json, + RecordingStudio.facilities_b: self.json, + } + ) + recording_studio.save().run_sync() + + instrument = Instrument( + { + Instrument.recording_studio: recording_studio, + Instrument.name: "Piccolo", + } + ) + instrument.save().run_sync() def tearDown(self): - RecordingStudio.alter().drop_table().run_sync() + drop_db_tables_sync(*self.tables) def test_select(self): - json = {"a": 123} - - RecordingStudio(facilities=json, facilities_b=json).save().run_sync() - - results = RecordingStudio.select().output(load_json=True).run_sync() - - if engine_is("cockroach"): - self.assertEqual( - results, - [ - { - "id": results[0]["id"], - "facilities": {"a": 123}, - "facilities_b": {"a": 123}, - } - ], + results = ( + RecordingStudio.select( + RecordingStudio.facilities, RecordingStudio.facilities_b ) - else: - self.assertEqual( - results, - [ - { - "id": 1, - "facilities": {"a": 123}, - "facilities_b": {"a": 123}, - } - ], + .output(load_json=True) + .run_sync() + ) + + self.assertEqual( + results, + [ + { + "facilities": self.json, + "facilities_b": self.json, + } + ], + ) + + def test_join(self): + """ + Make sure it works correctly when the JSON column is on a joined table. + + https://github.com/piccolo-orm/piccolo/issues/1001 + + """ + results = ( + Instrument.select( + Instrument.name, + Instrument.recording_studio._.facilities, ) + .output(load_json=True) + .run_sync() + ) - def test_objects(self): - json = {"a": 123} + self.assertEqual( + results, + [ + { + "name": "Piccolo", + "recording_studio.facilities": self.json, + } + ], + ) - RecordingStudio(facilities=json, facilities_b=json).save().run_sync() + def test_join_with_alias(self): + results = ( + Instrument.select( + Instrument.name, + Instrument.recording_studio._.facilities.as_alias( + "facilities" + ), + ) + .output(load_json=True) + .run_sync() + ) - results = RecordingStudio.objects().output(load_json=True).run_sync() + self.assertEqual( + results, + [ + { + "name": "Piccolo", + "facilities": self.json, + } + ], + ) - self.assertEqual(results[0].facilities, json) - self.assertEqual(results[0].facilities_b, json) + def test_objects(self): + results = RecordingStudio.objects().output(load_json=True).run_sync() + self.assertEqual(results[0].facilities, self.json) + self.assertEqual(results[0].facilities_b, self.json) class TestOutputNested(DBTestCase):