Persistence logic bails out on the wrong combination of source and relationship.

[BenBohannon]

I'm having difficulty with SDN frequently not persisting relationships between nodes upon calling .save(). Saving Nodes to the DB works as expected, but relationships do not appear to be written. Below is an example test that has the issue, along with the node and relationship classes.

Looking through the neo4j web console, the nodes are persisted without relations. Through the debugger, the relationships are in the rangeRelationsOut Sets of a and b nodes, but the relationship's id is null (which should be autogenerated on save).

The most unusual part of this is that it works sometimes. I can re-run the same test without changes, and it will either persist ALL relationships or not persist ANY relationships. I haven't seen any cases where it persists some relationships but not others.

I've seen the same behavior when running the Spring app locally (not tests), but still using the "neo4j:4.2.3" docker container for neo4j. The only fix is to repeatedly restart the Spring app until it decides to work.

Test:

// Neoj4 is running in a TestContainer Docker container.
    @Autowired
    SkuRepository skuRepo;

    @Test
    void testNewRelation() {
        Sku a = new Sku(0L, "A");
        Sku b = new Sku(1L, "B");
        Sku c = new Sku(2L, "C");
        Sku d = new Sku(3L, "D");

        a = skuRepo.save(a);
        b = skuRepo.save(b);
        c = skuRepo.save(c);
        d = skuRepo.save(d);

        a.rangeRelationTo(b, 1, 1 , RelationType.MULTIPLICATIVE);
        a.rangeRelationTo(c, 1, 1 , RelationType.MULTIPLICATIVE);
        a.rangeRelationTo(d, 1, 1 , RelationType.MULTIPLICATIVE);
        a = skuRepo.save(a);

        b = skuRepo.findById(b.getId()).get();
        b.rangeRelationTo(c, 1, 1, RelationType.MULTIPLICATIVE);
        skuRepo.save(b);

        System.out.println("meh!");
    }

Node:

@Node("SKU")
@Getter // lombok
@Setter
public class Sku {

    @Id @GeneratedValue
    private Long id;

    @Property("number")
    private Long number;

    @Property("name")
    private String name;

    @Relationship(type = "RANGE_RELATION_TO", direction = OUTGOING)
    private Set<RangeRelation> rangeRelationsOut = new HashSet<>();

    @ReadOnlyProperty
    @Relationship(type = "RANGE_RELATION_TO", direction = INCOMING)
    private Set<RangeRelation> rangeRelationsIn = new HashSet<>();

    public Sku(Long number, String name) {
        this.number = number;
        this.name = name;
    }


    public RangeRelation rangeRelationTo(Sku sku, double minDelta, double maxDelta, RelationType relationType) {
        RangeRelation relation = new RangeRelation(sku, minDelta, maxDelta, relationType);
        rangeRelationsOut.add(relation);
        return relation;
    }

Relationship:

@Data // lombok
@RelationshipProperties
public class RangeRelation {
    @Id @GeneratedValue private Long id;

    @Property private double minDelta;
    @Property private double maxDelta;
    @Property private RelationType relationType;

    @TargetNode private Sku targetSku;

    public RangeRelation(Sku targetSku, double minDelta, double maxDelta, RelationType relationType) {
        this.targetSku = targetSku;
        this.minDelta = minDelta;
        this.maxDelta = maxDelta;
        this.relationType = relationType;
    }

Repository:

@Repository
public interface SkuRepository extends Neo4jRepository<Sku, Long> {

[meistermeier]

The problem in your constellation is that you describe a bidirectional relationship just partially.
But let's start with the declaration:

@Relationship(type = "RANGE_RELATION_TO", direction = OUTGOING)
private Set<RangeRelation> rangeRelationsOut = new HashSet<>();

@Relationship(type = "RANGE_RELATION_TO", direction = INCOMING)
private Set<RangeRelation> rangeRelationsIn = new HashSet<>();

This ^^ defines the bidirectional dependency. Every relationship in the graph will get mapped either outgoing or incoming.
Setting the relationship via a.rangeRelationTo(b, 1, 1 , RelationType.MULTIPLICATIVE) and

public RangeRelation rangeRelationTo(Sku sku, double minDelta, double maxDelta, RelationType relationType) {
    RangeRelation relation = new RangeRelation(sku, minDelta, maxDelta, relationType);
    rangeRelationsOut.add(relation);
    return relation;
}

Will only set the (outgoing) relationship on this Sku.

When Spring Data Neo4j starts to persist a it will correctly create the relationship and related node. But at the moment it processes the empty (incoming) relationships it will make sure that there is nothing left in the database after the operation and delete it again.

What might look like something we could add as a "relaxed" bidirectional definition is more complex when you look more closely. SDN cannot determine who is right in this constellation. Is it a with the relationship or b without the relationship?

For this to solve you would have to also do something similar to sku.rangeRelationsIn.add(relation) in Sku#rangeRelationTo.

If there is something unclear (or I misunderstood anything), please ask, otherwise you can also close this issue ;)

// related issues #2282 and #2264

[BenBohannon]

I'm not sure I fully understand why "SDN cannot determine who is right in this constellation." When I create a relation from a to b, I intend for the incoming relationship set of a to remain empty. The result I expect is: a has an outgoing relationship, and b has an incoming one.

For this to solve you would have to also do something similar to sku.rangeRelationsIn.add(relation) in Sku#rangeRelationTo.

By this, do you mean I have to add an incoming relationship to b, in addition to the outgoing relationship on a? And would those have to be the same relation Object? I was concerned that that would persist two new relationships (since the autogenerated id is null on the relation in both sets), and SDN not be smart enough to realize its the same relationship.

Originally, I was also hoping that the @ReadOnlyProperty on rangeRelationsIn would allow nodes to own/persist only the outgoing relationships, while the incoming relationships would be ignored when saving. That way I can get the incoming relationships upon querying, but not have to deal with placing the same relationship on both nodes to save.

[BenBohannon]

Ah, after looking at the other issues I see you're saying that I should add an incoming relationship to b. I was hoping that @ReadOnlyProperty would handle that.

Last question: Upon persisting the sku with the relationship, .save() normally returns an updated a node with the persisted relationship complete with autogenerated id. Will the relationship on b also be updated with the persisted relationship from .save(), or will I need to rehydrate b?

Edit: Upon trying it out, adding the same relationship to b incoming with sku.rangeRelationsIn.add(relation) doesn't make sense, since the relationship has a @targetNode that is different for incoming vs outgoing.

Edit 2: Even though it doesn't make logical sense, it still works. I just have to hydrate b, so that the wrong @targetNode is replaced with the correct one. I can deal with that. Thanks!

[meistermeier]

SDN does not take @ReadOnlyProperty into consideration. Maybe we should at least discuss this. I will create another ticket for this.

SDN will automatically process the relationship from a to b. In the graph will only be one relationship in the end. Most important thing is: because the relationship back to a is set, it won't get deleted.

For edit2: Yes, this is a side effect. Usually I try to advice against bidirectional relationships at all (within Spring Data). That's the reason why the @RelationshipProperties have the "directed" targetNode in themself.
On load, you should hopefully the the right hydrated ones.
The cleanest way would be to create two relationship properties objects with the right target nodes inside, of course.

[BenBohannon]

Sorry, @meistermeier but I was mistaken. It only appeared to work because I got lucky 5+ times. The behavior is still the same even when adding the relation to the b incoming relation set. The following change had no effect:

