You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by dh...@apache.org on 2017/04/26 19:37:39 UTC
[1/4] beam git commit: LocalFileSystem: create parent directories if
needed
Repository: beam
Updated Branches:
refs/heads/master 85aaa6297 -> 869465edd
LocalFileSystem: create parent directories if needed
And improve testing to confirm.
Project: http://git-wip-us.apache.org/repos/asf/beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/080c3391
Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/080c3391
Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/080c3391
Branch: refs/heads/master
Commit: 080c33914410a95953f1e9b796f464ad7ab1d556
Parents: bdee44f
Author: Dan Halperin <dh...@google.com>
Authored: Tue Apr 25 20:48:02 2017 -0700
Committer: Dan Halperin <dh...@google.com>
Committed: Wed Apr 26 12:27:42 2017 -0700
----------------------------------------------------------------------
.../org/apache/beam/sdk/io/LocalFileSystem.java | 18 +++++-
.../apache/beam/sdk/io/LocalFileSystemTest.java | 66 +++++++++++++++-----
.../apache/beam/sdk/io/LocalResourceIdTest.java | 6 +-
3 files changed, 72 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/beam/blob/080c3391/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
index 1cad4b3..8349a35 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
@@ -107,6 +107,14 @@ class LocalFileSystem extends FileSystem<LocalResourceId> {
LocalResourceId src = srcResourceIds.get(i);
LocalResourceId dst = destResourceIds.get(i);
LOG.debug("Copying {} to {}", src, dst);
+ File parent = dst.getCurrentDirectory().getPath().toFile();
+ if (!parent.exists()) {
+ checkArgument(
+ parent.mkdirs() || parent.exists(),
+ "Unable to make output directory %s in order to copy into file %s",
+ parent,
+ dst.getPath());
+ }
// Copy the source file, replacing the existing destination.
// Paths.get(x) will not work on Windows OSes cause of the ":" after the drive letter.
Files.copy(
@@ -131,6 +139,14 @@ class LocalFileSystem extends FileSystem<LocalResourceId> {
LocalResourceId src = srcResourceIds.get(i);
LocalResourceId dst = destResourceIds.get(i);
LOG.debug("Renaming {} to {}", src, dst);
+ File parent = dst.getCurrentDirectory().getPath().toFile();
+ if (!parent.exists()) {
+ checkArgument(
+ parent.mkdirs() || parent.exists(),
+ "Unable to make output directory %s in order to move into file %s",
+ parent,
+ dst.getPath());
+ }
// Rename the source file, replacing the existing destination.
Files.move(
src.getPath(),
@@ -143,7 +159,7 @@ class LocalFileSystem extends FileSystem<LocalResourceId> {
@Override
protected void delete(Collection<LocalResourceId> resourceIds) throws IOException {
for (LocalResourceId resourceId : resourceIds) {
- LOG.debug("deleting file {}", resourceId);
+ LOG.debug("Deleting file {}", resourceId);
Files.delete(resourceId.getPath());
}
}
http://git-wip-us.apache.org/repos/asf/beam/blob/080c3391/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalFileSystemTest.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalFileSystemTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalFileSystemTest.java
index bb5928e..d335974 100644
--- a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalFileSystemTest.java
+++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalFileSystemTest.java
@@ -17,15 +17,16 @@
*/
package org.apache.beam.sdk.io;
-import static com.google.common.base.Preconditions.checkArgument;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import com.google.common.base.Function;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
import com.google.common.io.Files;
import com.google.common.io.LineReader;
import java.io.File;
@@ -121,36 +122,71 @@ public class LocalFileSystemTest {
.close();
}
+ private void assertContents(List<Path> destFiles, List<String> contents)
+ throws Exception {
+ for (int i = 0; i < destFiles.size(); ++i) {
+ assertThat(
+ Files.readLines(destFiles.get(i).toFile(), StandardCharsets.UTF_8),
+ containsInAnyOrder(contents.get(i)));
+ }
+ }
+
@Test
public void testCopyWithExistingSrcFile() throws Exception {
Path srcPath1 = temporaryFolder.newFile().toPath();
Path srcPath2 = temporaryFolder.newFile().toPath();
- Path destPath1 = srcPath1.resolveSibling("dest1");
+ Path destPath1 =
+ temporaryFolder.getRoot().toPath().resolve("nonexistentdir").resolve("dest1");
Path destPath2 = srcPath2.resolveSibling("dest2");
createFileWithContent(srcPath1, "content1");
createFileWithContent(srcPath2, "content2");
- testCopy(
- ImmutableList.of(srcPath1, srcPath2),
+ localFileSystem.copy(
+ toLocalResourceIds(ImmutableList.of(srcPath1, srcPath2), false /* isDirectory */),
+ toLocalResourceIds(ImmutableList.of(destPath1, destPath2), false /* isDirectory */));
+
+ assertContents(
ImmutableList.of(destPath1, destPath2),
ImmutableList.of("content1", "content2"));
}
- private void testCopy(List<Path> srcFiles, List<Path> destFiles, List<String> contents)
- throws Exception {
- checkArgument(srcFiles.size() == destFiles.size());
+ @Test
+ public void testMoveWithExistingSrcFile() throws Exception {
+ Path srcPath1 = temporaryFolder.newFile().toPath();
+ Path srcPath2 = temporaryFolder.newFile().toPath();
- localFileSystem.copy(
- toLocalResourceIds(srcFiles, false /* isDirectory */),
- toLocalResourceIds(destFiles, false /* isDirectory */));
- for (int i = 0; i < srcFiles.size(); ++i) {
- assertThat(
- Files.readLines(destFiles.get(i).toFile(), StandardCharsets.UTF_8),
- containsInAnyOrder(contents.get(i)));
- }
+ Path destPath1 =
+ temporaryFolder.getRoot().toPath().resolve("nonexistentdir").resolve("dest1");
+ Path destPath2 = srcPath2.resolveSibling("dest2");
+
+ createFileWithContent(srcPath1, "content1");
+ createFileWithContent(srcPath2, "content2");
+
+ localFileSystem.rename(
+ toLocalResourceIds(ImmutableList.of(srcPath1, srcPath2), false /* isDirectory */),
+ toLocalResourceIds(ImmutableList.of(destPath1, destPath2), false /* isDirectory */));
+
+ assertContents(
+ ImmutableList.of(destPath1, destPath2),
+ ImmutableList.of("content1", "content2"));
+
+ assertFalse(srcPath1 + "exists", srcPath1.toFile().exists());
+ assertFalse(srcPath2 + "exists", srcPath2.toFile().exists());
+ }
+
+ @Test
+ public void testDelete() throws Exception {
+ File f1 = temporaryFolder.newFile("file1");
+ File f2 = temporaryFolder.newFile("file2");
+ File f3 = temporaryFolder.newFile("other-file");
+ localFileSystem.delete(
+ toLocalResourceIds(Lists.newArrayList(f1.toPath(), f2.toPath()), false /* isDirectory */));
+ assertFalse(f1.exists());
+ assertFalse(f2.exists());
+ assertTrue(f3.exists());
}
@Test
http://git-wip-us.apache.org/repos/asf/beam/blob/080c3391/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
index 7a5f0be..5adfae6 100644
--- a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
+++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
@@ -226,12 +226,14 @@ public class LocalResourceIdTest {
@Test
public void testToString() throws Exception {
File someFile = tmpFolder.newFile("somefile");
- LocalResourceId fileResource = LocalResourceId.fromPath(someFile.toPath(), false);
+ LocalResourceId fileResource =
+ LocalResourceId.fromPath(someFile.toPath(), /* isDirectory */ false);
assertThat(fileResource.toString(), not(endsWith(File.separator)));
assertThat(fileResource.toString(), containsString("somefile"));
assertThat(fileResource.toString(), startsWith(tmpFolder.getRoot().getAbsolutePath()));
- LocalResourceId dirResource = LocalResourceId.fromPath(someFile.toPath(), true);
+ LocalResourceId dirResource =
+ LocalResourceId.fromPath(someFile.toPath(), /* isDirectory */ true);
assertThat(dirResource.toString(), endsWith(File.separator));
assertThat(dirResource.toString(), containsString("somefile"));
assertThat(dirResource.toString(), startsWith(tmpFolder.getRoot().getAbsolutePath()));
[4/4] beam git commit: This closes #2697
Posted by dh...@apache.org.
This closes #2697
Project: http://git-wip-us.apache.org/repos/asf/beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/869465ed
Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/869465ed
Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/869465ed
Branch: refs/heads/master
Commit: 869465edd4cc61e5e7dca1d6a42c4c7bd6622316
Parents: 85aaa62 080c339
Author: Dan Halperin <dh...@google.com>
Authored: Wed Apr 26 12:27:45 2017 -0700
Committer: Dan Halperin <dh...@google.com>
Committed: Wed Apr 26 12:27:45 2017 -0700
----------------------------------------------------------------------
.../org/apache/beam/sdk/io/FileSystems.java | 55 ++++++++++------
.../org/apache/beam/sdk/io/LocalFileSystem.java | 18 +++++-
.../org/apache/beam/sdk/io/LocalResourceId.java | 4 +-
.../apache/beam/sdk/io/LocalFileSystemTest.java | 66 +++++++++++++++-----
.../apache/beam/sdk/io/LocalResourceIdTest.java | 25 ++++++++
5 files changed, 132 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
[2/4] beam git commit: FileSystems: make tolerant of and more
efficient for empty lists
Posted by dh...@apache.org.
FileSystems: make tolerant of and more efficient for empty lists
Project: http://git-wip-us.apache.org/repos/asf/beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/bdee44f1
Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/bdee44f1
Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/bdee44f1
Branch: refs/heads/master
Commit: bdee44f1a675591b6a67883217e6669c5bb7b3b7
Parents: e07ba68
Author: Dan Halperin <dh...@google.com>
Authored: Tue Apr 25 20:48:23 2017 -0700
Committer: Dan Halperin <dh...@google.com>
Committed: Wed Apr 26 12:27:42 2017 -0700
----------------------------------------------------------------------
.../org/apache/beam/sdk/io/FileSystems.java | 55 +++++++++++++-------
1 file changed, 36 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/beam/blob/bdee44f1/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java
index aa247c3..0b50070 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java
@@ -214,22 +214,22 @@ public class FileSystems {
* @param destResourceIds the references of the destination resources
*/
public static void copy(
- List<ResourceId> srcResourceIds,
- List<ResourceId> destResourceIds,
- MoveOptions... moveOptions) throws IOException {
- validateOnlyScheme(srcResourceIds, destResourceIds);
+ List<ResourceId> srcResourceIds, List<ResourceId> destResourceIds, MoveOptions... moveOptions)
+ throws IOException {
+ validateSrcDestLists(srcResourceIds, destResourceIds);
+ if (srcResourceIds.isEmpty()) {
+ // Short-circuit.
+ return;
+ }
- List<ResourceId> srcToCopy;
- List<ResourceId> destToCopy;
+ List<ResourceId> srcToCopy = srcResourceIds;
+ List<ResourceId> destToCopy = destResourceIds;
if (Sets.newHashSet(moveOptions).contains(
MoveOptions.StandardMoveOptions.IGNORE_MISSING_FILES)) {
KV<List<ResourceId>, List<ResourceId>> existings =
filterMissingFiles(srcResourceIds, destResourceIds);
srcToCopy = existings.getKey();
destToCopy = existings.getValue();
- } else {
- srcToCopy = srcResourceIds;
- destToCopy = destResourceIds;
}
if (srcToCopy.isEmpty()) {
return;
@@ -252,22 +252,22 @@ public class FileSystems {
* @param destResourceIds the references of the destination resources
*/
public static void rename(
- List<ResourceId> srcResourceIds,
- List<ResourceId> destResourceIds,
- MoveOptions... moveOptions) throws IOException {
- validateOnlyScheme(srcResourceIds, destResourceIds);
- List<ResourceId> srcToRename;
- List<ResourceId> destToRename;
+ List<ResourceId> srcResourceIds, List<ResourceId> destResourceIds, MoveOptions... moveOptions)
+ throws IOException {
+ validateSrcDestLists(srcResourceIds, destResourceIds);
+ if (srcResourceIds.isEmpty()) {
+ // Short-circuit.
+ return;
+ }
+ List<ResourceId> srcToRename = srcResourceIds;
+ List<ResourceId> destToRename = destResourceIds;
if (Sets.newHashSet(moveOptions).contains(
MoveOptions.StandardMoveOptions.IGNORE_MISSING_FILES)) {
KV<List<ResourceId>, List<ResourceId>> existings =
filterMissingFiles(srcResourceIds, destResourceIds);
srcToRename = existings.getKey();
destToRename = existings.getValue();
- } else {
- srcToRename = srcResourceIds;
- destToRename = destResourceIds;
}
if (srcToRename.isEmpty()) {
return;
@@ -288,6 +288,11 @@ public class FileSystems {
*/
public static void delete(
Collection<ResourceId> resourceIds, MoveOptions... moveOptions) throws IOException {
+ if (resourceIds.isEmpty()) {
+ // Short-circuit.
+ return;
+ }
+
Collection<ResourceId> resourceIdsToDelete;
if (Sets.newHashSet(moveOptions).contains(
MoveOptions.StandardMoveOptions.IGNORE_MISSING_FILES)) {
@@ -329,6 +334,12 @@ public class FileSystems {
private static KV<List<ResourceId>, List<ResourceId>> filterMissingFiles(
List<ResourceId> srcResourceIds, List<ResourceId> destResourceIds) throws IOException {
+ validateSrcDestLists(srcResourceIds, destResourceIds);
+ if (srcResourceIds.isEmpty()) {
+ // Short-circuit.
+ return KV.of(Collections.<ResourceId>emptyList(), Collections.<ResourceId>emptyList());
+ }
+
List<ResourceId> srcToHandle = new ArrayList<>();
List<ResourceId> destToHandle = new ArrayList<>();
@@ -342,13 +353,19 @@ public class FileSystems {
return KV.of(srcToHandle, destToHandle);
}
- private static void validateOnlyScheme(
+ private static void validateSrcDestLists(
List<ResourceId> srcResourceIds, List<ResourceId> destResourceIds) {
checkArgument(
srcResourceIds.size() == destResourceIds.size(),
"Number of source resource ids %s must equal number of destination resource ids %s",
srcResourceIds.size(),
destResourceIds.size());
+
+ if (srcResourceIds.isEmpty()) {
+ // nothing more to validate.
+ return;
+ }
+
Set<String> schemes = FluentIterable.from(srcResourceIds)
.append(destResourceIds)
.transform(new Function<ResourceId, String>() {
[3/4] beam git commit: LocalResourceId: make toString end in '/' for
directories
Posted by dh...@apache.org.
LocalResourceId: make toString end in '/' for directories
Project: http://git-wip-us.apache.org/repos/asf/beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/e07ba68e
Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/e07ba68e
Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/e07ba68e
Branch: refs/heads/master
Commit: e07ba68ea380064ecb8bb9c838e712e2457214c6
Parents: 85aaa62
Author: Dan Halperin <dh...@google.com>
Authored: Tue Apr 25 20:48:52 2017 -0700
Committer: Dan Halperin <dh...@google.com>
Committed: Wed Apr 26 12:27:42 2017 -0700
----------------------------------------------------------------------
.../org/apache/beam/sdk/io/LocalResourceId.java | 4 +++-
.../apache/beam/sdk/io/LocalResourceIdTest.java | 23 ++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/beam/blob/e07ba68e/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalResourceId.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalResourceId.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalResourceId.java
index 091e955..9aa765b 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalResourceId.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalResourceId.java
@@ -21,6 +21,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
+import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;
@@ -45,7 +46,8 @@ class LocalResourceId implements ResourceId {
}
private LocalResourceId(Path path, boolean isDirectory) {
- this.pathString = path.normalize().toString();
+ this.pathString = path.toAbsolutePath().normalize().toString()
+ + (isDirectory ? File.separatorChar : "");
this.isDirectory = isDirectory;
}
http://git-wip-us.apache.org/repos/asf/beam/blob/e07ba68e/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
----------------------------------------------------------------------
diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
index 37bd303..7a5f0be 100644
--- a/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
+++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/io/LocalResourceIdTest.java
@@ -17,9 +17,15 @@
*/
package org.apache.beam.sdk.io;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThat;
+import java.io.File;
import java.nio.file.Paths;
import org.apache.beam.sdk.io.fs.ResolveOptions.StandardResolveOptions;
import org.apache.beam.sdk.io.fs.ResourceId;
@@ -27,6 +33,7 @@ import org.apache.commons.lang3.SystemUtils;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -40,6 +47,8 @@ public class LocalResourceIdTest {
@Rule
public ExpectedException thrown = ExpectedException.none();
+ @Rule
+ public TemporaryFolder tmpFolder = new TemporaryFolder();
@Test
public void testResolveInUnix() throws Exception {
@@ -214,6 +223,20 @@ public class LocalResourceIdTest {
toResourceIdentifier("/root/tmp/"));
}
+ @Test
+ public void testToString() throws Exception {
+ File someFile = tmpFolder.newFile("somefile");
+ LocalResourceId fileResource = LocalResourceId.fromPath(someFile.toPath(), false);
+ assertThat(fileResource.toString(), not(endsWith(File.separator)));
+ assertThat(fileResource.toString(), containsString("somefile"));
+ assertThat(fileResource.toString(), startsWith(tmpFolder.getRoot().getAbsolutePath()));
+
+ LocalResourceId dirResource = LocalResourceId.fromPath(someFile.toPath(), true);
+ assertThat(dirResource.toString(), endsWith(File.separator));
+ assertThat(dirResource.toString(), containsString("somefile"));
+ assertThat(dirResource.toString(), startsWith(tmpFolder.getRoot().getAbsolutePath()));
+ }
+
private LocalResourceId toResourceIdentifier(String str) throws Exception {
boolean isDirectory;
if (SystemUtils.IS_OS_WINDOWS) {