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

Working with HttpRequest content body is unwieldy. #8834

Closed
DartBot opened this issue Feb 27, 2013 · 12 comments
Closed

Working with HttpRequest content body is unwieldy. #8834

DartBot opened this issue Feb 27, 2013 · 12 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Milestone

Comments

@DartBot
Copy link

DartBot commented Feb 27, 2013

This issue was originally filed by @butlermatt


Currently an HttpRequest (even when not persistent) is a Stream of List<int>. As such to get the entire content body we need to do something like this:

List<int> body = new List<int>();
request.listen(body.addAll, onDone: () {
  var str = new String.fromCharCodes(body);
  // Now do something with the string of data
});

While there some modifications we can do like transform it to a StringDecoder then convert to a list then join said list of strings into the result.. ultimately that's just as (if not more) unwieldy. And neither option seems very dart-y.

I'm wondering if HttpRequest should provide a helper method or property to provide the 'body' of the HttpRequest. Something similar to:

httpRequest.content.then((List<int> body) {
  // Optional since we may not always want strings
  var str = new String.fromCharCodes(body);
  // now do stuff.
});

@sgjesse
Copy link
Contributor

sgjesse commented Feb 27, 2013

I was actually thinking about this earlier today, and came up with the following:

How about having a class which can do this either as a transformer (for the server) or through a future (usable on both client and server).

Something like this - see example usage below.

class HttpBody implements StreamTransformer<HttpRequest, HttpRequestBody> {
  static int BINARY = 0;
  static int TEXT = 1;
  static int JSON = 2;
  static int FORM = 3;

  static Future<HttpRequestBody> processRequestBody(HttpRequest request);

  static Future<HttpResponseBody> processResponseBody(HttpResponse response)

  // The content type e.g. application/json, application/octet-stream, application/x-www-form-urlencoded, ...
  String get mimeType;

  // Something better that mimeType, e.g. HttpBody.JSON, HttpBody.BINARY, HttpBody.FORM, HttpBody.TEXT ...
  int get type ...

  // The actual body, type of this depending on type (Map for JSON and FROM, List<int> for BINARY etc.
  dynamic get body;

  // Map if JSON, null otherwise
  Map get json;

  // List<int> if BINARY, null otherwise
  List<int>> get binary;

  // Map if FORM, null otherwise
  Map get form;

  // String if TEXT, null otherwise
  String get text;
}

class HttpRequestBody extends HttpBody {
  // The original request for access to all headers etc.
  HttpRequest get request;
}

class HttpResponseBody extends HttpBody {
  // The original response for access to all headers etc.
  HttpResponse get request;
}

HttpServer.bind().then((server) {
  server
      .transform(new HttpBodyHandler())
      .listen((HttpRequestBody body) {
        body.mimeType ... // The content type e.g. application/json, application/octet-stream, application/x-www-form-urlencoded, ...
        body.type ... // Something better that mimeType, e.g. HttpBody.JSON, HttpBody.BINARY, HttpBody.FORM, HttpBody.TEXT ...
        body.body ... // The actual body, type of this depending on type (Map for JSON and FROM, List<int> for BINARY etc.
        body.json ... // Map if JSON, null otherwise
        body.binary ... // List<int> if BINARY, null otherwise
        body.form ... // Map if FORM, null otherwise
        body.text ... // String if TEXT, null otherwise
        body.request ... // The original request for access to all headers etc.
  }

  HttpClient client = new HttpClient();
  client.getUrl(...)
      .then((HttpClientRequest request) {
        return request.close();
      })
      .then((HttpClientResponse response) {
        HttpBodyHandler.processBody(response).then((HttpResponseBody body) {
          body.contentType ... // As above
          body.body ... // As above
          body.response ... // the original response for access to all headers etc.
        }
      }
}


Added this to the M4 milestone.
Added Area-IO, Accepted labels.

@sethladd
Copy link
Contributor

For the record, yes! :) Also, for WebSocketTransformer. Thanks!

@DartBot
Copy link
Author

DartBot commented Feb 28, 2013

This comment was originally written by @butlermatt


Sounds very good. Though perhaps also add a getter to the HttpRequestBody to provide the response (ie HttpResponse get response => request.response;), otherwise we'd need to write something like:

body.request.response.addString(replyJson);

better to be just:

body.response...

Otherwise, yes I love the idea. Especially since it would work so well with many of the already existing routing implementations (or at least mine ;).

But definitely the key point for me is having the body content available synchronously instead of as a stream of data I have to 'collect'. All the extra functionality (body.type) is just the cherry on top ;)

@sgjesse
Copy link
Contributor

sgjesse commented Apr 3, 2013

Removed this from the M4 milestone.
Added this to the M5 milestone.

@andersjohnsen
Copy link

Landed a few days ago. We'll look into user-defined types down the road.


Set owner to @Skabet.
Removed this from the M5 milestone.
Added this to the M4 milestone.
Added Fixed label.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 12, 2013

The change landed in https://code.google.com/p/dart/source/detail?r=21202 support mime types text/* and application/json.

Built-in form support (for both application/x-www-form-urlencoded and multipart/form-data) and configuration will be added.


Removed the owner.
Removed this from the M4 milestone.
Added this to the M5 milestone.
Added Accepted label.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Issue #2488 has been merged into this issue.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Marked this as blocking #7391.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Issue #2787 has been merged into this issue.

@sgjesse
Copy link
Contributor

sgjesse commented Apr 16, 2013

Issue #5040 has been merged into this issue.

@sgjesse
Copy link
Contributor

sgjesse commented May 15, 2013

With the addition of HttpBodyHandler in dart:io the body handling has become much simpler.


Added Fixed label.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Area-Library, Library-IO labels.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 14, 2014
@DartBot DartBot added this to the M5 milestone May 14, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Projects
None yet
Development

No branches or pull requests

5 participants