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

Mutable NodeList or modifying query results #66

Open
robert-mac-wbt opened this issue Oct 30, 2023 · 6 comments
Open

Mutable NodeList or modifying query results #66

robert-mac-wbt opened this issue Oct 30, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@robert-mac-wbt
Copy link

robert-mac-wbt commented Oct 30, 2023

Hello 👋

I wanted to ask if it's possible to do something following with this crate.
I want to be able to run multiple queries on some json in order to eventually find a node that I'd like to modify/remove.
In the following example, I'm trying to remove genre of the second book in the list.

use serde_json::Value;
use serde_json_path::{JsonPath, JsonPathExt};

fn main() {
    let data = r#"
    {
        "books": [
            {
                "title": "Title 1",
                "details": {
                    "description": "Description 1",
                    "genre": "Genre 1"
                }

            },
            {
                "title": "Title 2",
                "details": {
                    "description": "Description 1",
                    "genre": "Genre 2"
                }
            }
        ]
    }
  "#;

    let some_json: Value = serde_json::from_str(data).expect("failed to read file");
    let books_path = JsonPath::parse("$.books").unwrap();
    let genre_path = JsonPath::parse("$.details.genre").unwrap();

    let nodes = some_json.json_path(&books_path).exactly_one().unwrap().as_array().unwrap();
    let second_node = nodes.get(1).unwrap();
    let genre_node = second_node.json_path(&genre_path).exactly_one().unwrap();

    //  remove genre_node
}

Do you have any suggestion how to achieve this?
Even after changing second_node to an object, I'm getting an error saying that it's not mutable.
Or maybe adding mutable nodelists could help here? Currently after querying &Value are returned.

Thanks!

@hiltontj
Copy link
Owner

Hey, thanks for raising the issue. Currently, yielding mutable references through the query is not supported by the crate.

I will keep this issue open to track and discuss that possibility, but I can't make any guarantees on whether/when it can be implemented.

For now, the closest option is to clone the references of the node list, however, that may not be desirable.

@robert-mac-wbt
Copy link
Author

Thank you! I'll try out this approach 😉

@hiltontj
Copy link
Owner

Mutable NodeList

The current NodeList is just a wrapper around a list of Value references, like so:

struct NodeList<'a>(Vec<&'a Value>);

Where the lifetime 'a comes from the lifetime of the original Value that was queried. This works just fine because we can hold as many immutable references to that Value as we want.

I suppose I could try it, but I am almost certain that this would not translate to mutable references, i.e., the following would not be possible:

struct NodeListMut<'a>(Vec<&'a mut Value>);

because we would be trying to hold multiple mutable references to the same data, which wouldn't compile.

Alternatively, we could take an iterative approach, whereby we yield mutable references one by one, to ensure exclusive access. This might look like so:

let path = JsonPath::parse("$.foo.*")?;
let mut value = json!({ "foo": [1, 2, 3] });
for node in path.query_iter_mut() {
  // node is &mut Value, you can mutate it as you please
}

I think this would be a nice feature for the crate, but could be fairly involved to get it implemented, if it is possible. I may start by toying with immutable iteration, i.e., a query_iter method, and go from there. As mentioned, I cannot make any guarantees on if/when this would be released.

@hiltontj hiltontj added the enhancement New feature or request label Nov 10, 2023
@hiltontj
Copy link
Owner

Alternatively, we could take an iterative approach, whereby we yield mutable references one by one, to ensure exclusive access.

After some digging, I am not sure that this is possible without the use of unsafe, nor that it can be done safely, as it is in the standard library. There is a good discussion about this here. One of the key points in that discussion that I am concerned about, when writing our own mutable iterator:

[...] don't create multiple mutable references that alias. If you did, this would violate the Rust rules and invoke undefined behavior.

JSONPath allows for queries that yield the same thing more than once, i.e., that alias.

@hiltontj
Copy link
Owner

@robert-mac-wbt - the recent release of 0.6.5, although it does not close this issue, may provide a solution for you.

The new NormalizedPath type, which can be used to produce a JSON Pointer, can therefore be used alongside the serde_json::pointer_mut method in a way that allows you to mutate the original serde_json::Value.

Here is an example,

use serde_json::json;
use serde_json_path::JsonPath;

fn main() {
    // Some JSON data:
    let mut v = json!({
        "foo": {
            "bar": {
                "baz": 1,
            },
            "baz": 2,
        },
        "baz": 3,
    });
    // locate a set of nodes using your JSONPath query:
    let path = JsonPath::parse("$..baz").expect("valid JSONPath");
    // get their locations, as JSON Pointer strings:
    let locations = path
        .query_located(&v)
        .locations()
        .map(|l| l.to_json_pointer())
        .collect::<Vec<_>>();
    // iterate over each location, and get a mutable ref to the node in the original object:
    for l in locations {
        if let Some(n) = v.pointer_mut(&l) {
            *n = false.into(); // <-- update your nodes in-place here
        }
    }
    println!("{}", serde_json::to_string_pretty(&v).unwrap());
}

This will print the following:

{
  "baz": false,
  "foo": {
    "bar": {
      "baz": false
    },
    "baz": false
  }
}

@robert-mac-wbt
Copy link
Author

Thank you!
This looks pretty promising, I'm going to need a bit of time to play with it, but thank you so much for following up on that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants