Async evaluation bug

[xogeny]

I've created a set of tests to reproduce a problem I've been having with evaluate. This demonstrates that if the input value is a Promise, things work mostly. But one case (involving sorting) seems to somehow deviate from the synchronous case.

Here is the code of the tests (you should be able to just drop it into jsonata-test.js):

const base_input = [
    { uri: "/foo/bar", metadata: { classes: ["foo", "bar"] } },
    { uri: "/bar", metadata: { classes: ["bar"] } },
  ];

describe.only("Bug in async?", () => {
    const asyncEval = (expr, input) => {
        return new Promise((resolve, reject) => {
            expr.evaluate(input, {}, (err, result) => {
                if (err) reject(err);
                else resolve(result);
            })
        })
    }
    it("should return an array synchronously", () => {
        var expr = jsonata("$");
        var result = expr.evaluate(base_input);
        expect(result).to.deep.equal(base_input);
    });
    it("should return a filtered array synchronously", () => {
        var expr = jsonata("$[\"bar\" in metadata.classes]");
        var result = expr.evaluate(base_input);
        expect(result).to.deep.equal([
            { uri: "/foo/bar", metadata: { classes: ["foo", "bar"] } },
            { uri: "/bar", metadata: { classes: ["bar"] } },
          ]);
    });
    it("should return a filtered and sorted array synchronously", () => {
        var expr = jsonata("$[\"bar\" in metadata.classes]^(>uri)");
        var result = expr.evaluate(base_input);
        expect(result).to.deep.equal([
            { uri: "/foo/bar", metadata: { classes: ["foo", "bar"] } },
            { uri: "/bar", metadata: { classes: ["bar"] } },
          ]);
    });
    it("should return an array asynchronously", () => {
        var expr = jsonata("$");
        return asyncEval(expr, base_input)
        .then((result) => {
            expect(result).to.deep.equal(base_input);            
        })
    });
    it("should return a filtered array asynchronously", () => {
        var expr = jsonata("$[\"bar\" in metadata.classes]");
        return asyncEval(expr, Promise.resolve(base_input))
        .then((result) => {
            expect(result).to.deep.equal([
                { uri: "/foo/bar", metadata: { classes: ["foo", "bar"] } },
                { uri: "/bar", metadata: { classes: ["bar"] } },
              ]);            
        })
    });
    it("should return a filtered and sorted array asynchronously", () => {
        var expr = jsonata("$[\"bar\" in metadata.classes]^(uri)");
        return asyncEval(expr, Promise.resolve(base_input))
        .then((result) => {
            expect(result).to.deep.equal([
                { uri: "/foo/bar", metadata: { classes: ["foo", "bar"] } },
                { uri: "/bar", metadata: { classes: ["bar"] } },
              ]);            
        })
    });
});

[xogeny]

To add a bit more detail (now that I have some time), the issue here is between the 3rd ("should return a filtered and sorted array synchronously") and the 6th ("should return a filtered and sorted array asynchronously").

The only difference between them is that the input for the 6th test is a Promise to the input used in the 3rd test. As you can see from tests 4 and 5, the Promise itself doesn't seem to cause problems in other cases. It is only with the addition of the ^(uri) that we get the issue.

The error message I get for the 6th case (causing it to fail) is:

The expressions within an order-by clause must evaluate to numeric or string values

It seems likely that the ^ operator is getting the Promise instead of the resolved value of the promise. I'll try to dig into the code tomorrow to see what is special about the ^ operator unless somebody beats me to it...

[xogeny]

So I can confirm the issue is that the value of input when evaluateSortExpresssion is called is actually a Promise and not a value. I must confess I don't quite understand how native JS values, iterators and promise are all handled in the evaluate function. I'll continue to dig to understand how these asynchronous values are being handled in other parts of the code.

[xogeny]

In digging into this, I extended the test suite to perform all tests in both synchronous and asynchronous modes. This uncovered lots of cases where the synchronous evaluation worked, but if the input was passed in as a Promise, I got errors during evaluation. So it seems clear that async support is a little uneven. After speaking with @andrew-coleman, I'm planning to take a shot at improving the support for async if I can muster the time. I'll provide updates here when there is something to report.

[andrew-coleman]

This should now be resolved by PR #219

[mattpardee]

@xogeny or anyone else: I hate to piggyback onto a tangentially related issue but it seems that the test cases here and the related solution in #219 get at an issue I'm having with async functions I've registered with registerFunction and I wanted to see if anyone had a tip.

I have a function like below that uses an external API:

expression.registerFunction('getTimezoneForLocation', async location => {
 const timezone = await remoteCall(location);
 return timezone;
})

Calling this fn through the jsonata evaluator always results in a plain object {} - I've tried:

• Reducing the innards to a simple promise
• Using the callback pattern for the evaluator (in combination with the promise)

...but nothing seems to work. Of course, removing the use of async at all and returning a dummy string value does work. Am I right to assume that this case isn't handled by jsonata or have I just made a simple mistake in my approach? Has anyone built a workaround? Thank you for any help!

[mattpardee]

Shoot, I wasn't comprehensive enough in my solution. Using the callback technique of execution did result in the correct behavior, you can ignore my above issue. e.g. using this strategy coupled with the example code above:

template.evaluate({}, data, (err, result) => {

Apologies for the back and forth!