Merge pull request #996 from spumko/issue/987

Remove Directory and View from cacheable responses
hapijs · Jul 22, 2013 · b3825cb · b3825cb
2 parents a724125 + 5f5bcab
commit b3825cb
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 23 deletions.
diff --git a/lib/request.js b/lib/request.js
@@ -470,7 +470,7 @@ internals.Request.handler = function (request, next) {
 
     var lookup = function () {
 
-        // Lookun in cache
+        // Lookup in cache
 
         request._route.cache.getOrGenerate(request.url.path, generate, function (err, value, cached, report) {  // request.url.path contains query
 

diff --git a/lib/response/directory.js b/lib/response/directory.js
@@ -3,8 +3,9 @@
 var Fs = require('fs');
 var Path = require('path');
 var Async = require('async');
-var Cacheable = require('./cacheable');
+var Generic = require('./generic');
 var Redirection = require('./redirection');
+var Text = require('./text');
 var Boom = require('boom');
 var File = require('./file');
 var Utils = require('../utils');
@@ -15,14 +16,14 @@ var Utils = require('../utils');
 var internals = {};
 
 
-// File response  (Generic -> Cacheable -> Directory)
+// File response  (Generic -> Directory)
 
 exports = module.exports = internals.Directory = function (paths, options) {
 
     Utils.assert(this.constructor === internals.Directory, 'Directory must be instantiated using new');
     Utils.assert(options, 'Options must exist');
 
-    Cacheable.call(this);
+    Generic.call(this);
     this.variety = 'directory';
     this.varieties.directory = true;
 
@@ -37,7 +38,7 @@ exports = module.exports = internals.Directory = function (paths, options) {
     this._hasTrailingSlash = this._resource && (this._resource[this._resource.length - 1] === '/');
 };
 
-Utils.inherits(internals.Directory, Cacheable);
+Utils.inherits(internals.Directory, Generic);
 
 
 internals.Directory.prototype._prepare = function (request, callback) {
@@ -167,10 +168,7 @@ internals.Directory.prototype._generateListing = function (path, request, callba
 
         html += '</ul></body></html>';
 
-        self._payload = [html];
-        self._headers['Content-Type'] = 'text/html';
-
-        return Cacheable.prototype._prepare.call(self, request, callback);
+        return callback(new Text(html, 'text/html'));
     });
 };
 

diff --git a/lib/response/index.js b/lib/response/index.js
@@ -16,17 +16,15 @@ var internals = {
 /*
                /-- Stream -----|--- File
               |
-    Generic --|--- Buffer       /-- Text ----|-- Redirection
+    Generic --|--- Buffer
+              |
+              |--- Directory    /-- Text ----|-- Redirection
               |                |
-               \-- Cacheable --|--- Empty
-                               |
-                               |-- Directory
-                               |
-                               |--- Object --|-- Error
-                               |
-                               |--- Cached
+              |--- Cacheable --|--- Empty
+              |                |
+               \-- View        |--- Object --|-- Error
                                |
-                                \-- View
+                                \-- Cached
 */
 
 // Prototype response types

diff --git a/lib/response/view.js b/lib/response/view.js
@@ -1,6 +1,6 @@
 // Load modules
 
-var Cacheable = require('./cacheable');
+var Generic = require('./generic');
 var Utils = require('../utils');
 
 
@@ -9,11 +9,11 @@ var Utils = require('../utils');
 var internals = {};
 
 
-// View response (Generic -> Cacheable -> View)
+// View response (Generic -> View)
 
 module.exports = internals.View = function (manager, template, context, options) {
 
-    Cacheable.call(this);
+    Generic.call(this);
     this.variety = 'view';
     this.varieties.view = true;
 
@@ -25,7 +25,7 @@ module.exports = internals.View = function (manager, template, context, options)
     };
 };
 
-Utils.inherits(internals.View, Cacheable);
+Utils.inherits(internals.View, Generic);
 
 
 internals.View.prototype._prepare = function (request, callback) {
@@ -49,7 +49,7 @@ internals.View.prototype._prepare = function (request, callback) {
             self._flags.encoding = config.encoding;
         }
 
-        return Cacheable.prototype._prepare.call(self, request, callback);
+        return Generic.prototype._prepare.call(self, request, callback);
     });
 };
 
diff --git a/test/integration/response.js b/test/integration/response.js
@@ -1421,6 +1421,12 @@ describe('Response', function () {
             return request.reply.view('test.xyz', { message: "Hello World!" });
         };
 
+        var cached = 1;
+        var cachedHandler = function (request) {
+
+            request.reply.view('test', { message: cached++ });
+        };
+
         describe('Default', function (done) {
 
             var server = new Hapi.Server({
@@ -1435,6 +1441,7 @@ describe('Response', function () {
             server.route({ method: 'GET', path: '/views/insecure', config: { handler: insecureHandler } });
             server.route({ method: 'GET', path: '/views/nonexistent', config: { handler: nonexistentHandler } });
             server.route({ method: 'GET', path: '/views/invalid', config: { handler: invalidHandler } });
+            server.route({ method: 'GET', path: '/views/cache', config: { handler: cachedHandler, cache: { mode: 'server', expiresIn: 2000 } } });
 
             it('returns a compiled Handlebars template reply', function (done) {
 
@@ -1497,6 +1504,24 @@ describe('Response', function () {
                     done();
                 });
             });
+
+            it('returns a fresh reply on second request when caching enabled', function (done) {
+
+                server.inject('/views/cache', function (res) {
+
+                    expect(res.result).to.exist;
+                    expect(res.result).to.equal('<div>\n    <h1>1</h1>\n</div>\n');
+                    expect(res.statusCode).to.equal(200);
+
+                    server.inject('/views/cache', function (res) {
+
+                        expect(res.result).to.exist;
+                        expect(res.result).to.equal('<div>\n    <h1>2</h1>\n</div>\n');
+                        expect(res.statusCode).to.equal(200);
+                        done();
+                    });
+                });
+            });
         });
 
         describe('Layout', function (done) {