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

[java_http] send request body #995

Merged
merged 10 commits into from
Jul 31, 2023
1 change: 1 addition & 0 deletions pkgs/java_http/jnigen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class_path:

classes:
- 'java.io.InputStream'
- 'java.io.OutputStream'
- 'java.lang.System'
- 'java.net.HttpURLConnection'
- 'java.net.URL'
Expand Down
39 changes: 30 additions & 9 deletions pkgs/java_http/lib/src/java_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,29 @@ class JavaClient extends BaseClient {
// See https://github.com/dart-lang/http/pull/980#discussion_r1253700470.
_initJVM();

final (statusCode, reasonPhrase, responseHeaders, responseBody) =
await Isolate.run(() {
request.finalize();
// We can't send a StreamedRequest to another Isolate.
// But we can send Map<String, String>, String, UInt8List, Uri.
final requestBody = await request.finalize().toBytes();
final requestHeaders = request.headers;
final requestMethod = request.method;
final requestUrl = request.url;

final (statusCode, reasonPhrase, responseHeaders, responseBody) =
await Isolate.run(() async {
final httpUrlConnection = URL
.ctor3(request.url.toString().toJString())
.ctor3(requestUrl.toString().toJString())
.openConnection()
.castTo(HttpURLConnection.type, deleteOriginal: true);

request.headers.forEach((headerName, headerValue) {
requestHeaders.forEach((headerName, headerValue) {
httpUrlConnection.setRequestProperty(
headerName.toJString(), headerValue.toJString());
});

httpUrlConnection.setRequestMethod(request.method.toJString());
httpUrlConnection.setRequestMethod(requestMethod.toJString());
_setRequestBody(httpUrlConnection, requestBody);

final statusCode = _statusCode(request, httpUrlConnection);
final statusCode = _statusCode(requestUrl, httpUrlConnection);
final reasonPhrase = _reasonPhrase(httpUrlConnection);
final responseHeaders = _responseHeaders(httpUrlConnection);
final responseBody = _responseBody(httpUrlConnection);
Expand All @@ -78,12 +84,27 @@ class JavaClient extends BaseClient {
reasonPhrase: reasonPhrase);
}

int _statusCode(BaseRequest request, HttpURLConnection httpUrlConnection) {
void _setRequestBody(
HttpURLConnection httpUrlConnection,
Uint8List requestBody,
) {
if (requestBody.isEmpty) return;

httpUrlConnection.setDoOutput(true);
final outputStream = httpUrlConnection.getOutputStream();
requestBody.forEach(outputStream.write);

outputStream
Copy link
Member

Choose a reason for hiding this comment

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

[optional] Is there any value in minimizing the uses of callMethodWithArgs and avoiding interop calls in a loop?

(untested suggestion)

Suggested change
httpUrlConnection.setDoOutput(true);
final outputStream = httpUrlConnection.getOutputStream();
requestBody.forEach(outputStream.write);
outputStream
httpUrlConnection.setDoOutput(true);
final outputStream = httpUrlConnection.getOutputStream();
final bodyArray = JArray(jbyte.type, requestBody.length)
..setRange(0, requestBody.length, requestBody);
outputStream
..write1(bodyArray)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea to me!

I've pushed a commit with the suggestions and the tests were all passing locally.

Thank you 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been learning about extension methods recently, could we add an extension method to Uint8List?

extension on Uint8List {
  JArray<jbyte> toJArray() =>
      JArray(jbyte.type, length)..setRange(0, length, this);
}

Then, the code would go from:

    final bodyArray = JArray(jbyte.type, requestBody.length)
      ..setRange(0, requestBody.length, requestBody);

    httpUrlConnection.getOutputStream()
      ..write1(bodyArray)
      ..flush()
      ..close();

to:

    httpUrlConnection.getOutputStream()
      ..write1(requestBody.toJArray())
      ..flush()
      ..close();

My intuition is that adding an extension method to Uint8List is unnecessary at the moment. But if it becomes a common occurence to convert Uint8List to JArray<jbyte> in java_http then it would be a nice idea to add an extension method.

Copy link
Member

Choose a reason for hiding this comment

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

could we add an extension method to Uint8List?

This extension method does look like a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! Let's go for the extension method toJArray() on Uint8List.

..flush()
..close();
}

int _statusCode(Uri requestUrl, HttpURLConnection httpUrlConnection) {
final statusCode = httpUrlConnection.getResponseCode();

if (statusCode == -1) {
throw ClientException(
'Status code can not be discerned from the response.', request.url);
'Status code can not be discerned from the response.', requestUrl);
}

return statusCode;
Expand Down
4 changes: 3 additions & 1 deletion pkgs/java_http/lib/src/third_party/java/io/InputStream.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import "dart:ffi" as ffi;
import "package:jni/internal_helpers_for_jnigen.dart";
import "package:jni/jni.dart" as jni;

import "OutputStream.dart" as outputstream_;

/// from: java.io.InputStream
class InputStream extends jni.JObject {
@override
Expand Down Expand Up @@ -192,7 +194,7 @@ class InputStream extends jni.JObject {

/// from: public long transferTo(java.io.OutputStream outputStream)
int transferTo(
jni.JObject outputStream,
outputstream_.OutputStream outputStream,
) {
return jni.Jni.accessors.callMethodWithArgs(reference, _id_transferTo,
jni.JniCallType.longType, [outputStream.reference]).long;
Expand Down
136 changes: 136 additions & 0 deletions pkgs/java_http/lib/src/third_party/java/io/OutputStream.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Autogenerated by jnigen. DO NOT EDIT!

// ignore_for_file: annotate_overrides
// ignore_for_file: camel_case_extensions
// ignore_for_file: camel_case_types
// ignore_for_file: constant_identifier_names
// ignore_for_file: file_names
// ignore_for_file: no_leading_underscores_for_local_identifiers
// ignore_for_file: non_constant_identifier_names
// ignore_for_file: overridden_fields
// ignore_for_file: unnecessary_cast
// ignore_for_file: unused_element
// ignore_for_file: unused_field
// ignore_for_file: unused_import
// ignore_for_file: unused_shown_name

import "dart:isolate" show ReceivePort;
import "dart:ffi" as ffi;
import "package:jni/internal_helpers_for_jnigen.dart";
import "package:jni/jni.dart" as jni;

/// from: java.io.OutputStream
class OutputStream extends jni.JObject {
@override
late final jni.JObjType<OutputStream> $type = type;

OutputStream.fromRef(
jni.JObjectPtr ref,
) : super.fromRef(ref);

static final _class = jni.Jni.findJClass(r"java/io/OutputStream");

/// The type which includes information such as the signature of this class.
static const type = $OutputStreamType();
static final _id_ctor =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"<init>", r"()V");

/// from: public void <init>()
/// The returned object must be deleted after use, by calling the `delete` method.
factory OutputStream() {
return OutputStream.fromRef(jni.Jni.accessors
.newObjectWithArgs(_class.reference, _id_ctor, []).object);
}

static final _id_nullOutputStream = jni.Jni.accessors.getStaticMethodIDOf(
_class.reference, r"nullOutputStream", r"()Ljava/io/OutputStream;");

/// from: static public java.io.OutputStream nullOutputStream()
/// The returned object must be deleted after use, by calling the `delete` method.
static OutputStream nullOutputStream() {
return const $OutputStreamType().fromRef(jni.Jni.accessors
.callStaticMethodWithArgs(_class.reference, _id_nullOutputStream,
jni.JniCallType.objectType, []).object);
}

static final _id_write =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"write", r"(I)V");

/// from: public abstract void write(int i)
void write(
int i,
) {
return jni.Jni.accessors.callMethodWithArgs(reference, _id_write,
jni.JniCallType.voidType, [jni.JValueInt(i)]).check();
}

static final _id_write1 =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"write", r"([B)V");

/// from: public void write(byte[] bs)
void write1(
jni.JArray<jni.jbyte> bs,
) {
return jni.Jni.accessors.callMethodWithArgs(reference, _id_write1,
jni.JniCallType.voidType, [bs.reference]).check();
}

static final _id_write2 =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"write", r"([BII)V");

/// from: public void write(byte[] bs, int i, int i1)
void write2(
jni.JArray<jni.jbyte> bs,
int i,
int i1,
) {
return jni.Jni.accessors.callMethodWithArgs(
reference,
_id_write2,
jni.JniCallType.voidType,
[bs.reference, jni.JValueInt(i), jni.JValueInt(i1)]).check();
}

static final _id_flush =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"flush", r"()V");

/// from: public void flush()
void flush() {
return jni.Jni.accessors.callMethodWithArgs(
reference, _id_flush, jni.JniCallType.voidType, []).check();
}

static final _id_close =
jni.Jni.accessors.getMethodIDOf(_class.reference, r"close", r"()V");

/// from: public void close()
void close() {
return jni.Jni.accessors.callMethodWithArgs(
reference, _id_close, jni.JniCallType.voidType, []).check();
}
}

class $OutputStreamType extends jni.JObjType<OutputStream> {
const $OutputStreamType();

@override
String get signature => r"Ljava/io/OutputStream;";

@override
OutputStream fromRef(jni.JObjectPtr ref) => OutputStream.fromRef(ref);

@override
jni.JObjType get superType => const jni.JObjectType();

@override
final superCount = 1;

@override
int get hashCode => ($OutputStreamType).hashCode;

@override
bool operator ==(Object other) {
return other.runtimeType == ($OutputStreamType) &&
other is $OutputStreamType;
}
}
1 change: 1 addition & 0 deletions pkgs/java_http/lib/src/third_party/java/io/_package.dart
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export "InputStream.dart";
export "OutputStream.dart";
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import "URL.dart" as url_;

import "../io/InputStream.dart" as inputstream_;

import "../io/OutputStream.dart" as outputstream_;

/// from: java.net.URLConnection
class URLConnection extends jni.JObject {
@override
Expand Down Expand Up @@ -467,9 +469,10 @@ class URLConnection extends jni.JObject {

/// from: public java.io.OutputStream getOutputStream()
/// The returned object must be deleted after use, by calling the `delete` method.
jni.JObject getOutputStream() {
return const jni.JObjectType().fromRef(jni.Jni.accessors.callMethodWithArgs(
reference, _id_getOutputStream, jni.JniCallType.objectType, []).object);
outputstream_.OutputStream getOutputStream() {
return const outputstream_.$OutputStreamType().fromRef(jni.Jni.accessors
.callMethodWithArgs(reference, _id_getOutputStream,
jni.JniCallType.objectType, []).object);
}

static final _id_toString1 = jni.Jni.accessors
Expand Down
2 changes: 2 additions & 0 deletions pkgs/java_http/test/java_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import 'package:test/test.dart';

void main() {
group('java_http client conformance tests', () {
testIsolate(JavaClient.new);
testResponseBody(JavaClient(), canStreamResponseBody: false);
testResponseHeaders(JavaClient());
testRequestBody(JavaClient());
testRequestHeaders(JavaClient());
testMultipleClients(JavaClient.new);
});
Expand Down