Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for removing and unloading a record from a hasMany relationship #4987

Closed
wants to merge 2 commits into from

Conversation

sudowork
Copy link
Contributor

@sudowork sudowork commented May 16, 2017

In relation to #4986.

Note: This test is intentionally failing right now. It is passing if run against a branch prior to 6d96eda.

@sudowork
Copy link
Contributor Author

Rebased to latest master (ac01921), checking to see if #5011 would resolve this issue. However, I still see the following test errors.

The test I just added:

integration/relationships/has_many - Has-Many Relationships: After removing and unloading a record, a hasMany relationship should still be valid (1, 1, 2)
2. Died on test #2     at testInDebug (http://localhost:4200/assets/tests.js:433:19)
    at Module.callback (http://localhost:4200/assets/tests.js:18328:28)
    at Module.exports (http://localhost:4200/assets/vendor.js:123:32)
    at requireModule (http://localhost:4200/assets/vendor.js:38:18)
    at TestLoader.require (http://localhost:4200/assets/test-support.js:4675:7)
    at TestLoader.loadModules (http://localhost:4200/assets/test-support.js:4667:14)
    at Function.TestLoader.load (http://localhost:4200/assets/test-support.js:4697:22): Assertion Failed: You looked up the 'comments' relationship on a 'post' with id 1 but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async ('DS.hasMany({ async: true })')@ 8 ms
Source: 	
    at assert (http://localhost:4200/assets/vendor.js:16279:13)
    at Object.assert (http://localhost:4200/assets/vendor.js:27947:34)
    at ManyRelationship.getRecords (http://localhost:4200/assets/vendor.js:80951:22)
    at Class.get (http://localhost:4200/assets/vendor.js:89241:58)
    at ComputedPropertyPrototype.get (http://localhost:4200/assets/vendor.js:27603:28)
    at get (http://localhost:4200/assets/vendor.js:32519:19)
    at _getPath (http://localhost:4200/assets/vendor.js:32540:13)
    at Object.get (http://localhost:4200/assets/vendor.js:32515:14)
    at Class.get (http://localhost:4200/assets/vendor.js:45239:26)
    at Backburner.run (http://localhost:4200/assets/vendor.js:11285:23)

A new test failure since

integration/deletedRecord - Deleting Records: Will resolve destroy and save in same loop (3, 1, 4)
2. Died on test #2     at Module.callback (http://localhost:4200/assets/tests.js:10865:19)
    at Module.exports (http://localhost:4200/assets/vendor.js:123:32)
    at requireModule (http://localhost:4200/assets/vendor.js:38:18)
    at TestLoader.require (http://localhost:4200/assets/test-support.js:4675:7)
    at TestLoader.loadModules (http://localhost:4200/assets/test-support.js:4667:14)
    at Function.TestLoader.load (http://localhost:4200/assets/test-support.js:4697:22)
    at http://localhost:4200/assets/test-support.js:4580:18: Assertion Failed: normalizeResponse must return a valid JSON API document:
	* One or more of the following keys must be present: "data", "errors", "meta".@ 3 ms
Source: 	
    at assert (http://localhost:4200/assets/vendor.js:16279:13)
    at Object.assert (http://localhost:4200/assets/vendor.js:27947:34)
    at normalizeResponseHelper (http://localhost:4200/assets/vendor.js:84257:20)
    at http://localhost:4200/assets/vendor.js:88804:19
    at Backburner.run (http://localhost:4200/assets/vendor.js:11296:23)
    at Backburner.join (http://localhost:4200/assets/vendor.js:11322:23)
    at http://localhost:4200/assets/vendor.js:88800:23
    at tryCatch (http://localhost:4200/assets/vendor.js:69108:14)
    at invokeCallback (http://localhost:4200/assets/vendor.js:69123:15)
    at publish (http://localhost:4200/assets/vendor.js:69091:9)
3. afterEach failed on Will resolve destroy and save in same loop: Assertion Failed: You can only unload a record which is not inFlight. `<person:null>`@ 4 ms
Source: 	
    at assert (http://localhost:4200/assets/vendor.js:16279:13)
    at Object.assert (http://localhost:4200/assets/vendor.js:27947:34)
    at Object.assertAgainstUnloadRecord [as unloadRecord] (http://localhost:4200/assets/vendor.js:77332:18)
    at InternalModel.send (http://localhost:4200/assets/vendor.js:83296:30)
    at InternalModel.unloadRecord (http://localhost:4200/assets/vendor.js:83063:10)
    at InternalModelMap.clear (http://localhost:4200/assets/vendor.js:84067:15)
    at IdentityMap.clear (http://localhost:4200/assets/vendor.js:84175:16)
    at Class.unloadAll (http://localhost:4200/assets/vendor.js:87780:25)
    at Class.willDestroy (http://localhost:4200/assets/vendor.js:88701:10)
    at Class.superWrapper [as willDestroy] (http://localhost:4200/assets/vendor.js:51006:22)
4. Expected 1 assertions, but 3 were run@ 4 ms
Source: 	
    at Module.callback (http://localhost:4200/assets/tests.js:10865:19)
    at Module.exports (http://localhost:4200/assets/vendor.js:123:32)
    at requireModule (http://localhost:4200/assets/vendor.js:38:18)
    at TestLoader.require (http://localhost:4200/assets/test-support.js:4675:7)
    at TestLoader.loadModules (http://localhost:4200/assets/test-support.js:4667:14)
    at Function.TestLoader.load (http://localhost:4200/assets/test-support.js:4697:22)
    at http://localhost:4200/assets/test-support.js:4580:18

@@ -1788,6 +1788,37 @@ testInDebug('A sync hasMany errors out if there are unlaoded records in it', fun
}, /You looked up the 'comments' relationship on a 'post' with id 1 but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async \('DS.hasMany\({ async: true }\)'\)/);
});

testInDebug('After removing and unloading a record, a hasMany relationship should still be valid', function(assert) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may as well test this always

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to always test.

stefanpenner added a commit that referenced this pull request Jul 25, 2017
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

 
For an async relationship [x, y] with x.unloadRecord(), now adjusts only the relationship’s currentState, leaving that relationship’s canonical state alone, ensuring the existing client-side delete semantics are preserved. But when that relationship is reloaded, the canonicalState consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState and canonical state are updated. This is to mirror the client-side delete semantics. But since we cannot reload a sync relationship we must assume this to be the new canonical state and rely on subsequent `push` or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload


note: if both sides of a relationships are unloaded, the above doesn’t apply. This is largely just when members of a loaded relationship are themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
stefanpenner added a commit that referenced this pull request Jul 25, 2017
…rd()

For an async relationship [x, y] with x.unloadRecord(), now adjusts only
the relationship’s currentState, leaving that relationship’s canonical
state alone, ensuring the existing client-side delete semantics are
preserved. But when that relationship is reloaded, the canonicalState
consulted.

For sync relationship [x, y] with x.unloadRecord(), both currentState
and canonical state are updated. This is to mirror the client-side
delete semantics. But since we cannot reload a sync relationship we must
assume this to be the new canonical state and rely on subsequent `push`
or `adapterPayloads` or manual `store.push` to update.

This aims to:

* [FIX] hasMany arrays never contain dematerialized records (so they no longer become broken)
* [FIX] using unloadRecord as a type of client side delete is restored
* [PRESERVE] the garbage collector pass to cleanup orphaned models
* [PRESERVE] second access to a relationship which did contain an unloadRecord to cause a reload

note: if both sides of a relationships are unloaded, the above doesn’t
apply. This is largely just when members of a loaded relationship are
themselves unloaded.

[fixes #4986 #5052 #4987 #4996]
@sudowork
Copy link
Contributor Author

Abandoning since it was merged as part of #5092

@sudowork sudowork closed this Aug 24, 2017
@sudowork sudowork deleted the unload-test branch August 24, 2017 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants