Liquid context debugging policy

[davidor]

Closes #838

[codeclimate]

cyclomatic complexity of function 'value_from' is too high (8 > 7)

[codeclimate]

unused argument 'self'

[mikz]

I'm quite worried about the cjson sparse array change.
It looks like it is global setting for any cjson serialization, so it either should be set globally by APIcast or figure out some workaround.
If it is "raise exception" or "serialize them as string" then I think it is safe to use this feature, but it should be set by APIcast init process itself.

[mikz]

This could be:

local typ = type(object)

if accepted_types_for_keys[t] then return object end

right?

[mikz]

I think this can be a helper function value_from_table. That would reduce cyclomatic complexity.

Building on top of the previous comment we can have a dispatch table:

local ident = function(...) return ... end
local value_from_fun = {
 string = ident, number = ident,
table = function(table)
  local res = {}
  for k, v in pairs(table) do
     local wanted_types = accepted_types_for_keys[type(k)] and
                          accepted_types_for_values[type(v)]

     if wanted_types then
       res[k] = value_from(v)
     end
   end

  return res
end

And value_from would just do dispatch:

local function value_from(object)
  local fun = value_from_fun[type(object)]

  if fun then return fun(object) end
end

[davidor]

It looks a bit better 👍

[mikz]

In theory we could add __pairs metamethod to the linked list that understands this.
http://lua-users.org/wiki/GeneralizedPairsAndIpairs

Then you could just iterate through the context.

[davidor]

That's a good idea, but I'd rather leave it for a future PR as it involves adding the functionality in the LinkedList module, test it, and check whether we can use it in some of the existing callers.

[mikz]

This is global setting, right? That is quite dangerous. Maybe we should enable this in init for everyone.

[mikz]

Maybe we should always encode table keys as strings when building the context content ? But that would probably break actual arrays.

[davidor]

Right, I didn't notice. It wasn't my intention to set this global setting there.

I'll just probably implement some ad-hoc solution in ContextContent. If the key is an integer and is greater than X then convert it to string.

[davidor]

Here's how I implemented this: https://github.com/3scale/apicast/pull/849/files#diff-d126ed9fb83c6261c4bb9fefa80fd029R22

[mikz]

We also should add the ngx_variable, right?

[davidor]

You're right. And to do that I needed to modify ngx_variable so it behaves as a context (linked list): 822345d

[davidor]

Also, the policy takes that into account now: 98d3d8a

[mikz]

I think arrays with 10 elements are common. Maybe better to set this to a something like 1000 ?

[davidor]

OK. I'll raise the limit to 1k. I think that in practice large service IDs are going to be the problem.

[mikz]

Looking good 👍