Correcting ConstraintError in `create_or_update` when modifying `__required_properties__`

[turukawa]

I have data where there are unique reference fields, and required title fields. Titles are not intended to be immutable, and can be modified.

When using bulk create_or_update, updates fail with:

ConstraintError: {code: Neo.ClientError.Schema.ConstraintValidationFailed} {message: Node(6) already exists with 
label `Project` and property `identifier` = 'b812a28ffd844c7a954e03588315a002'}`

I found a number of references to similar experiences (cf https://github.com/neo4j-contrib/neomodel/issues/438#issuecomment-553346476), but no solutions. This is a minimally-invasive change I'd like to suggest, unless there is an already-existing solution I should be using instead:

In _build_merge_query in core:

https://github.com/neo4j-contrib/neomodel/blob/bfeae3e10498006dc0aae6bab2885a3c0f194dc3/neomodel/core.py#L291

There is a specific check for both required and unique_index properties of a node using __required_properties__. My understanding is that required fields are mutable, but unique_index fields are assigned on creation as immutable references. This is something that probably doesn't catch everyone (depending on how you decide to use required), but it catches me.

My suggestion is as follows:

1. Create a new __unique_indexed_properties__ class property:

    @classproperty
    def __unique_indexed_properties__(cls):
        return tuple(
            name
            for name, property in cls.defined_properties(aliases=False, rels=False).items()
            if property.unique_index
        )

This could be added to NodeMeta.__new__ instead along with the other property tests:

https://github.com/neo4j-contrib/neomodel/blob/bfeae3e10498006dc0aae6bab2885a3c0f194dc3/neomodel/core.py#L162

2. Update _build_merge_query as follows (replace L289-L291 with these rows):

        required_properties = cls.__required_properties__
        if update_existing:
            required_properties = cls.__unique_indexed_properties__
        n_merge = "n:{0} {{{1}}}".format(
            ":".join(cls.inherited_labels()),
            ", ".join("{0}: params.create.{0}".format(getattr(cls, p).db_property or p) for p in required_properties),
        )

That then shouldn't interfere with get_or_create. If this is of interest, I can submit an update, otherwise simply inherit from StructuredNode and update accordingly.

[whatSocks]

@turukawa yes please, this seems like a good suggestion.

[turukawa]

@whatSocks one thing to check, though, is this where you check for required fields as well? Technically, required vs update should be different checks - an update should already have passed a required field check - but combining them (as with this) may leave a gap.

Ultimately, I'd say that's the responsibility of the developer - and there are great tools like Pydantic that can do safe sanity checks before hitting the database - but it would be good to know what is lost with this approach.