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

Why are flattened empty objects/arrays not representable? #1874

Closed
adamshapiro0 opened this issue Dec 18, 2019 · 8 comments
Closed

Why are flattened empty objects/arrays not representable? #1874

adamshapiro0 opened this issue Dec 18, 2019 · 8 comments
Labels
kind: question state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@adamshapiro0
Copy link

When you call flatten() on an object with an empty array, the flattened value is null instead of []. As a result, when you unflatten() the data back, you do not get the original object. Unfortunately, this makes it difficult to take advantage of flattening to operate on the object tree directly without having to traverse it (e.g., remove all entries where the path contains "x").

The flatten() documentation (https://nlohmann.github.io/json/classnlohmann_1_1basic__json_ab838f000d76662917ffd6ec529569e03.html#ab838f000d76662917ffd6ec529569e03) notes that this will be the case, but it's not clear why the values must be primitives since the flattened object is just a JSON object itself. Is there a reason the values need to be primitives?

For example:

  json a = json::parse("{\"foo\": []}");
  std::cout << "Before: " << a.dump(4) << std::endl;
  json a_flat = a.flatten();
  std::cout << "Flattened: " << a_flat.dump(4) << std::endl;
  a = a_flat.unflatten();
  std::cout << "After: " << a.dump(4) << std::endl;

produces the following:

Before: {
    "foo": []
}
Flattened: {
    "/foo": null
}
After: {
    "foo": null
}

Compiler: GCC 7.4
Library version: 3.7.3 single include

@nlohmann
Copy link
Owner

The reason is that JSON pointers only allow for primitive values. Therefore, you describe exactly the edge case that is not properly representable.

@adamshapiro0
Copy link
Author

adamshapiro0 commented Dec 19, 2019

Ah, I suppose that makes sense. Thanks so much for the quick response!

It's unfortunate it seems that there a few features missing from JSON pointer/patch that would be so useful. This is certainly one - seems like it would be pretty trivial to allow a pointer to refer to a structured object and have the value for the "flattened" output above be [] in the case of an empty array.

On a possibly related side note, flattening an empty json::object() outputs an object with size 1 ({"": null}), not 0 ({}). Not sure if that's intentional?

A second related feature would be to allow add operations where the pointer ends in /- to create an array if it doesn't exist already. Technically - is a valid key so it's ambiguous if you are referencing an array or object if it doesn't exist, but the JSON patch spec doesn't specify that I can find. This would allow patches to create and extend arrays without requiring someone to define some base structure ahead of time. In my opinion, using - to reference an array in an add operation is probably a lot more likely than an object.

The third missing piece as I see it is to have a merge operation for JSON patch so, for example, you could do:

Base: {
    "foo": [
        {"a": 1}
    ]
}
Patch: [{"op": "merge", "path": "/foo/0", "value": {"b": 2, "c": 3}}]
Result: {
    "foo": [
        {"a": 1, "b": 2, "c": 3}
    ]
}

You could definitely break this patch up into two add operations, one for b and one for c, but that is a lot less sustainable when you want to modify a large number of elements on an existing object. It also wouldn't work if the values in the patch were objects instead of primitives, and needed to be merged with existing keys. JSON merge can do it, but unfortunately not if you need to reference an array element like the example above.

Obviously these aren't necessarily spec, but I'm curious what you think?

@nlohmann
Copy link
Owner

I do not understand your proposal. Could you elaborate?

@stale
Copy link

stale bot commented Jan 22, 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 Jan 22, 2020
@adamshapiro0
Copy link
Author

Apologies for the delayed reply! Happy to elaborate. I was thinking the following:

Patch add Array Creation

Currently, doing an add to an array that doesn't exist does not create an array:

Starting object:
{}

Patch:
[{"op": "add", "path": "/foo/-", "value": 123}]

Expected result:
{
    "foo": [123]
}

Actual result:
{
    "foo": {
        "-": 123
    }
}

Since the JSON patch spec doesn't specify what to do in the case of - for a non-existent object, I think creating an array is a reasonable behavior.

Patch merge Operation

This would be a bit outside the JSON patch standard, but would be very useful. JSON patch has a replace operation, but not a merge or modify operation to partially modify an existing element. It would be nice to have a merge operator that worked similarly to JSON merge, merging the value object into the existing element:

Starting object:
{
    "foo": [
        {"a": 1}
    ]
}

Patch:
[{"op": "merge", "path": "/foo/0", "value": {"b": 2, "c": 3}}]

Result:
{
    "foo": [
        {"a": 1, "b": 2, "c": 3}
    ]
}

@stale stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jan 22, 2020
@nlohmann
Copy link
Owner

I need to check whether the described behavior for

[{"op": "add", "path": "/foo/-", "value": 123}]

is standard compliant.

I am afraid adding a merge would not be compliant, so adding it would not be portable.

@nlohmann
Copy link
Owner

http://jsonpatch.com states:

Finally, if you need to refer to the end of an array you can use - instead of an index. For example, to refer to the end of the array of biscuits above you would use /biscuits/-. This is useful when you need to insert a value at the end of an array.

But in your example, you start with an empty object {}. As - is also a valid object key, I think the current behavior is less surprising as converting an object to an array and interpreting - as the end of it. Applying the patch to an empty array has the expected behavior:

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

using json = nlohmann::json;

int main() {
    json ja = json::array();
    json jo = json::object();

    json p = R"([{"op": "add", "path": "/-", "value": 123}])"_json;
    
    std::cout << ja.patch(p) << std::endl;
    std::cout << jo.patch(p) << std::endl;
}

Output:

[123]
{"-":123}

@stale
Copy link

stale bot commented Mar 17, 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 Mar 17, 2020
@stale stale bot closed this as completed Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question 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

2 participants