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

feat: implement get_by_schema #110

Merged
merged 15 commits into from
Sep 19, 2024
Merged

Conversation

CPunisher
Copy link
Contributor

@CPunisher CPunisher commented Sep 8, 2024

Continuation of #108

  • array index needs specification Not support yet
  • test_failed_4 is not pass because I just do one step of skip_one in this case
  • object.get_mut(&key.deref()) could be a slow path
  • document

@liuq19
Copy link
Collaborator

liuq19 commented Sep 9, 2024

  1. we can add the jsonPointer into the field as the index, and make it easy to write the nested schema, and also support the array index.

The example can as follows:

use sonic_rs::json;

fn main() {
    let schema = json!({
        "a": null, // default value is `null`
        "b": {
            "b1": {},
            "b2": "default string" // default value is string
        },
        "c": "/1", // jsonpointer is "/1"
    });

    let data = r#"
    {
        "a": {},
        "b": {
            "b1": 123
        },
        "c": [1, 2, 3],
        "d": "balabala..."
    }"#;

    // parse json data by schem, we can parse into the schema value inplace
    let got = sonic_rs::get_by_schema(data, schema).unwrap();
    assert_eq!(
        got,
        json!({
            "a": {},
            "b": {
                "b1": 123,
                "b2": "default string"
            },
            "c": 2 // The 1 index from the array
        })
    );
}
  1. the skip one will not validate the trailing chars, so its result is expected.

@CPunisher CPunisher changed the base branch from main to v0.4 September 9, 2024 07:41
@CPunisher
Copy link
Contributor Author

CPunisher commented Sep 9, 2024

"c": "/1", // jsonpointer is "/1"

I think we can use "/c/1": "default value" instead to keep the consistance of the semantic of member values.
That means:
"c": "default value" == "/c": "default value"
"/c/1": "default value" == array index query or object member query

Here "/" is like ./ in file system. It could be somehow misleading and contradict the RFC:

Evaluation of a JSON Pointer begins with a reference to the root value of a JSON document and completes with a reference to some value within the document. Each reference token in the JSON Pointer is evaluated sequentially.

But I think it's ok because It is allowed to start with any (nested) Value and query with pointer.

Another question is that should we expand the results or keep the pointers. For example, "/c/d": 1 => "c": { "d": 2 } or "/c/d": 2, but pay attention that the query with any array index is not expandable, like "/c/1": 1 => "c": [??, 2]. I prefer the latter.

@liuq19
Copy link
Collaborator

liuq19 commented Sep 9, 2024

For the second question, if we do not expand it is a JSON pointer. The problem is how to name the key, the key is still the JSON pointer, such as "/c/d": 1 or the lastest name ``"d": 1` ? If the jsonpointer is only an index, the key name is?

Does the problem become complicated? Maybe we will not need to support the array index.

@liuq19
Copy link
Collaborator

liuq19 commented Sep 10, 2024

I think we do not need to support the array index in the schema now.

the object.get_mut(&key.deref()) is slow

are there more details?

@CPunisher
Copy link
Contributor Author

I think we do not need to support the array index in the schema now.

the object.get_mut(&key.deref()) is slow

are there more details?

Since the time complexity of object.get_mut is O(n), I use a HashMap to save the key value results.

@CPunisher CPunisher marked this pull request as ready for review September 10, 2024 06:32
src/parser.rs Show resolved Hide resolved
src/parser.rs Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
src/parser.rs Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
@liuq19 liuq19 merged commit a46586a into cloudwego:v0.4 Sep 19, 2024
13 checks passed
liuq19 pushed a commit that referenced this pull request Dec 13, 2024
liuq19 pushed a commit that referenced this pull request Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants