From 89415b52f6a6c52f8475c5eb5e0e80083fd6ddc3 Mon Sep 17 00:00:00 2001
From: jedrzejchalubek <jedrzej.chalubek@gmail.com>
Date: Thu, 19 Oct 2017 10:11:50 +0200
Subject: [PATCH] Fixes bug with templates context variables

Template contextual variables should be extracted within a render method instead setting up in query_vars, because they leaked bettwen multiple rendering the same template.
---
 src/Gin/Template/Template.php       | 25 +++---------
 tests/Gin/Template/TemplateTest.php | 59 +++++++++++++++--------------
 tests/fixtures/template.tpl.php     |  1 +
 3 files changed, 37 insertions(+), 48 deletions(-)
 create mode 100644 tests/fixtures/template.tpl.php

diff --git a/src/Gin/Template/Template.php b/src/Gin/Template/Template.php
index fbc97e0..764886b 100644
--- a/src/Gin/Template/Template.php
+++ b/src/Gin/Template/Template.php
@@ -41,30 +41,17 @@ public function __construct(ConfigInterface $config)
      */
     public function render(array $context = [])
     {
-        if (locate_template($path = $this->getRelativePath(), false, false)) {
-            $this->setContext($context);
+        if ($template = locate_template($path = $this->getRelativePath(), false, false)) {
             $this->doActions();
 
-            return locate_template($path, true, false);
-        }
+            extract(apply_filters("tonik/gin/template/context/{$this->getFilename()}", $context));
 
-        throw new FileNotFoundException("Template file [{$this->getRelativePath()}] cannot be located.");
-    }
+            require $template;
 
-    /**
-     * Sets context dataset on query.
-     *
-     * @param array $context
-     *
-     * @return void
-     */
-    public function setContext(array $context)
-    {
-        $context = apply_filters("tonik/gin/template/context/{$this->getFilename()}", $context);
-
-        foreach ($context as $key => $value) {
-            set_query_var($key, $value);
+            return;
         }
+
+        throw new FileNotFoundException("Template file [{$this->getRelativePath()}] cannot be located.");
     }
 
     /**
diff --git a/tests/Gin/Template/TemplateTest.php b/tests/Gin/Template/TemplateTest.php
index 0e07809..e728db1 100644
--- a/tests/Gin/Template/TemplateTest.php
+++ b/tests/Gin/Template/TemplateTest.php
@@ -62,18 +62,21 @@ public function test_centext_filter_on_template_rendering()
     {
         $config = $this->getConfig();
 
-        Functions::expect('locate_template')->atLeast()->once()->andReturn(true);
-        Functions::expect('set_query_var')->atLeast()->once()->with('key', 'changed')->andReturn(null);
+        Functions::expect('locate_template')->andReturn($this->getFixtureTemplatePath());
 
         $template = $this->getTemplate($config, 'sample_template');
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', null);
         Filters::expectApplied('tonik/gin/template/context/sample_template.php')->once()->with(['key' => 'value'])->andReturn(['key' => 'changed']);
+        ob_start();
         $template->render(['key' => 'value']);
+        $this->assertEquals('<div>changed</div>', $this->removeNewLineAtEOF(ob_get_clean()));
 
         $template = $this->getTemplate($config, ['sample_template', 'named']);
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', 'named');
         Filters::expectApplied('tonik/gin/template/context/sample_template-named.php')->once()->with(['key' => 'value'])->andReturn(['key' => 'changed']);
+        ob_start();
         $template->render(['key' => 'value']);
+        $this->assertEquals('<div>changed</div>', $this->removeNewLineAtEOF(ob_get_clean()));
     }
 
     /**
@@ -88,7 +91,7 @@ public function it_should_throw_exception_on_render_if_file_is_no_located()
 
         $this->expectException(FileNotFoundException::class);
 
-        $template->render();
+        $template->render(['key' => 'value']);
     }
 
     /**
@@ -135,10 +138,12 @@ public function it_should_do_get_template_part_action_on_render_with_no_named_te
         $config = $this->getConfig();
         $template = $this->getTemplate($config, 'sample_template');
 
-        Functions::expect('locate_template')->twice()->andReturn(true);
+        Functions::expect('locate_template')->once()->andReturn($this->getFixtureTemplatePath());
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', null);
 
-        $template->render();
+        ob_start();
+        $template->render(['key' => 'value']);
+        ob_get_clean();
     }
 
     /**
@@ -148,38 +153,24 @@ public function it_should_do_get_template_part_action_on_render_with_named_templ
     {
         $config = $this->getConfig();
 
+        ob_start();
+
         $template = $this->getTemplate($config, ['sample_template', 'named']);
-        Functions::expect('locate_template')->twice()->andReturn(true);
+        Functions::expect('locate_template')->once()->andReturn($this->getFixtureTemplatePath());
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', 'named');
-        $template->render();
+        $template->render(['key' => 'value']);
 
         $template = $this->getTemplate($config, ['sample_template', false]);
-        Functions::expect('locate_template')->twice()->andReturn(true);
+        Functions::expect('locate_template')->once()->andReturn($this->getFixtureTemplatePath());
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', false);
-        $template->render();
+        $template->render(['key' => 'value']);
 
         $template = $this->getTemplate($config, ['sample_template', null]);
-        Functions::expect('locate_template')->twice()->andReturn(true);
+        Functions::expect('locate_template')->once()->andReturn($this->getFixtureTemplatePath());
         Actions::expectFired('get_template_part_sample_template')->once()->with('sample_template', null);
-        $template->render();
-    }
-
-    /**
-     * @test
-     */
-    public function it_should_set_up_context_to_the_query_var()
-    {
-        $config = $this->getConfig();
-        $template = $this->getTemplate($config, ['sample_template', 'named']);
-
-        Functions::expect('locate_template')->twice()->andReturn(true);
-        Functions::expect('set_query_var')->once()->with('key1', 'value1')->andReturn(null);
-        Functions::expect('set_query_var')->once()->with('key2', 'value2')->andReturn(null);
+        $template->render(['key' => 'value']);
 
-        $template->render([
-            'key1' => 'value1',
-            'key2' => 'value2'
-        ]);
+        ob_get_clean();
     }
 
     public function getConfig()
@@ -201,4 +192,14 @@ public function getTemplate($config, $name)
     {
         return (new Template($config))->setFile($name);
     }
-}
\ No newline at end of file
+
+    public function getFixtureTemplatePath()
+    {
+        return dirname(__DIR__) . '/../fixtures/template.tpl.php';
+    }
+
+    public function removeNewLineAtEOF($string)
+    {
+        return trim(preg_replace('/\s\s+/', ' ', $string));
+    }
+}
diff --git a/tests/fixtures/template.tpl.php b/tests/fixtures/template.tpl.php
new file mode 100644
index 0000000..be4dfbc
--- /dev/null
+++ b/tests/fixtures/template.tpl.php
@@ -0,0 +1 @@
+<div><?= $key ?></div>