On Fri, 6 Jun 2025 07:38:21 GMT, Volkan Yazici <vyaz...@openjdk.org> 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 specified and `NoSuchFileException` is not then >> that's probably a good change of behaviour. >> >> Could be worth a release note (if that's the case and the behaviour changed) >> - but probably not a CSR. > > 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::new` throws `NSFE` when passed a not-regular file, e.g., a directory) > was thrown at `subscribe()`. Moving this check from `subscribe()` to > `create()` breaks the `RelayingPublishers` test, since failure gets moved > from subscription-time to [reactive pipeline] assembly-time. > > Would you prefer me to > > 1. keep things as is, > 2. move `isRegularFile()` to `create()`, or > 3. move `isRegularFile()` to `create()` and file a CSR? > > Note that I don't think we can avoid the `NSFE`-to-`FNFE` translation in > `subscribe()`, since, whatever earlier check we do before `Files::newIS`, > file might get changed afterwards, and `NSFE` can still be thrown at the > `Files::newIS` invocation. > 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't support `Path::toFile`<sup>1</sup>, earlier users would get `Files::newIS` exceptions (which were indeed undocumented), whereas others would get `FIS::new` exceptions. <sup>1</sup> Allow me to note that I intentionally avoid using "default file system" term, since, IMHO, that distracts us from the problem: file systems that don't support `Path::toFile`. The "default file system" is required to support that feature, but there can be other file systems that support that too. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25662#discussion_r2131690023
