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

Feature Request: ability to add custom operators which are not automatically traversed for evaluation #116

Open
Sykander opened this issue Nov 11, 2022 · 9 comments · May be fixed by #120

Comments

@Sykander
Copy link

Sykander commented Nov 11, 2022

Some operators, for example if, map, reduce etc. have modified scope for evaluation, so that the parameters passed into these operators aren't automatically evaluated but instead conditionally evaluated based on the operators control structure.

{ "if"[true, "only evaluated after if is run", "never gets evaluated"] }

However, it appears the ability to add such behaviour with a custom operator is not currently supported

if (op === "if" || op == "?:") {

There is already this large if-else chain within the apply method which could be easily modified to allow for custom operators to added and executed here (perhaps via some configuration when registering the custom operator), which would allow users to define whether their custom operator should automatically traverse or not.

js-logic-engine another library for json-logic also has an implementation of a similar configuration when registering a custom operator with their traverse option.

Would it be possible, to add a similar sort of functionality to this library when registering custom operators?

@Sykander
Copy link
Author

Just to be clear, I'm happy to do the work for this change, just wondering what the appetite is for new functionality like this?

@jwadhams
Copy link
Owner

OMG, sorry for the late response. Yes I think this is a really cool idea and I'd be happy to review (and consult and advise) a PR that achieves it.

@Sykander
Copy link
Author

Hey @jwadhams ,

Thanks for the response 🙏 I've raised this PR #117 which implements the feature. I've tried to match coding standards and testings styles where I could.

Please take a look and leave feedback :)

@xn330125
Copy link

Hey @jwadhams ,
is there any progress in this pr? I need this feature too. I would appreciate it if you can review this pr soon. 😀

@josephdpurcell
Copy link

I have been evaluating JSON Logic for a use case and I want the ability to insert data into the logic. You can do this with primitives, but not objects. An example of how you can do it with primitives is very simple:

"if": [
  true,
  "a",
  "b"
]

Where "a" and "b" could be any primitive, even nested arrays! Just not objects.

To prove this out, I did the following test:

jsonLogic.add_operation('constant', (value) => {
  return value;
});

Then:

console.log(jsonLogic.apply({"constant": "hello"}));

prints "hello", but this:

console.log(jsonLogic.apply({"constant": {"key": "hello"}));

throws an error: Error: Unrecognized operation key.

That took me to this ticket and the related PR. I can confirm that if I use the JSON Logic version from the PR I get the results I desire, which is:

jsonLogic.add_operation('constant', (value) => {
  return value;
}, {
  controlledExecution: true
});
console.log(jsonLogic.apply({"constant": "hello"}));
console.log(jsonLogic.apply({"constant": {"key": "hello"}));

prints:

[ "hello" ]
[ { "key": "hello" } ]

One very interesting point is that controlledExecution: false gives a different behavior than if it is true in the primitive scenario. When it is false, the returned value is the exact value passed, e.g. "hello", but when the value is true you get a modified value, "hello" is returned as ["hello"]. I presume this is because when controlledExecution is false, it returns the primitive right away at

json-logic-js/logic.js

Lines 213 to 216 in 0653b36

// You've recursed to a primitive, stop!
if ( ! jsonLogic.is_logic(logic) ) {
return logic;
}
with the depth-first evaluation, but when it is true you get the unary-ified value because of

json-logic-js/logic.js

Lines 226 to 229 in 0653b36

// easy syntax for unary operators, like {"var" : "x"} instead of strict {"var" : ["x"]}
if ( ! Array.isArray(values)) {
values = [values];
}
.

Overall, I am very interested in this controlledExecution feature. I personally would like to see two changes:

  1. A controlled execution op return the non-unary-ified value so that you can do something like a constant operation that just returns the value as-is. But, this could also simply be "fixed" with documentation that explains its up to the implementer of the custom op to de-unary-ify the value.
  2. The name controlledExecution isn't the phrasing that would make me think of this feature. The JSON Logic limitations section here: https://jsonlogic.com/add_operation.html says "Every other operator including custom operators, performs depth-first recursion." Maybe the flag could be called recursion or recurse as a bool? I wonder what other libs call such a "don't traverse me" feature.

@josephdpurcell
Copy link

I've been exploring an idea that I thought I'd share.

Let's say this feature is implemented that let's you disable the depth-first traversal. You could write your own operation that does its own traversal! Which, would let you do something like this:

console.log(jsonLogic.apply({
  "traverse": {
      "key1": "value1",
      "key2": { "var": "dataKey1" },
      "key3": {
          "if": [
              true,
              {
                  "var": "dataKey1"
              },
              "foo"
          ]
      },
      "key4": {
          "nestedKey": {
              "var": "dataKey1"
          }
      }
  }
}, { "dataKey1": "dataValue1" }));

And get back:

[
  {
    key1: 'value1',
    key2: 'dataValue1',
    key3: 'dataValue1',
    key4: { nestedKey: 'dataValue1' }
  }
]

That's pretty cool! It would be a way of having a "template" JSON structure, then traversing it to replace certain parts of it using JSON Logic.

The traversal I implemented to achieve the above did require adding an is_operation feature. I'll attach the code I scratched together here; I don't recommend using it! It's just for entertainment at this point.

Expand to view "is_operation" feature and custom traversal op
jsonLogic.is_operation = function (op) {
  if (['if', '?:', 'and', 'or', 'filter', 'map', 'reduce', 'all', 'none', 'some'].indexOf(op) !== -1) {
    return true;
  }
  if (operations.hasOwnProperty(op) && typeof operations[op] === 'function') {
    return true;
  } else if (op.indexOf('.') > 0) {
    const sub_ops = String(op).split('.');
    let operation = operations;
    for (let i = 0; i < sub_ops.length; i++) {
      if (!operation.hasOwnProperty(sub_ops[i])) {
        return false;
      }
      // Descending into operations
      operation = operation[sub_ops[i]];
    }
    return true;
  }
  return false;
};

function traverseJsonLogic(value, data, jsonlogic) {
  if (Array.isArray(value)) {
    const ret = [];
    for (const i in value) {
      if (typeof value[i] === 'object') {
        ret[i] = traverseJsonLogic(value[i], data, jsonlogic);
      } else {
        ret[i] = value;
      }
    }
    return ret;
  }
  if (typeof value === 'object' && Object.keys(value).length > 1) {
    const ret = {};
    for (const i in value) {
      if (jsonLogic.is_operation(i)) {
        throw new Error(`Operation key found on object: ${i}! An object that has multiple keys cannot have a key that is an operation.`);
      }
      ret[i] = traverseJsonLogic({[i]: value[i]}, data, jsonlogic)[i];
    }
    return ret;
  }
  if (typeof value === 'object' && !jsonLogic.is_operation(Object.keys(value)[0])) {
    const op = Object.keys(value)[0];
    return {
      [op]: traverseJsonLogic(value[op], data, jsonlogic),
    };
  }
  return jsonlogic.apply(value, data);
}

jsonLogic.add_operation('traverse', (value, data, jsonlogic) => {
  return traverseJsonLogic(value, data, jsonlogic);;
}, {
  controlledExecution: true
});

All that to say, this feature would open up significant flexibility / extensibility for JSON Logic.

@josephdpurcell
Copy link

I did a very brief look to see what "is traversable" and found:

This isn't earth shattering, but I do like the idea of simply traverse.

@josephdpurcell
Copy link

I might be having too much fun with this traverse flag idea. I did an example of a JSON filter and one with JSON Patch over here: #15 (comment).

@Sykander
Copy link
Author

Hey @jwadhams,

Any updates on when you might be able to review the linked PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants