Build all test payloads as JSON strings directly

[oliverbarnes]

To avoid uncaught missed serde camelcases

[marcoow]

What does this mean? What payloads are not JSON?

[oliverbarnes]

Earlier we were using struct representations for the request payloads, eg InitialPlaceInput (from the top of my head)

[marcoow]

I mean you're always using strictly that represent the payload – the structure get deserialized from JSON with serde

[oliverbarnes]

Yes, problem is when we forget to add the serde field attribute to camelcase the field name when serializing into json.

Tests don't catch that when the struct request payload is used, the payload gets accepted whether it's camelcased or not.

We were bitten by this once and started using pure json since. It ensures that the field attribute is in place

[pablobm]

To clarify: this is about the tests. We have two styles going on at https://github.com/mainmatter/reStations/blob/main/web/tests/api/places_test.rs.

One style builds the payload as a Rust struct and relies on serde to turn it into JSON:

    let payload = json!(OsdmPlaceRequest {
        restrictions: None,
        place_input: Some(OsdmInitialPlaceInput {
            name: Some(String::from("Berlin")),
            geo_position: None,
        }),
    });

Another style gives the payload as a string, already in JSON:

    let payload = r#"
        {
            "restrictions": { "numberOfResults": 1 },
            "placeInput": {
                "geoPosition": {
                    "latitude": 51.508362,
                    "longitude": -0.123835
                }
            }
        }
    "#;

The preference is for the second style:

• We had an instance where doing it with structs+serde made us miss an issue with deserialisation.
• We wouldn't need to add new elements each time that OsdmPlaceRequest or its children change.
• It's easier to write.

Originally raised at #94 (comment)

[marcoow]

oh, I see – we should always use the first style though; you need to leave your Ruby behind and embrace Rust :)

[oliverbarnes]

We're married with kids to Rust at this point :)

We'd prefer to use types of course. If there's a more idiomatic way to catch the camelization bug, all ears.

[pablobm]

I'd be interested to know what the Rust conventions are around this, but I find the first style counterproductive in the context of writing tests (not production code).

The first style involves these transformations:

Rust struct --(json!, serde)--> JSON string --(request.send, serde)--> Rust struct (at the controller)

Visually speaking, this goes: snake_case, camelCase, snake_case (I know the snake_case is actually a Rust structure, but just talking from the perspective of the code reader). The second style sheds the first step:

JSON string --(request.send, serde)--> Rust struct (at the controller)

With the first style, at the test level we are working with a data structure that is one step removed from the "real" data that is sent to the application (the camelCased JSON string). For one, we are forced to work with snake_case, confusing the reader who may not realize the API uses camelCase. This was the source of a bug some weeks ago, when we didn't realize that we were accepting the wrong variant because serde was hiding this from us.

All this is in the context of writing tests. If instead we discussed production code, other priorities would apply and it typically would make sense to write in terms of Rust structures that can be validated statically.

Plus additionally: it's easier to write and doesn't need changing everywhere when a new field is added.