Serialization should respect the 'includeNullFields' configuration

[tbowley]

Hi there

I have limited experience working with JSON and webservices, so maybe I am missing something, but shouldn't the serializer respect the includeNullFields configuration option for all serialization?

If so, here is my PR to fix that... 😄

/ Thomas

[bamkrs]

Hey Thomas!

Thanks for your PR! Does your serialiser does not respect the config?
Normally, if a null field is included or not is already handled here

oatpp/src/oatpp/parser/json/mapping/Serializer.cpp

Line 168 in 5d263b8

if(value || serializer->getConfig()->includeNullFields) {

So the code you patched should only be reached if including null fields are enabled. Can you check that?

Next, I may see a potential bug with your changes: Checking a field for null at your edited lines would break JSON. Let me try to explain it.
Correct:

{
    "FieldA": "foo",
    "FieldB": null,
    "FieldC": "bar"
}

Your lines would result in the following JSON:

{
    "FieldA": "foo",
    "FieldB":,
    "FieldC": "bar
}

Observe how the key "FieldB" is still in the resulting JSON but has no value.

Could you double check my concerns?
Best Regards
Benedikt

[tbowley]

Hi Benedikt

I am aware that JSON can't have empty values, though I might have not been clear in what I was trying to solve.
The reason I started looking into this is because my serialized responses includes a single ´null´ string in the response body as the top-level object. As a side note I have also been able to generate 200 responses with only '{}' in the response body.

// Simple reproduction of issue, I am aware of the createResponse(..., oatpp::String) method
...
createDtoResponse(Status::CODE_200, {});
...

So maybe the missing checks for 'includeNullFields' in my PR are too far reaching, I honestly couldn't figure out where it should have been to fix the issue.
Basically I am trying to make sure that I don't get either an empty JSON object or a 'null' string in my responses.

This is how I have setup the object mapper, I assume that is correct?

OATPP_CREATE_COMPONENT(std::shared_ptr<oatpp::data::mapping::ObjectMapper>, apiObjectMapper)([] {
    auto objectMapper =  oatpp::parser::json::mapping::ObjectMapper::createShared();
    objectMapper->getDeserializer()->getConfig()->allowUnknownFields = false;
    objectMapper->getSerializer()->getConfig()->useBeautifier = true;
    objectMapper->getSerializer()->getConfig()->includeNullFields = false;
    return objectMapper;
}());

Does that clarify your question?

[bamkrs]

Oh, now I seem to get it.

createDtoResponse(Status::CODE_200, {});

Just returns null as body but you are expecting {} as body, correct?

That's a different story. iirc Passing {} to the function basically tells c++ to initialize and DTO::Wrapper without any content. It's kinda like an empty std::shared_ptr<> (which is a nullptr).
That it prints null with null-fields disabled can actually be considered a bug but should be handled at a way higher level.

How do you end up in this situation? Do you just want to create an empty Response?

Anyhow, if you want to have {} in your response you have to create any DTO but leave it empty:

auto dto = MessageDto::createShared();
// Do not populate any fields
createDtoResponse(Status::CODE_200, dto);

There are more generic ways like creating an Fields<Any> object, leaving it empty and passing it to createDtoResponse.