FIX static-handler middleware for well known files #638

Merged
merged 4 commits into from Oct 17, 2012

Projects

None yet

4 participants

@imalberto
Contributor

Appears that static middleware does not quite work for the following: /robots.txt, /crossdomain.xml and /favicon.ico.

This PR addresses this, with unit tests.

@drewfish drewfish commented on an outdated diff Oct 16, 2012
lib/app/middleware/mojito-handler-static.js
}
if (!resource && (path === '/crossdomain.xml')) {
- resource = store.getResource('client', {}, 'asset-xml-crossdomain');
+ resource = store.getResources('client', {}, 'asset-xml-crossdomain');
@drewfish
drewfish Oct 16, 2012 Member

Good catch. However, store.getResources() returns a list of resources, so you might need to [0] on it. Also, I think the 'asset-xml-crossdomain' argument might instead need to be {id: 'asset-xml-crossdomain'}. (Said another way: I think I rather bungled the previous version of the code 😞 .)

@rwaldura
Contributor

see also #565 and follow-up #623 which dealt with these special files

@caridy

Isn't cleaner to store and restore original methods using setUp/tearDown instead of doing the same over and over again in different tests?

Owner

Only the newly added tests needed to customized the store instance with "wrappers". The other tests works with the "store" and "_handler" as-is, so these are setup in the setUp/tearDown.

@caridy

why not A.areEqual? that will give us a better message, telling us the callCount number when failure.

@caridy

Why are we deleting a member here?

Owner

This was fixed in the next commit (7b24220)

@caridy
Collaborator
caridy commented Oct 17, 2012

Aside from those minor details in the tests +1

@caridy
Collaborator
caridy commented Oct 17, 2012

+1

@imalberto imalberto merged commit 75fd4a0 into yahoo:develop-perf Oct 17, 2012

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment