Skip to content

Added Webclient, refactored Webserver to share streamed body sending and receiving logic.#2771

Draft
jurgenvinju wants to merge 69 commits into
mainfrom
feat/webclient
Draft

Added Webclient, refactored Webserver to share streamed body sending and receiving logic.#2771
jurgenvinju wants to merge 69 commits into
mainfrom
feat/webclient

Conversation

@jurgenvinju

@jurgenvinju jurgenvinju commented May 12, 2026

Copy link
Copy Markdown
Member
  • adds Webclient.rsc which offers Response fetch(Request)
  • mirrors Webserver.rsc exactly, reusing the same Request and Response data-type
  • full implementation of HTTP1.1 client
  • restructuring of Webserver implementation to accomodate the client API: split Body type into send and receive kind:
    • send is used for server response and client put and get
    • receive is used for server put and get and client response
  • factored WebBody.java to reuse receiving and sending HTTP bodies in both the client and the server
  • Server POST/PUT fully implemented now.
  • Added JSON, HTML, XML (de)serialization for sending and receiving from streamed input (avoids local string copying)
  • add tests for the client
  • add tests for the server

Unfortunately Undertow requires at least a Java 17 JVM.

@jurgenvinju jurgenvinju self-assigned this May 12, 2026
@jurgenvinju jurgenvinju requested a review from DavyLandman May 12, 2026 10:28
@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 284 lines in your changes missing coverage. Please review.
✅ Project coverage is 46%. Comparing base (6212f69) to head (748dcc7).
⚠️ Report is 91 commits behind head on main.

Files with missing lines Patch % Lines
src/org/rascalmpl/library/util/Webclient.java 0% 283 Missing ⚠️
.../rascalmpl/exceptions/RuntimeExceptionFactory.java 0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##              main   #2771    +/-   ##
========================================
- Coverage       46%     46%    -1%     
+ Complexity    6718    6714     -4     
========================================
  Files          794     795     +1     
  Lines        65937   66220   +283     
  Branches      9889    9918    +29     
========================================
+ Hits         30760   30761     +1     
- Misses       32797   33079   +282     
  Partials      2380    2380            

☔ 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.

@DavyLandman DavyLandman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

I think this needs tests.

Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.rsc Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.rsc Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
Comment thread src/org/rascalmpl/library/util/Webclient.java Outdated
@jurgenvinju

jurgenvinju commented May 13, 2026

Copy link
Copy Markdown
Member Author

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.

@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

}
catch (InvocationTargetException e) {
throw new JavaMethodLink(className, e.getMessage(), e);
String cause = e.getTargetException().getMessage();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is superfluous now

private IFunction formatters;
private boolean explicitConstructorNames = false;
private boolean explicitDataTypes;
private boolean fileLocationsAsPathOnly = true;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

check why this is gone

return this;
}

public JsonValueWriter setFileLocationsAsPathOnly(boolean setting) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

why is this gone?

this.tf = tf;
this.vf = vf;
this.monitor = monitor;
this.html = null; // initialize later from reified type

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this can be done eagerly, now that we cut off the dependency on TypeStore instances from the ModuleEnvironments.

*/
public IFunction createBodyReceiver(InputStream input, ISourceLocation url, String contentType, String charset) {
TypeStore store = new TypeStore();
var BodyKind = tf.abstractDataType(store, "BodyKind");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

move these to final fields


IConstructor options = kind.asWithKeywordParameters().getParameter("jOptions");

try {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

check if these streams need closing. that was not allowed in the NanoHTTP times, but things may be different now with Undertow.

}
}

// exchange.endExchange();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

check if we need this

}

private String getMimeType(HeaderMap headers) {
String contenType = headers.getFirst(Headers.CONTENT_TYPE);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

contentType

if (parts.length > 1) {
String[] assign = parts[1].split("=");

if (assign[0].equals("charset")) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

trim


private IMap makeMap(Map<String, Deque<String>> headers) {
IMapWriter writer = vf.mapWriter();
for (Entry<String, Deque<String>> entry : headers.entrySet()) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what about the other elements in the Deque?

Response response(Status status, str mimeType, map[str, str] headers, Body body)
= response(status, body[mimeType=mimeType], headers=headers);

@synopsis{Bodies can be sent or received, depending on the context (client or)}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

server

}
}
else {
else if (!kwFormals.isBottom()) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is an unrelated fix that was annoying during testing of the client and server.


public static String yield(IConstructor tree, int limit) {
return yield(tree, false, limit);
return TreeAdapter.yield(tree, false, limit);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Java 17 fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants