Skip to content

Commit

Permalink
Code review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
cmnbroad committed Aug 19, 2024
1 parent edb4168 commit e45cd78
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 41 deletions.
66 changes: 43 additions & 23 deletions src/main/java/htsjdk/beta/io/IOPathUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,43 +73,63 @@ public static void writeStringToPath(final IOPath ioPath, final String contents)
}

/**
* Takes an IOPath and returns a new IOPath object that keeps the same basename as the original but has
* a new extension. If append is set to false, only the last component of an extension will be replaced.
* e.g. "my.fasta.gz" -> "my.fasta.tmp"
* Takes an IOPath and returns a new IOPath object that keeps the same basename as the original but with
* a new extension. Only the last component of the extension will be replaced, e.g. ("my.fasta.gz", ".tmp") ->
* "my.fasta.tmp". If the original path has no extension, an exception will be thrown.
*
* If the input IOPath was created from a rawInputString that specifies a relative local path, the new path will
* have a rawInputString that specifies an absolute path.
*
* Examples:
* - (test_na12878.bam, .bai) -> test_na12878.bai (append = false)
* - (test_na12878.bam, .md5) -> test_na12878.bam.md5 (append = true)
* Examples
* - ("test_na12878.bam", ".bai") -> "test_na12878.bai"
* - ("test_na12878.bam", "bai") -> "test_na12878.bai"
* - ("test_na12878.ext.bam, ".bai") -> "test_na12878.ext.md5"
*
* @param path The original path
* @param newExtension A new file extension. Must include the leading dot e.g. ".txt", ".bam"
* @param append If set to true, append the new extension to the original basename. If false, replace the original extension
* with the new extension. If append = false and the original name has no extension, an exception will be thrown.
* @param newExtension The new file extension. If no leading "." is provided as part of the new extension, one will be added.
* @param ioPathConstructor a function that takes a string and returns an IOPath-derived class of type <T>
* @return A new IOPath object with the new extension
*/
public static <T extends IOPath> T replaceExtension(
final IOPath path,
final String newExtension,
final boolean append,
final Function<String, T> ioPathConstructor){
ValidationUtils.validateArg(newExtension.startsWith("."), "newExtension must start with a dot '.'");

final String oldFileName = path.toPath().getFileName().toString();

String newFileName;
if (append){
newFileName = oldFileName + newExtension;
} else {
final Optional<String> oldExtension = path.getExtension();
if (oldExtension.isEmpty()){
throw new RuntimeException("The original path must have an extension when append = false: " + path.getURIString());
}
newFileName = oldFileName.replaceAll(oldExtension.get() + "$", newExtension);
final String extensionToUse = newExtension.startsWith(".") ?
newExtension :
"." + newExtension;
final Optional<String> oldExtension = path.getExtension();
if (oldExtension.isEmpty()){
throw new RuntimeException("The original path has no extension to replace" + path.getURIString());
}
final String oldFileName = path.toPath().getFileName().toString();
final String newFileName = oldFileName.replaceAll(oldExtension.get() + "$", extensionToUse);
return ioPathConstructor.apply(path.toPath().resolveSibling(newFileName).toUri().toString());
}

/**
* Takes an IOPath and returns a new IOPath object that keeps the same name as the original, but with
* the new extension added. If no leading "." is provided as part of the new extension, one will be added.
*
* If the input IOPath was created from a rawInputString that specifies a relative local path, the new path will
* have a rawInputString that specifies an absolute path.
*
* Examples:
* - ("test_na12878.bam", ".bai") -> "test_na12878.bam.bai"
* - ("test_na12878.bam", "md5") -> "test_na12878.bam.md5"
*
* @param path The original path
* @param extension The file extension to add. If no leading "." is provided as part of the extension, one will be added.
* @param ioPathConstructor a function that takes a string and returns an IOPath-derived class of type <T>
* @return A new IOPath object with the new extension
*/
public static <T extends IOPath> T appendExtension(
final IOPath path,
final String extension,
final Function<String, T> ioPathConstructor){
final String oldFileName = path.toPath().getFileName().toString();
final String newExtension = extension.startsWith(".") ?
extension :
"." + extension;
return ioPathConstructor.apply(path.toPath().resolveSibling(oldFileName + newExtension).toUri().toString());
}
}
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/beta/io/bundle/BundleJSON.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static <T extends IOPath> Bundle toBundle(
String.format("A JSON string with more than one bundle was provided but only a single bundle is allowed in this context (%s)",
e.getMessage()));
}
return bundles.stream().findFirst().get();
return bundles.get(0);
} catch (JSONException | UnsupportedOperationException e2) {
throw new IllegalArgumentException(
String.format("The JSON can be interpreted neither as an individual bundle (%s) nor as a bundle collection (%s)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public static <T extends IOPath> Optional<T> resolveIndex(
final Function<String, T> ioPathConstructor) {
final Set<String> indexExtensions = Set.of(FileExtensions.TRIBBLE_INDEX, FileExtensions.TABIX_INDEX);
for (final String extension : indexExtensions) {
final T putativeIndexPath = IOPathUtils.replaceExtension(variantsHtsPath, extension, true, ioPathConstructor);
final T putativeIndexPath = IOPathUtils.appendExtension(variantsHtsPath, extension, ioPathConstructor);
if (Files.exists(putativeIndexPath.toPath())) {
return Optional.of(putativeIndexPath);
}
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/htsjdk/io/IOPath.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,6 @@ default String getScheme() {
return getURI().getScheme();
}

// default String resolveSibling(final String sibling) {
// return getSiblingPath(sibling).toString();
// }
//
// default String getSiblingPath(final String sibling) {
// return toPath().resolveSibling(sibling).toString();
// }

/**
* @return an {@link Optional} containing the extension of the last component of the hierarchical part of the
* scheme-specific part of the URI, if any, including the ".", or Optional.empty() if the hierarchical name ends
Expand Down
48 changes: 40 additions & 8 deletions src/test/java/htsjdk/io/IOPathUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,55 @@ public class IOPathUtilsTest extends HtsjdkTest {
@DataProvider(name = "replaceExtensionTestData")
public Object[][] getReplaceExtensionTestData() {
return new Object[][]{
{"file:///somepath/a.vcf", ".idx", true, "file:///somepath/a.vcf.idx"},
{"file:///somepath/a.vcf", ".idx", false, "file:///somepath/a.idx"},
{"file:///a.vcf/a.vcf", ".idx", true, "file:///a.vcf/a.vcf.idx"},
{"file:///a.vcf/a.vcf", ".idx", false, "file:///a.vcf/a.idx"},
{"file:///somepath/a.vcf.gz", ".tbi", true, "file:///somepath/a.vcf.gz.tbi"},
{"file:///somepath/a.vcf.gz", ".tbi", false, "file:///somepath/a.vcf.tbi"},
{"file:///somepath/a.vcf", ".idx", "file:///somepath/a.idx"},
{"file:///somepath/a.vcf", "idx", "file:///somepath/a.idx"},
{"file:///a.vcf/a.vcf", ".idx", "file:///a.vcf/a.idx"},
{"file:///a.vcf/a.vcf", "idx", "file:///a.vcf/a.idx"},
{"file:///somepath/a.vcf.gz", ".tbi", "file:///somepath/a.vcf.tbi"},
{"file:///somepath/a.vcf.gz", "tbi", "file:///somepath/a.vcf.tbi"},
};
}

@Test(dataProvider = "replaceExtensionTestData")
public void testReplaceExtension(
final String basePath,
final String extension,
final boolean append,
final String resolvedPath) {
Assert.assertEquals(
IOPathUtils.replaceExtension(new HtsPath(basePath), extension, append, HtsPath::new),
IOPathUtils.replaceExtension(new HtsPath(basePath), extension, HtsPath::new),
new HtsPath(resolvedPath));
}

@Test(expectedExceptions = {RuntimeException.class})
public void testThrowOnMissingExtension() {
try {
IOPathUtils.replaceExtension(new HtsPath("file:///somepath/a"), "idx", HtsPath::new);
Assert.fail("Expected exception");
} catch (final RuntimeException e) {
Assert.assertTrue(e.getMessage().contains("The original path has no extension to replace"));
throw e;
}
}

@DataProvider(name = "appendExtensionTestData")
public Object[][] getAppendExtensionTestData() {
return new Object[][]{
{"file:///somepath/a.vcf", ".idx", "file:///somepath/a.vcf.idx"},
{"file:///somepath/a.vcf", "idx", "file:///somepath/a.vcf.idx"},
{"file:///a.vcf/a.vcf", ".idx", "file:///a.vcf/a.vcf.idx"},
{"file:///a.vcf/a.vcf", "idx", "file:///a.vcf/a.vcf.idx"},
{"file:///somepath/a.vcf.gz", ".tbi", "file:///somepath/a.vcf.gz.tbi"},
{"file:///somepath/a.vcf.gz", "tbi", "file:///somepath/a.vcf.gz.tbi"},
};
}

@Test(dataProvider = "appendExtensionTestData")
public void testAppendExtension(
final String basePath,
final String extension,
final String resolvedPath) {
Assert.assertEquals(
IOPathUtils.appendExtension(new HtsPath(basePath), extension, HtsPath::new),
new HtsPath(resolvedPath));
}

Expand Down

0 comments on commit e45cd78

Please sign in to comment.