data icon indicating copy to clipboard operation
data copied to clipboard

[4.6] Bug: sync hasMany relationships with pointers to unloaded data do not recompute if the data loads later

Open patrick-auditboard opened this issue 2 years ago • 2 comments

This bug is present in at least 4.6.

Given the following setup: @hasMany('foo', { async: false, inverse: null }) foos; And given an initial payload of relationships: { foos: { data: [ { type: 'foo', id: '1' } ] } } but which does not include resource data for foo:1

  • The cache and graph will enter the correct state

Possibly related to reproduction:

  • a notification will be sent to the record
  • the notification is discarded because the record is not yet materialized

Then

  • The parent record becomes materialized
  • The foos relationship is accessed on the parent record
  • the ManyArray is instantiated
  • retrieveLatest is called from init
  • retrieveLatest filters out the record as it has no resource data loaded https://github.com/emberjs/data/blob/4f866a0d4539187a99de8664fdd6300b80a9e999/packages/model/addon/-private/many-array.ts#L312-L319

Later

  • The related record is loaded, nothing invalidates the ManyArray to resync

Unrelated to bug but of Note: in our app, when the related record is loaded we also reload the relationship data for the primary record; however, since the data is unchanged no change notification is sent so the ManyArray does not invalidate.

patrick-auditboard avatar Sep 06 '23 00:09 patrick-auditboard

Some additional context here:

This bug can only be hit in production because its for unspecified behavior that occurs when an assert that would normally be triggered in development is removed.

but

This is a situation where we should probably attempt to not totally wreck the system when the error case is entered. How to handle that is unclear. The situation only presents itself when applications choose to give identifiers for relationships to records that are unloadable the user's lack of permission to see the records. Ostensibly the application should either be modeling this differently or using links instead of identifiers for managing relationship state.

runspired avatar Sep 19 '23 04:09 runspired

We added tests to codify what happens in 4.6 when we enter these situations in production, not because we consider this supported or the behavior good but simply because we do want to know when we've changed what happens.

Tests for 4.6: #9115 We have ported those tests to 4.12 where we used them to minimize the impacts of some changes: #9120 We will similarly port forward the 4.12 state to the main branch and 5.3

runspired avatar Nov 17 '23 23:11 runspired