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

Remove akka from runtime #8953

Merged
merged 36 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0b38e89
Add withDebug commands to some projects
Akirathan Feb 2, 2024
9f8832f
Add MultipartBodyPublisher
Akirathan Feb 2, 2024
0c9719b
URIBuilder uses java.net.URI
Akirathan Feb 2, 2024
3688cc8
Refactor HTTPRequestBuilder to java.net from akka
Akirathan Feb 2, 2024
bd71fd9
Migrate downloader to JDK HttpClient
Akirathan Feb 9, 2024
b17f86b
fmt
Akirathan Feb 9, 2024
8444663
Merge branch 'develop' into wip/akirathan/8222-remove-akka-from-downl…
Akirathan Feb 9, 2024
0ef2ffd
fmt
Akirathan Feb 9, 2024
506c582
Merge branch 'develop' into wip/akirathan/8222-remove-akka-from-downl…
Akirathan Feb 12, 2024
dc1e582
Extract runServer method in HTTPTestHelperServer
Akirathan Feb 12, 2024
7d79c02
downloader uses http-test-helper for testing
Akirathan Feb 12, 2024
ed8a567
Ensure http-test-helper main method is blocked until interrupted
Akirathan Feb 12, 2024
28173fb
Add some HttpDownloader tests
Akirathan Feb 12, 2024
4163d29
Add bodyhandlers that report HttpReponse progress
Akirathan Feb 13, 2024
a681dc2
Add optional logging to simple-library-server tool
Akirathan Feb 13, 2024
b845057
Fix IOException in PathProgressBodyHandler
Akirathan Feb 13, 2024
3e6f3a4
Fix URIBuilder
Akirathan Feb 13, 2024
b9cdeea
Simplify posting files
Akirathan Feb 13, 2024
0bab2bb
fmt
Akirathan Feb 13, 2024
c746448
Merge branch 'develop' into wip/akirathan/8222-remove-akka-from-downl…
Akirathan Feb 13, 2024
62c1414
remove unused import
Akirathan Feb 13, 2024
ec44437
remove akka from downloader
Akirathan Feb 13, 2024
5eaef2e
remove akka from logging-service.
Akirathan Feb 13, 2024
8c63995
Split connected-lock-manager into two projects.
Akirathan Feb 13, 2024
553a785
Add no akka dependencies test to runtime.
Akirathan Feb 13, 2024
212070d
fmt
Akirathan Feb 13, 2024
f3f3606
Update license review
Akirathan Feb 13, 2024
4f1e54c
Change line-endings of some license files
Akirathan Feb 14, 2024
0d30555
Fix compilation of connected-lock-manager/Test
Akirathan Feb 14, 2024
0e2b1d6
Merge branch 'develop' into wip/akirathan/8222-remove-akka-from-downl…
Akirathan Feb 16, 2024
c5cc2dc
URIBuilder handles special characters
Akirathan Feb 16, 2024
743e167
Capture the output from native-image build.
Akirathan Feb 16, 2024
6a9fff2
fmt
Akirathan Feb 16, 2024
ba0a69d
Pass arguments to native-image via argfile
Akirathan Feb 19, 2024
148a64d
Merge branch 'develop' into wip/akirathan/8222-remove-akka-from-downl…
Akirathan Feb 19, 2024
9965c4c
Update report-state in license review
Akirathan Feb 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ lazy val enso = (project in file("."))
`library-manager`,
`library-manager-test`,
`connected-lock-manager`,
`connected-lock-manager-server`,
syntax,
testkit,
`common-polyglot-core-utils`,
Expand Down Expand Up @@ -734,8 +735,7 @@ lazy val `logging-service` = project
libraryDependencies ++= Seq(
"org.slf4j" % "slf4j-api" % slf4jVersion,
"com.typesafe" % "config" % typesafeConfigVersion,
"org.scalatest" %% "scalatest" % scalatestVersion % Test,
akkaHttp
"org.scalatest" %% "scalatest" % scalatestVersion % Test
)
)
.dependsOn(`logging-utils`)
Expand Down Expand Up @@ -1383,7 +1383,7 @@ lazy val `language-server` = (project in file("engine/language-server"))
.dependsOn(`json-rpc-server`)
.dependsOn(`task-progress-notifications`)
.dependsOn(`library-manager`)
.dependsOn(`connected-lock-manager`)
.dependsOn(`connected-lock-manager-server`)
.dependsOn(`edition-updater`)
.dependsOn(`logging-utils-akka`)
.dependsOn(`logging-service`)
Expand Down Expand Up @@ -1800,6 +1800,7 @@ lazy val `runtime-integration-tests` =
.dependsOn(`runtime-test-instruments`)
.dependsOn(`logging-service-logback` % "test->test")
.dependsOn(testkit % Test)
.dependsOn(`connected-lock-manager-server`)

/** A project that holds only benchmarks for `runtime`. Unlike `runtime-integration-tests`, its execution requires
* the whole `runtime-fat-jar` assembly, as we want to be as close to the enso distribution as possible.
Expand Down Expand Up @@ -2481,19 +2482,23 @@ lazy val editions = project
lazy val downloader = (project in file("lib/scala/downloader"))
.settings(
frgaalJavaCompilerSetting,
// Fork the tests to make sure that the withDebug command works (we can
// attach debugger to the subprocess)
(Test / fork) := true,
commands += WithDebugCommand.withDebug,
version := "0.1",
libraryDependencies ++= circe ++ Seq(
"com.typesafe.scala-logging" %% "scala-logging" % scalaLoggingVersion,
"commons-io" % "commons-io" % commonsIoVersion,
"org.apache.commons" % "commons-compress" % commonsCompressVersion,
"org.scalatest" %% "scalatest" % scalatestVersion % Test,
akkaActor,
akkaStream,
akkaHttp,
akkaSLF4J
"junit" % "junit" % junitVersion % Test,
"com.github.sbt" % "junit-interface" % junitIfVersion % Test,
"org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test
)
)
.dependsOn(cli)
.dependsOn(`http-test-helper`)

lazy val `edition-updater` = project
.in(file("lib/scala/edition-updater"))
Expand Down Expand Up @@ -2542,6 +2547,7 @@ lazy val `library-manager-test` = project
.settings(
frgaalJavaCompilerSetting,
Test / fork := true,
commands += WithDebugCommand.withDebug,
Test / javaOptions ++= testLogProviderOptions,
Test / test := (Test / test).tag(simpleLibraryServerTag).value,
libraryDependencies ++= Seq(
Expand All @@ -2557,6 +2563,21 @@ lazy val `library-manager-test` = project
lazy val `connected-lock-manager` = project
.in(file("lib/scala/connected-lock-manager"))
.configs(Test)
.settings(
frgaalJavaCompilerSetting,
libraryDependencies ++= Seq(
"com.typesafe.scala-logging" %% "scala-logging" % scalaLoggingVersion,
"org.scalatest" %% "scalatest" % scalatestVersion % Test
)
)
.dependsOn(`distribution-manager`)
.dependsOn(`polyglot-api`)

/** Unlike `connected-lock-manager` project, has a dependency on akka.
*/
lazy val `connected-lock-manager-server` = project
.in(file("lib/scala/connected-lock-manager-server"))
.configs(Test)
.settings(
frgaalJavaCompilerSetting,
libraryDependencies ++= Seq(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ object ProjectUploader {
showProgress: Boolean,
logLevel: Level
): Unit = {
import scala.concurrent.ExecutionContext.Implicits.global
val progressReporter = new ProgressReporter {
override def trackProgress(
message: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.enso.runtime.test;

import static org.junit.Assert.fail;

import org.junit.Test;

public class DependenciesTest {
@Test
public void noAkkaDependencies() {
try {
Class.forName("akka.actor.ActorSystem$");
fail("akka.actor.ActorSystem should not be on the classpath");
Copy link
Member Author

Choose a reason for hiding this comment

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

This test is a bit clumsy. If akka.actor.ActorSystem is not on the class-path, it only means that runtime/test does not have akka dependency, it does not check that there are no akka classes in runtime.jar. However, I still think that this test is useful. Moreover, I have manually checked that runtime.jar does not contain any akka classes.

} catch (ClassNotFoundException e) {
// expected
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.enso.downloader.http;

import java.io.IOException;
import java.net.http.HttpRequest;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

/**
* This utility class adds support for {@code multipart/form-data} content-type as there is no such
* support in {@code java.net.http} JDK.
*
* <p>Inspired by <a href="https://stackoverflow.com/a/56482187/4816269">SO</a>.
*/
public class FilesMultipartBodyPublisher {
public static HttpRequest.BodyPublisher ofMimeMultipartData(
Comment on lines +12 to +19
Copy link
Member

Choose a reason for hiding this comment

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

I don't like us implementing such utilities ourselves instead of using existing well-tested solutions.

This works nicely for simple cases, but how do we guarantee that it will work well in general? Should we try testing it as well as an existing solution would? That adds a lot of maintenance burden on us.

Example 1: the boundary.

I see it is generated as a UUID. I guess it is extremely unlikely that the same UUID would appear inside of the contents of one of the parts submitted as part of this form, but it is not 100% guaranteed.

I'm a bit nitpicky here as indeed it is going to be very unlikely to run into this kind of collision. But was this even considered when this code was written?

This is a seemingly simple operation but it may raise many uncertainities like the example above. IMO relying on an existing solution is better as it does not put the maintenance burden of ensuring this solution is bug free, on us.

Copy link
Member Author

Choose a reason for hiding this comment

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

The downloader project makes only a very specific kind of HTTP requests and expects a very specific and narrow set of HTTP responses. We are in a full control over these reqs and responses. This project is only used for library uploading and downloading, which is also covered by tests.

Implementing a very simple handling of multipart/form-data is a minimal price we have to pay here. Please keep in mind that we are trying very hard, in general, to reduce the number of classes that will eventually get into runtime.jar.

Collection<Path> files, String boundary) throws IOException {
// Result request body
List<byte[]> byteArrays = new ArrayList<>();

// Separator with boundary
byte[] separator =
("--" + boundary + "\r\nContent-Disposition: form-data; name=\"files\";")
.getBytes(StandardCharsets.UTF_8);

for (Path path : files) {
// Opening boundary
byteArrays.add(separator);

// If value is type of Path (file) append content type with file name and file binaries,
// otherwise simply append key=value
String mimeType = Files.probeContentType(path);
byteArrays.add(
(" filename=\"" + path.getFileName() + "\"\r\nContent-Type: " + mimeType + "\r\n\r\n")
.getBytes(StandardCharsets.UTF_8));
byteArrays.add(Files.readAllBytes(path));
byteArrays.add("\r\n".getBytes(StandardCharsets.UTF_8));
}

// Closing boundary
byteArrays.add(("--" + boundary + "--").getBytes(StandardCharsets.UTF_8));

return HttpRequest.BodyPublishers.ofByteArrays(byteArrays);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package org.enso.downloader.http;

import java.io.IOException;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodySubscriber;
import java.net.http.HttpResponse.ResponseInfo;
import java.nio.ByteBuffer;
import java.nio.channels.WritableByteChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.Flow;
import org.enso.cli.task.TaskProgressImplementation;
import scala.Option;
import scala.Some;

/** A {@link HttpResponse} body handler for {@link Path} that keeps track of the progress. */
class PathProgressBodyHandler implements HttpResponse.BodyHandler<Path> {
private final Path destination;
private final TaskProgressImplementation<Path> progress;
private Long total;

private PathProgressBodyHandler(
Path destination, TaskProgressImplementation<Path> progress, Long total) {
this.destination = destination;
this.progress = progress;
this.total = total;
}

static PathProgressBodyHandler of(
Path destination, TaskProgressImplementation<Path> progress, Long sizeHint) {
return new PathProgressBodyHandler(destination, progress, sizeHint);
}

@Override
public BodySubscriber<Path> apply(ResponseInfo responseInfo) {
if (total == null) {
var reportedLenOpt = responseInfo.headers().firstValueAsLong("Content-Length");
if (reportedLenOpt.isPresent()) {
total = reportedLenOpt.getAsLong();
}
}
if (total != null) {
progress.reportProgress(0, Some.apply(total));
} else {
progress.reportProgress(0, Option.empty());
}
WritableByteChannel destChannel;
try {
destChannel =
Files.newByteChannel(destination, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
} catch (IOException e) {
throw new IllegalStateException(e);
}
return new ProgressSubscriber(destChannel);
}

private class ProgressSubscriber implements BodySubscriber<Path> {
private Flow.Subscription subscription;
private final WritableByteChannel destChannel;
private final CompletableFuture<Path> result = new CompletableFuture<>();

ProgressSubscriber(WritableByteChannel destChannel) {
this.destChannel = destChannel;
}

@Override
public void onSubscribe(Flow.Subscription subscription) {
this.subscription = subscription;
this.subscription.request(1);
}

@Override
public void onNext(List<ByteBuffer> items) {
try {
for (ByteBuffer item : items) {
var len = item.remaining();
progress.reportProgress(len, total == null ? Option.empty() : Some.apply(total));
destChannel.write(item);
}
subscription.request(1);
} catch (IOException e) {
subscription.cancel();
throw new RuntimeException(e);
}
}

@Override
public void onError(Throwable throwable) {
try {
destChannel.close();
} catch (IOException e) {
throwable.addSuppressed(e);
}
result.completeExceptionally(throwable);
}

@Override
public void onComplete() {
try {
destChannel.close();
} catch (IOException e) {
throw new IllegalStateException(e);
}
result.complete(destination);
}

@Override
public CompletionStage<Path> getBody() {
return CompletableFuture.completedFuture(destination);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package org.enso.downloader.http;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodySubscriber;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.Flow;
import org.enso.cli.task.TaskProgressImplementation;
import scala.Option;
import scala.Some;

/** A {@link HttpResponse} body handler for {@link Path} that keeps track of the progress. */
public class StringProgressBodyHandler implements HttpResponse.BodyHandler<String> {
private final ByteArrayOutputStream destination = new ByteArrayOutputStream();
private final TaskProgressImplementation<?> progress;
private final Charset encoding;
private Long total;

private StringProgressBodyHandler(
TaskProgressImplementation<?> progress, Charset encoding, Long total) {
this.progress = progress;
this.encoding = encoding;
this.total = total;
}

public static StringProgressBodyHandler of(
TaskProgressImplementation<?> progress, Long sizeHint, Charset encoding) {
return new StringProgressBodyHandler(progress, encoding, sizeHint);
}

@Override
public HttpResponse.BodySubscriber<String> apply(HttpResponse.ResponseInfo responseInfo) {
if (total == null) {
var reportedLenOpt = responseInfo.headers().firstValueAsLong("Content-Length");
if (reportedLenOpt.isPresent()) {
total = reportedLenOpt.getAsLong();
}
}
if (total != null) {
progress.reportProgress(0, Some.apply(total));
} else {
progress.reportProgress(0, Option.empty());
}
return new ProgressSubscriber();
}

private class ProgressSubscriber implements BodySubscriber<String> {
private Flow.Subscription subscription;
private final CompletableFuture<String> result = new CompletableFuture<>();

@Override
public void onSubscribe(Flow.Subscription subscription) {
this.subscription = subscription;
this.subscription.request(Long.MAX_VALUE);
}

@Override
public void onNext(List<ByteBuffer> items) {
for (ByteBuffer item : items) {
var len = item.remaining();
progress.reportProgress(len, total == null ? Option.empty() : Some.apply(total));
byte[] bytes = new byte[len];
item.get(bytes);
destination.write(bytes, 0, bytes.length);
}
subscription.request(Long.MAX_VALUE);
}

@Override
public void onError(Throwable throwable) {
try {
destination.close();
} catch (IOException e) {
throwable.addSuppressed(e);
}
result.completeExceptionally(throwable);
}

@Override
public void onComplete() {
try {
destination.close();
} catch (IOException e) {
throw new IllegalStateException(e);
}
result.complete(destination.toString(encoding));
}

@Override
public CompletionStage<String> getBody() {
return result;
}
}
}
Loading
Loading