Running the item ancestors propagation in a separate transaction should not introduce a breach allowing to create a cycle in the item relations graph

[zenovich]

But unfortunately it does as of #1058 (April 2024).
This PR contains a test reproducing the issue:

1. There are two users: John and Jane. There are two skills: 1 and 2.
2. Both users have permissions to view content and edit children of both skills.
3. John creates a relation between the skills: 1 is a parent of 2.
4. Right after John created the relation, but before the item ancestors propagation is started, Jane tries to create a relation between the skills: 2 is a parent of 1 .
5. Jane's request should return 403 because it's not allowed to create a cycle.

The test fails on the step 5: instead of returning 403, the request creates a cycle in the item relations graph and then both requests get stuck in infinite propagation process.

[zenovich]

Failed
Loading environment: test
Loading environment: test
Loading environment: test
    propagation_integration_test.go:64: sanity check before all propagations
    propagation_verifier.go:127: before propagation step 1: "sanity check" (1)
    propagation_integration_test.go:64: triggering propagations
    propagation_verifier.go:127: before propagation step 2: "item ancestors: init" (1)
    propagation_integration_test.go:65: cannot send PUT request to /items/1 with token 'token_john': Put "http://127.0.0.1:41185/items/1": context deadline exceeded
    propagation_integration_test.go:64: after all propagations
    propagation_integration_test.go:56: cannot send PUT request to /items/2 with token 'token_jane': Put "http://127.0.0.1:41185/items/2": context deadline exceeded
    propagation_verifier.go:127: before propagation step 3: "item ancestors: main step" (1)
    propagation_verifier.go:127: before propagation step 4: "access: main step" (1)
    propagation_verifier.go:127: before propagation step 5: "access: main step" (2)
    propagation_verifier.go:127: before propagation step 6: "access: main step" (3)
    propagation_verifier.go:127: before propagation step 7: "access: main step" (4)
    propagation_verifier.go:127: before propagation step 8: "access: main step" (5)
    propagation_verifier.go:127: before propagation step 9: "access: main step" (6)
    propagation_verifier.go:127: before propagation step 10: "access: main step" (7)
    propagation_verifier.go:127: before propagation step 11: "access: main step" (8)
    propagation_verifier.go:127: before propagation step 12: "access: main step" (9)
    propagation_verifier.go:127: before propagation step 13: "access: main step" (10)
    propagation_verifier.go:124: looks like an infinite loop in propagation step "access: main step", called 11 times
    propagation_verifier.go:124: looks like an infinite loop in propagation step "access: main step", called 12 times

[smadbe]

Few ideas to solve the problem:

• As changing a parent-child relation is not so common, maybe there is a cheap way to do the check without using ancestors (but see comment below). If so we could re-move ancestor computation in sync.
• do the ancestor propagation immediately if we need to change the relationship and there is a propagation planned. So the request would be slower some (rare) times.
• let the request pass, but revert the conflicting change afterwards (optimistic approach)

Anyway, the ancestor propagation does not seem to be a long operation, so it looks reasonable to handle it "immediately" and so ensuring that the previous computation is done before changing the items_items relationship again (if this change the same part of "item tree" at least). But the main reason why this propagation was moved to full async is that changing item children is already a very slow operation... and so waiting the propagation to complete before responding to the http request (with a user waiting in front of his screen) does not work and does not really make sense. If the response could be sent as soon as the items_items has been changed and the propagation done immediately afterwards, that would be ok, but we still need a way to do it.