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

[BUG] JSONPath.set not working for array filter expressions #1469

Closed
akelsch opened this issue May 12, 2023 · 12 comments
Closed

[BUG] JSONPath.set not working for array filter expressions #1469

akelsch opened this issue May 12, 2023 · 12 comments
Labels
bug Something isn't working fixed
Milestone

Comments

@akelsch
Copy link
Contributor

akelsch commented May 12, 2023

Problem description

We expect JSONPath.set to successfully set a value into the first segment of a array filter expression but nothing happens.

Environment information

Version info: Fastjson2 2.0.31

Steps to reproduce

Example JSON file:

{
  "posts": [
    {
      "id": 1,
      "title": "ABC"
    },
    {
      "id": 2,
      "title": "DEF"
    }
  ]
}

Example Java code:

import com.alibaba.fastjson2.JSON;
import com.alibaba.fastjson2.JSONObject;
import com.alibaba.fastjson2.JSONPath;

public class JsonPathApp {
    public static void main(String[] args) {
        JSONObject posts = JSON.parseObject(Main.class.getResourceAsStream("/posts.json"));
        JSONObject target = new JSONObject();

        JSONPath jsonPath = JSONPath.of("$.posts[?(@.id == 1)]");
        Object source = jsonPath.eval(posts);
        jsonPath.set(target, source);

        System.err.println(target); // Actual: {} | Expected: {"posts":[{"id":1,"title":"ABC"}]}
    }
}

Expected correct result

In above code eval is evaluated correctly but set does nothing. Instead we have to call set using JSONPath.of("$.posts") which we do by modifying existing JSONPath string: StringUtils.substringBefore("$.posts[?(@.id == 1)]", "["). This might be unstable in case of nested array (multiple '[' character) so we expect the library to handle this case.

We could have handled the case ourselves if JSONPathTwoSegment class would expose value first which in this case is partners. There are no methods in JSONPath abstract class to 1) expose all segment paths and 2) determine if it is an array filter expression either so everything is internal to the library with no access by users.

@akelsch akelsch added the bug Something isn't working label May 12, 2023
@wenshao wenshao added this to the 2.0.32 milestone May 12, 2023
@wenshao
Copy link
Member

wenshao commented May 12, 2023

https://oss.sonatype.org/content/repositories/snapshots/com/alibaba/fastjson2/fastjson2/2.0.32-SNAPSHOT/
update version to 2.0.32-SNAPSHOT, use setCallBack, such as:

public class Issue1469 {
    @Test
    public void test() {
        JSONObject posts = JSON.parseObject(STR);

        JSONPath jsonPath = JSONPath.of("$.posts[?(@.id == 1)]");
        jsonPath.setCallback(posts, e -> {
            JSONObject object = ((JSONObject) e).clone();
            object.put("title", "XXX");
            return object;
        });

        assertEquals(
                "{\"posts\":[{\"id\":1,\"title\":\"XXX\"},{\"id\":2,\"title\":\"DEF\"}]}",
                posts.toJSONString()
        );
    }

    static final String STR = "{\n" +
            "  \"posts\": [\n" +
            "    {\n" +
            "      \"id\": 1,\n" +
            "      \"title\": \"ABC\"\n" +
            "    },\n" +
            "    {\n" +
            "      \"id\": 2,\n" +
            "      \"title\": \"DEF\"\n" +
            "    }\n" +
            "  ]\n" +
            "}";
}

@akelsch
Copy link
Contributor Author

akelsch commented May 12, 2023

Thanks @wenshao, I see setCallback is supported for JSONPathFilter in this version but I am not sure if it can fix our issue. During setCallback I am not aware of current path in JSON tree so that I can put it in the according target JSONObject.

Screenshot 2023-05-12 at 15 43 31

We basically try to achieve the opposite of JSONPath.remove because JSONPath.eval does return [{"id":1,"title":"ABC"}] instead of {"posts":[{"id":1,"title":"ABC"}]}. Is it possible for eval to return the matching JSON tree? Then we would not need support for set.

@wenshao
Copy link
Member

wenshao commented May 12, 2023

setCallback support BiFunction, such as :

  @Test
    public void test1() {
        JSONObject posts = JSON.parseObject(STR);

        JSONPath jsonPath = JSONPath.of("$.posts[?(@.id == 1)]");
        jsonPath.setCallback(posts, (parent, value) -> {
            JSONArray array = (JSONArray) parent;
            array.add(JSONObject.of("id", 3));
            return value;
        });

        assertEquals(
                "{\"posts\":[{\"id\":1,\"title\":\"ABC\"},{\"id\":2,\"title\":\"DEF\"},{\"id\":3}]}",
                posts.toJSONString()
        );
    }

