On Thu, 12 Jun 2025 14:42:27 GMT, Daniel Fuchs wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Simplify `FilePublisherTest`
>
> The person who wrote the test was probably concerned that
> `fs.getPath("non-existen
On Thu, 12 Jun 2025 09:21:08 GMT, Volkan Yazici 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`
>> wit
On Thu, 12 Jun 2025 08:34:02 GMT, Jaikiran Pai wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix `FilePublisherTest`
>
> test/jdk/java/net/httpclient/FilePublisher/FilePublisherTest.java line 168:
>
>> 166:
On Thu, 12 Jun 2025 10:05:14 GMT, Volkan Yazici wrote:
> Would that suffice?
Thank you, yes that's good enough. It's good to go ahead with the integration.
-
PR Comment: https://git.openjdk.org/jdk/pull/25662#issuecomment-2966111409
> 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 descripti
On Thu, 12 Jun 2025 09:33:38 GMT, Jaikiran Pai wrote:
> This looks good to me. I don't expect this latest change to cause a failure
> in CI, but it would be good to have this test run on all platforms before
> integrating (doesn't have to be the entire tier).
@jaikiran, I've already run the up
On Thu, 12 Jun 2025 09:21:08 GMT, Volkan Yazici 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`
>> wit
On Wed, 11 Jun 2025 12:32:13 GMT, Volkan Yazici 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`
>> wit
On Fri, 6 Jun 2025 08:25:44 GMT, Daniel Fuchs wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add back removed SM tests
>
> The new version LGTM. I wonder if we should add `@bug 8358688` to the various
> FilePubl
> 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 descripti
On Wed, 11 Jun 2025 10:49:48 GMT, Volkan Yazici 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`
>> wit
On Wed, 11 Jun 2025 10:49:48 GMT, Volkan Yazici 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`
>> wit
> 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 descripti
On Wed, 11 Jun 2025 09:50:32 GMT, Jaikiran Pai wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update tests
>
> src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java
> line 268:
>
>> 266:
On Fri, 6 Jun 2025 11:07:34 GMT, Volkan Yazici 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
On Wed, 11 Jun 2025 08:56:01 GMT, Volkan Yazici wrote:
>> Hello Volkan,
>>
>>> Earlier the regular file check was performed by FIS::new at subscription,
>>> hence, NSFE (yes, FIS::new throws NSFE when passed a not-regular file,
>>> e.g., a directory) was thrown at subscribe()
>>
>> The `NoSuc
On Fri, 6 Jun 2025 11:07:34 GMT, Volkan Yazici 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
On Sun, 8 Jun 2025 16:38:09 GMT, Jaikiran Pai wrote:
>> Thanks for the clarification.
>
> Hello Volkan,
>
>> Earlier the regular file check was performed by FIS::new at subscription,
>> hence, NSFE (yes, FIS::new throws NSFE when passed a not-regular file, e.g.,
>> a directory) was thrown at s
On Fri, 6 Jun 2025 11:49:31 GMT, Daniel Fuchs wrote:
>>> The "default file system" is required to support that feature, but there
>>> can be other file systems that support that too.
>>
>> I need to amend this statement: Per [`Path::toFile` specification](
>> https://docs.oracle.com/en/java/ja
On Fri, 6 Jun 2025 11:13:38 GMT, Volkan Yazici wrote:
>> OK
>
>> The "default file system" is required to support that feature, but there can
>> be other file systems that support that too.
>
> I need to amend this statement: Per [`Path::toFile` specification](
> https://docs.oracle.com/en/jav
On Fri, 6 Jun 2025 08:21:31 GMT, Daniel Fuchs wrote:
>>> I believe the old code was not always throwing `FileNotFoundException` -
>>> but might have thrown `NoSuchFileException` if using a non-default
>>> filesystem.
>>
>> @dfuch, correct. If `Path` is associated with a file system that doesn'
On Fri, 6 Jun 2025 11:07:00 GMT, Volkan Yazici wrote:
>> 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:
On Fri, 6 Jun 2025 11:07:34 GMT, Volkan Yazici 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
On Fri, 6 Jun 2025 08:28:30 GMT, Daniel Fuchs wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add back removed SM tests
>
> src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java
> line 269:
On Fri, 6 Jun 2025 08:25:44 GMT, Daniel Fuchs wrote:
> I wonder if we should add `@bug 8358688` to the various FilePublisher test -
> or just consider the fix as noreg-cleanup. On the one hand those tests might
> have failed if you hadn't catched and transformed NSFE. On the other hand
> they
> 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 descripti
On Fri, 6 Jun 2025 08:11:35 GMT, Volkan Yazici 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
On Fri, 6 Jun 2025 07:44:33 GMT, Volkan Yazici wrote:
>> Placing `isRegularFile()` check to `create()` indeed was my initial attempt
>> too. Though that resulted in a behavior change: Earlier the regular file
>> check was performed by `FIS::new` at subscription, hence, `NSFE` (yes,
>> `FIS::ne
On Fri, 6 Jun 2025 07:47:17 GMT, Volkan Yazici wrote:
> Such a test already exists: FilePublisherTest. I deleted its SM counterpart:
> FilePublisherPermsTest.
This test seems to test more things and less things than FilePublisherTest
(typically it does test FileNotFoundException). We shouldn't
> 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 descripti
On Thu, 5 Jun 2025 14:03:30 GMT, Jaikiran Pai wrote:
> it might still be better to remove those tests independently in a separate
> task.
@jaikiran, sure, will do.
> While at it, I think we should also introduce a regression test which does a
> `BodyPublishers.ofFile(Path)` against a `Path` p
On Fri, 6 Jun 2025 07:38:21 GMT, Volkan Yazici wrote:
>> Good idea. FWIW, and ulness I'm mistaken, I believe the old code was not
>> always throwing `FileNotFoundException` - but might have thrown
>> `NoSuchFileException` if using a non-default filesystem. If
>> `FileNotFoundException` is sp
On Thu, 5 Jun 2025 15:19:09 GMT, Daniel Fuchs wrote:
>> 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:
On Thu, 5 Jun 2025 13:30:10 GMT, Volkan Yazici 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 `Fi
On Thu, 5 Jun 2025 13:59:47 GMT, Jaikiran Pai 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
On Thu, 5 Jun 2025 13:30:10 GMT, Volkan Yazici 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 `Fi
On Thu, 5 Jun 2025 13:30:10 GMT, Volkan Yazici wrote:
> 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.
I think, if it's possible, then it might still be better to remove those
On Thu, 5 Jun 2025 13:30:10 GMT, Volkan Yazici 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 `Fi
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 t
39 matches
Mail list logo
