HDFS-16963. Remove the unnecessary use of Optional from DistributedFileSystem

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java

@@ -130,7 +130,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
-import java.util.Optional;
 import java.util.Set;
 
 import static org.apache.hadoop.fs.impl.PathCapabilitiesSupport.validatePathCapabilityArgs;
@@ -402,17 +401,15 @@ protected HdfsPathHandle createPathHandle(FileStatus st, HandleOpt... opts) {
 
     HdfsFileStatus hst = (HdfsFileStatus) st;
     final Path p;
-    final Optional<Long> inodeId;
+    final Long inodeId;
     if (loc.allowChange()) {
       p = DFSUtilClient.makePathFromFileId(hst.getFileId());
-      inodeId = Optional.empty();
+      inodeId = null;
     } else {
       p = hst.getPath();
-      inodeId = Optional.of(hst.getFileId());
+      inodeId = hst.getFileId();
     }
-    final Optional<Long> mtime = !data.allowChange()
-        ? Optional.of(hst.getModificationTime())
-        : Optional.empty();
+    final Long mtime = !data.allowChange()? hst.getModificationTime(): null;
     return new HdfsPathHandle(getPathName(p), inodeId, mtime);
   }
 
@@ -3876,10 +3873,9 @@ public boolean hasPathCapability(final Path path, final String capability)
       throws IOException {
     // qualify the path to make sure that it refers to the current FS.
     final Path p = makeQualified(path);
-    Optional<Boolean> cap = DfsPathCapabilities.hasPathCapability(p,
-        capability);
-    if (cap.isPresent()) {
-      return cap.get();
+    if (DfsPathCapabilities.hasPathCapability(p, capability)
+        && supportsSymlinks()) {
+      return true;
     }
     // this switch is for features which are in the DFS client but not
     // (yet/ever) in the WebHDFS API.

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/DfsPathCapabilities.java

@@ -18,8 +18,6 @@
 
 package org.apache.hadoop.hdfs.client;
 
-import java.util.Optional;
-
 import org.apache.hadoop.fs.CommonPathCapabilities;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
@@ -38,7 +36,7 @@ private DfsPathCapabilities() {
    * @return either a value to return or, if empty, a cue for the FS to
    * pass up to its superclass.
    */
-  public static Optional<Boolean> hasPathCapability(final Path path,
+  public static boolean hasPathCapability(final Path path,
       final String capability) {
     switch (validatePathCapabilityArgs(path, capability)) {
 
@@ -54,11 +52,11 @@ public static Optional<Boolean> hasPathCapability(final Path path,
     case CommonPathCapabilities.FS_STORAGEPOLICY:
     case CommonPathCapabilities.FS_XATTRS:
     case CommonPathCapabilities.FS_TRUNCATE:
-      return Optional.of(true);
+      return true;
     case CommonPathCapabilities.FS_SYMLINKS:
-      return Optional.of(FileSystem.areSymlinksEnabled());
+      return FileSystem.areSymlinksEnabled();
     default:
-      return Optional.empty();
+      return false;
     }
   }
 }

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/HdfsPathHandle.java

@@ -19,7 +19,6 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.Optional;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -42,11 +41,10 @@ public final class HdfsPathHandle implements PathHandle {
   private final Long mtime;
   private final Long inodeId;
 
-  public HdfsPathHandle(String path,
-      Optional<Long> inodeId, Optional<Long> mtime) {
+  public HdfsPathHandle(String path, Long inodeId, Long mtime) {
     this.path = path;
-    this.mtime = mtime.orElse(null);
-    this.inodeId = inodeId.orElse(null);
+    this.mtime = mtime;
+    this.inodeId = inodeId;
   }
 
   public HdfsPathHandle(ByteBuffer bytes) throws IOException {

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java

@@ -47,7 +47,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.concurrent.TimeUnit;
@@ -2207,10 +2206,9 @@ public boolean hasPathCapability(final Path path, final String capability)
       throws IOException {
     // qualify the path to make sure that it refers to the current FS.
     final Path p = makeQualified(path);
-    Optional<Boolean> cap = DfsPathCapabilities.hasPathCapability(p,
-        capability);
-    if (cap.isPresent()) {
-      return cap.get();
+    if (DfsPathCapabilities.hasPathCapability(p, capability)
+        && supportsSymlinks()) {
+      return true;
     }
     return super.hasPathCapability(p, capability);
   }

hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/client/TestDfsPathCapabilities.java

@@ -0,0 +1,91 @@
+/*
+ * 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.hdfs.client;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdfs.DistributedFileSystem;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.web.WebHdfsFileSystem;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.hadoop.fs.CommonPathCapabilities.FS_SYMLINKS;
+
+public class TestDfsPathCapabilities {
+  public static final Logger LOG = LoggerFactory.getLogger(TestDfsPathCapabilities.class);
+
+  static FileSystem getFileSystem(String url) throws Exception {
+    final HdfsConfiguration conf = new HdfsConfiguration();
+    conf.set(FileSystem.FS_DEFAULT_NAME_KEY, url);
+    final FileSystem fs = FileSystem.get(conf);
+    Assert.assertTrue(fs.supportsSymlinks());
+    return fs;
+  }
+
+  /**
+   * Test if {@link DistributedFileSystem#hasPathCapability(Path, String)}
+   * returns correct result for FS_SYMLINKS.
+   */
+  @Test
+  public void testSymlinks() throws Exception {
+    final DistributedFileSystem dfs = Mockito.spy(
+        (DistributedFileSystem) getFileSystem("hdfs://localhost/"));
+    final WebHdfsFileSystem webhdfs = Mockito.spy(
+        (WebHdfsFileSystem) getFileSystem("webhdfs://localhost/"));
+
+    final FileSystem[] fileSystems = {dfs, webhdfs};
+    final Path path = new Path("/");
+
+    for (FileSystem fs : fileSystems) {
+      LOG.info("fs is {}", fs.getClass().getSimpleName());
+      // Symlinks support disabled
+      Mockito.doReturn(false).when(fs).supportsSymlinks();
+      Assert.assertFalse(fs.supportsSymlinks());
+      Assert.assertFalse(FileSystem.areSymlinksEnabled());
+      Assert.assertFalse(fs.hasPathCapability(path, FS_SYMLINKS));
+
+      // Symlinks support enabled
+      Mockito.doReturn(true).when(fs).supportsSymlinks();
+      Assert.assertTrue(fs.supportsSymlinks());
+      Assert.assertFalse(FileSystem.areSymlinksEnabled());
+      Assert.assertFalse(fs.hasPathCapability(path, FS_SYMLINKS));
+    }
+
+    // Once it is enabled, it cannot be disabled.
+    FileSystem.enableSymlinks();
+
+    for (FileSystem fs : fileSystems) {
+      LOG.info("fs is {}", fs.getClass().getSimpleName());
+      // Symlinks enabled
+      Mockito.doReturn(true).when(fs).supportsSymlinks();
+      Assert.assertTrue(fs.supportsSymlinks());
+      Assert.assertTrue(FileSystem.areSymlinksEnabled());
+      Assert.assertTrue(fs.hasPathCapability(path, FS_SYMLINKS));
+
+      // Symlinks enabled but symlink-support is disabled
+      Mockito.doReturn(false).when(fs).supportsSymlinks();
+      Assert.assertFalse(fs.supportsSymlinks());
+      Assert.assertTrue(FileSystem.areSymlinksEnabled());
+      Assert.assertFalse(fs.hasPathCapability(path, FS_SYMLINKS));
+    }
+  }
+}