Entity Provider for java.nio.file.Path

[mkarg]

Closing #1274

This pull request provides the needed changes in the spec document and the TCK to support java.nio.file.Path entity providers.

[jim-krueger]

Looks good. I see no reason not to add this.

[jim-krueger]

Upon further review: I have some comments/questions.

[jim-krueger]

Question: All of these tests (including what was added here) have this @Tag. Do they really all require xml_binding? If not then this may be a problem (even for Jakarta Rest 4.0) since xml_binding is no longer required by the Platform, so none of these tests will run assuming tests tagged this way will be excluded. This applies to all new tests in this PR.

[jamezp]

+1, given Jakarta XML Binding is no longer required, implementations won't need to run these effectively allowing implementations to pass without even implementing the feature.

[mkarg]

I actually did a 1:1 copy from the "file" originals, just replacing "file" by "path". So I assume that why you mention was already wrong before. I hence kindly ask to separate that discussion from this PR, and address that issue in a separate PR.

[jim-krueger]

I've written Issue 1277 to document this. However, I don't see the point of adding additional tests that will not run. I understand that you may not have time to evaluate/address all of the tests in this class, but any new that you add should not have that dependency.

[mkarg]

So what is the correct solution?

• Simply removing @Tag("xml_binding") from the added test?
• Not adding the test at all?

[jim-krueger]

I see your point Markus. As long as Issue 1277 is addressed prior to the next release. Can you confirm that the new TCK tests you've added have run successfully (since they currently would be skipped as part of a Jakarta Rest 4.0 TCK execution correct?)

[jamezp]

I'm not sure we should be adding new TCK tests that don't run. It kind of defeats the purpose of a test. While I can understand that's how the old tests work, there's not reason to make new tests work the same way. There could very easily be a new jsonContent instance variable created that these tests use. I see no reason for it to to be XML content. It could even be plain text.

There would be new interceptors needed too, but again IMO that's not really a big deal.

[mkarg]

@jim-krueger I cannot confirm that they run, as there is not yet any implementation that fulfils the new API (or maybe I misunderstood your question).

[mkarg]

@jamezp For me it is a big deal, as it imposes unnecessary work for me now and it imposes a temporary deviation from the original code, which I want to prevent as good as possible. OTOH as you feel this is not a big deal, maybe you like to chime in and fix #1277 upfront, so I can rebase on the already fixed code base in a second step? We can leave open this PR for the time being. WDYT?

[mkarg]

How do we proceed? Apparently nobody chimed in to fix #1277, so could we please merge this feature and solve #1277 later?

[mkarg]

Will answer open question ASAP, unfortunately CORONA hit me, so it might take a few days. Sorry!

[jim-krueger]

Get well Markus, no rush. Also, it seems like now would be a good time to create a release plan for 4.1 (or 5.0 if needed) and add this to it.

@spericas / others, do you concur?

[spericas]

Get well Markus, no rush. Also, it seems like now would be a good time to create a release plan for 4.1 (or 5.0 if needed) and add this to it.

@spericas / others, do you concur?

Do we have any release drivers for 4.1?

[mkarg]

Get well Markus, no rush. Also, it seems like now would be a good time to create a release plan for 4.1 (or 5.0 if needed) and add this to it.
@spericas / others, do you concur?

Do we have any release drivers for 4.1?

As long as we all commit to really getting 5.0 thru the door in time, then I have no driver for 4.1. Looking at how things worked out in the past months, I doubt that this time it will work out better than the years before, so maybe we should start with 4.1 instead of 5.0...

[mkarg]

Kindly asking everybody to vote on this PR. Thanks! :-)

[jansupol]

lgtm

[spericas]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

[mkarg]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

Besides migrating JAX-RS's API to modern Java, it mostly is performance (not necessarily speed, but mostly power savings). In short, there is a huge difference how a file is handled by the JRE (all work done by JRE) internally to how a path is (if any possible, all work offloaded to OS). Falling back to files, when you actually have a path, is squandering valuable resources. In modern Java, nobody should ever go with file but always with path. If you are interested in more details, we should spin-off the thread.

[spericas]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

Besides migrating JAX-RS's API to modern Java, it mostly is performance (not necessarily speed, but mostly power savings). In short, there is a huge difference how a file is handled by the JRE (all work done by JRE) internally to how a path is (if any possible, all work offloaded to OS). Falling back to files, when you actually have a path, is squandering valuable resources. In modern Java, nobody should ever go with file but always with path. If you are interested in more details, we should spin-off the thread.

@jansupol How does Jersey handle java.nio.file.Path in the impl?

[spericas]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

Besides migrating JAX-RS's API to modern Java, it mostly is performance (not necessarily speed, but mostly power savings). In short, there is a huge difference how a file is handled by the JRE (all work done by JRE) internally to how a path is (if any possible, all work offloaded to OS). Falling back to files, when you actually have a path, is squandering valuable resources. In modern Java, nobody should ever go with file but always with path. If you are interested in more details, we should spin-off the thread.

@jansupol How does Jersey handle java.nio.file.Path in the impl?

From what I gather, the implementations for Path and File are basically identical:

• https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/PathProvider.java
• https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/FileProvider.java

Based on this, it's difficult to justify the advantages over the costs of needing to update all implementations, docs, etc. that this PR proposes.

[mkarg]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

Besides migrating JAX-RS's API to modern Java, it mostly is performance (not necessarily speed, but mostly power savings). In short, there is a huge difference how a file is handled by the JRE (all work done by JRE) internally to how a path is (if any possible, all work offloaded to OS). Falling back to files, when you actually have a path, is squandering valuable resources. In modern Java, nobody should ever go with file but always with path. If you are interested in more details, we should spin-off the thread.

@jansupol How does Jersey handle java.nio.file.Path in the impl?

From what I gather, the implementations for Path and File are basically identical:

* https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/PathProvider.java

* https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/FileProvider.java

Based on this, it's difficult to justify the advantages over the costs of needing to update all implementations, docs, etc. that this PR proposes.

No need to convince me. It was me who wrote "PathProvider". In fact, I would even go as far to say that support for "File" should become deprecated eventually. It simply is dead legacy.

As an OpenJDK NIO2 contributor I like to make everybody understand that "File" is a rather outdated concept, while "Path" is what every new software should go with. The difference is not what Jersey does, but what the JRE does. For those interested in the details, I would be more than happy to explain in a separate thread.

Edit: The core idea of this MR is to modernize JAX-RS, and using "Path" should be a no-brainer, actually. Sticking with File but not supporting Path is simply ridiculous and draws a picture of an old man's ancient not actively maintained API.

[spericas]

What is the real benefit in natively supporting java.nio.file.Path when you have java.nio.file.Path#toFile? Just trying to understand the rationale here.

Besides migrating JAX-RS's API to modern Java, it mostly is performance (not necessarily speed, but mostly power savings). In short, there is a huge difference how a file is handled by the JRE (all work done by JRE) internally to how a path is (if any possible, all work offloaded to OS). Falling back to files, when you actually have a path, is squandering valuable resources. In modern Java, nobody should ever go with file but always with path. If you are interested in more details, we should spin-off the thread.

@jansupol How does Jersey handle java.nio.file.Path in the impl?

From what I gather, the implementations for Path and File are basically identical:

* https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/PathProvider.java

* https://github.com/eclipse-ee4j/jersey/blob/3.1/core-common/src/main/java/org/glassfish/jersey/message/internal/FileProvider.java

Based on this, it's difficult to justify the advantages over the costs of needing to update all implementations, docs, etc. that this PR proposes.

No need to convince me. It was me who wrote "PathProvider". In fact, I would even go as far to say that support for "File" should become deprecated eventually. It simply is dead legacy.

As an OpenJDK NIO2 contributor I like to make everybody understand that "File" is a rather outdated concept, while "Path" is what every new software should go with. The difference is not what Jersey does, but what the JRE does. For those interested in the details, I would be more than happy to explain in a separate thread.

From an API perspective, I understand that Path may be desirable in some cases (yet, this is a bit of a weak argument IMO). From an implementation perspective, which seems to be your argument above, it seems that there can be zero difference if the implementation of File uses Path as the one in Jersey --again both providers seem to be doing exactly the same.

[mkarg]

From an API perspective, I understand that Path may be desirable in some cases (yet, this is a bit of a weak argument IMO). From an implementation perspective, which seems to be your argument above, it seems that there can be zero difference if the implementation of File uses Path as the one in Jersey --again both providers seem to be doing exactly the same.

Santiago, I think there is a misunderstanding, so let me clarify -- even if I still think that it would be better to start a new thread elsewhere, as this explanation is partly off-topic IMHO as it is about OpenJDK and Jersey, not about JAX-RS.

Maybe my reasoning was misleading. I do not see why you link this PR with my current implementation inside Jersey in particular (which is something which might change further over time, and which I actually changed in the NIO2-area several times just recently, so for this PR it plays absolutely no role how good or not-so-well done my solution in Jersey is currently or will be in future); the idea of this PR is to allow optimized implementations, which might or might not happen eventually or might or might not exist currently. Current Jersey itelf might or might not experience a benefit from Path-instead-of-File already, but that is simply irrelevant for opening the potential provided by native Path support. Other implementations might do better or never optimize at all; it is their choice - and even that is irrelevant. The point is, that the intended resource optimization happens not within any JAX-RS implementation, but it happens within the JRE!

Looking at OpenJDK's source code of File::toPath and Path::toFile we can see that these conversions imply (at least) the following drawbacks:

