jsonpath icon indicating copy to clipboard operation
jsonpath copied to clipboard

`.apply` skips the last path

Open PsyGik opened this issue 1 year ago • 7 comments

given the code below:

var jp = require("jsonpath")
var data = {
  "store": {
    "book": [ 
      {
        "category": "reference",
        "author": "Nigel Rees",
        "title": "Sayings of the Century",
        "price": 8.95
      }, {
        "category": "fiction",
        "author": "Evelyn Waugh",
        "title": "Sword of Honour",
        "price": 12.99
      }, {
        "category": "fiction",
        "author": "Herman Melville",
        "title": "Moby Dick",
        "isbn": "0-553-21311-3",
        "price": 8.99
      }, {
         "category": "fiction",
        "author": "J. R. R. Tolkien",
        "title": "The Lord of the Rings",
        "isbn": "0-395-19395-8",
        "price": 22.99
      }
    ],
    "bicycle": {
      "color": "red",
      "price": 19.95
    }
  }
}
var nodes = jp.apply(data, '$..author', function(value) { return value.toUpperCase() });
console.log(nodes)

Expected:

[
{"path":["$","store","book",0, "author"],"value":"NIGEL REES"},
{"path":["$","store","book",1, "author"],"value":"EVELYN WAUGH"},
{"path":["$","store","book",2, "author"],"value":"HERMAN MELVILLE"},
{"path":["$","store","book",3, "author"],"value":"J. R. R. TOLKIEN"}
]

Actual:

[
{"path":["$","store","book",0],"value":"NIGEL REES"},
{"path":["$","store","book",1],"value":"EVELYN WAUGH"},
{"path":["$","store","book",2],"value":"HERMAN MELVILLE"},
{"path":["$","store","book",3],"value":"J. R. R. TOLKIEN"}
]

Runkit example: https://runkit.com/psygik/6538cdcc485c4f000831f0dc

PsyGik avatar Oct 25 '23 08:10 PsyGik

can confirm. about to write the same bug report.

apiwat-chantawibul avatar Nov 08 '23 06:11 apiwat-chantawibul

might have something to do with the use of pop in this line:

https://github.com/dchester/jsonpath/blob/c1dd8ec74034fb0375233abb5fdbec51ac317b4b/lib/index.js#L42

apiwat-chantawibul avatar Nov 08 '23 07:11 apiwat-chantawibul

This hot fix seems to work. Need reviews though, I just learn nodejs 2 weeks ago.

var assert = require('assert');
var jp = require('jsonpath');

jp.apply = function(obj, string, fn) {

  assert.ok(obj instanceof Object, "obj needs to be an object");
  assert.ok(string, "we need a path");
  assert.equal(typeof fn, "function", "fn needs to be function")

  var nodes = this.nodes(obj, string).sort(function(a, b) {
    // sort nodes so we apply from the bottom up
    return b.path.length - a.path.length;
  });

  nodes.forEach(function(node) {
    var key = node.path.slice(-1);
    var parent = this.value(obj, this.stringify(node.path.slice(0, -1)));
    var val = node.value = fn.call(obj, parent[key]);
    parent[key] = val;
  }, this);

  return nodes;
}

apiwat-chantawibul avatar Nov 08 '23 07:11 apiwat-chantawibul

you can update the tests https://github.com/dchester/jsonpath/blob/master/test/query.js to ensure it works?

PsyGik avatar Nov 08 '23 11:11 PsyGik

you can update the tests https://github.com/dchester/jsonpath/blob/master/test/query.js to ensure it works?

Sure, but after looking around, I think https://github.com/dchester/jsonpath/blob/master/test/sugar.js might be a better place. The rationale is jp.apply probably counts as a syntactic sugar function. Plus, there already exists some other tests of jp.apply in the /test/sugar.js

would you agree?

apiwat-chantawibul avatar Nov 09 '23 03:11 apiwat-chantawibul

One more thing @PsyGik, I just noticed that the expected result in your opening post does not match the behaviour of jp.apply describe in README.

Specifically, the document said jp.apply should "Returns matching nodes with their updated values". Your opening post implies you expect the returned values to be the original values.

I'm going to stick with the README for now.

apiwat-chantawibul avatar Nov 09 '23 04:11 apiwat-chantawibul

One more thing @PsyGik, I just noticed that the expected result in your opening post does not match the behaviour of jp.apply describe in README.

Specifically, the document said jp.apply should "Returns matching nodes with their updated values". Your opening post implies you expect the returned values to be the original values.

I'm going to stick with the README for now.

Yeah you are right. The values should have been formatted to uppercase. I was focusing on the main issue i.e missing the last key from the paths array

PsyGik avatar Nov 09 '23 05:11 PsyGik