recovered webclient branch; needs a little clean up by jurgenvinju · Pull Request #2771 · usethesource/rascal

jurgenvinju · 2026-05-12T10:28:15Z

added initial webclient API that mirrors the Webserver API and uses the same Response and Request encodings
cleanup and added POST method
added the other methods
added progress bar
fixed post

…he same Response and Request encodings

codecov · 2026-05-12T10:38:10Z

Codecov Report

❌ Patch coverage is 0% with 184 lines in your changes missing coverage. Please review.
✅ Project coverage is 46%. Comparing base (bf08ac0) to head (ddf4bfa).

Files with missing lines	Patch %	Lines
src/org/rascalmpl/library/util/Webclient.java	0%	184 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##              main   #2771    +/-   ##
========================================
- Coverage       46%     46%    -1%     
  Complexity    6728    6728            
========================================
  Files          794     795     +1     
  Lines        65936   66120   +184     
  Branches      9888    9900    +12     
========================================
- Hits         30860   30854     -6     
- Misses       32693   32874   +181     
- Partials      2383    2392     +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sonarqubecloud · 2026-05-12T14:29:13Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

DavyLandman

I think this is a nice idea but not ready yet, I've written down my concerns.

I think this needs tests.

DavyLandman · 2026-05-12T17:33:15Z

