pkg/shelf - send Date header in shelf_io

pkg/shelf - send Date header in shelf_io

R=nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36709

[nweiz, 2014-05-27 19:52:43]

This needs a CHANGELOG entry. The adapter rules should also be updated to
explain how to handle Date.

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/lib/shelf_io.dart
File pkg/shelf/lib/shelf_io.dart (right):

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/lib/shelf_io.d...
pkg/shelf/lib/shelf_io.dart:58: /// `true`.
Why is this configurable? Any configuration that's included in shelf_io runs the
risk of becoming standard for all adapters, which increases the barrier to entry
of writing a new adapter.

I don't think we should make this configurable without a clear use case for
configuring it.

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:264: inInclusiveRange(nowEpoch - 1000,
nowEpoch));
This is potentially flaky. Instead of assuming the time will be within 1000ms,
record the time before the GET and use that as the range.

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:272: HttpHeaders.DATE:
parser.formatHttpDate(date)
Nit: I don't like using HttpHeaders over just a header string.

[kevmoo, 2014-05-27 20:50:58]

PTAL

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/lib/shelf_io.dart
File pkg/shelf/lib/shelf_io.dart (right):

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/lib/shelf_io.d...
pkg/shelf/lib/shelf_io.dart:58: /// `true`.
On 2014/05/27 19:52:44, nweiz wrote:
> Why is this configurable? Any configuration that's included in shelf_io runs
the
> risk of becoming standard for all adapters, which increases the barrier to
entry
> of writing a new adapter.
> 
> I don't think we should make this configurable without a clear use case for
> configuring it.

DONE

I guess we can always add this later if demand arises.

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:264: inInclusiveRange(nowEpoch - 1000,
nowEpoch));
On 2014/05/27 19:52:44, nweiz wrote:
> This is potentially flaky. Instead of assuming the time will be within 1000ms,
> record the time before the GET and use that as the range.

Done. I have to add in some rounding to account for HTTP date dropping
milliseconds.

https://codereview.chromium.org/300023003/diff/20001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:272: HttpHeaders.DATE:
parser.formatHttpDate(date)
On 2014/05/27 19:52:44, nweiz wrote:
> Nit: I don't like using HttpHeaders over just a header string.

We're already using it in shelf_io. Eh...

[nweiz, 2014-05-27 21:06:47]

lgtm

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/CHANGELOG.md
File pkg/shelf/CHANGELOG.md (right):

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/CHANGELOG.md#n...
pkg/shelf/CHANGELOG.md:3: * The `shelf_io` adapter nows send the `Date` HTTP
header.
"nows" -> "now"

I'd say "now automatically sends the `Date` HTTP by default."

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:258: // update beforeRequest to be one second
earlier. Accounts for second
"update" -> "Update"

"Accounts for second resolution in HTTP Date" -> "HTTP dates only have
second-level granularity, and the request will probably take less than a
second."

[kevmoo, 2014-05-27 21:10:35]

Committed patchset #4 manually as r36709 (presubmit successful).

[kevmoo, 2014-05-27 21:10:58]

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/CHANGELOG.md
File pkg/shelf/CHANGELOG.md (right):

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/CHANGELOG.md#n...
pkg/shelf/CHANGELOG.md:3: * The `shelf_io` adapter nows send the `Date` HTTP
header.
On 2014/05/27 21:06:47, nweiz wrote:
> "nows" -> "now"
> 
> I'd say "now automatically sends the `Date` HTTP by default."

Done.

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/test/shelf_io_...
File pkg/shelf/test/shelf_io_test.dart (right):

https://codereview.chromium.org/300023003/diff/40001/pkg/shelf/test/shelf_io_...
pkg/shelf/test/shelf_io_test.dart:258: // update beforeRequest to be one second
earlier. Accounts for second
On 2014/05/27 21:06:47, nweiz wrote:
> "update" -> "Update"
> 
> "Accounts for second resolution in HTTP Date" -> "HTTP dates only have
> second-level granularity, and the request will probably take less than a
> second."

Done.