tortoise / pypika-tortoise

Forked from pypika and streamline just for tortoise-orm
Apache License 2.0
7 stars 10 forks source link

fix MySQLValueWrapper #9

Closed KanagawaNezumi closed 1 year ago

KanagawaNezumi commented 1 year ago

When using MySQL's JSON field, the bulk_update method fails to properly escape non-ASCII characters.

long2ice commented 1 year ago

Thanks! Did you test that?

KanagawaNezumi commented 1 year ago

I haven't tested it thoroughly, just did some basic testing to ensure that it can update non-ascii characters properly and won't crash when the string includes backslashes.

# ...

class Tournament(Model):
    id = fields.IntField(pk=True)
    name = fields.TextField()
    metadata = fields.JSONField()

# ...

async def main():
    records = [Tournament(name=f"{i}", metadata={"title": f"锦标赛{i}"}) for i in range(3)]
    await Tournament.bulk_create(records)

    records = await Tournament.all()
    await Tournament.bulk_update(records, ["name", "metadata"])

    for record in await Tournament.all():
        print(record.metadata)

        # --- before ---

        # {'title': 'u9526u6807u8d5b0'}
        # {'title': 'u9526u6807u8d5b1'}
        # {'title': 'u9526u6807u8d5b2'}

        # --- after ---

        # {'title': '锦标赛0'}
        # {'title': '锦标赛1'}
        # {'title': '锦标赛2'}
KanagawaNezumi commented 1 year ago

I forked the tortoise-orm repo, but it didn't include any test cases for storing non-ASCII strings in the MySQL JSON field. So I added a test case and triggered CI, which failed initially. Then I updated the pypika-tortoise dependency to my latest commit and triggered CI again. This time it succeeded.

    async def test_bulk_update_json_value_non_ascii_string(self):
        objs = [
            await JSONFields.create(data={}),
            await JSONFields.create(data={}),
        ]
        objs[0].data = ["你好", "世界"]
        objs[1].data = {"标题": "测试"}
        rows_affected = await JSONFields.bulk_update(objs, fields=["data"])
        self.assertEqual(rows_affected, 2)
        self.assertEqual((await JSONFields.get(pk=objs[0].pk)).data, objs[0].data)
        self.assertEqual((await JSONFields.get(pk=objs[1].pk)).data, objs[1].data)

# PYTHONDEVMODE=1 TORTOISE_TEST_DB="***127.0.0.1:3306/test_\{\}?storage_engine=MYISAM" pytest -n auto --cov=tortoise --tb=native -q --cov-append --cov-report=
# ...
# AssertionError: Lists differ: ['u4f60u597d', 'u4e16u754c'] != ['你好', '世界']
# ...
# FAILED tests/test_update.py::TestUpdate::test_bulk_update_json_value_non_ascii_string - AssertionError: Lists differ: ['u4f60u597d', 'u4e16u754c'] != ['你好', '世界']

image

Is there anything else that needs to be tested?

long2ice commented 1 year ago

Thanks!