Skip to content

Ability to serialize request bodies from IIdentifiable for integration testing #1263

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

Closed
bjornharrtell opened this issue Mar 13, 2023 · 7 comments

Comments

@bjornharrtell
Copy link
Contributor

Some time ago generic client serialization was removed, which I used to implement roundtrip integration tests by serializing from mocked IIdentifiable objects.

It now looks like the only option to make such tests is to manually contruct request bodies fx. like at

var requestBody = new
{
data = new
{
type = "televisionBroadcasts",
attributes = new
{
title = newBroadcast.Title,
airedAt = newBroadcast.AiredAt,
archivedAt = newBroadcast.ArchivedAt
}
}
};
. In my opinion this is not optimal and can be difficult and error prone for more complex types.

I wish there was some (limited?) ability to serialize IIdentifiable for integration test purposes.

@bkoelman
Copy link
Member

which I used to implement roundtrip integration tests by serializing from mocked IIdentifiable objects

What do you mean by "roundtrip integration tests"? Is that sending requests to a JSON:API server and asserting on its response, or do you mean sending requests to a JSON:API client application, which calls into a JSON:API server and processes its response?

It now looks like the only option to make such tests is to manually contruct request bodies

There are many ways to write tests; what you're pointing to is just how we do it internally. And it works well for us. Alternative approaches include using dynamic and/or foreach-ing over your mocked objects to build a JsonDocument.

In my opinion this is not optimal and can be difficult and error prone for more complex types

What would you consider optimal, then? And what makes this approach difficult to use? What kind of errors are you concerned about? What do the complex types you're referring to look like?

It would help to include code demonstrating how you'd expect such a utility to be consumed, what benefits it provides etc. And what the utility takes as input and what output it produces exactly.

Asking for "I wish there was some ability" is not very actionable, because it's unclear what you expect from it or how it should work.

There's no simple conversion from IIdentifiable to a request body. Depending on the endpoint, the request body has a different shape (resource versus relationship endpoints), and due to partial post/patch, it makes a difference when sending a relationship with "null" versus omitting it (the former clears the relationship, whereas the latter leaves it unchanged). To create a resource, all required attributes and relationships must be sent for the operation to succeed. Atomic operations complicate this even further, because the targeted fields typically vary per individual operation. Then there's client-generated IDs that play a role, which are permitted or not based on options. A request may include meta at various levels, interpreted by user code to drive different behavior, so a client must be able to provide that. The JSON:API version can be set in the request body as well, which can be used to adapt the response or perform content negotiation. And there's obfuscated IDs, base64-encoded BLOBs, and composite primary/foreign keys. It's also unclear how dates, times, and floating point numbers would be converted to string format, as culture settings and different notations play a role. JsonApiDotNetCore supports several naming conventions, so property names need to be converted to the naming convention being used on the server. Resource classes may have non-exposed and/or non-mapped properties, some of which have only a getter or only a setter, which works well from the server side, but the client needs the reverse, which may not be available. We have an example that auto-encrypts/decrypts sensitive attribute values, which should be accounted for as well. Then there's resource inheritance. And to optimize network bandwidth, clients can use ETags with a HEAD request. JsonApiDotNetCore supports 0 as a stored ID value, when configured as such. And resources may be self-referencing, or their relationships may have cycles, leading to infinite recursion if not accounted for. This is just off the top of my head, but I've likely missed other cases.

Honestly, I don't see our team building something like that, which would take months. Our time is pretty limited, and we have a ton of work with higher priorities. It doesn't align with our goals, which is to support clients through OpenAPI. However, I'm willing to discuss and advise on how you can build your own simple mechanism that fits your needs.

@bjornharrtell
Copy link
Contributor Author

My issue is essentially that I'm a stuck in a 4.x to 5.x upgrade where I need to ensure API compatibility to the extent that is possible. Essentially what my old 4.x compatible tests was doing was prepare a fixture dbcontext and create some basic entities into it. Then I verify create, update by copying and modifying entities then using the IRequestSerializer to create request bodies POST/PATCH and then IResponseDeserializer get the actual result back as entity and then do assertions.

Without carefully reproducing these tests for 5.x I will most likely run into regressions that would have been caught. Besides the work involved in mocking the payloads manually I'm afraid I will make mistakes that could hide potential regressions. The entities I have are not very deep, only a single HasOne relationship that is required, but they have geometry as attribute (geojson) via NTS which makes the payloads a bit on the large side with real data which makes me hesistant to try and make correct manual body payloads of them as anon objects.

