Various fixes and improvements #1114
Merged
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
added 17 commits
November 18, 2021 15:38
…base on POST many-to-many and DELETE to-many requests
…g null for missing services This produces HTTP 405 (Method Not Allowed) instead of 404 (Not Found), which better reveals the intent
…pe parameter, used for non-generic or generic constructed interfaces
Apparently EF Core 5 did not always fail on missing required relationships, which was fixed in EF Core 6. I tried to update existing tests leaving the models intact, but that poluted lots of tests so I made them optional instead.
bart-degreed
force-pushed
the
various
branch
from
4ca9e50 to
0140070
Compare
…pReadOnly, NoHttpPost etc. because they interfere with relationship endpoints. For example, blocking POST would block creating resources, as well as adding to to-many relationships, which is not very useful. The replacement is to inject just the subset of exposed services, or simply use the Command/Query controllers. When an endpoint is not exposed, we now return HTTP 403 Forbidden instead of 404 or 405.
Codecov Report
@@ Coverage Diff @@
## master #1114 +/- ##
==========================================
+ Coverage 88.60% 89.00% +0.40%
==========================================
Files 255 249 -6
Lines 7160 7123 -37
==========================================
- Hits 6344 6340 -4
+ Misses 816 783 -33
Continue to review full report at Codecov.
|
bart-degreed
force-pushed
the
various
branch
2 times, most recently
from
e36e8ca to
2ef4daf
Compare
added 8 commits
November 19, 2021 15:31
…llows for more intelligence in the link builder, such as handling versioning or inheritance.
…ny relationship request
…lls back to it on failure. This produces more correct error responses in operations, compared to rolling back the entire transaction. For example, when an operations request creates a resource and the next operation fails, the resource service may incorrectly conclude that the resource from the first operation does not exist.
bart-degreed
force-pushed
the
various
branch
from
2ef4daf to
bcf1f18
Compare
bart-degreed
force-pushed
the
various
branch
from
f9cee9b to
3fe49f4
Compare
maurei
reviewed
Nov 22, 2021
src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs
Outdated
Show resolved
Hide resolved
…Type type` everywhere
maurei
approved these changes
Nov 22, 2021
bart-degreed
pushed a commit
that referenced
this pull request
Dec 3, 2021
* Fixed inconsistencies in naming of resource type * Fixed: Query to determine initial state is sent to the read-only database on POST many-to-many and DELETE to-many requests * Make command/query controllers inherit from JsonApiController, passing null for missing services This produces HTTP 405 (Method Not Allowed) instead of 404 (Not Found), which better reveals the intent * Added overload on TypeExtensions.IsOrImplementsInterface to take a type parameter, used for non-generic or generic constructed interfaces * Use ResourceType instead of public name in local-id tracker * Change IQueryStringParameterReader.AllowEmptyValue into normal interface member * Simplified startup in example project * Removed left-over overload with single type parameter in ResourceGraphBuilder * Various corrections in documentation, added #nullable where it makes a difference * Revert DocFx workaround * Updated version compatibility table * Added release notes and icon to NuGet package * Fix nullability warnings produced by .NET 6 with EF Core 6 * Fixed: Error when using EagerLoad on a relationship * Use VS2022 image in AppVeyor on Windows * Fixed broken tests on EF Core 6 Apparently EF Core 5 did not always fail on missing required relationships, which was fixed in EF Core 6. I tried to update existing tests leaving the models intact, but that poluted lots of tests so I made them optional instead. * Fixed redacted data when running tests * Breaking: Removed access-control action filter attributes such as HttpReadOnly, NoHttpPost etc. because they interfere with relationship endpoints. For example, blocking POST would block creating resources, as well as adding to to-many relationships, which is not very useful. The replacement is to inject just the subset of exposed services, or simply use the Command/Query controllers. When an endpoint is not exposed, we now return HTTP 403 Forbidden instead of 404 or 405. * Increase version number, use branch name in suffix * Pass the full resource to LinkBuilder, instead of just its ID. This allows for more intelligence in the link builder, such as handling versioning or inheritance. * Optimization: Only save when there are changes in a remove-from-to-many relationship request * Clarifications in doc-comments * Extract constant * Extract method * Cleanup SelectClauseBuilder.ToPropertySelectors * Since EF Core 5, SaveChanges automatically creates a savepoint and rolls back to it on failure. This produces more correct error responses in operations, compared to rolling back the entire transaction. For example, when an operations request creates a resource and the next operation fails, the resource service may incorrectly conclude that the resource from the first operation does not exist. * Improved unittests for populating IJsonApiRequest in middleware * Clarify intent in AtomicOperationObjectAdapter * Review feedback: use `ResourceType resourceType` instead of `ResourceType type` everywhere
bart-degreed
pushed a commit
that referenced
this pull request
Dec 3, 2021
* Fixed inconsistencies in naming of resource type * Fixed: Query to determine initial state is sent to the read-only database on POST many-to-many and DELETE to-many requests * Make command/query controllers inherit from JsonApiController, passing null for missing services This produces HTTP 405 (Method Not Allowed) instead of 404 (Not Found), which better reveals the intent * Added overload on TypeExtensions.IsOrImplementsInterface to take a type parameter, used for non-generic or generic constructed interfaces * Use ResourceType instead of public name in local-id tracker * Change IQueryStringParameterReader.AllowEmptyValue into normal interface member * Simplified startup in example project * Removed left-over overload with single type parameter in ResourceGraphBuilder * Various corrections in documentation, added #nullable where it makes a difference * Revert DocFx workaround * Updated version compatibility table * Added release notes and icon to NuGet package * Fix nullability warnings produced by .NET 6 with EF Core 6 * Fixed: Error when using EagerLoad on a relationship * Use VS2022 image in AppVeyor on Windows * Fixed broken tests on EF Core 6 Apparently EF Core 5 did not always fail on missing required relationships, which was fixed in EF Core 6. I tried to update existing tests leaving the models intact, but that poluted lots of tests so I made them optional instead. * Fixed redacted data when running tests * Breaking: Removed access-control action filter attributes such as HttpReadOnly, NoHttpPost etc. because they interfere with relationship endpoints. For example, blocking POST would block creating resources, as well as adding to to-many relationships, which is not very useful. The replacement is to inject just the subset of exposed services, or simply use the Command/Query controllers. When an endpoint is not exposed, we now return HTTP 403 Forbidden instead of 404 or 405. * Increase version number, use branch name in suffix * Pass the full resource to LinkBuilder, instead of just its ID. This allows for more intelligence in the link builder, such as handling versioning or inheritance. * Optimization: Only save when there are changes in a remove-from-to-many relationship request * Clarifications in doc-comments * Extract constant * Extract method * Cleanup SelectClauseBuilder.ToPropertySelectors * Since EF Core 5, SaveChanges automatically creates a savepoint and rolls back to it on failure. This produces more correct error responses in operations, compared to rolling back the entire transaction. For example, when an operations request creates a resource and the next operation fails, the resource service may incorrectly conclude that the resource from the first operation does not exist. * Improved unittests for populating IJsonApiRequest in middleware * Clarify intent in AtomicOperationObjectAdapter * Review feedback: use `ResourceType resourceType` instead of `ResourceType type` everywhere
bart-degreed
pushed a commit
that referenced
this pull request
Dec 3, 2021
* Fixed inconsistencies in naming of resource type * Fixed: Query to determine initial state is sent to the read-only database on POST many-to-many and DELETE to-many requests * Make command/query controllers inherit from JsonApiController, passing null for missing services This produces HTTP 405 (Method Not Allowed) instead of 404 (Not Found), which better reveals the intent * Added overload on TypeExtensions.IsOrImplementsInterface to take a type parameter, used for non-generic or generic constructed interfaces * Use ResourceType instead of public name in local-id tracker * Change IQueryStringParameterReader.AllowEmptyValue into normal interface member * Simplified startup in example project * Removed left-over overload with single type parameter in ResourceGraphBuilder * Various corrections in documentation, added #nullable where it makes a difference * Revert DocFx workaround * Updated version compatibility table * Added release notes and icon to NuGet package * Fix nullability warnings produced by .NET 6 with EF Core 6 * Fixed: Error when using EagerLoad on a relationship * Use VS2022 image in AppVeyor on Windows * Fixed broken tests on EF Core 6 Apparently EF Core 5 did not always fail on missing required relationships, which was fixed in EF Core 6. I tried to update existing tests leaving the models intact, but that poluted lots of tests so I made them optional instead. * Fixed redacted data when running tests * Breaking: Removed access-control action filter attributes such as HttpReadOnly, NoHttpPost etc. because they interfere with relationship endpoints. For example, blocking POST would block creating resources, as well as adding to to-many relationships, which is not very useful. The replacement is to inject just the subset of exposed services, or simply use the Command/Query controllers. When an endpoint is not exposed, we now return HTTP 403 Forbidden instead of 404 or 405. * Increase version number, use branch name in suffix * Pass the full resource to LinkBuilder, instead of just its ID. This allows for more intelligence in the link builder, such as handling versioning or inheritance. * Optimization: Only save when there are changes in a remove-from-to-many relationship request * Clarifications in doc-comments * Extract constant * Extract method * Cleanup SelectClauseBuilder.ToPropertySelectors * Since EF Core 5, SaveChanges automatically creates a savepoint and rolls back to it on failure. This produces more correct error responses in operations, compared to rolling back the entire transaction. For example, when an operations request creates a resource and the next operation fails, the resource service may incorrectly conclude that the resource from the first operation does not exist. * Improved unittests for populating IJsonApiRequest in middleware * Clarify intent in AtomicOperationObjectAdapter * Review feedback: use `ResourceType resourceType` instead of `ResourceType type` everywhere
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.
Various fixes and improvements that I wrote down over the last few weeks while working on other things. See the individual commits for details.
Fixes #1110 and #1106.
QUALITY CHECKLIST