[ https://issues.apache.org/jira/browse/HADOOP-13191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15299346#comment-15299346 ]
John Zhuge edited comment on HADOOP-13191 at 5/25/16 2:44 AM: -------------------------------------------------------------- Patch 001: * Update {{listStatus}} doc in filesystem.md * Annotate {{FileSystem#listStatus}} with @Nonnull (from javax.annotation supported by IntelliJ) * Restrict {{FileSystem#listStatus}} throw list to {{IOException}} since {{FileNotFoundException}} is a subclass of {{IOException}} * Add {{AccessControlException}} to {{FileSystem#listStatus}} javadoc * Fix {{RawLocalFileSystem#listStatus}} to return empty list when {{localf.list()}} returns null * Fix {{listStatus}} in subclasses of {{FileSystem}} to return empty list instead of null. Only found them in test classes. * Parse 276 callers of {{FileSystem#listStatus}}: ** If it only handles {{FileNotFoundException}} but not {{IOException}}, fix it. Only found 2 cases. ** If it does not have any catch clause, do nothing. ** If it checks null return and list length == 0, essentially no-op, do nothing. Needs discussion: * I am ok not to add @Nonnull annotations although it is nice to have. * What to expect from {{listStatus(path)}} when the path is an accessible file? was (Author: jzhuge): Patch 001: * Update {{listStatus}} doc in filesystem.md * Annotate {{FileSystem#listStatus}} with @Nonnull * Restrict {{FileSystem#listStatus}} throw list to {{IOException}} only since {{FileNotFoundException}} is a subclass of {{IOException}} * Add {{AccessControlException}} to {{FileSystem#listStatus}} javadoc * Fix {{RawLocalFileSystem#listStatus}} to return empty list when {{localf.list()}} returns null * Fix {{listStatus}} in subclasses of {{FileSystem}} to return empty list instead of null. Only found them in tests. * Parse 276 callers of {{FileSystem#listStatus}}: ** If it only handles {{FileNotFoundException}} but not {{IOException}}, fix it. Only found 2 cases. ** If it does not have any catch clause, do nothing. ** If it check null return and list length == 0, essentially no-op, do nothing. Needs discussion: * I am ok not to add @Nonnull annotations although it is nice to have. * What to expect from {{listStatus(path)}} when the path is an accessible file? > FileSystem#listStatus should not return null > -------------------------------------------- > > Key: HADOOP-13191 > URL: https://issues.apache.org/jira/browse/HADOOP-13191 > Project: Hadoop Common > Issue Type: Bug > Components: fs > Affects Versions: 2.6.0 > Reporter: John Zhuge > Assignee: John Zhuge > Priority: Minor > Attachments: HADOOP-13191.001.patch > > > This came out of discussion in HADOOP-12718. The {{FileSystem#listStatus}} > contract does not indicate {{null}} is a valid return and some callers do not > test {{null}} before use: > AbstractContractGetFileStatusTest#testListStatusEmptyDirectory: > {code} > assertEquals("ls on an empty directory not of length 0", 0, > fs.listStatus(subfolder).length); > {code} > ChecksumFileSystem#copyToLocalFile: > {code} > FileStatus[] srcs = listStatus(src); > for (FileStatus srcFile : srcs) { > {code} > SimpleCopyLIsting#getFileStatus: > {code} > FileStatus[] fileStatuses = fileSystem.listStatus(path); > if (excludeList != null && excludeList.size() > 0) { > ArrayList<FileStatus> fileStatusList = new ArrayList<>(); > for(FileStatus status : fileStatuses) { > {code} > IMHO, there is no good reason for {{listStatus}} to return {{null}}. It > should throw IOExceptions upon errors or return empty list. > To enforce the contract that null is an invalid return, update javadoc and > leverage @Nullable/@NotNull/@Nonnull annotations. > So far, I am only aware of the following functions that can return null: > * RawLocalFileSystem#listStatus -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org