diegoholiveira / jsonlogic

Go Lang implementation of JsonLogic
MIT License
159 stars 49 forks source link

get the variable that causes a false output on jsonlogic evaluation #67

Open FlorianRuen opened 1 year ago

FlorianRuen commented 1 year ago

Hey @diegoholiveira

I'm using JsonLogic in Go backend, with sometimes complex logics (lot of sums, conditional ...) And in order to help my end users to debug in case of false result, I'm asking if there is a way to output the variable that cause the false output

For example, output just need to be a string with variable name

{
  "and":[
    { "==":[{ "var":"VariableA" }, true] },
    { ">=":[{ "var":"VariableC" }, 17179869184] },
  ]
}

If VariableA = false and VariableC = 17179869184 The jsonlogic.Apply() output can be result, variable, err := jsonlogic.Apply() With variable = "VariableA"(or maybe an array in case of multiple var, or maybe only the first that cause false result)

Do you think, this kind of behavior can be achieve, or because of the operation of jsonlogic which works with json, this will not be possible?

If yes, if you have some tips, maybe I can achieve this, and make another PR !

diegoholiveira commented 1 year ago

It would be super nice and useful to have something like this, but I'm not sure how we can achieve it. I do think it would require another data structure to achieve this, one that allow the output of each step of the json logic eval.

I will think about it, but I have not idea how to do it right now. I'll be glad to accept a PR.

FlorianRuen commented 1 year ago

With another json logic eval, it can be achieve using something like

{
  "if": [
    { "<": [{ "var": "VariableA" }, 5] },
    "VariableA is less than 5",
    { ">": [{ "var": "VariableA" }, 10] },
    "VariableA is greater than 10",
    null
  ]
}

So the output will be a clear message, but my idea is to achieve this without changing the json logic, because, it can be really tricky when the json logic is built using an UI side (the users can create the jsonlogic)

diegoholiveira commented 1 year ago

I was thinking about it and I do think a good API to this problem is:

parsed := jsonlogic.New(logic).With(jsonlogic.EXPLAIN).parse(data)

// an alternative
// parsed := jsonlogic.New(logic).With(jsonlogic.TRACE).parse(data)

What do you think? @joaoandre please take a look into this thread.

FlorianRuen commented 1 year ago

@diegoholiveira Can be a very good way to achieve this And the parsed variable in output, the result should be the entire jsonlogic ? or only the concerned variable ?

joaoandre commented 1 year ago

@diegoholiveira I do like the API you propose(I like TRACE or TRACEBACK better than EXPLAIN).

Maybe the result of parse could be a map with both the result and the traceback of apply. Something linke this:

// It would have to return an error as well to be compliance with the apply api
parsed, err := jsonlogic.New(logic).With(jsonlogic.TRACE).parse(data)
// parsed would be like
// {"result": ..., "trace": ...}

But maybe it would be too complex to combine both at the moment. I still not quite sure how this would be implemented.

diegoholiveira commented 1 year ago

I'm thinking about the output to be like this:

interface Output {
    Value() any
    Trace() any
    Error() error
}

func (e *Engine) Parse(data any) Output {
    // TODO
}

It's very close to your proposal (an interface instead of map), but sure, it would require a full rewrite to introduce this tracing. It could be a good moment to also apply generics and reduce the usage of reflection.

FlorianRuen commented 1 year ago

That suggestion can be a very good way! Maybe the output using @joaoandre suggestion can be better, but both of them answer to the main issue

interface Output {
    Value() any
    Trace() any
}

result, err := jsonlogic.New(logic).With(jsonlogic.TRACE).parse(data)
// parsed would be like
// {"result": ..., "trace": ...}

func (e *Engine) Parse(data any) (Output, error) {
    // TODO
}
FlorianRuen commented 6 months ago

Hello @diegoholiveira

Is this improvement still in progress/or perhaps scheduled ? Maybe it still requires a little more thought.

diegoholiveira commented 6 months ago

Hey @FlorianRuen . Unfortunately I'm not being able to deep thought about it yet. It's on my mind to do it but I'm little busy and taking care of the small issues only.

Do you have any proposal? I would be glad to hear from you because you're the first real user of this feature.