From 8bfaa80037365c0790083313a905d1e7d88b0682 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Mon, 28 Mar 2016 14:13:48 -0700 Subject: [PATCH] HADOOP-10965. Print fully qualified path in CommandWithDestination error messages. Contributed by John Zhuge. --- .../org/apache/hadoop/fs/PathIOException.java | 9 ++ .../fs/shell/CommandWithDestination.java | 3 +- .../org/apache/hadoop/fs/shell/Touch.java | 3 +- .../org/apache/hadoop/fs/TestFsShellCopy.java | 52 +++++++++-- .../apache/hadoop/fs/TestFsShellTouch.java | 88 +++++++++++++++++++ 5 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellTouch.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIOException.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIOException.java index 459a83669a..deb3880ee4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIOException.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIOException.java @@ -33,6 +33,7 @@ public class PathIOException extends IOException { // uris with no authority private String operation; private String path; + private String fullyQualifiedPath; private String targetPath; /** @@ -68,6 +69,11 @@ protected PathIOException(String path, String error, Throwable cause) { this.path = path; } + public PathIOException withFullyQualifiedPath(String fqPath) { + fullyQualifiedPath = fqPath; + return this; + } + /** Format: * cmd: {operation} `path' {to `target'}: error string */ @@ -85,6 +91,9 @@ public String getMessage() { if (getCause() != null) { message.append(": " + getCause().getMessage()); } + if (fullyQualifiedPath != null && !fullyQualifiedPath.equals(path)) { + message.append(": ").append(formatPath(fullyQualifiedPath)); + } return message.toString(); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java index 4b8d3bc11a..5fcfdf88ef 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java @@ -220,7 +220,8 @@ protected void processArguments(LinkedList args) throw new PathExistsException(dst.toString()); } } else if (!dst.parentExists()) { - throw new PathNotFoundException(dst.toString()); + throw new PathNotFoundException(dst.toString()) + .withFullyQualifiedPath(dst.path.toUri().toString()); } super.processArguments(args); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Touch.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Touch.java index 72a463af7b..a6c751ea6f 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Touch.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Touch.java @@ -72,7 +72,8 @@ protected void processPath(PathData item) throws IOException { @Override protected void processNonexistentPath(PathData item) throws IOException { if (!item.parentExists()) { - throw new PathNotFoundException(item.toString()); + throw new PathNotFoundException(item.toString()) + .withFullyQualifiedPath(item.path.toUri().toString()); } touchz(item); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java index 6b5de745c0..c00ed66d6f 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellCopy.java @@ -18,18 +18,28 @@ package org.apache.hadoop.fs; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +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 static org.junit.Assume.assumeTrue; import java.io.File; import java.io.IOException; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.util.StringUtils; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -public class TestFsShellCopy { +public class TestFsShellCopy { + static final Log LOG = LogFactory.getLog(TestFsShellCopy.class); + static Configuration conf; static FsShell shell; static LocalFileSystem lfs; @@ -42,9 +52,10 @@ public static void setup() throws Exception { lfs = FileSystem.getLocal(conf); testRootDir = lfs.makeQualified(new Path( System.getProperty("test.build.data","test/build/data"), - "testShellCopy")); + "testFsShellCopy")); - lfs.mkdirs(testRootDir); + lfs.mkdirs(testRootDir); + lfs.setWorkingDirectory(testRootDir); srcPath = new Path(testRootDir, "srcFile"); dstPath = new Path(testRootDir, "dstFile"); } @@ -62,6 +73,16 @@ public void prepFiles() throws Exception { assertTrue(lfs.exists(lfs.getChecksumFile(srcPath))); } + private void shellRun(int n, String ... args) throws Exception { + assertEquals(n, shell.run(args)); + } + + private int shellRun(String... args) throws Exception { + int exitCode = shell.run(args); + LOG.info("exit " + exitCode + " - " + StringUtils.join(" ", args)); + return exitCode; + } + @Test public void testCopyNoCrc() throws Exception { shellRun(0, "-get", srcPath.toString(), dstPath.toString()); @@ -95,10 +116,6 @@ private void checkPath(Path p, boolean expectChecksum) throws IOException { assertEquals(expectChecksum, hasChecksum); } - private void shellRun(int n, String ... args) throws Exception { - assertEquals(n, shell.run(args)); - } - @Test public void testCopyFileFromLocal() throws Exception { Path testRoot = new Path(testRootDir, "testPutFile"); @@ -571,4 +588,23 @@ private String pathAsString(Path p) { String s = (p == null) ? Path.CUR_DIR : p.toString(); return s.isEmpty() ? Path.CUR_DIR : s; } + + /** + * Test copy to a path with non-existent parent directory. + */ + @Test + public void testCopyNoParent() throws Exception { + final String noDirName = "noDir"; + final Path noDir = new Path(noDirName); + lfs.delete(noDir, true); + assertThat(lfs.exists(noDir), is(false)); + + assertThat("Expected failed put to a path without parent directory", + shellRun("-put", srcPath.toString(), noDirName + "/foo"), is(not(0))); + + // Note the trailing '/' in the target path. + assertThat("Expected failed copyFromLocal to a non-existent directory", + shellRun("-copyFromLocal", srcPath.toString(), noDirName + "/"), + is(not(0))); + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellTouch.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellTouch.java new file mode 100644 index 0000000000..988a57c295 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellTouch.java @@ -0,0 +1,88 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.fs; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.util.StringUtils; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +public class TestFsShellTouch { + static final Log LOG = LogFactory.getLog(TestFsShellTouch.class); + + static FsShell shell; + static LocalFileSystem lfs; + static Path testRootDir; + + @BeforeClass + public static void setup() throws Exception { + Configuration conf = new Configuration(); + shell = new FsShell(conf); + lfs = FileSystem.getLocal(conf); + testRootDir = lfs.makeQualified(new Path( + System.getProperty("test.build.data","test/build/data"), + "testFsShell")); + + lfs.mkdirs(testRootDir); + lfs.setWorkingDirectory(testRootDir); + } + + @Before + public void prepFiles() throws Exception { + lfs.setVerifyChecksum(true); + lfs.setWriteChecksum(true); + } + + private int shellRun(String... args) throws Exception { + int exitCode = shell.run(args); + LOG.info("exit " + exitCode + " - " + StringUtils.join(" ", args)); + return exitCode; + } + + @Test + public void testTouchz() throws Exception { + // Ensure newFile does not exist + final String newFileName = "newFile"; + final Path newFile = new Path(newFileName); + lfs.delete(newFile, true); + assertThat(lfs.exists(newFile), is(false)); + + assertThat("Expected successful touchz on a new file", + shellRun("-touchz", newFileName), is(0)); + shellRun("-ls", newFileName); + + assertThat("Expected successful touchz on an existing zero-length file", + shellRun("-touchz", newFileName), is(0)); + + // Ensure noDir does not exist + final String noDirName = "noDir"; + final Path noDir = new Path(noDirName); + lfs.delete(noDir, true); + assertThat(lfs.exists(noDir), is(false)); + + assertThat("Expected failed touchz in a non-existent directory", + shellRun("-touchz", noDirName + "/foo"), is(not(0))); + } +}