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

Merge master into openapi #1086

Merged
merged 8 commits into from
Sep 20, 2021
Merged

Merge master into openapi #1086

merged 8 commits into from
Sep 20, 2021

Conversation

bart-degreed
Copy link
Contributor

@bart-degreed bart-degreed commented Sep 18, 2021

This PR merges the latest changes from the master branch into the openapi branch, which primarily contains the switch from Newtonsoft to System.Text.Json.

It merges cleanly but produces build errors for IResourceContextProvider and NamingStrategy, which I've fixed. I got stuck on NewtonsoftDataContractResolver usage and need your help @maurei. Feel free to take it from here.

I also found a reference from OpenApiTests to JsonApiDotNetCore.OpenApi.Client, which should not exist, so I removed it.

Bart Koelman added 6 commits September 16, 2021 13:04
* Removed Serialization.Client.Internal

* Removed undocumented ?nulls and ?defaults query string support

* Refactor: use interpolated strings instead of concatenation

* Updated tests to use string value for IDs; centralized pseudo-constants

* Added tests for pascal casing

* Optimized attribute/relationship lookups

* Breaking: Made IResourceContextProvider.GetResourceContext() throw when not found; added TryGetResourceContext() that returns null

* Optimized resource graph lookups

* Breaking: Merged IResourceContextProvider into IResourceGraph

* Switched to STJ in assertions

Note we need JsonDateTimeOffsetFormatSpecifier now, because STJ never tries to infer the CLR type from JSON values between quotes, while Newtonsoft does. So Newtonsoft would convert both values to date/time, effectively hiding the textual difference that was always there.

* Switched to STJ in rendering exception stack traces

* Switched to STJ in rendering CLR objects as part of tracing. STJ properly handles self-referencing EF Core objects when enabling reference tracking, as opposed to Newtonsoft.

* Switched to STJ in attribute change tracking. This used to take options into account, which is unneeded because we only care about whether there's a diff, not so much what that diff looks like. And we don't expect self-references here (it would have crashed in the past, and will now too).

* Switched to STJ in Microservices example

* Removed re-indent of response body on HTTP status code mismatch in tests, because we already use indenting in TestableStartup, so this is no longer needed.

* Use STJ naming convention on special-cased code paths

* Renamed RelationshipEntry to RelationshipObject, Error to ErrorObject

* Fix broken test in cibuild

* Fixed broken tests in cibuild due to different line endings

* Package updates

* Refactor serialization objects
- Simplified error objects, so they are similar to the other serialization objects. This means no default instances, constructors (exception: ErrorObject) or conditional serialization logic. And explicit names to overrule naming conventions. And annotations to skip serialization when null.
- Added missing members from JSON:API v1.1 spec: ErrorDocument.Meta, ErrorLinks.Type, ErrorSource.Header, ResourceIdentifierObject.Meta
- Normalized collection types
- Updated documentation: Link to v1.1 of JSON:API spec instead of copy/pasted text

* Merged ErrorDocument and AtomicOperationsDocument into Document
Bugfix: jsonapi/version was missing in error responses

* Fill error.source.header where applicable

* Breaking: Renamed "total-resources" meta key to "total" because thats what Ember.js expects it to be named (see https://guides.emberjs.com/release/models/handling-metadata/)

* Removed unneeded StringEnumConverter usage. Also removed it from the defaults for tests, because that hides the problem when we forget to put it on a member that needs it.

* Use configured STJ options for null/default value inclusion
Bugfix: do not break out of method on first attribute

* Fixed data type in json request body

* Added missing type, which is a required element

* Converted core code to use System.Text.Json
- Added various converters to steer JsonSerializer in the right direction
- JsonApiDotNetCore.Serialization.Objects
  - Removed inheritance in JsonApiDotNetCore.Serialization.Objects, so we're in control of element write order
  - Moved "meta" to the end in all types (it is secondary information)
  - Consistently set IgnoreCondition on all properties, so we don't need to override global options anymore

* Updated documentation

* Fixed broken example-generation. Set launchBrowser to true, so it shows sample data on F5.

* Inlined properties on serializable objects

* Add test for incompatible ID value.
By default, this produces:
```
The JSON value could not be converted to JsonApiDotNetCore.Serialization.Objects.SingleOrManyData`1[JsonApiDotNetCore.Serialization.Objects.ResourceObject]. Path: $.data | LineNumber: 3 | BytePositionInLine: 11.
```
which is totally unhelpful. Because this is so likely to hit users, we special-case here to produce a better error.

* Removed misplaced launchsettings.json

* Review feedback: use base class instead of static helper
@codecov
Copy link

codecov bot commented Sep 18, 2021

Codecov Report

Merging #1086 (4f5ee42) into openapi (362bf5f) will increase coverage by 0.15%.
The diff coverage is 94.80%.

Impacted file tree graph

@@             Coverage Diff             @@
##           openapi    #1086      +/-   ##
===========================================
+ Coverage    88.82%   88.98%   +0.15%     
===========================================
  Files          306      298       -8     
  Lines         8057     8117      +60     
===========================================
+ Hits          7157     7223      +66     
+ Misses         900      894       -6     
Impacted Files Coverage Δ
...NetCoreExample/Controllers/NonJsonApiController.cs 0.00% <0.00%> (ø)
src/Examples/JsonApiDotNetCoreExample/Startup.cs 0.00% <0.00%> (ø)
...tCore/Configuration/ServiceCollectionExtensions.cs 100.00% <ø> (+15.38%) ⬆️
src/JsonApiDotNetCore/Configuration/TypeLocator.cs 86.48% <0.00%> (ø)
...rrors/ResourcesInRelationshipsNotFoundException.cs 100.00% <ø> (ø)
...NetCore/Serialization/Objects/RelationshipLinks.cs 100.00% <ø> (ø)
...iDotNetCore/Serialization/Objects/ResourceLinks.cs 100.00% <ø> (ø)
src/JsonApiDotNetCore/Queries/QueryLayer.cs 87.23% <50.00%> (ø)
...ore/Serialization/Objects/AtomicOperationObject.cs 80.00% <50.00%> (+5.00%) ⬆️
...NetCore/Resources/Internal/RuntimeTypeConverter.cs 88.57% <60.00%> (-11.43%) ⬇️
... and 118 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 362bf5f...4f5ee42. Read the comment docs.

@maurei maurei marked this pull request as ready for review September 18, 2021 17:00
@maurei
Copy link
Member

maurei commented Sep 18, 2021

@bart-degreed this should do, go ahead and merge if it looks okay to you

@bart-degreed bart-degreed merged commit 0c0690e into openapi Sep 20, 2021
@bart-degreed bart-degreed deleted the merge-master-into-openapi branch September 20, 2021 10:35
@bart-degreed bart-degreed restored the merge-master-into-openapi branch September 20, 2021 10:35
@bart-degreed bart-degreed deleted the merge-master-into-openapi branch September 20, 2021 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants