Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12 - Add tests in util/resource for alternate FileSystem implementations #9149

Merged

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Jan 10, 2023

This adds some jetty-util Resource tests for alternate FileSystem providers, to ensure that we are not doing anything awkward in our implementation that would prevent it.

This uses the https://github.com/google/jimfs implementation of an In-Memory FileSystem implementation to accomplish these tests.
(This filesystem uses scheme jimfs and has a URI authority representing the named FileSystem)

This was initiated as a response to a question about our support for alternate FileSystem implementations in PR #9136

URL url = ResourceFactoryInternals.class.getResource("/org/eclipse/jetty/version/build.properties");
if ((url != null) && !RESOURCE_FACTORIES.contains(url.getProtocol()))
{
RESOURCE_FACTORIES.put(url.getProtocol(), mountedPathResourceFactory);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using a new MountedPathResourceFactory instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No real reason to, the implementation wouldn't change with each new protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An argument could be made that there should be a test here, to see if the URL points to a mounted FileSystem or not, and adding the appropriate ResourceFactory based on results.

Or using some variation of the mountIfNeeded() logic always here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If adding some logic to figure out if mounting is needed or not doesn't take too much, let's add it. Otherwise let's get these changes make it in and eventually open an enhancement bug.

@joakime joakime requested a review from lorban January 10, 2023 17:17
@joakime joakime merged commit 8374860 into jetty-12.0.x Jan 13, 2023
gregpoulos pushed a commit to gregpoulos/jetty.project that referenced this pull request Jan 16, 2023
…x-document-modules

* upstream/jetty-12.0.x:
  Issue jetty#9167 - making assumption in flaky test
  jetty 12.0.x cleanup duplicate osgi pom metadata (jetty#9093)
  Jetty 12 - Add tests in util/resource for alternate FileSystem implementations (jetty#9149)
  Cleanup non-retainable `Retainable`s (jetty#9159)
  Fixes retainability of special Chunks (jetty#9073)
  TCK: Dispatch forward and includes attributes do not meet the spec (jetty#9074)
  re-enable h3 tests (jetty#8773)
  More fundamental test case
  Reorganization of jetty-client classes. (jetty#9127)
  Removing @disabled from SslUploadTest
  Removing @disabled from jetty-start
  jetty#9134 added test
  ee10: DefaultServlet: Replace checks for isStreaming() by !isWriting()
  jetty#9078 make HttpContent.getByteBuffer() implementations return new ByteBuffer instances and document that contract
  Fixes jetty#9141 - Thread-safe Content.Chunk#slice (jetty#9142)
  Remove `@Disabled` from `jetty-jmx` (jetty#9143)
  Bump maven.version from 3.8.6 to 3.8.7
  Bump maven.version from 3.8.6 to 3.8.7
gregpoulos pushed a commit to gregpoulos/jetty.project that referenced this pull request Jan 16, 2023
… into jetty-12.0.x-documentation-lowercase-jetty_home-attribute

* 'jetty-12.0.x' of https://github.com/eclipse/jetty.project:
  Issue jetty#9167 - making assumption in flaky test
  jetty 12.0.x cleanup duplicate osgi pom metadata (jetty#9093)
  Jetty 12 - Add tests in util/resource for alternate FileSystem implementations (jetty#9149)
  Cleanup non-retainable `Retainable`s (jetty#9159)
  Fixes retainability of special Chunks (jetty#9073)
  TCK: Dispatch forward and includes attributes do not meet the spec (jetty#9074)
  re-enable h3 tests (jetty#8773)
  More fundamental test case
  Reorganization of jetty-client classes. (jetty#9127)
  Removing @disabled from SslUploadTest
  Removing @disabled from jetty-start
  Bump maven.version from 3.8.6 to 3.8.7
  Bump maven.version from 3.8.6 to 3.8.7
@joakime joakime deleted the fix/jetty-12.0.x/support-alternate-filesytem-mounts branch January 23, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants