plone / guillotina

Python AsyncIO data API to manage billions of resources
https://guillotina.readthedocs.io/en/latest/
Other
187 stars 51 forks source link

Transaction improvement #1054

Open Qiwn opened 3 years ago

Qiwn commented 3 years ago

Transaction only considers the objects in pg, it doesn't consider those objects are not stored in pg yet (they are in txn.deleted/added dict). It would be great if this kind of cases are contemplated:

async def test_txn_add_and_delete(container_requester):
    async with container_requester as requester:
        async with transaction(db=requester.db) as txn:
            root = await txn.get(ROOT_ID)
            container = await root.async_get("guillotina")
            await create_content_in_container(
                container, "Item", "foobar", check_security=False, __uuid__="foobar"
            )
            await container.async_del("foobar")
            obj = await container.async_get("foobar")  # should be deleted at this point
            assert obj is None

        async with transaction(db=requester.db) as txn:
            obj = await container.async_get("foobar")
            # object still exists
            assert obj is None

Another case: https://github.com/plone/guillotina/pull/1053

masipcat commented 3 years ago

@bloodbare @vangheem @jordic What do you think about this? IMO this is something interesting to implement in G7

jordic commented 3 years ago

From my point of view, this is cleary a bug, if we support "virtual transactions" we should do the correct way, and data within boundaries should be consistent!

bloodbare commented 3 years ago

Looks like a good idea to clean up the g transaction flow.

In your use case https://github.com/plone/guillotina/blob/master/guillotina/content.py#L358 is the responsible that at async_get gets None because its not on db, so fixing that async_get can retrieve from txn objects (we will need to store the parent and id on the txn virtual state) may be enought to fix this issue.

bloodbare commented 3 years ago

After the PR the main scope will be fixed. There is still an inconsistency on async_len, aync_keys, async_values .... all this functions will not be consistent with modified objects and may be the scope for a future cache refactor.