wenshao added a commit that referenced this issue May 12, 2023
@akelsch
Copy link
Contributor Author

akelsch commented May 15, 2023

@wenshao thanks, I understand, but we do not try to modify the existing object. We want a copy of it including tree structure/schema. The goal is to create a JSONObject of {"posts":[{"id":1,"title":"ABC"}]}. How could we achieve that without modifying the original JSONPath of $.posts[?(@.id == 1)]? Currently, we have to cut everything after '[' character for it to work.

@wenshao wenshao modified the milestones: 2.0.32, 2.0.33 May 16, 2023
@335184181
Copy link

@wenshao @akelsch 是我们同事,你可能没有理解我们的问题,我们能不能通过微信或者钉钉通话一下 给你解释一下; 如果方便的话,您可以留一下您的微信或者钉钉号吗

@wenshao
Copy link
Member

wenshao commented May 16, 2023

https://oss.sonatype.org/content/repositories/snapshots/com/alibaba/fastjson2/fastjson2/2.0.33-SNAPSHOT/
update version to 2.0.33-SNAPSHOT, JSONPath now support getParent, such as:

JSONPath path = JSONPath.of("$.posts[?(@.id == 1)]");
JSONPath p1 = path.getParent();
assertEquals("$.posts", p1.toString());

JSONPath p2 = p1.getParent();
assertEquals("$", p2.toString());

JSONPath p3 = p1.getParent();
assertNull(p3);

@akelsch is it what you want?

@wenshao
Copy link
Member

wenshao commented May 16, 2023

@wenshao @akelsch 是我们同事,你可能没有理解我们的问题,我们能不能通过微信或者钉钉通话一下 给你解释一下; 如果方便的话,您可以留一下您的微信或者钉钉号吗

我的微信: wenshaojin

@akelsch
Copy link
Contributor Author

akelsch commented May 17, 2023

https://oss.sonatype.org/content/repositories/snapshots/com/alibaba/fastjson2/fastjson2/2.0.33-SNAPSHOT/ update version to 2.0.33-SNAPSHOT, JSONPath now support getParent, such as:

JSONPath path = JSONPath.of("$.posts[?(@.id == 1)]");
JSONPath p1 = path.getParent();
assertEquals("$.posts", p1.toString());

JSONPath p2 = p1.getParent();
assertEquals("$", p2.toString());

JSONPath p3 = p1.getParent();
assertNull(p3);

@akelsch is it what you want?

Actually yes, this is really useful because we can use JSONPath.getParent for JSONPath.set operation. I did quickly test it and it seems to work 👍 For "$.abc.posts[?(@.id == 1)]" is also returns "$.abc.posts" which is exactly what I would expect. Thank you!

How do you suggest should we determine if it is an array filter expression and we have to use getParent? We cannot use instanceof JSONPathFilter or similar because the class is not public right? We could check raw JSONPath string for "[" I guess.

@wenshao
Copy link
Member

wenshao commented May 17, 2023

https://oss.sonatype.org/content/repositories/snapshots/com/alibaba/fastjson2/fastjson2/2.0.33-SNAPSHOT/
update snapshot version 2.0.33-SNAPSHOT, JSONPath now support endsWithFilter, such as:

JSONPath path = JSONPath.of("$.posts[?(@.id == 1)].id");

assertFalse(path.endsWithFilter());

JSONPath p0 = path.getParent();
assertEquals("$.posts[?(@.id == 1)]", p0.toString());

assertTrue(p0.endsWithFilter());

JSONPath p1 = p0.getParent();
assertEquals("$.posts", p1.toString());
assertFalse(p1.endsWithFilter());

JSONPath p2 = p1.getParent();
assertEquals("$", p2.toString());
assertFalse(p2.endsWithFilter());

@wenshao wenshao added the fixed label May 20, 2023
@akelsch
Copy link
Contributor Author

akelsch commented May 22, 2023

@wenshao sorry for the delay. This works perfectly and covers our use case, thank you. Please release it to Maven Central for productive usage.

@wenshao
Copy link
Member

wenshao commented May 29, 2023

https://github.com/alibaba/fastjson2/releases/tag/2.0.33
2.0.23 released

@wenshao wenshao closed this as completed May 29, 2023
@akelsch
Copy link
Contributor Author

akelsch commented May 31, 2023

@wenshao it seems not to work with published version anymore?

JSONPath p3 = JSONPath.of("$.posts[?(@.id == 1)]");
assertTrue(p3.endsWithFilter());

This fails because p3 is instance of JSONPathTwoSegment but implementation is only done for JSONPathMulti. Could you have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed
Projects
None yet
Development

No branches or pull requests

3 participants