    public RangeRelation rangeRelationTo(Sku sku, double minDelta, double maxDelta, RelationType relationType) {
        RangeRelation relation = new RangeRelation(sku, minDelta, maxDelta, relationType);
        rangeRelationsOut.add(relation);
        sku.rangeRelationsIn.add(relation);
        return relation;
    }

I also tried making two separate relationship objects with the correct @targetNode on each, but it only works if I save the correct node.

    public RangeRelation rangeRelationTo(Sku sku, double minDelta, double maxDelta, RelationType relationType) {
        RangeRelation relationOut = new RangeRelation(sku, minDelta, maxDelta, relationType);
        RangeRelation relationIn = new RangeRelation(this, minDelta, maxDelta, relationType);
        rangeRelationsOut.add(relationOut);
        sku.rangeRelationsIn.add(relationIn);
        return relationOut;
    }

I assume if Spring decides to look at outgoing relations last, then saving a works. If Spring decides to look at incoming relations last, then saving b works...

[michael-simons]

Hi.

The last snippet you added @BenBohannon is the one I personally would use and we would recommend. That it doesn't work is a bug.

Why doesn't it work?

You are right in your assumption it is about ordering: Sometimes we get the properties in different order from Spring Data Commons, that is a given.
Now, when the persisting logic sees a relationship, it looks if it has seen the obverse already, but, and that's the error, from this side of things. This is wrong in relationships going back and forth, as in this case.
If we encounter such a thing, we must not bail out like we are doing now.

This will be fixed at least in 6.1.x, but we try to back port it to 6.0.x, too.

Thanks for your report and the discussion.

[michael-simons]

Hi @BenBohannon Would you be willing to try out our snapshot build? This bug is fixed in

We publish snapshots at

<repositories>
  <repository>
    <id>spring-snapshots</id>
    <name>Spring Snapshots</name>
    <url>https://repo.spring.io/snapshot</url>
    <releases>
      <enabled>false</enabled>
    </releases>
  </repository>
</repositories>

See

• https://repo.spring.io/snapshot/org/springframework/data/spring-data-neo4j/6.0.10-SNAPSHOT/
• https://repo.spring.io/snapshot/org/springframework/data/spring-data-neo4j/6.1.2-SNAPSHOT/
• https://repo.spring.io/snapshot/org/springframework/data/spring-data-neo4j/6.2.0-SNAPSHOT/

Please let us know if there's more we have overseen.

[BenBohannon]

I tried snapshot build 6.0.10, and it's closer but seems to behave strangely. In the same test from above:

@Autowired
    SkuRepository skuRepo;

    @Test
    void testNewRelation() {
        Sku a = new Sku(0L, "A");
        Sku b = new Sku(1L, "B");
        Sku c = new Sku(2L, "C");
        Sku d = new Sku(3L, "D");

        a = skuRepo.save(a);
        b = skuRepo.save(b);
        c = skuRepo.save(c);
        d = skuRepo.save(d);

        a.rangeRelationTo(b, 1, 1 , RelationType.MULTIPLICATIVE);
        a.rangeRelationTo(c, 1, 1 , RelationType.MULTIPLICATIVE);
        a.rangeRelationTo(d, 1, 1 , RelationType.MULTIPLICATIVE);
        a = skuRepo.save(a);

        b = skuRepo.findById(b.getId()).get();
        b.rangeRelationTo(c, 1, 1, RelationType.MULTIPLICATIVE);
        skuRepo.save(b);

        System.out.println("meh!");
    }

the first save on a will result in the correct relations persisting 100% of the time, which is an improvement. But saving b can overwrite all the relations on a, which is not desirable.

I can't get 6.1.2-SNAPSHOT or 6.2.0-SNAPSHOT to correctly load the repository beans to test them.

[michael-simons]

Thanks @BenBohannon we can reproduce this and investigate further.