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

merge result not as espected #1279

Closed
makubica opened this issue Oct 5, 2018 · 8 comments
Closed

merge result not as espected #1279

makubica opened this issue Oct 5, 2018 · 8 comments
Labels
state: needs more info the author of the issue needs to provide more details

Comments

@makubica
Copy link

makubica commented Oct 5, 2018

Hi,
If I try to do the following with the attached table in the header zip I didnt get the expected result.
Do This:

    auto j = nlohmann::json::parse((const char*)j_data);
    nlohmann::json patch = R"({"UniqueId":"9999"})"_json;
    std::cout << j.dump();
    j.merge_patch(patch);
    std::cout << j.dump();
    nlohmann::json patch2 = R"({"Id":"9999"})"_json;
    j.merge_patch(patch2);

The result is different. Sometimes the whole table is missing in the result and sometimes the field UniqueId appears twice.

Header.zip

@nlohmann
Copy link
Owner

nlohmann commented Oct 6, 2018

I simplified your program and the JSON value (used an empty array so that the changes easier to spot):

#include "json.hpp"
#include <iostream>

using nlohmann::json;

int main()
{
    const char* j_data = R"(
    {
        "UniqueId": "",
        "_200": []
    }
    )";
    
    auto j = nlohmann::json::parse(j_data);
    std::cout << j << std::endl;

    // patch: set/change "UniqueId" to "9999"
    nlohmann::json patch = R"({"UniqueId":"9999"})"_json;
    j.merge_patch(patch);
    std::cout << j << std::endl;

    // patch: set/change "Id" to "9999"
    nlohmann::json patch2 = R"({"Id":"9999"})"_json;
    j.merge_patch(patch2);
    std::cout << j << std::endl;
}

The patch {"UniqueId":"9999"} asks to set/change "UniqueId" to "9999". The result:

{"UniqueId":"9999","_200":[]}

The patch {"Id":"9999"} asks to set/change "Id" to "9999". The result:

{"Id":"9999","UniqueId":"9999","_200":[]}

I cannot reproduce your findings "Sometimes the whole table is missing in the result and sometimes the field UniqueId appears twice.". What result are you getting and what do you expect?

@nlohmann nlohmann added the state: needs more info the author of the issue needs to provide more details label Oct 6, 2018
@makubica
Copy link
Author

makubica commented Oct 7, 2018

The point is that you should not simplify the things. The problem appears only if you work with large tables/arrays.
The result was in one place that the UniqueId appears twice in the merge result. Sometimes the whole array was not there.
Play with your function with the table and you will see what I mean.

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2018

I did. Several times. What is the exact problem you encounter?

@makubica
Copy link
Author

makubica commented Oct 7, 2018

As I said above I merged with the UniqueId and this field appears twice in the result Json stream.
Now I made a simple console app and this problem do not appear there.
I will try to extract the problem from my project to an console app.

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2018

Thanks!

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2018

Note the library uses std::map for objects, so a duplicate key is impossible.

@makubica
Copy link
Author

makubica commented Oct 7, 2018

I think I found the problem. I use internally wchar_t and before I call your lib I translate them to ansi char. In the routine WideCharToMultiByte I made a mistake with the length of the unicode string. Now it works.
Thanks!

@nlohmann
Copy link
Owner

nlohmann commented Oct 7, 2018

Good catch! Thanks for reporting back!

@nlohmann nlohmann closed this as completed Oct 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs more info the author of the issue needs to provide more details
Projects
None yet
Development

No branches or pull requests

2 participants