TotalTechGeek / json-logic-engine

Construct complex rules with JSON & process them.
MIT License
43 stars 9 forks source link

[bug] Conditional then/else only works with primitives #6

Closed scott-wyatt closed 1 year ago

scott-wyatt commented 1 year ago

@TotalTechGeek awesome job with the traversal stuff! We made the switch to this library for that functionality.

if/and/or works as expected if it returns a string, number, or boolean. However, if the if conditional returns an object, it returns undefined because https://github.com/TotalTechGeek/json-logic-engine/blob/master/defaultMethods.js#L53 returns this as a context for another rule. It seems to me that the expected behavior would be that if the context is not a rule, it should just return the object.

TLDR; If I use JSON logic conditional that returns an object that is not a rule then I get the object back, if I use JSON Logic Engine I do not.

This might all be fixed if _parse would just return data if not satisfied.

Quick way to reproduce the issue is:


    const data = {
      first_name: true
    }

    const rule = {
      if: [
        {
          "===": [
            {
              var: "first_name"
            },
            true
          ]
        },
        { first_name: "scott" },
        { first_name: "no idea" }
      ]
    }

    const result = logic.run(rule, data)

    console.log(result) // 'undefined'
TotalTechGeek commented 1 year ago

Hi!

I may explore a configuration that allows for this behavior in the near future, though I'm a bit hesitant to make it a default behavior, since I like the syntax remaining consistent as: { func: args }

If I may suggest a workaround for now, I'd recommend using preserve to describe structures that you wish to keep intact.

const rule = {
    if: [
      {
        "===": [
          {
            var: "first_name"
          },
          true
        ]
      },
      { preserve: { first_name: "scott" } },
      { preserve: { first_name: "no idea" } }
    ]
  }

In the default configuration, I may want to design it to throw an exception when it encounters a structure it does not recognize, but allow for a permissive mode that executes things how you're requesting (though I think it'll terminate traversal then & there, similar to preserve).

TotalTechGeek commented 1 year ago

Also, where you can, I recommend trying

const f = engine.build(rule)

// and invoking with
f(data)

As it will try to optimize your function & do further checks on it. Building will actually throw the exception:

Method 'first_name' was not found in the Logic Engine.
scott-wyatt commented 1 year ago

Wow! Thanks for the quick response!

Your library, your rules. I can't use preserve unfortunately, but I tested it and it does work.

We are using these serialized rules across multiple programming languages, and we were testing this library to see if we could use this for the javascript runtime for the traversal feature and port over a few more languages to be able to do the same. The way we are using it though is as an AST to generate runtime functions in target languages.

I would say though, the preserve works, but the mental model breaks. If I'm trying to simulate a language, the extra keyword is kind of "magic" in this context, particularly if the object I was going to return had the keyword preserve in it :-p So I hope you decide to support returning the value given in the future if it is not a rule by default.

IE:


if (true) {
  return { first_name:  "scott" }
}
else {
  return {  first_name: "no idea" }
} // { first_name: "scott" }

// vs

if (true) {
  return { preserve: { first_name:  "scott" } }
}
else {
  return {  preserve: { first_name: "no idea" } }
} // magic { first_name: "scott" }
TotalTechGeek commented 1 year ago

Okay. I will do my best to accommodate. I will add a permissive mode. ETA: Sometime in the next hour :)

scott-wyatt commented 1 year ago

That is very very kind of you!

TotalTechGeek commented 1 year ago

The new version should contain the desired functionality,

// I know the undefined to ignore the methods is a bit weird, I may want to rework that. 
const logic = new LogicEngine(undefined, { permissive: true }) 
const data = {
    first_name: true
  }

  const rule = {
    if: [
      {
        "===": [
          {
            var: "first_name"
          },
          true
        ]
      },
      { first_name: "scott" },
      { first_name: "no idea" }
    ]
  }

  const result = logic.run(rule, data)

  console.log(result) // { first_name: 'scott' }
scott-wyatt commented 1 year ago

Closing issue. Well done, and you are a wonderful person! Hit me up if I can ever do the same for you.

scott-wyatt commented 1 year ago

Sorry to re-open this!

So, the current implementation only returns the first key.

const logic = new LogicEngine(undefined, { permissive: true }) 
const data = {
    first_name: true,
    last_name: true
  }

  const rule = {
    if: [
      {
        and: [
          {
            "===": [
               {
                 var: "first_name"
               },
              true
             ]
           },
           {
             "===": [
                {
                  var: "first_name"
                },
               true
              ]
            }
        ]
      },
      { first_name: "scott", last_name: "wyatt" },
      { first_name: "no", last_name: "idea" }
    ]
  }

  const result = logic.run(rule, data)

  console.log(result) // should return { first_name: 'scott', last_name: 'wyatt' }, is only returning { first_name: 'scott' }
TotalTechGeek commented 1 year ago

🙃 Good catch, my apologies! If you use .build it should work. Patch incoming shortly. 🤦

TotalTechGeek commented 1 year ago

Apologies for the late response, but a patch has been deployed that should resolve the described issue.

Thanks!

scott-wyatt commented 1 year ago

Confirmed, thanks!

TotalTechGeek commented 1 year ago

Please let me know if there's anything else I can help with! Cool use case by the way!

TotalTechGeek commented 1 year ago

@scott-wyatt Also, while this is beyond the scope of the issue, feel free to reach out if you're looking for someone to bounce ideas off of regarding using JSON Logic as an AST & developing compilers / grammars around that.

I have a bit of experience doing so:

Also you might give the AsyncLogicEngine a try if applicable.

While the interpreted version has a chunk of overhead, the "built" execution path tends to not have very much slowdown.