+    private HttpRequest makeGetRequest(IConstructor input) {
+        var params = input.asWithKeywordParameters();
+        var host = ((ISourceLocation) params.getParameter("host"));
+        host = host == null ? URIUtil.assumeCorrectLocation("http://www.example.com") : host;


I think a missing host should not default to a request against http://www.example.com but instead throw an exception.

what if the scheme is not http/https?

If the scheme is not https then the host will not be resolved and you get the appropriate http error code.

okay, but the fallback to example.com should not be there, but it should throw an exception for a missing host.

DavyLandman · 2026-05-12T17:36:13Z

+
+            Thread writer = new Thread(() -> {
+                try (OutputStream os = out; Writer w = new OutputStreamWriter(out)) {
+                    JsonWriter jsonWriter = new JsonWriter(w);


not all POST request have JSON bodies, we should think of a design that doesn't enforce json, but allows the user to specify the body format.

Yes that's possible. The mirror design in response body's is that we have text with arbitrary mime types, files with mime types decided by file extension and JSON values as arbitrary serialized rascal values. That seems to be general enough.

DavyLandman · 2026-05-12T18:23:38Z

+            PipedOutputStream out = new PipedOutputStream();
+            PipedInputStream in = new PipedInputStream(out, 64 * 1024);
+
+            Thread writer = new Thread(() -> {


I don't really like this pattern of starting a separate thread just to invert the stream direction. Looking at the BodyPublishers, it should be okayish to write a class that wraps a ByteBuffer around a buffer that's getting flushed.

Here is such an implementation:

public class OutputStreamBodySupplier extends BufferedOutputStream implements BodyPublisher { private final List<Subscriber<? super ByteBuffer>> subscribers; public OutputStreamBodySupplier() { super(new PublishingStream()); this.subscribers = ((PublishingStream)super.out).subscribers; } /** * The buffed outputstream will take care to collect the bytes untill there's a decent chunk to forward to the consumers */ private static class PublishingStream extends OutputStream { private final List<Subscriber<? super ByteBuffer>> subscribers = new CopyOnWriteArrayList<>(); @Override public void write(int b) throws IOException { for (var sub: subscribers) { sub.onNext(ByteBuffer.wrap(new byte[] { (byte)(b & 0xFF) })); } } @Override public void write(byte[] b, int off, int len) throws IOException { for (var sub: subscribers) { sub.onNext(ByteBuffer.wrap(b, off, len).asReadOnlyBuffer()); } } @Override public void close() throws IOException { for (var sub: subscribers) { sub.onComplete(); } } } @Override public void subscribe(Subscriber<? super ByteBuffer> subscriber) { this.subscribers.add(subscriber); } @Override public long contentLength() { return -1; } }

Me too. Thanks for the code!

DavyLandman · 2026-05-12T18:28:39Z

+            PipedInputStream in = new PipedInputStream(out, 64 * 1024);
+
+            Thread writer = new Thread(() -> {
+                try (OutputStream os = out; Writer w = new OutputStreamWriter(out)) {


the encoding should be set for the writer, and the same encoding should also be set in the http headers.

nit: the writer will close os/out no need to add that to the try with resources.

DavyLandman · 2026-05-12T18:30:33Z

+                    jsonOut.write(jsonWriter, val);
+                }
+                catch (Exception e) {
+                    throw RuntimeExceptionFactory.io(e.getMessage());


this exception will never pop up in rascal, as it's running on a separate thread.

That's another negative aspect of this thread loop indeed. Let's get rid of it.

DavyLandman · 2026-05-12T19:32:02Z

+    | unsupportedMediaType()
+    ;
+
+data Request(


I'm missing the option to set headers in the request, quite some API's require this.

That's provided in the Content module. Here you see only the things we need extra to make the client api work. The goal is to make Request and Response be shared as much as possible between server and client API.

okay, but I'm also missing that mapping in the java side. that's why I remarked that here.

DavyLandman · 2026-05-12T19:34:43Z

+
+@synopsis{Short-hand for construction of JSON post bodies}
+Request jsonPost(str path, &T content, loc host=|http://www.example.com|, BodyExpectation body = textBody()) 
+    = post(path, &T (type[&T] _expected) { return content; }, host=host, body=body);


Shouldn't this also change the content type to application/json?

Yes and also there are missing parameters for the headers, query, etc.

DavyLandman · 2026-05-12T19:36:48Z

+            .build();
+    }
+
+    private HttpRequest makePutRequest(IConstructor input) {


content-type is missing?

That should be one of the header settings in the keyword field of this constructor. I'll check.

DavyLandman · 2026-05-12T19:39:21Z

+            });
+
+            writer.start();
+


missing content-type?

DavyLandman · 2026-05-12T19:42:02Z

+        Type respCons;
+        IString body;
+
+        switch (expect != null ? expect.getName() : "textBody") {


I'm a bit surprised, from the Webclient.rsc I got the expression the ExpectedBody was about the body of the http request. but here it looks like it's about the expected type of the response? I think that is not well aligned in the naming and the feature of the post implementation.

It is for all requests, get, put, post, etc. When the server sends a response it has the three options: text, file or JSON (serialized from any rascal value). Here the client mirrors that design exactly where it can receive text, a file or json (parsed to a rascal value).

See above: posts also have bodies, but there you get the server perspective/direction so I'm inclined to see if we can factor the common parts. BodyProviders and BodyConsumers.

This will break backward compatibility for post in webserver so I wanted to push this to a later PR.

jurgenvinju · 2026-05-13T19:32:11Z

Great review @DavyLandman I'll try and improve this PR and let's have a look together later again.

I've tested this client so far with a couple of web APIs and some very large test downloads. It seems very natural apart from the host/path/query separation. I think some shorthands where a full URL can be used which is split up internally would be nice. Also there is no support for fragments yet.

jurgenvinju added 5 commits May 11, 2026 14:58

added initial webclient API that mirrors the Webserver API and uses t…

10d71ba

…he same Response and Request encodings

cleanup and added POST method

645d334

added the other methods

61cadea

added progress bar

6cce6de

fixed post

17723ff

jurgenvinju self-assigned this May 12, 2026

jurgenvinju requested a review from DavyLandman May 12, 2026 10:28

constructor typo

b9c1e6e

jurgenvinju added 2 commits May 12, 2026 16:25

fix post bug

8c9c0da

added path to other requests kinds but GET and POST

ddf4bfa

DavyLandman reviewed May 12, 2026

View reviewed changes

Conversation

jurgenvinju commented May 12, 2026

Uh oh!

codecov Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud Bot commented May 12, 2026

Quality Gate passed

Uh oh!

DavyLandman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jurgenvinju commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented May 12, 2026 •

edited

Loading

jurgenvinju commented May 13, 2026 •

edited

Loading