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

Handle flat JSON escape dot syntax #37

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Conversation

gquittet
Copy link
Contributor

@gquittet gquittet commented Jun 3, 2024

The goal is to support this kind of JSON

{"firstName.lastName": "Guillaume"}

using this rule:

{"===": [{"var": "firstName\\.lastName"}, "Guillaume"]}

It's already implemented in the json-logic-engine: https://github.com/TotalTechGeek/json-logic-engine
See:

I already made the code for you but I'm not a Rust developer so feel free to edit the code as you wish 😉

I tested my code using the following Python code:

import jsonlogic_rs


def main() -> None:
    schema = {"===": [{"var": "firstName\\.lastName"}, "Guillaume"]}
    data = {"firstName.lastName": "Guillaume"}
    assert jsonlogic_rs.apply(schema, data) is True


if __name__ == "__main__":
    main()

@gquittet
Copy link
Contributor Author

gquittet commented Jun 3, 2024

Hello @jstewmon @mplanchard 👋

Can you look at it when you have time? 👀

Thank you very much! 🙏

@gquittet
Copy link
Contributor Author

gquittet commented Jun 3, 2024

I also tested it using this code and it's also working great!

import jsonlogic_rs


def main() -> None:
    schema = {
        "or": [
            {"===": [{"var": "a\\.b"}, "Guillaume"]},
            {"===": [{"var": "a\\.b.Name"}, "Guillaume"]},
        ]
    }
    data1 = {"a.b": "Guillaume"}
    data2 = {"a.b": {"Name": "Guillaume"}}
    assert jsonlogic_rs.apply(schema, data1) is True
    assert jsonlogic_rs.apply(schema, data2) is True


if __name__ == "__main__":
    main()

@gquittet gquittet force-pushed the master branch 2 times, most recently from d7b8eea to 82e3b77 Compare June 3, 2024 13:04
@gquittet
Copy link
Contributor Author

Hello @jstewmon and @mplanchard 👋

Can you look at this pull request when you have time? 👀

Thank you! 🤟

@gquittet
Copy link
Contributor Author

gquittet commented Jul 1, 2024

Hello @jstewmon and @mplanchard 👋

Can you look at this pull request when you have time? 👀

Thank you! 🤟

I'm using this version in prod since 1 month and everything is running fine 💪

Is it possible to be a co-maintainer to be able to update to project with new Python version and to help you to review + merge incoming pull requests?

Copy link
Contributor

@jstewmon jstewmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we want to use \ as an escape character, the solution should be a little more robust.

For example, a key whose unescaped value is foo\.bar should be supported. The JSON value for foo\.bar would be "foo\\\\\\\\\\\\.bar" 😵‍💫 .

Here's a set of test cases we should add (expressed as unescaped values):

foo\.bar => [foo.bar]
foo\\.bar => [foo\, bar]
foo\\\.bar => [foo\.bar]
foo\\bar => [foo\bar]

I asked Copilot "rust function to split a string by a character, using backslash as escape character", and it suggested the following:

pub fn split_with_escape(input: &str, delimiter: char) -> Vec<String> {
    let mut result = Vec::new();
    let mut current = String::new();
    let mut escape = false;

    for c in input.chars() {
        if escape {
            current.push(c);
            escape = false;
        } else if c == '\\' {
            escape = true;
        } else if c == delimiter {
            result.push(current.clone());
            current.clear();
        } else {
            current.push(c);
        }
    }

    if !current.is_empty() {
        result.push(current);
    }

    result
}

The Copilot suggestion LGTM if you'd like to use it to amend your PR.

gquittet added a commit to gquittet/json-logic-rs that referenced this pull request Jul 1, 2024
gquittet added a commit to gquittet/json-logic-rs that referenced this pull request Jul 1, 2024
@gquittet
Copy link
Contributor Author

gquittet commented Jul 1, 2024

@jstewmon

Great idea! 💡

I just implemented it and I added some unit tests. Thanks for the review! 🤟

@gquittet gquittet requested a review from jstewmon July 1, 2024 17:04
gquittet added a commit to gquittet/json-logic-rs that referenced this pull request Jul 1, 2024
gquittet added a commit to gquittet/json-logic-rs that referenced this pull request Jul 1, 2024
@jstewmon jstewmon merged commit 24f490a into Bestowinc:master Jul 1, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants