Add redirect method to HttpResponse

Add redirect method to HttpResponse

BUG=http://dartbug.com/9612

R=ajohnsen@google.com

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

[Anders Johnsen, 2013-08-02 08:22:03]

lgtm

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart
File sdk/lib/io/http.dart (right):

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart#newcode848
sdk/lib/io/http.dart:848: * Send a redirect to [location] to the client.
"to the client." may be redundant.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart#newcode866
sdk/lib/io/http.dart:866: Future redirect(location, [int status =
HttpStatus.MOVED_TEMPORARILY]);
I think it should be a named argument.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:711: Future redirect(location, [int status =
HttpStatus.MOVED_TEMPORARILY]) {
Add check for _headersWritten.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:712: if (location is! String && location is !Uri)
throw new ArgumentError();
"location is not a String or Uri".

Could also be written as:

  if (location is String) uri = ...;
  if (location is! Uri) throw ...;

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:713: if (location is String) location
=Uri.parse(location);
Nit: add space after =.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:717: host: _httpRequest._httpServer._address,
Host field of request.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:718: port: _httpRequest._httpServer.port,
Port field of request.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:720: print(base);
No print ;)

[Bill Hesse, 2013-08-02 08:27:36]

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:713: if (location is String) location
=Uri.parse(location);
= Uri.parse

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:717: host: _httpRequest._httpServer._address,
_address can be either a String or an InternetAddress, I think.
Uri only takes a String, according to the documentation.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:720: print(base);
remove print statement.

https://codereview.chromium.org/21411003/diff/1/tests/standalone/io/http_redi...
File tests/standalone/io/http_redirect_test.dart (right):

https://codereview.chromium.org/21411003/diff/1/tests/standalone/io/http_redi...
tests/standalone/io/http_redirect_test.dart:488: 
One line between functions?

[Søren Gjesse, 2013-08-02 09:01:05]

Ended up removing the code for generating an absolute URI.

Also changed the type of location from dynamic to Uri.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart
File sdk/lib/io/http.dart (right):

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart#newcode848
sdk/lib/io/http.dart:848: * Send a redirect to [location] to the client.
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> "to the client." may be redundant.

Done.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http.dart#newcode866
sdk/lib/io/http.dart:866: Future redirect(location, [int status =
HttpStatus.MOVED_TEMPORARILY]);
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> I think it should be a named argument.

Done.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart
File sdk/lib/io/http_impl.dart (right):

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:711: Future redirect(location, [int status =
HttpStatus.MOVED_TEMPORARILY]) {
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> Add check for _headersWritten.

Done.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:712: if (location is! String && location is !Uri)
throw new ArgumentError();
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> "location is not a String or Uri".
> 
> Could also be written as:
> 
>   if (location is String) uri = ...;
>   if (location is! Uri) throw ...;

Removed and changed type to Uri.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:713: if (location is String) location
=Uri.parse(location);
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> Nit: add space after =.

Line removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:713: if (location is String) location
=Uri.parse(location);
On 2013/08/02 08:27:36, Bill Hesse wrote:
> = Uri.parse

Line removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:717: host: _httpRequest._httpServer._address,
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> Host field of request.

Line removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:717: host: _httpRequest._httpServer._address,
On 2013/08/02 08:27:36, Bill Hesse wrote:
> _address can be either a String or an InternetAddress, I think.
> Uri only takes a String, according to the documentation.

Line removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:718: port: _httpRequest._httpServer.port,
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> Port field of request.

Line removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:720: print(base);
On 2013/08/02 08:22:03, Anders Johnsen wrote:
> No print ;)

Removed.

https://codereview.chromium.org/21411003/diff/1/sdk/lib/io/http_impl.dart#new...
sdk/lib/io/http_impl.dart:720: print(base);
On 2013/08/02 08:27:36, Bill Hesse wrote:
> remove print statement.

Done.

[Søren Gjesse, 2013-08-02 09:14:32]

Committed patchset #2 manually as r25730 (presubmit successful).