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

UniqueIdProperty makes get_or_create() function does not work as expected #788

Open
agnoam opened this issue Apr 4, 2024 · 2 comments
Open

Comments

@agnoam
Copy link

agnoam commented Apr 4, 2024

Expected Behavior (Mandatory)

This function is expected to find matching nodes by a filter object.
In case no nodes were found, The function should create a node.

Actual Behavior (Mandatory)

The function creates a duplicated node, even if the matching object returns a node (by manual check)

Simple Example

Declarations

class BaseNodeModel(StructuredNode):
    uid = UniqueIdProperty()
    created_at = DateTimeProperty(True)
    updated_at = DateTimeProperty(True)

class Person(BaseNodeModel):
    name = StringProperty(required=True)

Example

first_tim = Person.get_or_create({'name': 'Tim'})[0]
should_be_same_tim = Person.get_or_create({'name': 'Tim'})[0]

assert first_tim.uid == should_be_same_tim.uid

Specifications (Mandatory)

Versions

  • OS: Linux
  • Library: neomodel==5.2.1
@mariusconjeaud
Copy link
Collaborator

To extend on this, I did some testing like below ; I thought the inheritance might be the problem, but no, it is indeed the presence of a UniqueIdProperty() that you don't pass at creation time.

async def test_issue_788():
    class BaseNodeModel(AsyncStructuredNode):
        uid = UniqueIdProperty()
        created_at = DateTimeProperty(True)
        updated_at = DateTimeProperty(True)
        base_prop = StringProperty()

    class InheritedPerson(BaseNodeModel):
        name = StringProperty(required=True)

    base_tim = (await BaseNodeModel.get_or_create({"base_prop": "Tim"}))[0]
    should_be_same_base_tim = (await BaseNodeModel.get_or_create({"base_prop": "Tim"}))[
        0
    ]
    # This will pass if uid is commented out, but fail otherwise
    assert base_tim.element_id == should_be_same_base_tim.element_id

    first_tim = (await InheritedPerson.get_or_create({"name": "Tim"}))[0]
    should_be_same_tim = (await InheritedPerson.get_or_create({"name": "Tim"}))[0]
    
    # This will pass if uid is commented out, but fail otherwise
    assert first_tim.element_id == should_be_same_tim.element_id

Meanwhile, this test works fine ; the difference being that on get_or_create, we do pass the uid explicitly, so it looks like that is the key.

async def test_unique_id_property_batch():
    users = await UniqueUser.create(
        {"name": "bob", "age": 2}, {"name": "ben", "age": 3}
    )

    assert users[0].uid != users[1].uid

    users = await UniqueUser.get_or_create(
        {"uid": users[0].uid}, {"name": "bill", "age": 4}
    )

    assert users[0].name == "bob"
    assert users[1].uid

@mariusconjeaud
Copy link
Collaborator

Source of the issue is that the generate MERGE query looks like this : 'UNWIND $merge_params as params\n MERGE (n:BaseNodeModel {uid: params.create.uid})\n ON CREATE SET n = params.create\n RETURN n'

The key for the MERGE is the list of required properties ; in my above test, that is the uid property, which is auto-generated by the neomodel => does not match any existing node in the database => create instead of get

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

2 participants