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

Extend url protocols #361

Merged
merged 7 commits into from Aug 19, 2012
Merged

Extend url protocols #361

merged 7 commits into from Aug 19, 2012

Conversation

ghost
Copy link

@ghost ghost commented Jul 17, 2012

Solution to #360: Unable to extend URL protocols handled by ClasspathIterable

I replaced the hard-coded dependencies on FileResourceIterator and ZipResourceIterator in the ClasspathIterable with a factory for loading resource iterators. Resource iterators are now created using ResourceIteratorFactory implementations and making these implementations available to the ClasspathIterable only requires a META-INF/services/cucumber.io.ResourceIteratorFactory file with the names of the implementing classes on each line.

The factory used by ClasspathIterable will try to find an appropriate factory to create resource iterators for each URL. If no appropriate factory is found, it will fall back to the ZipResourceIteratorFactory and then the FileResourceIteratorFactory. This means that existing behavior is not impacted by the changes.

There is a proof-of-concept test, cucumber.io.DelegatingResourceIteratorFactoryTest, which shows the META-INF/services/cucumber.io.ResourceIteratorFactory file in action. Other tests for fall-back capability with ZIP/File resource iterator factories would have been inconclusive because any test resources could exist either as a file or within a JAR depending on the test runtime environment.

Other changes:

  • I added URL-decoding to the ResourceLoaderTest class's dir field because I'm on Windows and spaces in my path were causing the test to fail
  • I added explicit generic argument to line 70 of cucumber.table.DataTable because I couldn't get the code to compile even with modifications to the compiler

Logan McGrath and others added 7 commits July 16, 2012 20:46
Windows and your source code sits in a directory with spaces
* Raised visibility of ClasspathIterable.getPath(URL)
* Created ResourceIteratorFactory interface
* Factored FileResourceIterator creation into ResourceIteratorFactory
implementation
* Added services file for ResourceIteratorFactory
* Factored resource iterators into separate factories
* There seems to be a classpath issue with the ZipResourceIterator,
because when it's created by a factory loaded using the ServiceLoader
class, it's unable to find some ZIP files (this was a problem in the
cucumber-jython tests) the solution was to create the
ZipThenFileResourceIteratorFallback class
* Cosmetic tweaks
* Removed hard-coded dependency on ZipResourceIteratorFactory in
ClasspathIterable
@aslakhellesoy
Copy link
Contributor

I didn't know about the ServiceLoader class!! Maybe that can be used to load the various Backend implementations as well. I see only empty constructors are allowed, but I can live with some setters...

Anyway - nice patch. I'll take a closer look when I have more time.

@ghost
Copy link
Author

ghost commented Jul 17, 2012

I know right?? I didn't know about the ServiceLoader class until I started working with Arquillian several months ago. It loads all its extensions using it.

You can probably follow a similar pattern for the Backend implementations like what I did in the patch and just load factories for creating the back-ends themselves, that way you can get around the no-args constructor problem.

aslakhellesoy added a commit that referenced this pull request Aug 19, 2012
@aslakhellesoy aslakhellesoy merged commit 4d2b84a into cucumber:master Aug 19, 2012
@aslakhellesoy aslakhellesoy mentioned this pull request Jun 24, 2013
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant