Add LocationResolver framework #311
Conversation
gab1one
force-pushed
the
uri-to-location
branch
from
11ece51 to
c85c5eb
Compare
|
@ctrueden I updated the code based on our discussion on Friday, should be good to merge now. |
gab1one
force-pushed
the
uri-to-location
branch
from
c85c5eb to
4a697a1
Compare
|
@ctrueden I moved the implementation of the resolve method into the interface, as discussed. |
ctrueden
left a comment
ctrueden
left a comment
There was a problem hiding this comment.
Looking really good! Two main things: 1) get rid of the resolve subpackage; and 2) change the FileLocationResolver-related tests to not use getClass().getResource(...).
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
There was a problem hiding this comment.
Let's put this in org.scijava.io.location without the resolve subpackage. The service, while focused on location resolution right now, might do other things as well in the future; it is called LocationService after all and not LocationResolverService.
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
| * #L% | ||
| */ | ||
|
|
||
| package org.scijava.io.location.resolve; |
| public void testURIResolve() throws URISyntaxException { | ||
| URI uri = new URI(getClass().getResource("/").toString()); | ||
| Location loc = resolver.resolve(uri); | ||
| assertTrue(loc instanceof FileLocation); |
There was a problem hiding this comment.
Does that really work always? It seems dicey. Maybe use ClassUtils.getLocation instead? Or just hardcode something like new File(".").getAbsolutePath() which is guaranteed to work on every platform?
| final URI uri = getClass().getResource("/").toURI(); | ||
| final LocationResolver res = loc.getResolver(uri); | ||
|
|
||
| assertTrue(res instanceof FileLocationResolver); |
There was a problem hiding this comment.
Again, not convinced that getClass().getResource(...) URIs will always end up as files. Let's be safer about this.
gab1one
force-pushed
the
uri-to-location
branch
from
4a697a1 to
e8cf3e3
Compare
|
@ctrueden I included your suggestions |
The LocationService manages LocationResolver plugins that provide translation from URI to Location. The LocationService itself contains convenience methods to translate from String to Location.
ctrueden
force-pushed
the
uri-to-location
branch
from
e8cf3e3 to
e3babc8
Compare
The LocationResolverService manages LocationResolver plugins that
provide translation from URI to Location.