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

[BUG] Inconsistent insertion behaviour depending on method used #1093

Open
tom-whitehead opened this issue Dec 20, 2024 · 1 comment
Open

Comments

@tom-whitehead
Copy link

tom-whitehead commented Dec 20, 2024

Describe the bug
Transient fields and null fields get inserted into the database differently based on the insertion method used. See my example below. I'm new to Beanie, so apologies if I'm missing something obvious.

To Reproduce
Given the following example data model:

class MyDoc(Document):
    creation_date: datetime = Field(default_factory=datetime.now)
    opt_field: Optional[List[str]] = Field(default=None)
    transient_field: Optional[List[str]] = Field(
        exclude=True,
        default=None,
        description="This field should not be persisted to the DB!"
    )

    class Settings:
        name = "my_collection"
        keep_nulls = False

    @before_event(Insert)
    def remove_transient(self) -> None:
        self.transient_fields = None

Running these three insertions:

# 1 - insert one
doc1 = MyDoc(transient_field=["one"])
await doc1.insert()

# 2 - insert many
doc2 = MyDoc(transient_field=["two"])
await MyDoc.insert_many([doc2])

# 3 - pymongo operation
doc3 = MyDoc(transient_field=["three"])
ops = [InsertOne(doc3.model_dump())]
await MyDoc.get_motor_collection().bulk_write(ops)

Produces three inconsistent results:

  1. Inserts as I would expect, excluding the transient field from the insertion and also not inserting nulls
  2. Nulls are excluded as I would expect, however the transient field is inserted in spite of the pre-event hook (I assume the exclude in the pydantic field only applies when serialising to a dictionary with model_dump).
  3. The pymongo operation does nothing right - nulls are inserted, the transient field is also inserted (and for good measure an "id": null field is inserted in addition to the default mongo "_id" field - I think i'm supposed to specify this gets excluded in model_dump).

Expected behavior

  • I expect the transient field to be excluded from the insert, either due to the pydantic field specifying the exclusion or the pre-event hook.
  • I expect nulls to not be inserted because I have specified this in the settings.
@tom-whitehead
Copy link
Author

Having looked at the source code a bit, the more generalised issue seems to be that actions are not called for the Document class methods like insert_many, insert_one and replace_many, where they are for other instance methods via the wrap_with_actions decorator. Is this a deliberate design choice?

If not, couldn't we use a decorator like the following to wrap these class methods to ensure the actions get run. Untested and type hints removed, so take with a grain of salt.

Code:
def wrap_classmethod_with_actions(event_type):
    def decorator(f):
        @wraps(f)
        async def wrapper(cls, target, *args, skip_actions, **kwargs):
            if skip_actions is None:
                skip_actions = []

            if isinstance(target, Document):
                await ActionRegistry.run_actions(
                    target,
                    event_type=event_type,
                    action_direction=ActionDirections.BEFORE,
                    exclude=skip_actions,
                )
            elif isinstance(target, Iterable):
                await asyncio.gather(
                    *[
                        ActionRegistry.run_actions(
                            document,
                            event_type=event_type,
                            action_direction=ActionDirections.BEFORE,
                            exclude=skip_actions,
                        )
                        for document in target
                    ]
                )

            result = await f(
                cls,
                documents,
                *args,
                skip_actions=skip_actions,
                **kwargs,
            )

            if isinstance(target, Document):
                await ActionRegistry.run_actions(
                    target,
                    event_type=event_type,
                    action_direction=ActionDirections.AFTER,
                    exclude=skip_actions,
                )
            elif isinstance(target, Iterable):
                await asyncio.gather(
                    *[
                        ActionRegistry.run_actions(
                            document,
                            event_type=event_type,
                            action_direction=ActionDirections.AFTER,
                            exclude=skip_actions,
                        )
                        for document in target
                    ]
                )

            return result

        return wrapper

    return decorator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant