8358688: HttpClient: Simplify file streaming in RequestPublishers.FilePublisher
Reviewed-by: dfuchs, jpai
This commit is contained in:
parent
8d33ea7395
commit
610a18e7b3
@ -25,7 +25,6 @@
|
|||||||
|
|
||||||
package jdk.internal.net.http;
|
package jdk.internal.net.http;
|
||||||
|
|
||||||
import java.io.FileInputStream;
|
|
||||||
import java.io.FileNotFoundException;
|
import java.io.FileNotFoundException;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
@ -35,6 +34,7 @@ import java.net.http.HttpRequest.BodyPublisher;
|
|||||||
import java.nio.ByteBuffer;
|
import java.nio.ByteBuffer;
|
||||||
import java.nio.charset.Charset;
|
import java.nio.charset.Charset;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
|
import java.nio.file.NoSuchFileException;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
@ -48,7 +48,6 @@ import java.util.concurrent.Flow;
|
|||||||
import java.util.concurrent.Flow.Publisher;
|
import java.util.concurrent.Flow.Publisher;
|
||||||
import java.util.concurrent.atomic.AtomicReference;
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
import java.util.concurrent.locks.ReentrantLock;
|
import java.util.concurrent.locks.ReentrantLock;
|
||||||
import java.util.function.Function;
|
|
||||||
import java.util.function.Supplier;
|
import java.util.function.Supplier;
|
||||||
|
|
||||||
import jdk.internal.net.http.common.Demand;
|
import jdk.internal.net.http.common.Demand;
|
||||||
@ -228,30 +227,16 @@ public final class RequestPublishers {
|
|||||||
|
|
||||||
private final Path path;
|
private final Path path;
|
||||||
private final long length;
|
private final long length;
|
||||||
private final Function<Path, InputStream> inputStreamSupplier;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Factory for creating FilePublisher.
|
* Factory for creating FilePublisher.
|
||||||
*/
|
*/
|
||||||
public static FilePublisher create(Path path)
|
public static FilePublisher create(Path path)
|
||||||
throws FileNotFoundException {
|
throws FileNotFoundException {
|
||||||
boolean defaultFS = true;
|
|
||||||
|
|
||||||
try {
|
|
||||||
path.toFile().getPath();
|
|
||||||
} catch (UnsupportedOperationException uoe) {
|
|
||||||
// path not associated with the default file system provider
|
|
||||||
defaultFS = false;
|
|
||||||
}
|
|
||||||
|
|
||||||
// existence check must be after FS checks
|
|
||||||
if (Files.notExists(path))
|
if (Files.notExists(path))
|
||||||
throw new FileNotFoundException(path + " not found");
|
throw new FileNotFoundException(path + " not found");
|
||||||
|
|
||||||
boolean finalDefaultFS = defaultFS;
|
|
||||||
Function<Path, InputStream> inputStreamSupplier = (p) ->
|
|
||||||
createInputStream(p, finalDefaultFS);
|
|
||||||
|
|
||||||
long length;
|
long length;
|
||||||
try {
|
try {
|
||||||
length = Files.size(path);
|
length = Files.size(path);
|
||||||
@ -259,26 +244,12 @@ public final class RequestPublishers {
|
|||||||
length = -1;
|
length = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
return new FilePublisher(path, length, inputStreamSupplier);
|
return new FilePublisher(path, length);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static InputStream createInputStream(Path path,
|
private FilePublisher(Path name, long length) {
|
||||||
boolean defaultFS) {
|
|
||||||
try {
|
|
||||||
return defaultFS
|
|
||||||
? new FileInputStream(path.toFile())
|
|
||||||
: Files.newInputStream(path);
|
|
||||||
} catch (IOException io) {
|
|
||||||
throw new UncheckedIOException(io);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
private FilePublisher(Path name,
|
|
||||||
long length,
|
|
||||||
Function<Path, InputStream> inputStreamSupplier) {
|
|
||||||
path = name;
|
path = name;
|
||||||
this.length = length;
|
this.length = length;
|
||||||
this.inputStreamSupplier = inputStreamSupplier;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@ -286,7 +257,14 @@ public final class RequestPublishers {
|
|||||||
InputStream is = null;
|
InputStream is = null;
|
||||||
Throwable t = null;
|
Throwable t = null;
|
||||||
try {
|
try {
|
||||||
is = inputStreamSupplier.apply(path);
|
// Throw `FileNotFoundException` to match the specification of `BodyPublishers::ofFile
|
||||||
|
if (!Files.isRegularFile(path)) {
|
||||||
|
throw new FileNotFoundException(path + " (Not a regular file)");
|
||||||
|
}
|
||||||
|
is = Files.newInputStream(path);
|
||||||
|
} catch (NoSuchFileException nsfe) {
|
||||||
|
// Throw `FileNotFoundException` to match the specification of `BodyPublishers::ofFile`
|
||||||
|
t = new FileNotFoundException(path + " (No such file or directory)");
|
||||||
} catch (UncheckedIOException | UndeclaredThrowableException ue) {
|
} catch (UncheckedIOException | UndeclaredThrowableException ue) {
|
||||||
t = ue.getCause();
|
t = ue.getCause();
|
||||||
} catch (Throwable th) {
|
} catch (Throwable th) {
|
||||||
|
@ -1,5 +1,5 @@
|
|||||||
/*
|
/*
|
||||||
* Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
|
* Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
|
||||||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
||||||
*
|
*
|
||||||
* This code is free software; you can redistribute it and/or modify it
|
* This code is free software; you can redistribute it and/or modify it
|
||||||
@ -23,9 +23,9 @@
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* @test
|
* @test
|
||||||
* @bug 8235459
|
* @bug 8235459 8358688
|
||||||
* @summary Confirm that HttpRequest.BodyPublishers#ofFile(Path)
|
* @summary Verifies `HttpRequest.BodyPublishers#ofFile(Path)` against file
|
||||||
* assumes the default file system
|
* systems that support `Path#toFile()` and also those that don't
|
||||||
* @library /test/lib /test/jdk/java/net/httpclient/lib
|
* @library /test/lib /test/jdk/java/net/httpclient/lib
|
||||||
* @build jdk.httpclient.test.lib.common.HttpServerAdapters
|
* @build jdk.httpclient.test.lib.common.HttpServerAdapters
|
||||||
* jdk.test.lib.net.SimpleSSLContext
|
* jdk.test.lib.net.SimpleSSLContext
|
||||||
@ -39,11 +39,10 @@ import org.testng.annotations.DataProvider;
|
|||||||
import org.testng.annotations.Test;
|
import org.testng.annotations.Test;
|
||||||
|
|
||||||
import javax.net.ssl.SSLContext;
|
import javax.net.ssl.SSLContext;
|
||||||
|
import java.io.FileNotFoundException;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import java.io.OutputStream;
|
import java.io.OutputStream;
|
||||||
import java.net.InetAddress;
|
|
||||||
import java.net.InetSocketAddress;
|
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
import java.net.http.HttpClient;
|
import java.net.http.HttpClient;
|
||||||
import java.net.http.HttpRequest;
|
import java.net.http.HttpRequest;
|
||||||
@ -61,6 +60,7 @@ import static java.net.http.HttpClient.Builder.NO_PROXY;
|
|||||||
import static java.net.http.HttpClient.Version.HTTP_1_1;
|
import static java.net.http.HttpClient.Version.HTTP_1_1;
|
||||||
import static java.net.http.HttpClient.Version.HTTP_2;
|
import static java.net.http.HttpClient.Version.HTTP_2;
|
||||||
import static org.testng.Assert.assertEquals;
|
import static org.testng.Assert.assertEquals;
|
||||||
|
import static org.testng.Assert.fail;
|
||||||
|
|
||||||
public class FilePublisherTest implements HttpServerAdapters {
|
public class FilePublisherTest implements HttpServerAdapters {
|
||||||
SSLContext sslContext;
|
SSLContext sslContext;
|
||||||
@ -156,6 +156,26 @@ public class FilePublisherTest implements HttpServerAdapters {
|
|||||||
send(uriString, path, expectedMsg, sameClient);
|
send(uriString, path, expectedMsg, sameClient);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testFileNotFound() throws Exception {
|
||||||
|
out.printf("\n\n--- testFileNotFound(): starting\n");
|
||||||
|
try (FileSystem fs = newZipFs()) {
|
||||||
|
Path fileInZip = fs.getPath("non-existent.txt");
|
||||||
|
BodyPublishers.ofFile(fileInZip);
|
||||||
|
fail();
|
||||||
|
} catch (FileNotFoundException e) {
|
||||||
|
out.println("Caught expected: " + e);
|
||||||
|
}
|
||||||
|
var path = Path.of("fileNotFound.txt");
|
||||||
|
try {
|
||||||
|
Files.deleteIfExists(path);
|
||||||
|
BodyPublishers.ofFile(path);
|
||||||
|
fail();
|
||||||
|
} catch (FileNotFoundException e) {
|
||||||
|
out.println("Caught expected: " + e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static final int ITERATION_COUNT = 3;
|
private static final int ITERATION_COUNT = 3;
|
||||||
|
|
||||||
private void send(String uriString,
|
private void send(String uriString,
|
||||||
@ -193,9 +213,6 @@ public class FilePublisherTest implements HttpServerAdapters {
|
|||||||
zipFs = newZipFs();
|
zipFs = newZipFs();
|
||||||
zipFsPath = zipFsFile(zipFs);
|
zipFsPath = zipFsFile(zipFs);
|
||||||
|
|
||||||
InetSocketAddress sa =
|
|
||||||
new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
|
|
||||||
|
|
||||||
httpTestServer = HttpServerAdapters.HttpTestServer.create(HTTP_1_1);
|
httpTestServer = HttpServerAdapters.HttpTestServer.create(HTTP_1_1);
|
||||||
httpTestServer.addHandler(new HttpEchoHandler(), "/http1/echo");
|
httpTestServer.addHandler(new HttpEchoHandler(), "/http1/echo");
|
||||||
httpURI = "http://" + httpTestServer.serverAuthority() + "/http1/echo";
|
httpURI = "http://" + httpTestServer.serverAuthority() + "/http1/echo";
|
||||||
|
@ -1,5 +1,5 @@
|
|||||||
/*
|
/*
|
||||||
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
|
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
|
||||||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
||||||
*
|
*
|
||||||
* This code is free software; you can redistribute it and/or modify it
|
* This code is free software; you can redistribute it and/or modify it
|
||||||
@ -42,7 +42,7 @@ import static org.testng.Assert.assertEquals;
|
|||||||
* @test
|
* @test
|
||||||
* @summary Verifies that some of the standard BodyPublishers relay exception
|
* @summary Verifies that some of the standard BodyPublishers relay exception
|
||||||
* rather than throw it
|
* rather than throw it
|
||||||
* @bug 8226303
|
* @bug 8226303 8358688
|
||||||
* @library /test/lib
|
* @library /test/lib
|
||||||
* @run testng/othervm RelayingPublishers
|
* @run testng/othervm RelayingPublishers
|
||||||
*/
|
*/
|
||||||
|
Loading…
x
Reference in New Issue
Block a user