Restructured eager loading tests and fixed two bugs #882
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1] During update, eager loads without parse fieldsets were not loaded at all places, which would make the resource change tracker incorrectly detect changes and return 200 instead of 204. 2] Creating a resource with a required EagerLoad (not exposed as relationship) was not possible, as there was no place for user code to create the related entity. Fixed this by making the create logic more similar to update logic. The change tracker scans through the initally created resource, invoking all property getters, enabling them to create related entities on the fly. See usage in Building.cs.
added 2 commits
November 18, 2020 17:55
Yesterday I was trying to solve the wrong problem. The problem is not how to prevent a crash when creating a new resource that has a required EagerLoad relationship. Instead, the problem is that api developers need a way to assign relationships (EagerLoads or not) on resource creation. Whether such a relationship is required or not is irrelevant, it depends on business logic which relationships must be assigned. And that may be an already-existing entity or a new one. If the goal is to assign an already-existing entity, the api developer needs an async method to load that entity and assign it to the resource being created. This commit adds `IResourceWriteRepository.GetForCreateAsync`, which is analog to `GetForUpdateAsync`. This makes the resource creation logic in resource service very similar to update. This also fixes the tests, where [NotMapped] was missing on calculated properties. Except for request body deserialization, there is no more need to handle nulls in calculated properties that access a required related EagerLoad entity. And by treating the deserialized resource object just as an input source (which is never saved), we get by with just storing the deserialized data for later use, knowing it will be read and copied into the resource instance we're going to save. Also reordered repository members in a more logical order (used to be verb-based).
maurei
approved these changes
Nov 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Restructured eager loading tests and fixed two bugs: