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

Redesign conversion between JSON objects and ASP.NET models #1091

Merged
merged 49 commits into from
Oct 27, 2021

Commits on Sep 20, 2021

  1. Replaced suppression with fix

    Bart Koelman committed Sep 20, 2021
    Configuration menu
    Copy the full SHA
    8534c41 View commit details
    Browse the repository at this point in the history

Commits on Sep 29, 2021

  1. Fixed: do not duplicate the list of JSON:API errors in meta stack tra…

    …ce, but do write them when logging
    Bart Koelman committed Sep 29, 2021
    Configuration menu
    Copy the full SHA
    f502628 View commit details
    Browse the repository at this point in the history
  2. Breaking: Added option to write request body in meta when unable to r…

    …ead it (false by default)
    Bart Koelman committed Sep 29, 2021
    Configuration menu
    Copy the full SHA
    afe119d View commit details
    Browse the repository at this point in the history
  3. Breaking: Added option to allow unknown attribute/relationship keys i…

    …n request body (false by default)
    Bart Koelman committed Sep 29, 2021
    Configuration menu
    Copy the full SHA
    2b47438 View commit details
    Browse the repository at this point in the history
  4. Fixed invalid test

    Bart Koelman committed Sep 29, 2021
    Configuration menu
    Copy the full SHA
    2b1c32b View commit details
    Browse the repository at this point in the history

Commits on Sep 30, 2021

  1. Rewrite of reading the request body and converting it into models.

    The existing validation logic inside controllers, JsonApiReader and RequestDeserializer/BaseDeserializer has been consolidated into a set of adapters that delegate to each other, passing along the current state. The dependency on `HttpContext` has been removed and similar errors are now grouped, in preparation to unify them. Because in this commit we always track the location of errors and write them to `error.source.pointer`, the error messages can be unified and simplified in future commits.
    
    In this commit, I've gone through great lenghts to preserve the existing error messages in our tests as much as possible, while keeping all tests green. All error tests in ReadWrite and Operations now have assertions on the source pointer. Added tests for missing/invalid 'data' in resource requests. Removed outdated unit tests and added new one for handling attributes of various CLR types.
    
    Updated benchmark code. The synthetic BenchmarkDotNet reports it has become 3-7% slower (we're talking microseconds here) compared to the old code. But when running full requests with our pipeline-instrumentation turned on, the difference falls in the range of background noise and is thus unmeasurable.
    
    ```
    BEFORE:
              Read request body ...............................  0:00:00:00.0000167 ...   1.08%  } 167 + 434 = 601
                JsonSerializer.Deserialize ....................  0:00:00:00.0000560 ...   3.62%  +
                Deserializer.Build (single) ...................  0:00:00:00.0000434 ...   2.80%  }
    
    AFTER:
              Read request body ...............................  0:00:00:00.0000511 ...   3.43%
                JsonSerializer.Deserialize ....................  0:00:00:00.0000537 ...   3.61%
    ```
    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    0192e39 View commit details
    Browse the repository at this point in the history
  2. Updated error message for unknown attribute/relationship

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    d247ba2 View commit details
    Browse the repository at this point in the history
  3. Updated error message for unknown resource type

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    8dbafd2 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    d243dc3 View commit details
    Browse the repository at this point in the history
  5. Unified error messages about the absense of 'type'

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    4c3bf71 View commit details
    Browse the repository at this point in the history
  6. Unified error messages about incompatible types

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    1700c00 View commit details
    Browse the repository at this point in the history
  7. Revert the use of different exception, because this way the request b…

    …ody does not get added to the error response meta
    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    d4549e8 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    c2e31d3 View commit details
    Browse the repository at this point in the history
  9. Additional unification of error messages

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    5ea961f View commit details
    Browse the repository at this point in the history
  10. Unified error messages for failed type conversion

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    6244ef7 View commit details
    Browse the repository at this point in the history
  11. Fix cibuild

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    72f02b0 View commit details
    Browse the repository at this point in the history
  12. Unified error messages about data presense and its value: null/object…

    …/array
    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    fe10147 View commit details
    Browse the repository at this point in the history
  13. Unified remaining error messages

    Bart Koelman committed Sep 30, 2021
    Configuration menu
    Copy the full SHA
    2322ac8 View commit details
    Browse the repository at this point in the history

Commits on Oct 1, 2021

  1. Sealed types and reduced dependencies

    Bart Koelman committed Oct 1, 2021
    Configuration menu
    Copy the full SHA
    d254ac9 View commit details
    Browse the repository at this point in the history
  2. Fixed broken test on linux

    Bart Koelman committed Oct 1, 2021
    Configuration menu
    Copy the full SHA
    0923c8b View commit details
    Browse the repository at this point in the history
  3. Adapter renames:

    - IOperationsDocumentAdapter -> IDocumentInOperationsRequestAdapter
    - IResourceDocumentAdapter -> IDocumentInResourceOrRelationshipRequestAdapter
    - IOperationResourceDataAdapter -> IResourceDataInOperationsRequestAdapter
    Bart Koelman committed Oct 1, 2021
    Configuration menu
    Copy the full SHA
    6e0d287 View commit details
    Browse the repository at this point in the history
  4. Added missing assertions on request body in error meta

    Bart Koelman committed Oct 1, 2021
    Configuration menu
    Copy the full SHA
    169acc7 View commit details
    Browse the repository at this point in the history

Commits on Oct 4, 2021

  1. Refactorings:

    - Changed deserializer to accept Error(s) instead of Document (used to be ErrorDocument, but they were merged)
    - Write request body in error meta instead of top-level meta
    - Fixed: Log at Error instead of Info when processing an operation throws unknown error
    Bart Koelman committed Oct 4, 2021
    Configuration menu
    Copy the full SHA
    317f293 View commit details
    Browse the repository at this point in the history
  2. Enhanced existing tests: Assert on resource type when included cont…

    …ains mixed types
    Bart Koelman committed Oct 4, 2021
    Configuration menu
    Copy the full SHA
    65b1f03 View commit details
    Browse the repository at this point in the history

Commits on Oct 5, 2021

  1. Configuration menu
    Copy the full SHA
    5198265 View commit details
    Browse the repository at this point in the history
  2. Refactor: removed OperationContainer.Kind because IJsonApiRequest.Wri…

    …teOperation is now populated consistently; applied c# pattern usage
    Bart Koelman committed Oct 5, 2021
    Configuration menu
    Copy the full SHA
    64cc4e2 View commit details
    Browse the repository at this point in the history
  3. Added test to capture the current behavior to return data:null for vo…

    …id operations. The spec allows an empty object too, but I don't consider this important enough to add yet another JSON converter with all its overhead.
    Bart Koelman committed Oct 5, 2021
    Configuration menu
    Copy the full SHA
    1e6d18e View commit details
    Browse the repository at this point in the history

Commits on Oct 6, 2021

  1. Improved tests for includes

    Bart Koelman committed Oct 6, 2021
    Configuration menu
    Copy the full SHA
    cab3dc6 View commit details
    Browse the repository at this point in the history
  2. Rewrite of rendering the response body from models

    - Added temporary bridge class to toggle between old/new code
    - Fixed: Missing top-level link in error- and operations response
    - Fixed: `ILinkBuilder.GetResourceLinks` was also used to render relationship links
    - IJsonApiRequest management: restore backup after processing operations
    - Refreshed serialization benchmarks
    
    Measurement results for GET http://localhost:14140/api/v1/todoItems?include=owner,assignee,tags:
        Write response body ............................  0:00:00:00.0010385 ->  0:00:00:00.0013343 = 130%
    
    Measurement results for GET http://localhost:14140/api/v1/todoItems?filter=and(startsWith(description,'T'),equals(priority,'Low'),not(equals(owner,null)),not(equals(assignee,null))):
        Write response body ............................  0:00:00:00.0006601 -> 0:00:00:00.0004624 = 70%
    
    Measurement results for POST http://localhost:14140/api/v1/operations (10x add-resource):
        Write response body ............................  0:00:00:00.0003432 -> 0:00:00:00.0003289 = 95%
    
    |                            Method |     Mean |   Error |  StdDev |
    |---------------------------------- |---------:|--------:|--------:|
    |       SerializeOperationsResponse | 153.8 us | 0.72 us | 0.60 us |
    | LegacySerializeOperationsResponse | 239.0 us | 3.17 us | 2.81 us |
    
    |                          Method |     Mean |   Error |  StdDev |
    |-------------------------------- |---------:|--------:|--------:|
    |       SerializeResourceResponse | 101.3 us | 0.31 us | 0.29 us |
    | LegacySerializeResourceResponse | 177.6 us | 0.56 us | 0.50 us |
    Bart Koelman committed Oct 6, 2021
    Configuration menu
    Copy the full SHA
    d8510aa View commit details
    Browse the repository at this point in the history
  3. Avoid closure in hot code path to reduce allocations

    Bart Koelman committed Oct 6, 2021
    Configuration menu
    Copy the full SHA
    055b93f View commit details
    Browse the repository at this point in the history
  4. Cleanup reader and writer

    Bart Koelman committed Oct 6, 2021
    Configuration menu
    Copy the full SHA
    a7c6009 View commit details
    Browse the repository at this point in the history

Commits on Oct 8, 2021

  1. Removed old code

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    428b06b View commit details
    Browse the repository at this point in the history
  2. Fixed: crash in test serializer on assertion failure

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    05f155e View commit details
    Browse the repository at this point in the history
  3. Removed RequestScopedServiceProvider

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    fbdcefb View commit details
    Browse the repository at this point in the history
  4. Use sets for include expressions

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    7fcb58e View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    f8d71f2 View commit details
    Browse the repository at this point in the history
  6. Reorganized JADNC.Serialization namespace

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    ff763f1 View commit details
    Browse the repository at this point in the history
  7. Created custom exception for remaining errors

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    80fe2b2 View commit details
    Browse the repository at this point in the history
  8. Fixed: call ResourceDefinition.OnApplyIncludes for all children, even…

    … when empty
    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    f6caf3a View commit details
    Browse the repository at this point in the history
  9. Renamed ResourceContext to ResourceType and exposed it through relati…

    …onship left/right. This enabled to reduce many resource graph lookups from the codebase.
    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    0efde1b View commit details
    Browse the repository at this point in the history
  10. Moved logic to build resource graph from DbContext into ResourceGraph…

    …Builder for easier reuse.
    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    f7e7da4 View commit details
    Browse the repository at this point in the history
  11. Opened up ResponseModelAdapter for extensibility

    Bart Koelman committed Oct 8, 2021
    Configuration menu
    Copy the full SHA
    3d98057 View commit details
    Browse the repository at this point in the history

Commits on Oct 11, 2021

  1. Check off roadmap entry

    Bart Koelman committed Oct 11, 2021
    Configuration menu
    Copy the full SHA
    2e3659f View commit details
    Browse the repository at this point in the history

Commits on Oct 20, 2021

  1. Review feedback

    Bart Koelman committed Oct 20, 2021
    Configuration menu
    Copy the full SHA
    2c4e75c View commit details
    Browse the repository at this point in the history

Commits on Oct 25, 2021

  1. Simplified existing tests

    Bart Koelman committed Oct 25, 2021
    Configuration menu
    Copy the full SHA
    7158595 View commit details
    Browse the repository at this point in the history
  2. Added extra test for data:null in relationship

    Bart Koelman committed Oct 25, 2021
    Configuration menu
    Copy the full SHA
    7c6684f View commit details
    Browse the repository at this point in the history
  3. Added test for broken resource linkage

    Bart Koelman committed Oct 25, 2021
    Configuration menu
    Copy the full SHA
    4a8bfdd View commit details
    Browse the repository at this point in the history

Commits on Oct 26, 2021

  1. Ported existing unit tests and changed how included[] is built.

    It now always emits related resources in relationship declaration order, even in deeply nested circular chains where a subsequent inclusion chain is deeper and adds more relationships to an already converted resource.
    
    Performance impact, summary:
    - In the endpoint test, this commit improves performance for rendering includes, while slightly decreasing it for the other two scenarios. All are still faster or the same compared to the master branch.
    - In BenchmarkDotNet, this commit slightly increases rendering time, compared to earlier commits in this PR, but it is still faster than the master branch.
    
    Measurement results for GET http://localhost:14140/api/v1/todoItems?include=owner,assignee,tags:
        Write response body .............................  0:00:00:00.0010385 -> 0:00:00:00.0008060 ...  77% (was: 130%)
    
    Measurement results for GET http://localhost:14140/api/v1/todoItems?filter=and(startsWith(description,'T'),equals(priority,'Low'),not(equals(owner,null)),not(equals(assignee,null))):
        Write response body .............................  0:00:00:00.0006601 -> 0:00:00:00.0005629 ...  85% (was: 70%)
    
    Measurement results for POST http://localhost:14140/api/v1/operations (10x add-resource):
        Write response body .............................  0:00:00:00.0003432 -> 0:00:00:00.0003411 ...  99% (was: 95%)
    
    |                            Method |     Mean |   Error |  StdDev |
    |---------------------------------- |---------:|--------:|--------:|
    | LegacySerializeOperationsResponse | 239.0 us | 3.17 us | 2.81 us |
    |       SerializeOperationsResponse | 153.8 us | 0.72 us | 0.60 us |
    | (new) SerializeOperationsResponse | 168.6 us | 1.74 us | 1.63 us |
    
    |                          Method |     Mean |   Error |  StdDev |
    |-------------------------------- |---------:|--------:|--------:|
    | LegacySerializeResourceResponse | 177.6 us | 0.56 us | 0.50 us |
    |       SerializeResourceResponse | 101.3 us | 0.31 us | 0.29 us |
    | (new) SerializeResourceResponse | 123.7 us | 1.12 us | 1.05 us |
    Bart Koelman committed Oct 26, 2021
    Configuration menu
    Copy the full SHA
    0f41f85 View commit details
    Browse the repository at this point in the history
  2. Fixed cibuild

    Bart Koelman committed Oct 26, 2021
    Configuration menu
    Copy the full SHA
    5dfb6ea View commit details
    Browse the repository at this point in the history