Skip to content

Commit

Permalink
Incorporate review feedback (more to follow later)
Browse files Browse the repository at this point in the history
- use uri instead of response.uri()/redirected when constructing ResolvedModuleKey
- add docs for HttpClient implementations
  • Loading branch information
odenix committed Feb 23, 2024
1 parent a46e0fc commit b26b91f
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.net.http.HttpResponse.BodyHandler;
import javax.annotation.concurrent.ThreadSafe;

/** An {@code HttpClient} implementation that throws {@code AssertionError} on every send. */
@ThreadSafe
final class DummyHttpClient implements HttpClient {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.pkl.core.util.ErrorMessages;
import org.pkl.core.util.Exceptions;

/** An {@code HttpClient} implementation backed by {@link java.net.http.HttpClient}. */
@ThreadSafe
final class JdkHttpClient implements HttpClient {
// non-private for testing
Expand Down
5 changes: 5 additions & 0 deletions pkl-core/src/main/java/org/pkl/core/http/LazyHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;

/**
* An {@code HttpClient} implementation that defers creating the underlying HTTP client until the
* first send. A potential drawback of using {@code LazyHttpClient} is that any {@link
* HttpClientInitException} thrown by the underlying client is equally deferred.
*/
@ThreadSafe
class LazyHttpClient implements HttpClient {
private final Supplier<HttpClient> supplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.concurrent.ThreadSafe;

/**
* An {@code HttpClient} decorator that
*
* <ul>
* <li>overrides the {@code User-Agent} header of {@code HttpRequest}s
* <li>sets a request timeout if none is present
* <li>ensures that {@link #close()} is idempotent.
* </ul>
*
* <p>Both {@code User-Agent} header and default request timeout are configurable through {@link
* HttpClient.Builder}.
*/
@ThreadSafe
final class RequestRewritingClient implements HttpClient {
// non-private for testing
Expand Down
6 changes: 4 additions & 2 deletions pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,8 @@ public ResolvedModuleKey resolve(SecurityManager securityManager)
HttpUtils.checkHasStatusCode200(response);
securityManager.checkResolveModule(response.uri());
String text = IoUtils.readString(body);
return ResolvedModuleKeys.virtual(this, response.uri(), text, true);
// intentionally use uri instead of response.uri()
return ResolvedModuleKeys.virtual(this, uri, text, true);
}
}

Expand All @@ -509,7 +510,8 @@ public ResolvedModuleKey resolve(SecurityManager securityManager)
}
securityManager.checkResolveModule(redirected);
var text = IoUtils.readString(stream);
return ResolvedModuleKeys.virtual(this, redirected, text, true);
// intentionally use uri instead of redirected
return ResolvedModuleKeys.virtual(this, uri, text, true);
}
}
}
Expand Down

0 comments on commit b26b91f

Please sign in to comment.