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

Better support for others who want to create implementations #93

Open
gregsdennis opened this issue Dec 18, 2020 · 7 comments
Open

Better support for others who want to create implementations #93

gregsdennis opened this issue Dec 18, 2020 · 7 comments

Comments

@gregsdennis
Copy link

This isn't an issue for this library so much as for JsonLogic in general.

I started on a .Net implementation of JsonLogic, but found there were too many inconsistencies in the online implementation. I've complained about described some of these inconsistencies in my issue to add this support.

Ideally, I'd like to see:

  • A more formal document or website describing in detail the expected behavior(s) for each of the operators.
  • A test suite that others can use to ensure compliance to said document.
@jwadhams
Copy link
Owner

jwadhams commented Dec 18, 2020

OK, well the good news is that there is a common test suite. The unit tests in this repo are downloading http://jsonlogic.com/tests.json and treating the whole thing as an array, and each row of that array as
[rule, data, expected]

That gives me a common 275 item test suite across the two implementations I maintain, PHP and JavaScript.

But you're right, most operators are implemented to just delegate the decision to the language in the fewest lines of code. The entire JavaScript implementation of max is

    "max": function() {
      return Math.max.apply(this, arguments);
    },

The one place I found I just couldn't live with the ambiguity was truthiness, which is why JsonLogic has its own truth table: https://jsonlogic.com/truthy.html It's certainly possible that you could define the same thing for "what is JsonLogic's opinion of -3 > 't'" but it hasn't affected my day job and it doesn't sound like a fun time.

I think if you wanted to make a .NET version that passed the common tests and then went absolutely berserk on type inconsistencies (e.g., if you call {"max":[-1,"t",-3]} it throws an exception like "all arguments must be of the same type" or "all arguments must be numeric") there would absolutely be an audience for that. JsonLogic is loose because the two languages I implemented it in are loose (and in vaguely compatible ways) but you are absolutely not alone in being frustrated by it.

I also think I screwed up the spec when I allowed single-argument operators to skip the [] around args, e.g. {"var":"a"} is treated as identical to {"var":["a"]}. You're not even the first person this month to be thrown by that one. All to save myself two characters in var statements! But I think it's gonna be a colossal breaking change to rules in the wild to remove it now, I haven't even begun to think how I'd migrate my uses in my day job, let alone how I'd advise the community.

@gregsdennis
Copy link
Author

Thanks for the explanation and the link to the tests. I might pick it up again.

I suppose the max issue I mentioned is more about casting than truthyness (== vs ===). I'll probably constrain all of the things that aren't explicitly required by the tests. Loose equality isn't something that .Net does natively. It might be interesting to do an in-depth comparison (like this one for JSONPath) of your JS and my .Net implementations (and/or others) to determine which edge cases are supported where.

@gregsdennis
Copy link
Author

More on the casting issue, cases like {"/":["1",1]} (from the test suite) are going to be quite difficult for strongly-typed languages.

@marvindv
Copy link

I created a port to Rust a while ago (jsonlogic_rs) and faced the same problems. In the end I actually reimplemented the way JavaScript handles types and their coercion and created tests for all examples on jsonlogic.com and all other funny edge cases I could imagine, so stuff like {"max":[-1,"t",-3]} should work just like with the original JsonLogic implementation in JavaScript. Good to know there is http://jsonlogic.com/tests.json, which can be used for other languages.

Even if tedious, it can be done by "just" following the language spec and it was actually a nice insight for me. My personal "highlight" is the specification of abstract equality (==) and its implementation.

@gregsdennis
Copy link
Author

gregsdennis commented Dec 22, 2020

Okay. I have all the tests passing except two of the all tests. I can't figure out what they're supposed to do.

#1

logic: {"all":[{"var":"integers"},{"<":[{"var":""},1]}]}
data: {"integers":[]}
expected: false
 { "all" :
  [
    { "var" : "integers" },    // (a)
    { "<" :                    // (b)
      [
        { "var" : "" },
        1
      ]
    }
  ]
}

(a) This returns an empty array, so there's nothing to apply the rule to. Do we require that all must have at least one "truthy" element?
(b) "" < 1? how an empty string compare to a number? Do the comparison operators follow loose equality (i.e. ==)?

#2

logic: {"all":[{"var":"items"},{">=":[{"var":"qty"},1]}]}
data: {"items":[]}
expected: false

Basically the same test, except we have null >= 1. Again, there are no items to run the test on.


Note that if all does require at least one "truthy" element, then an empty array will always fail, and we can put whatever ridiculous rule we want in the logic and it doesn't matter because it'll never run.

@gregsdennis
Copy link
Author

Alright... I've published my first version. Passes the entire test suite (with the assumptions above).

@gregsdennis
Copy link
Author

gregsdennis commented Jan 12, 2021

How does one go about getting added to the site as an implementation? Would a PR here suffice?

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

No branches or pull requests

3 participants