Skip to content

Add new etaging options#30

Merged
kanongil merged 9 commits intohapijs:masterfrom
kanongil:etag-option
Oct 18, 2015
Merged

Add new etaging options#30
kanongil merged 9 commits intohapijs:masterfrom
kanongil:etag-option

Conversation

@kanongil
Copy link
Copy Markdown
Contributor

This PR adds a new etagMethod option to the file and directory handlers, as suggested in #29:

  • etagMethod - specifies the method used to calculate the ETag header response. Available values:
    • 'hash' - SHA1 sum of the file contents, suitable for distributed deployments. Default value.
    • 'simple' - Hex encoded size and modification date, suitable when files are only stored on a single server.
    • false - Disable ETag computation.

Note that this changes the behavior of the default hashing method to always return an ETag header. This requires uncached etags to be computed in the prepare step, which translates to some extra latency in such requests, while the file is hashed. I don't believe that this should cause problems for most common use-cases and can actually improve performance in some cases (eg. cached if-none-match requests against a freshly started server can now serve a 304 response).

@kanongil kanongil added the feature New functionality or improvement label Jul 13, 2015
@kanongil
Copy link
Copy Markdown
Contributor Author

PR updated with a deferred callback mechanism to prevent multiple hashings on the same file.

lib/etag.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change callback to next since it is not always called on next tick or if it needs to be consistent, use Hoek.nextTick().

@hueniverse
Copy link
Copy Markdown
Contributor

See comments. Otherwise looks good.

@hueniverse hueniverse self-assigned this Jul 29, 2015
@hueniverse hueniverse added this to the 2.1.7 milestone Jul 29, 2015
@kanongil kanongil modified the milestones: 3.0.0, 3.1.0 Aug 8, 2015
kanongil added a commit that referenced this pull request Oct 18, 2015
@kanongil kanongil merged commit 478e66c into hapijs:master Oct 18, 2015
@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants