On Thu, 5 Jun 2025 13:30:10 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
> Simplifies content streaming for `HttpRequest.BodyPublishers::ofFile(Path)`, > which delegates to `RequestPublishers.FilePublisher::create(Path)`. This > effectively replaces all usages of `FileInputStream::new` in `HttpClient` > with `Files::newInputStream(Path)`. See the JBS ticket description for the > full story. > > Also removing certain test files which were rendered redundant after > `SecurityManager` removal. We do this in this PR, since tests were added due > to touched lines. > > `tier1,2` results on b5f3c8dc6f4 are attached to the ticket. src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java line 269: > 267: // The old code was using `FileInputStream::new`, which > throws `FNFE` if file doesn't exist. > 268: // Preserving that behaviour after migrating to > `Files::newInputStream`: > 269: t = new FileNotFoundException(path + " (No such file or > directory)"); Hello Volkan, just some initial comments. I think this `Files.isRegularFile(...)` check and the `Files.newInputStream(...)` call should be done in `RequestPublishers.create(...)` instead of here in subscribe. That way the checks happen when the `BodyPublishers.ofFile(...)` gets invoked instead of at subscription time. That then means that this `FilePublisher` will continue to accept a `InputStream` in its constructor, created out of a successful call to `Files.newInputStream()`. Moving these checks and call into `RequestPublishers.create(...)` will also mean that we won't need the code comments explaining why a `FileNotFoundException` gets thrown, because both `BodyPublishers.ofFile(...)` and the (internal) `RequestPublishers.create(...)` are already specified to throw `FileNotFoundException` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25662#discussion_r2128933747
