fix: Setting 2 full path properties fails SOFIE-2628

[Julusian]

I think what is happening is:

• Receive getDirectory for '1.1' (first setValue)
  • update node 1.1
  • report change to 1.1
  • create node 1.1.1
• Receive getDirectory for '1.1' (second setValue)
  • update node 1.1
  • report change to 1.1
  • update node 1.1.1
  • report change to 1.1.1

The change detection logic is pretty naive and isnt factoring in that getDirectory is expecting the children property to be loaded.
So the first 'change' which is updating solely the contents is completing the request, and confusing the getElementByPath which is expecting the children object to have been loaded by the time that the getDirectory call resolves.

---

The change here introduces a simple enum, so that the getDirectory request can flag as needing the children to be loaded before it can be resolved. Other calls should behave the same as before, with the previous hasResponse being replaced with values in this enum

[codecov-commenter]

Codecov Report

Attention: 41 lines in your changes are missing coverage. Please review.

Comparison is base (7682c4e) 60.94% compared to head (e5698e5) 60.76%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
- Coverage   60.94%   60.76%   -0.19%     
==========================================
  Files          69       69              
  Lines        6058     6076      +18     
  Branches      555      555              
==========================================
  Hits         3692     3692              
- Misses       2355     2373      +18     
  Partials       11       11              

Files	Coverage Δ
src/Ember/Client/index.ts	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mint-dewit]

The change detection logic is pretty naive and isnt factoring in that getDirectory is expecting the children property to be loaded.

I understand what the nature of the bug is now.

There's in essence 3 requests at the same time going on.
A) A getDirectory on the parent
B) A second getDirectory on the parent
C) A getDirectory on the child

C is fired directly after A is done, but at that point B isn't complete yet. The completion of B also means that the child is reinserted. This creates a change on the path of the child and resolves C even though it hasn't completed yet.

I can see this solution fixing the issue specifically for the getElementByPath case but in essence the race condition is not fundamentally fixed. Unfortunately, given how emberplus reports all its responses through updates to the tree it seems we would have to do very specific matching of how the tree is updated to whether that corresponds with the contents of the command to do a fundamental fix. (Or move the more execution synchronous model which would sadden me a bit)

[Julusian]

I can see this solution fixing the issue specifically for the getElementByPath case but in essence the race condition is not fundamentally fixed. Unfortunately, given how emberplus reports all its responses through updates to the tree it seems we would have to do very specific matching of how the tree is updated to whether that corresponds with the contents of the command to do a fundamental fix. (Or move the more execution synchronous model which would sadden me a bit)

Yes, I haven't attempted to fix the underlying race condition. I don't think I know enough about ember+ to be comfortable making that large of a change much here.
But I am not sure that the race condition is really an issue, and more importantly I dont know how it could be avoided.
Even if this library was more synchronous, there is still a risk of a subscription update during a request which could trip up the state handling logic. So without a way to match a request to a response, I think that this new approach of validating an update matches what a request asked for is the way to go

[mint-dewit]

I agree. Another optimisation would be to implement some logic in the getElementByPath to not create these duplicate requests but this PR is fine as is by me.