I can fully understand that complete/flexible client serialization is out of scope for JADNC. And perhaps there is no easy way to get back what I could do. But I note you do have some auto serialization testing in SerializationBenchmarkBase involving the ResponseModelAdapter but I find it rather difficult to use, as of yet I've failed to get it to produce payloads that work.

@bjornharrtell
Copy link
Contributor Author

bjornharrtell commented Mar 13, 2023

A concrete example of one of the most simple tests I have is:

var route = "/objekter";
// get entity from fixture db
var entity = _fixture.Context.DmpFrededeOmrBk.Where(o => o.Id == _objectFixture.FrededeOmrBk.Id).First();
// convert to general form (this is what we transport in and out of JSON-API instead of exposing all "subclasses")
var objekt = entity.ToObjekt();
// reset identifiers to make it look like new object
objekt.Id = 0;
objekt.ObjektId = null;
objekt.VersionId = null;
// post through integration test host, the Post method uses IRequestSerializer and IResponseDeserializer to produce and parse the body back to entity object
var result = await _fixture.Post<Objekt>(route, objekt);
Assert.Equal(HttpStatusCode.Created, result.Response.StatusCode);
return result.Body.VersionId; // used in other test that compose this test

The payload that is produced by IRequestSerializer is in this case:

{
  "data": {
    "type": "objekter",
    "attributes": {
      "temaattributter": {
        "fred-tkode-id": 1,
        "reg-nr": "0810401",
        "fred-navn": null,
        "aendr-kode-id": 0,
        "aar-fredn": null,
        "gyldig-fra": "2023-03-13T19:36:26.4131894+01:00",
        "gyldig-til": null
      },
      "objekt-id": null,
      "version-id": null,
      "systid-fra": "2023-03-13T19:36:27.111+01:00",
      "systid-til": null,
      "oprettet": "2023-03-13T19:36:27.111+01:00",
      "oprindkode-id": 8,
      "statuskode-id": 3,
      "off-kode-id": 1,
      "cvr-kode-id": 20228806,
      "bruger-id": "123e4567-e89b-12d3-a456-426652340000",
      "link": null,
      "shape": {
        "type": "MultiPolygon",
        "coordinates": [
          [
            [
              [488884.0, 6114493.0],
              [488884.0, 6114393.0],
              [488984.0, 6114393.0],
              [488984.0, 6114493.0],
              [488884.0, 6114493.0]
            ]
          ]
        ]
      }
    },
    "relationships": {
      "temakode": { "data": { "type": "temakoder", "id": "2140" } },
      "vedhaeftede-filer": { "data": [] }
    }
  }
}

So in this case not overly complex but I have a fair amount of tests with this being a very basic one.. and the devil is in the details on what exactly these tests excerise.

@bkoelman
Copy link
Member

Without carefully reproducing these tests for 5.x I will most likely run into regressions that would have been caught. Besides the work involved in mocking the payloads manually I'm afraid I will make mistakes that could hide potential regressions.

The common denominator of 4.x and 5.x are the input/output bodies as strings. So to ensure you'll observe all changes during upgrade:

  1. Run your existing tests against v4.x and add assertions on the existing request body and response body strings.
  2. Replace client serialization with your own mechanism (could be anonymous types).
  3. Run the tests again, ensuring they produce the exact same strings.
  4. Upgrade to v5.x, removing the client serialization (keep only your new mechanism).
  5. Run the tests again. If they look fine, remove the string assertions.

But I note you do have some auto serialization testing in SerializationBenchmarkBase involving the ResponseModelAdapter but I find it rather difficult to use, as of yet I've failed to get it to produce payloads that work.

That does the opposite of what you need. The benchmarks serialize request bodies and deserialize response bodies. What you need instead, is something that deserializes request bodies and serializes response bodies.

If I were to implement what you need, I'd start by re-adding the deleted files in the first commit of https://github.com/json-api-dotnet/JsonApiDotNetCore/pull/1075/commits, which is where client serialization was removed.

@bjornharrtell
Copy link
Contributor Author

@bkoelman thanks for the input. I've spent the last couple of hours taking a new stab on my own serialization logic. I based on some parts of ResponseModelAdapter and get the same payload as above so it's looking good. I could repurpose ConvertAttributes for ResponseModelAdapter more or less without changes and hacked the relationship part to work without a generic treenode and instead introspect the resource object directly. I don't think it's far fetched it could be cleaned up for reuse, but is still perhaps out of scope of the project and will likely have many limitations that you mentioned which will be difficult to document and maintain. So I will close this for now.

@bjornharrtell
Copy link
Contributor Author

Just for the record it did work out, got all tests working at this point. :)

@bkoelman
Copy link
Member

Awesome, great to hear you were able to wrap up something that unblocks you from upgrading!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants