Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use key variable instead of pathname #123

Merged
merged 1 commit into from
Jun 12, 2014
Merged

Use key variable instead of pathname #123

merged 1 commit into from
Jun 12, 2014

Conversation

IonicaBizau
Copy link
Contributor

key variable is not used, but I guess that instead of pathname it should be key that is pathname || files[0].

Is that correct?

@phstc
Copy link
Collaborator

phstc commented Jun 12, 2014

@IonicaBizau hmmm makes sense. Is that related to a specific bug? Are the tests still passing?

@IonicaBizau
Copy link
Contributor Author

It's not related to a bug. Actually, I was implementing caching in statique module, following the node-static code related to that, and I noticed that key is not used.

The tests are passing. Relevant log:

ionicabizau@laptop:~/Documents/node-static$ git remote add fork [email protected]:IonicaBizau/node-static.git
ionicabizau@laptop:~/Documents/node-static$ git pull fork patch-1
remote: Counting objects: 4, done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 4 (delta 0), reused 2 (delta 0)
Unpacking objects: 100% (4/4), done.
From github.com:IonicaBizau/node-static
 * branch            patch-1    -> FETCH_HEAD
 * [new branch]      patch-1    -> fork/patch-1
Updating 4c64f0c..3bdf0d7
Fast-forward
 lib/node-static.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ionicabizau@laptop:~/Documents/node-static$ git diff 4c64f0c..3bdf0d7
diff --git a/lib/node-static.js b/lib/node-static.js
index 1287115..3b0e8f0 100644
--- a/lib/node-static.js
+++ b/lib/node-static.js
@@ -269,7 +269,7 @@ Server.prototype.respondNoGzip = function (pathname, status, contentType, _heade
     } else {
         res.writeHead(status, headers);

-        this.stream(pathname, files, new(buffer.Buffer)(stat.size), res, function (e, buffer) {
+        this.stream(key, files, new(buffer.Buffer)(stat.size), res, function (e, buffer) {
             if (e) { return finish(500, {}) }
             finish(status, headers);
         });
ionicabizau@laptop:~/Documents/node-static$ npm test

> [email protected] test /home/ionicabizau/Documents/node-static
> vows --spec --isolate


  ♢ node-static 

  once an http server is listening with a callback
    ✓ should be listening
  streaming a 404 page
    ✓ should respond with 404
    ✓ should respond with the streamed content
  once an http server is listening without a callback
    ✓ should be listening
  requesting a file not found
    ✓ should respond with 404
  requesting a malformed URI
    ✓ should respond with 400
  serving hello.txt
    ✓ should respond with 200
    ✓ should respond with text/plain
    ✓ should respond with hello world
  serving directory index
    ✓ should respond with 200
    ✓ should respond with text/html
  serving index.html from the cache
    ✓ should respond with 200
    ✓ should respond with text/html
  requesting with If-None-Match
    ✓ should respond with 304
  requesting with If-None-Match and If-Modified-Since
    ✓ should respond with a 200
  requesting POST
    ✓ should respond with 200
    ✓ should not be empty
  requesting HEAD
    ✓ should respond with 200
    ✓ head must has no body
  requesting headers
    ✓ should respond with node-static/0.7.3
  addings custom mime types
    ✓ should add woff
  serving subdirectory index
    ✓ should respond with 200
    ✓ should respond with text/html
  redirecting to subdirectory index
    ✓ should respond with 301
    ✓ should respond with location header
    ✓ should respond with empty string body
  requesting a subdirectory (with trailing slash) not found
    ✓ should respond with 404
  requesting a subdirectory (without trailing slash) not found
    ✓ should respond with 404
-
✓ OK » 28 honored (0.153s) 

phstc added a commit that referenced this pull request Jun 12, 2014
Use key variable instead of pathname
@phstc phstc merged commit 7f655b1 into cloudhead:master Jun 12, 2014
@phstc
Copy link
Collaborator

phstc commented Jun 12, 2014

Awesome! Added to master. 😄

@IonicaBizau
Copy link
Contributor Author

Thanks! 😄

@phstc
Copy link
Collaborator

phstc commented Jun 12, 2014

BTW node-static doesn't support cache in memory anymore. Many issues/comments asking to remove that, because it was blowing the memory when serving big files. :/

If you are going to implement an in memory cache, I would recommend to check the file size before caching it. Add some configurable constraints.

@IonicaBizau
Copy link
Contributor Author

Thank you for tip. It's first time when I used cache responses, but I will keep that in mind. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants