[liquid] rendering template is idempotent and thread safe

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Some policies did not have access to the vars exposed when using Liquid (`uri`, `path`, etc.) [PR #891](https://github.com/3scale/apicast/pull/891)
 - Fix error when loading certain configurations that use OIDC [PR #893](https://github.com/3scale/apicast/pull/893)
 - Fix error that appeared when combining the liquid context debug policy with policies that contain liquid templates [PR #895](https://github.com/3scale/apicast/pull/895)
+- Thread safety issues when rendering Liquid templates [PR #896](https://github.com/3scale/apicast/pull/896)
 
 ### Added
 

gateway/src/apicast/template_string.lua

@@ -1,5 +1,4 @@
 local Liquid = require 'liquid'
-local LiquidTemplate = Liquid.Template
 local LiquidInterpreterContext = Liquid.InterpreterContext
 local LiquidFilterSet = Liquid.FilterSet
 local LiquidResourceLimit = Liquid.ResourceLimit
@@ -68,15 +67,33 @@ end
 -- Set resource limits to avoid loops
 local liquid_resource_limit = LiquidResourceLimit:new(nil, nil, 0)
 
+-- TODO: we should move this to liquid-lua and fix it's broken Template interface
+local CachedParser = { }
+local CachedParser_mt = { __index = CachedParser }
+
+function CachedParser.new(parser)
+  local doc = parser:document()
+  return setmetatable({ doc = doc }, CachedParser_mt)
+end
+
+function CachedParser:document() return self.doc end
+
+local function liquid_parser(text)
+  local lexer = Liquid.Lexer:new(text)
+  local parser = Liquid.Parser:new(lexer)
+
+  return CachedParser.new(parser)
+end
+
 function LiquidTemplateString.new(string)
-  return setmetatable({ template = LiquidTemplate:parse(string) },
+  return setmetatable({ parser = liquid_parser(string) },
                       liquid_template_string_mt)
 end
 
 function LiquidTemplateString:render(context)
   local available_context = ngx_variable.available_context(context)
 
-  return self.template:render(
+  return Liquid.Interpreter:new(self.parser):interpret(
     LiquidInterpreterContext:new(available_context),
     liquid_filter_set,
     liquid_resource_limit

spec/template_string_spec.lua

@@ -33,6 +33,35 @@ describe('template string', function()
     assert.equals(ngx.md5('something'), liquid_template:render({}))
   end)
 
+  describe(':render', function()
+    it('it allows context to be garbage collected #gc', function()
+      local template =  TemplateString.new('string', 'liquid')
+      local context = { 'value' }
+
+      template:render(context)
+
+      -- store the object so we can verify it was GC'd later
+      local gc = setmetatable({ context = context }, { __mode = 'vk' })
+      -- luassert stub takes a reference to parameters passed through
+      ngx_variable.available_context:revert()
+      context = nil
+
+      collectgarbage()
+      assert.is_nil(gc.context)
+    end)
+
+    it('does not parse the document twice', function()
+      local template = TemplateString.new('string', 'liquid')
+
+      stub(require('liquid').Parser, 'document', function()
+        error('this should not be called')
+      end)
+
+      assert.same('string', template:render({}))
+      assert.same('string', template:render({}))
+    end)
+  end)
+
   describe('.new', function()
     it('returns nil and an error with invalid type', function()
       local template, err = TemplateString.new('some_string', 'invalid_type')