• Path::toFile only works with paths created by the default provider. All other providers will throw UnsupportedOperationException. This means, as of today, JAX-RS is incompatible with these providers! Or in other words, the missing native Path support in JAX-RS is (more or less) a showstopper for applications that work with these providers (yes, there is always the workaround of using raw streams instead of typed entities, but not using JAX-RS features is definitively not what we want people to do).
• Due to different parsing and toString algorithms, it might happen that the string after conversion actually differs from the original string. This might lead to weird bugs at runtime which are (more or less) hard to find and/or hard to work around. Path and File are even applying different rulesets what strings they can hold, so it could happen, that a string is not convertable at all (the reason is that a Path MAY be a C-implementation which is dependent of the provider and/or filesystem, while a File is more or less a simple FS/OS-independent Java-implemented string wrapper). This implies (besides other problems) that File::toPath might fail with InvalidPathException (another hard-to-workaround showstopper we won't people to experience at runtime)!
• File::toPath is synchronized, at least for the first invocation of each file instance, which implies a speed penalty. As each invocation of Path::toFile creates a new instance, effectively this means that conversion-and-reconversion always induces that speed penalty!
• File::toPath accesses a volatile variable as part of the aforementioned caching algorithm. As a result, all invocations of File::toPath imply passing-by the cache, or in other words, are pretty slow.
• Creating a Path or File instance implies (in most cases) one or many C calls via JNI, which implies a speed penalty, which sums up when performing conversion-and-reconversion.

Jersey: I already optimized Jersey (at least in part) for NIO2, i. e. for native use of Path, so it omits (in many cases, but not in all - something I like to fix in future) most of these drawbacks. As I hope is now clear, the optimization is not obvious to see in the Jersey source code, as Jersey only paves the way for not running into these JRE-internal obstacles by preventing unnecessary / multiple conversion. As a result, Jersey is able to do the same work with less resources (which might or might not lead to performance optimizations depending on the actual use case and environment). If you look carefully, you will notice that the two source codes you refer to are slightly different: One is using conversion (which is a bad thing, as we learned now), the other is not (or at least, not that often - until I eventually fixed that). Also see that the Path based provider bears potential for future optimization (which I would be happen to contribute), while typically no more optimizations will flow into the File based provider - just as the OpenJDK Team frequently optimizes the modern NIO2 API under the hood, while optimiztations to the 30yrs old IO API happen only sparingly).

Regarding your non-technical arguments, I like to say the following:

• Driver: The major driver for adopting this PR is allowing implementations to spare resources (whether or not sparing actually happens is not under my control, but at least implementors have a chance to optimize it, as there is no more force to convert-and-reconvert). Nevertheless, from the aspect of an OpenJDK contributor, also a major driver definitively is to trigger people to use Path everywhere and get rid of File ASAP. This includes triggering JAX-RS implementors to replace files by paths as a side effect of implementing this PR, but it also includes triggering JAX-RS users to do the same in their applications. NIO2 is key for future performance optimizations measured in both, speed and resource consumption. The earlier everybody switches from IO to NIO2, the better - particular in I/O-savvy applications like REST servers.
• Efforts: The work in Jersey is already done mostly (I did that in very few hours), and I am willing to contribute the missing bricks; Jan even already voted +1 for this PR. The work in other implementations should be few hours only, and I could contribute if needed; Jim already wrote "Looks good. I see no reason not to add this.". The work in JAX-RS itself is already done in this PR. So I actually cannot see what or whom you mean with "efforts".
• Users more and more experience Java, and in particular JAX-RS, as "ancient". If we want to stop that trend, we need to adopt modern features from current JREs. This implies, among other things, Path. I am really concerned you started this discussion as apparently readers could think the project lead is not planning to adapt JAX-RS to Java's future at all, leading to more people going away from JAX-RS. So I really beg you to please not stand in the way of the adoption of modern Java features. Thank you.

[spericas]

From an API perspective, I understand that Path may be desirable in some cases (yet, this is a bit of a weak argument IMO). From an implementation perspective, which seems to be your argument above, it seems that there can be zero difference if the implementation of File uses Path as the one in Jersey --again both providers seem to be doing exactly the same.

Santiago, I think there is a misunderstanding, so let me clarify -- even if I still think that it would be better to start a new thread elsewhere, as this explanation is partly off-topic IMHO as it is about OpenJDK and Jersey, not about JAX-RS.

The discussion is very much about JAX-RS, the side note about implementations was prompted by your performance claims.

Maybe my reasoning was misleading. I do not see why you link this PR with my current implementation inside Jersey in particular (which is something which might change further over time, and which I actually changed in the NIO2-area several times just recently, so for this PR it plays absolutely no role how good or not-so-well done my solution in Jersey is currently or will be in future); the idea of this PR is to allow optimized implementations, which might or might not happen eventually or might or might not exist currently. Current Jersey itelf might or might not experience a benefit from Path-instead-of-File already, but that is simply irrelevant for opening the potential provided by native Path support. Other implementations might do better or never optimize at all; it is their choice - and even that is irrelevant. The point is, that the intended resource optimization happens not within any JAX-RS implementation, but it happens within the JRE!

JAX-RS implementations drive the JRE, the JRE does not drive itself. I'm not opposed to this new feature, but as I stated above, I'm not convinced it is worth the effort. Hopefully others can comment and we can proceed with it.

[jansupol]

Given we support the File, I would not want to restrain the customers from using NIO API,

On the other hand, I see the real usage of Path much lower than the usage of the Status Codes defined in RFC 9110, which still did not make it to Jakarta REST Response.Status.

[mkarg]

Now that we all have exchanged our personal opinions, in the name of modern Java, I do beg all committers to vote +1. Thanks.