Skip to content

Commit

Permalink
Rename Location.parse() to Location.of()
Browse files Browse the repository at this point in the history
  • Loading branch information
@electrum
electrum committed May 3, 2023
1 parent 0c58983 commit cd4d618
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 73 deletions.
2 changes: 1 addition & 1 deletion lib/trino-filesystem/src/main/java/io/trino/filesystem/Location.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public final class Location
private final OptionalInt port;
private final String path;

public static Location parse(String location)
public static Location of(String location)
{
requireNonNull(location, "location is null");
checkArgument(!location.isEmpty(), "location is empty");
Expand Down
3 changes: 1 addition & 2 deletions lib/trino-filesystem/src/main/java/io/trino/filesystem/local/LocalFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import java.util.Optional;

import static com.google.common.base.Preconditions.checkArgument;
import static io.trino.filesystem.Location.parse;
import static io.trino.filesystem.local.LocalUtils.handleException;

/**
Expand Down Expand Up @@ -173,7 +172,7 @@ private Path toDirectoryPath(String directoryLocation)

private static Location parseLocalLocation(String locationString)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
checkArgument(location.scheme().equals(Optional.of("local")), "Only 'local' scheme is supported: %s", locationString);
checkArgument(location.userInfo().isEmpty(), "Local location cannot contain user info: %s", locationString);
checkArgument(location.host().isEmpty(), "Local location cannot contain a host: %s", locationString);
Expand Down
3 changes: 1 addition & 2 deletions lib/trino-filesystem/src/main/java/io/trino/filesystem/memory/MemoryFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.concurrent.ConcurrentMap;

import static com.google.common.base.Preconditions.checkArgument;
import static io.trino.filesystem.Location.parse;

/**
* A blob file system for testing.
Expand Down Expand Up @@ -173,7 +172,7 @@ private static String toBlobPrefix(String location)

private static Location parseMemoryLocation(String locationString)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
checkArgument(location.scheme().equals(Optional.of("memory")), "Only 'memory' scheme is supported: %s", locationString);
checkArgument(location.userInfo().isEmpty(), "Memory location cannot contain user info: %s", locationString);
checkArgument(location.host().isEmpty(), "Memory location cannot contain a host: %s", locationString);
Expand Down
131 changes: 65 additions & 66 deletions lib/trino-filesystem/src/test/java/io/trino/filesystem/TestLocation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.util.Optional;
import java.util.OptionalInt;

import static io.trino.filesystem.Location.parse;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

Expand Down Expand Up @@ -82,76 +81,76 @@ void testParse()
assertLocation("file:/some@what/path", "file", "some@what/path");

// invalid locations
assertThatThrownBy(() -> parse(null))
assertThatThrownBy(() -> Location.of(null))
.isInstanceOf(NullPointerException.class);

assertThatThrownBy(() -> parse(""))
assertThatThrownBy(() -> Location.of(""))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("location is empty");
assertThatThrownBy(() -> parse(" "))
assertThatThrownBy(() -> Location.of(" "))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("location is blank");
assertThatThrownBy(() -> parse("x"))
assertThatThrownBy(() -> Location.of("x"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("scheme");
assertThatThrownBy(() -> parse("scheme://host:invalid/path"))
assertThatThrownBy(() -> Location.of("scheme://host:invalid/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("port");
assertThatThrownBy(() -> parse("scheme:/path"))
assertThatThrownBy(() -> Location.of("scheme:/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("scheme");

// fragment is not allowed
assertThatThrownBy(() -> parse("scheme://userInfo@host/some/path#fragement"))
assertThatThrownBy(() -> Location.of("scheme://userInfo@host/some/path#fragement"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Fragment");
assertThatThrownBy(() -> parse("scheme://userInfo@ho#st/some/path"))
assertThatThrownBy(() -> Location.of("scheme://userInfo@ho#st/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Fragment");
assertThatThrownBy(() -> parse("scheme://user#Info@host/some/path"))
assertThatThrownBy(() -> Location.of("scheme://user#Info@host/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Fragment");
assertThatThrownBy(() -> parse("sc#heme://userInfo@host/some/path"))
assertThatThrownBy(() -> Location.of("sc#heme://userInfo@host/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Fragment");

// query component is not allowed
assertThatThrownBy(() -> parse("scheme://userInfo@host/some/path?fragement"))
assertThatThrownBy(() -> Location.of("scheme://userInfo@host/some/path?fragement"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("query");
assertThatThrownBy(() -> parse("scheme://userInfo@ho?st/some/path"))
assertThatThrownBy(() -> Location.of("scheme://userInfo@ho?st/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("query");
assertThatThrownBy(() -> parse("scheme://user?Info@host/some/path"))
assertThatThrownBy(() -> Location.of("scheme://user?Info@host/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("query");
assertThatThrownBy(() -> parse("sc?heme://userInfo@host/some/path"))
assertThatThrownBy(() -> Location.of("sc?heme://userInfo@host/some/path"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("query");
}

private static void assertLocation(String locationString, String scheme, Optional<String> userInfo, String host, String path)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
Optional<String> expectedHost = host.isEmpty() ? Optional.empty() : Optional.of(host);
assertLocation(location, locationString, Optional.of(scheme), userInfo, expectedHost, OptionalInt.empty(), path);
}

private static void assertLocation(String locationString, String scheme, String path)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
assertLocation(location, locationString, Optional.of(scheme), Optional.empty(), Optional.empty(), OptionalInt.empty(), path);
}

private static void assertLocation(String locationString, String scheme, String host, int port, String path)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
assertLocation(location, locationString, Optional.of(scheme), Optional.empty(), Optional.of(host), OptionalInt.of(port), path);
}

private static void assertLocation(String locationString, String path)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
assertLocation(location, locationString, Optional.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(), path);
}

Expand All @@ -170,21 +169,21 @@ private static void assertLocation(Location location, String locationString, Opt
assertThat(location.path()).isEqualTo(path);

assertThat(location).isEqualTo(location);
assertThat(location).isEqualTo(parse(locationString));
assertThat(location).isEqualTo(Location.of(locationString));
assertThat(location.hashCode()).isEqualTo(location.hashCode());
assertThat(location.hashCode()).isEqualTo(parse(locationString).hashCode());
assertThat(location.hashCode()).isEqualTo(Location.of(locationString).hashCode());

assertThat(location.toString()).isEqualTo(locationString);
}

@Test
void testVerifyFileLocation()
{
parse("scheme://userInfo@host/name").verifyValidFileLocation();
parse("scheme://userInfo@host/path/name").verifyValidFileLocation();
Location.of("scheme://userInfo@host/name").verifyValidFileLocation();
Location.of("scheme://userInfo@host/path/name").verifyValidFileLocation();

parse("/name").verifyValidFileLocation();
parse("/path/name").verifyValidFileLocation();
Location.of("/name").verifyValidFileLocation();
Location.of("/path/name").verifyValidFileLocation();

assertInvalidFileLocation("scheme://userInfo@host", "File location must contain a path");
assertInvalidFileLocation("scheme://userInfo@host/", "File location must contain a path");
Expand All @@ -198,7 +197,7 @@ void testVerifyFileLocation()

private static void assertInvalidFileLocation(String locationString, String expectedErrorMessage)
{
Location location = parse(locationString);
Location location = Location.of(locationString);
assertThatThrownBy(location::verifyValidFileLocation)
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining(locationString)
Expand Down Expand Up @@ -229,27 +228,27 @@ void testFileName()
private static void assertFileName(String locationString, String fileName)
{
// fileName method only works with valid file locations
Location location = parse(locationString);
Location location = Location.of(locationString);
location.verifyValidFileLocation();
assertThat(location.fileName()).isEqualTo(fileName);
}

@Test
void testParentDirectory()
{
assertParentDirectory("scheme://userInfo@host/path/name", parse("scheme://userInfo@host/path"));
assertParentDirectory("scheme://userInfo@host:1234/name", parse("scheme://userInfo@host:1234"));
assertParentDirectory("scheme://userInfo@host/path/name", Location.of("scheme://userInfo@host/path"));
assertParentDirectory("scheme://userInfo@host:1234/name", Location.of("scheme://userInfo@host:1234"));

assertParentDirectory("scheme://userInfo@host/path//name", parse("scheme://userInfo@host/path/"));
assertParentDirectory("scheme://userInfo@host/path///name", parse("scheme://userInfo@host/path//"));
assertParentDirectory("scheme://userInfo@host/path:/name", parse("scheme://userInfo@host/path:"));
assertParentDirectory("scheme://userInfo@host/path//name", Location.of("scheme://userInfo@host/path/"));
assertParentDirectory("scheme://userInfo@host/path///name", Location.of("scheme://userInfo@host/path//"));
assertParentDirectory("scheme://userInfo@host/path:/name", Location.of("scheme://userInfo@host/path:"));

assertParentDirectory("/path/name", parse("/path"));
assertParentDirectory("/name", parse("/"));
assertParentDirectory("/path/name", Location.of("/path"));
assertParentDirectory("/name", Location.of("/"));

assertParentDirectory("/path//name", parse("/path/"));
assertParentDirectory("/path///name", parse("/path//"));
assertParentDirectory("/path:/name", parse("/path:"));
assertParentDirectory("/path//name", Location.of("/path/"));
assertParentDirectory("/path///name", Location.of("/path//"));
assertParentDirectory("/path:/name", Location.of("/path:"));

// all valid file locations must have a parent directory
// invalid file locations are tested in testVerifyFileLocation
Expand All @@ -258,7 +257,7 @@ void testParentDirectory()
private static void assertParentDirectory(String locationString, Location parentLocation)
{
// fileName method only works with valid file locations
Location location = parse(locationString);
Location location = Location.of(locationString);
location.verifyValidFileLocation();
Location parentDirectory = location.parentDirectory();

Expand All @@ -268,59 +267,59 @@ private static void assertParentDirectory(String locationString, Location parent
@Test
void testAppendPath()
{
assertAppendPath("scheme://userInfo@host", "name", parse("scheme://userInfo@host/name"));
assertAppendPath("scheme://userInfo@host/", "name", parse("scheme://userInfo@host/name"));
assertAppendPath("scheme://userInfo@host", "name", Location.of("scheme://userInfo@host/name"));
assertAppendPath("scheme://userInfo@host/", "name", Location.of("scheme://userInfo@host/name"));

assertAppendPath("scheme://userInfo@host:1234/path", "name", parse("scheme://userInfo@host:1234/path/name"));
assertAppendPath("scheme://userInfo@host/path/", "name", parse("scheme://userInfo@host/path/name"));
assertAppendPath("scheme://userInfo@host:1234/path", "name", Location.of("scheme://userInfo@host:1234/path/name"));
assertAppendPath("scheme://userInfo@host/path/", "name", Location.of("scheme://userInfo@host/path/name"));

assertAppendPath("scheme://userInfo@host/path//", "name", parse("scheme://userInfo@host/path//name"));
assertAppendPath("scheme://userInfo@host/path:", "name", parse("scheme://userInfo@host/path:/name"));
assertAppendPath("scheme://userInfo@host/path//", "name", Location.of("scheme://userInfo@host/path//name"));
assertAppendPath("scheme://userInfo@host/path:", "name", Location.of("scheme://userInfo@host/path:/name"));

assertAppendPath("scheme://", "name", parse("scheme:///name"));
assertAppendPath("scheme:///", "name", parse("scheme:///name"));
assertAppendPath("scheme://", "name", Location.of("scheme:///name"));
assertAppendPath("scheme:///", "name", Location.of("scheme:///name"));

assertAppendPath("scheme:///path", "name", parse("scheme:///path/name"));
assertAppendPath("scheme:///path/", "name", parse("scheme:///path/name"));
assertAppendPath("scheme:///path", "name", Location.of("scheme:///path/name"));
assertAppendPath("scheme:///path/", "name", Location.of("scheme:///path/name"));

assertAppendPath("/", "name", parse("/name"));
assertAppendPath("/path", "name", parse("/path/name"));
assertAppendPath("/", "name", Location.of("/name"));
assertAppendPath("/path", "name", Location.of("/path/name"));
}

private static void assertAppendPath(String locationString, String newPathElement, Location expected)
{
Location location = parse(locationString).appendPath(newPathElement);
Location location = Location.of(locationString).appendPath(newPathElement);
assertLocation(location, expected);
}

@Test
void testAppendSuffix()
{
assertAppendSuffix("scheme://userInfo@host", ".ext", parse("scheme://userInfo@host/.ext"));
assertAppendSuffix("scheme://userInfo@host/", ".ext", parse("scheme://userInfo@host/.ext"));
assertAppendSuffix("scheme://userInfo@host", ".ext", Location.of("scheme://userInfo@host/.ext"));
assertAppendSuffix("scheme://userInfo@host/", ".ext", Location.of("scheme://userInfo@host/.ext"));

assertAppendSuffix("scheme://userInfo@host:1234/path", ".ext", parse("scheme://userInfo@host:1234/path.ext"));
assertAppendSuffix("scheme://userInfo@host/path/", ".ext", parse("scheme://userInfo@host/path/.ext"));
assertAppendSuffix("scheme://userInfo@host:1234/path", ".ext", Location.of("scheme://userInfo@host:1234/path.ext"));
assertAppendSuffix("scheme://userInfo@host/path/", ".ext", Location.of("scheme://userInfo@host/path/.ext"));

assertAppendSuffix("scheme://userInfo@host/path//", ".ext", parse("scheme://userInfo@host/path//.ext"));
assertAppendSuffix("scheme://userInfo@host/path:", ".ext", parse("scheme://userInfo@host/path:.ext"));
assertAppendSuffix("scheme://userInfo@host/path//", ".ext", Location.of("scheme://userInfo@host/path//.ext"));
assertAppendSuffix("scheme://userInfo@host/path:", ".ext", Location.of("scheme://userInfo@host/path:.ext"));

assertAppendSuffix("scheme://", ".ext", parse("scheme:///.ext"));
assertAppendSuffix("scheme:///", ".ext", parse("scheme:///.ext"));
assertAppendSuffix("scheme://", ".ext", Location.of("scheme:///.ext"));
assertAppendSuffix("scheme:///", ".ext", Location.of("scheme:///.ext"));

assertAppendSuffix("scheme:///path", ".ext", parse("scheme:///path.ext"));
assertAppendSuffix("scheme:///path/", ".ext", parse("scheme:///path/.ext"));
assertAppendSuffix("scheme:///path", ".ext", Location.of("scheme:///path.ext"));
assertAppendSuffix("scheme:///path/", ".ext", Location.of("scheme:///path/.ext"));

assertAppendSuffix("scheme:///path", "/foo", parse("scheme:///path/foo"));
assertAppendSuffix("scheme:///path/", "/foo", parse("scheme:///path//foo"));
assertAppendSuffix("scheme:///path", "/foo", Location.of("scheme:///path/foo"));
assertAppendSuffix("scheme:///path/", "/foo", Location.of("scheme:///path//foo"));

assertAppendSuffix("/", ".ext", parse("/.ext"));
assertAppendSuffix("/path", ".ext", parse("/path.ext"));
assertAppendSuffix("/", ".ext", Location.of("/.ext"));
assertAppendSuffix("/path", ".ext", Location.of("/path.ext"));
}

private static void assertAppendSuffix(String locationString, String suffix, Location expected)
{
Location location = parse(locationString).appendSuffix(suffix);
Location location = Location.of(locationString).appendSuffix(suffix);
assertLocation(location, expected);
}
}
4 changes: 2 additions & 2 deletions plugin/trino-hudi/src/main/java/io/trino/plugin/hudi/partition/HiveHudiPartitionInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ public void loadPartitionInfo(Optional<Partition> partition)
throw new HoodieIOException(format("Cannot find partition in Hive Metastore: %s", hivePartitionName));
}
this.relativePartitionPath = getRelativePartitionPath(
Location.parse(table.getStorage().getLocation()),
Location.parse(partition.get().getStorage().getLocation()));
Location.of(table.getStorage().getLocation()),
Location.of(partition.get().getStorage().getLocation()));
this.hivePartitionKeys = buildPartitionKeys(partitionColumns, partition.get().getValues());
}

Expand Down

0 comments on commit cd4d618

Please sign in to comment.