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

Add support of nullable fields to NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE and NLOHMANN_DEFINE_TYPE_INTRUSIVE #2356

Closed
2 of 3 tasks
Kotodevochka opened this issue Aug 15, 2020 · 7 comments
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@Kotodevochka
Copy link

Kotodevochka commented Aug 15, 2020

What is the issue you have?

When I try to deserialize JSON which contains nullable string fields into an object with (de)serializers generated by NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE macro, the program throws an exception at this point:

from_json.hpp line 80
error:
Exception thrown at 0x764A4DBD in TrelloBackup.exe: Microsoft C++ exception: nlohmann::detail::type_error at memory location 0x00FAE7B8.
Unhandled exception at 0x764A4DBD in TrelloBackup.exe: Microsoft C++ exception: nlohmann::detail::type_error at memory location 0x00FAE7B8.

Exception thrown: read access violation.
_Right was nullptr.

Please describe the steps to reproduce the issue.

Create a json like this:

{
      "cover":{
         "idAttachment":null,
         "color":null,
         "idUploadedBackground":null,
         "size":"normal",
         "brightness":"light"
      }
}

Define classes like this:

#pragma once

#include <string>
#include <vector>
#include <nlohmann/json.hpp>

using namespace std;
using json = nlohmann::json;

namespace TrelloDtos {

	class CoverDto {
	public:
		string idAttachment;
		string size;
	};

	class CardDto {
	public:
		CoverDto cover;
	};
		NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(CoverDto, idAttachment, size)
		NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(CardDto, cover)
}

Then try to deserialize:


voidTrelloApi::findCards(string response) {
    json jsonObj = json::parse(response);
    CardDto cardDto = jsonObj .get<CardDto>(); <--- at this point the exception is thrown
}

I had to write custom (de)serailizers to handle nullable fields like this:

void TrelloDtos::to_json(json& j, const CoverDto& p) {
	j = json{ {"idAttachment", p.idAttachment},{"size", p.size} };
}

void TrelloDtos::from_json(const json& j, CoverDto& p) {
	auto idAttachment = j.at("idAttachment");
	if (!idAttachment.is_null()) {
		idAttachment.get_to(p.idAttachment);
	}
	auto size = j.at("size");
	if (!size.is_null()) {
		size.get_to(p.size);
	}
}

What is the expected behavior?

I would expect the logic generated by macros not to crash if it stumbles upon a nullable field, but set nullpointr or some default value to the field.

And what is the actual behavior instead?

Exception is thrown.

Which compiler and operating system are you using?

  • Compiler: Win32
  • Operating system: Windows 8.1

Which version of the library did you use?

  • latest release version 3.9.1

If you experience a compilation error: can you compile and run the unit tests?

  • yes
  • no - please copy/paste the error message below
@nlohmann
Copy link
Owner

This is not a bug, but the expected behavior of the macros. If you need anything beyond a mapping of members to object keys, you need to define your own conversion functions, just as you did.

@Kotodevochka
Copy link
Author

Kotodevochka commented Aug 16, 2020

Thank you for the clarification!
I am curious, would it make sense to improve those macros to handle possible nullability (or absence) of fields?

@nlohmann
Copy link
Owner

I think changing the existing macros' behavior would not be helpful as this would break existing code. I'm not sure whether adding another pair of macros like NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_NULLABLE would be a good idea. Any thoughts?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Aug 16, 2020
@SahibYar
Copy link

can we use std::optional for C++ data types used in nlohmann::json This way we can add support of std::nullopt for values atleast.

Datatypes are mentioned here

@nlohmann
Copy link
Owner

Optionals are a C++17 feature. We are working on support for built-in support for them, see #2229.

@serg06
Copy link

serg06 commented Aug 25, 2020

I don't think NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_NULLABLE would be very useful after #2229 is merged in?

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 4, 2020
@stale stale bot closed this as completed Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

4 participants