IBM / JSONata4Java

Open Source Java version of JSONata
Apache License 2.0
90 stars 37 forks source link

Object sorting grouping is not implemented #15

Open wnm3 opened 5 years ago

wnm3 commented 5 years ago

As described at https://docs.jsonata.org/sorting-grouping the object grouping using a key : value filter is not implemented.

mgarrigo commented 3 years ago

I was about to open a new issue and found this. Was trying to evaluate the following expression:

Account.Order{OrderID:Product}

For the Invoice JSON from the playground, and got an empty JsonNode. Any alternative to do such transformation with current implementation?

PS: Sample code attached jsonata-test.zip

wnm3 commented 3 years ago

Thank you for the example. The problem is related to how I'm processing the object references. I'm getting the keys from the ObjectID (order103 and order104) and then I'm getting the values from the Product, but I'm not doing the later in the context of the former. So, I'm getting 4 Products and 2 keys but I don't gather the information needed to align which products went with which order.

I think the entire approach needs to be rethought as this relates to another issue #139 (see my final comments there).

I'll have to sleep on this and may be able to work on it this weekend.

mgarrigo commented 3 years ago

Excellent @wnm3! Thanks for the quick response

Some additional info that might be of help, I've tried with different versions of the JSONata4Java lib, and had different results: 1.5.2 returns an empty JsonNode as explained in my previous comment:

{ }

1.5.1 throws an exception:

Exception in thread "main" java.lang.ClassCastException: class com.api.jsonata4java.expressions.generated.MappingExpressionParser$IdContext cannot be cast to class org.antlr.v4.runtime.tree.TerminalNodeImpl (com.api.jsonata4java.expressions.generated.MappingExpressionParser$IdContext and org.antlr.v4.runtime.tree.TerminalNodeImpl are in unnamed module of loader 'app')
    at com.api.jsonata4java.expressions.ExpressionsVisitor.visitFieldList(ExpressionsVisitor.java:1727)
    at com.api.jsonata4java.expressions.ExpressionsVisitor.visitFieldList(ExpressionsVisitor.java:93)
    at com.api.jsonata4java.expressions.generated.MappingExpressionParser$FieldListContext.accept(MappingExpressionParser.java:1394)
    at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visitChildren(AbstractParseTreeVisitor.java:46)
    at com.api.jsonata4java.expressions.generated.MappingExpressionBaseVisitor.visitObject(MappingExpressionBaseVisitor.java:230)
    at com.api.jsonata4java.expressions.generated.MappingExpressionParser$ObjectContext.accept(MappingExpressionParser.java:783)
    at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18)
    at com.api.jsonata4java.expressions.ExpressionsVisitor.visit(ExpressionsVisitor.java:799)
    at com.api.jsonata4java.expressions.ExpressionsVisitor.visitTree(ExpressionsVisitor.java:2524)
    at com.api.jsonata4java.expressions.Expressions.evaluate(Expressions.java:185)
    at com.mgarrigo.jsonata.JsonataTest.main(JsonataTest.java:91)

1.5.0 actually returns something:

[ {
  "OrderID" : "order103",
  "Product" : [ {
    "Product Name" : "Bowler Hat",
    "ProductID" : 858383,
    "SKU" : "0406654608",
    "Description" : {
      "Colour" : "Purple",
      "Width" : 300,
      "Height" : 200,
      "Depth" : 210,
      "Weight" : 0.75
    },
    "Price" : 34.45,
    "Quantity" : 2
  }, {
    "Product Name" : "Trilby hat",
    "ProductID" : 858236,
    "SKU" : "0406634348",
    "Description" : {
      "Colour" : "Orange",
      "Width" : 300,
      "Height" : 200,
      "Depth" : 210,
      "Weight" : 0.6
    },
    "Price" : 21.67,
    "Quantity" : 1
  } ]
}, {
  "OrderID" : "order104",
  "Product" : [ {
    "Product Name" : "Bowler Hat",
    "ProductID" : 858383,
    "SKU" : "040657863",
    "Description" : {
      "Colour" : "Purple",
      "Width" : 300,
      "Height" : 200,
      "Depth" : 210,
      "Weight" : 0.75
    },
    "Price" : 34.45,
    "Quantity" : 4
  }, {
    "ProductID" : 345664,
    "SKU" : "0406654603",
    "Product Name" : "Cloak",
    "Description" : {
      "Colour" : "Black",
      "Width" : 30,
      "Height" : 20,
      "Depth" : 210,
      "Weight" : 2
    },
    "Price" : 107.99,
    "Quantity" : 1
  } ]
} ]

Notice that the products correspond to the OrderID. Although it's different from the playground as It returns an Array with the structure:

[
  {
    "OrderID" : "id",
    "Product" : [ products... ]
  },
  ...
]

Instead of being:

{
  "id1" : [ products from order with id1 ],
  "id2" : [ products from order with id2 ],
  ...
}
aandres-medallia commented 3 years ago

hi @wnm3, I've taken a quick look into this issue and I have a proposed fix. I'm attaching a patch, would you mind taking a look into it? issue15-patch.txt (you can apply it by doing patch -p 1 -i issue15-patch.txt in the repository)

wnm3 commented 3 years ago

Thank you very much @aandres-medallia . I really appreciate your looking into this. I've added your patch and will release 1.5.3 in a few hours. If you are interested, there is still a case where we don't report the correct result:

Account.Order.Product { Product Name: $.{"Price": Price, "Qty": Quantity} } returns:

{
  "Bowler Hat" : {
    "Price" : 34.45,
    "Qty" : 4
  },
  "Trilby hat" : {
    "Price" : 21.67,
    "Qty" : 1
  },
  "Cloak" : {
    "Price" : 107.99,
    "Qty" : 1
  }
}

but should return an additional Bowler Hat element (in bold below)

{ "Bowler Hat": [ { "Price": 34.45, "Qty": 2 }, { "Price": 34.45, "Qty": 4 } ], "Trilby hat": { "Price": 21.67, "Qty": 1 }, "Cloak": { "Price": 107.99, "Qty": 1 } }

wnm3 commented 3 years ago

Essentially, if the key exists, make it an array and add the additional object...

aandres-medallia commented 3 years ago

thanks @wnm3 for looking into this and providing a case that was not supported (indeed, I was not aware of such behavior un jsonata) I'll send you a patch for the remaining part in a few minutes (so, if you haven't released yet, maybe you can wait some minutes)

aandres-medallia commented 3 years ago

@wnm3 I'm sending you a patch when we have multiple keys with the same name. Maybe we can include it in the same release: issue15_multiple_keys_patch.txt Let me know if that works for you. Thank you again!

wnm3 commented 3 years ago

I already pushed 1.5.3 so will push as part of 1.5.4 asap (later today)

wnm3 commented 3 years ago

@aandres-medallia I admit, I'm taking advantage of you ;^) but love that you are willing to help. I'll go ahead and release this update as 1.5.4 (knowing there are still some errors in compatibility).

I've taken this opportunity to try to upgrade the testing to be compliant with jsonata 1.8.4 (was at 1.8.3 see pom.xml to see changes).

I've also taken this opportunity to upgrade the Eclipse environment to 2021-03.

I've uncommented out some of the test cases for object construction in the AgnosticTestSuite and in 1.8.4 these problems remain... You can see the tests and datasets they reference in the target/jsonata-1.8.4/test/test-suite/groups or datasets. You need to execute a Maven Build using goals: clean install (could do in JSONata4Java project directory from a command line using:

mvn clean install

Note: you can pass the path of a dataset into the Tester to use as its data for testing (I've begun collecting datasets and arrays in the root of the Project but in many cases can use the datasets from the target/jsonata... area)

Please note: the fixes have allowed us to pass cases: 008, 009, 010, 012, 014, 015, 016, 018, 019, and 020 so this is a tremendous improvement. Thank you.

You can see the skipped cases for object construction between lines 126 and 133 in AgnosticTestSuite.java:

SKIP_CASES("object-constructor", /* "case008", "case009", "case010", */ 
  "case011", 
  /* "case012", */
  "case013", 
  /* "case014", "case015", "case016", */
  "case017", 
  /* "case018", "case019", "case020", */
  "case022", "case025");

target/jsonata-1.8.4/test/test-suite/groups/object-constructor/case011.json:

Account.Order.Product{$string(ProductID): (Price)[0]} expects to return:

{
        "345664": 107.99,
        "858236": 21.67,
        "858383": 34.45
}

but we are returning an extra price now:

{
  "858383" : [ 34.45, 34.45 ],
  "858236" : 21.67,
  "345664" : 107.99
}

target/jsonata-1.8.4/test/test-suite/groups/object-constructor/case013.json: Account.Order.Product{ProductID: "Product Name"} expects to return an error message because the key should be a String (we probably converted it to a String automatically but should not):

Key in object structure must evaluate to a string; got: 858383

but we returned:

{
  "858383" : [ "Product Name", "Product Name" ],
  "858236" : "Product Name",
  "345664" : "Product Name"
}

target/jsonata-1.8.4/test/test-suite/groups/object-constructor/case017.json:

Account.Order.Product{$."Product Name": Price, $."Product Name": Price} using ./target/jsonata/jsonata-1.8.4/test/test-suite/datasets/dataset5.json should return an error: Multiple key definitions evaluate to same key: "Bowler Hat" but it returns:

{
  "Bowler Hat" : [ 34.45, 34.45 ],
  "Trilby hat" : 21.67,
  "Cloak" : 107.99
}

which is the expected return from: Account.Order.Product{$."Product Name": Price} so, there must be some different rules for when to create the array and when to complain about overwriting an existing key. It would seem this test case protects against the 2nd set of values being added to the object should not overwrite existing keys, whereas the example with only one specification for what should be added allows for aggregation of the prices.

target/jsonata-1.8.4/test/test-suite/groups/object-constructor/case022.json:

Phone{type: $join(number, ", "), "phone":number} using ./target/jsonata/jsonata-1.8.4/test/test-suite/datasets/dataset1.json expects:

{
        "home": "0203 544 1234",
        "phone": [
            "0203 544 1234",
            "01962 001234",
            "01962 001235",
            "077 7700 1234"
        ],
        "office": "01962 001234, 01962 001235",
        "mobile": "077 7700 1234"
}

but we return the office as an array of strings rather than a concatenation of strings

{
  "home" : "0203 544 1234",
  "phone" : [ "0203 544 1234", "01962 001234", "01962 001235", "077 7700 1234" ],
  "office" : [ "01962 001234", "01962 001235" ],
  "mobile" : "077 7700 1234"
}

target/jsonata-1.8.4/test/test-suite/groups/object-constructor/case025.json: Uses lambda's which we don't support yet...