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

Accessing value with json pointer adds key if not existing #1600

Closed
sdaly2107 opened this issue May 14, 2019 · 9 comments
Closed

Accessing value with json pointer adds key if not existing #1600

sdaly2107 opened this issue May 14, 2019 · 9 comments
Assignees
Labels
kind: enhancement/improvement release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@sdaly2107
Copy link

  • What is the issue you have?

When accessing a key using json pointer, if the key doesn't exist, it is added. Is this expected behaviour? If so, is there some alternative const accessor we can use? For now, we are working around this by copying the object.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?

      json json;
      json["something"]["else"] = "me";
    
      auto dumpA = json.dump(4);
    
      auto value = json["/something/else"_json_pointer].get<std::string>();
      auto dumpB = json.dump(4);
      REQUIRE(dumpA == dumpB); //ok
    
      auto blah = json["/xyz"_json_pointer];
      auto dumpC = json.dump(4);
      REQUIRE(dumpB == dumpC); //fail, because new key has been added
    
  • What is the expected behavior?

{
"something": {
"else": "me"
},
"xyz": null <-- didn't expect this to be added when accessing via json pointer
}

  • And what is the actual behavior instead?

Key added.

MSVC + Win 10
GCC + CentOS 7

  • Did you use a released version of the library or the version from the develop branch?

Yes

@brouer
Copy link

brouer commented May 15, 2019

I use code like this:

    try
    {
        last_page = j.at("isLastPage");
    } catch (...)
    {
        //"isLastPage" was not present
        last_page = true;
    }

Quote from README.md:

If you are not sure whether an element in an object exists, use checked access with the at() function.

@sdaly2107
Copy link
Author

So looks like "at" might be the solution. I'd rather not be throwing and catching in normal paths though - would be nice if count() worked with json pointer too, then I could do if(count(json_pointer) then at(json_pointer).

@brouer
Copy link

brouer commented May 15, 2019

I only try/catch around optional entries that might not be present, in which case I may provide my own default as shown.

I don't try/catch with entries I expect to be there. If that happens, I let the exception propagate back up to the caller, as I consider that a parse error.

YMMV.

@nickaein
Copy link
Contributor

When accessing a key using json pointer, if the key doesn't exist, it is added. Is this expected behaviour?

I believe yes, as basic_json::operator[] documentation states. This mimics the behavior of operator[] on std::map which also adds an object when the key does not already exist.

If so, is there some alternative const accessor we can use? For now, we are working around this by copying the object.

AFAIK, this is not such API. To avoid copying, your best bet is probably a similar solution mentioned by @brouer. Here is documentation of basic_json::at(json_pointer&) that covers different scenarios.

I'd rather not be throwing and catching in normal paths though - would be nice if count() worked with json pointer too.

That's very good point. Though I think adding an overload to basic_json::contains() that accepts json_pointer would be more fitting. @nlohmann, @garethsb-sony Any thoughts?

@nlohmann
Copy link
Owner

I understand the issue. The implementation of JSON Pointers is a bit hairy. I agree that contains or count should be available for JSON Pointers too.

@stale
Copy link

stale bot commented Jun 23, 2019

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 Jun 23, 2019
@nlohmann nlohmann added good first issue and removed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated labels Jun 24, 2019
@nlohmann
Copy link
Owner

I added a contains function for JSON pointers, see 258fa79. Feedback is greatly appreciated! @sdaly2107 @brouer @nickaein

@nlohmann nlohmann self-assigned this Jun 30, 2019
@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed good first issue labels Jun 30, 2019
@sdaly2107
Copy link
Author

Looks good to me, thank you very much :D

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2019


🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description


@nlohmann nlohmann added this to the Release 3.6.2 milestone Jul 9, 2019
@nlohmann nlohmann closed this as completed Jul 9, 2019
@nlohmann nlohmann modified the milestones: Release 3.6.2, Release 3.7.0 Jul 9, 2019
@nlohmann nlohmann added this to the Release 3.7.0 milestone Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement/improvement release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

4 participants