Fixes

apache · Sep 22, 2022 · f28c025 · f28c025
1 parent b78a873
commit f28c025
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 83 deletions.
diff --git a/...roject/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java b/...roject/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
@@ -1888,25 +1888,21 @@ public BlockLocation[] getFileBlockLocations(final Path p, final long offset,
       final long length) throws IOException {
     statistics.incrementReadOps(1);
     storageStatistics.incrementOpCounter(OpType.GET_FILE_BLOCK_LOCATIONS);
-    BlockLocation[] locations = null;
+    BlockLocation[] locations;
     try {
       if (isServerHCFSCompatible) {
-        locations =
-            getFileBlockLocations(GetOpParam.Op.GETFILEBLOCKLOCATIONS, p, offset, length);
+        locations = getFileBlockLocations(GetOpParam.Op.GETFILEBLOCKLOCATIONS, p, offset, length);
       } else {
-        locations = getFileBlockLocations(GetOpParam.Op.GET_BLOCK_LOCATIONS, p,
-            offset, length);
+        locations = getFileBlockLocations(GetOpParam.Op.GET_BLOCK_LOCATIONS, p, offset, length);
       }
     } catch (RemoteException e) {
-      // parsing the exception is needed only if the client thinks the service
-      // is compatible
+      // parsing the exception is needed only if the client thinks the service is compatible
       if (isServerHCFSCompatible && isGetFileBlockLocationsException(e)) {
         LOG.warn("Server does not appear to support GETFILEBLOCKLOCATIONS." +
                 "Fallback to the old GET_BLOCK_LOCATIONS. Exception: " +
             e.getMessage());
         isServerHCFSCompatible = false;
-        locations = getFileBlockLocations(GetOpParam.Op.GET_BLOCK_LOCATIONS, p,
-            offset, length);
+        locations = getFileBlockLocations(GetOpParam.Op.GET_BLOCK_LOCATIONS, p, offset, length);
       } else {
         throw e;
       }
@@ -1915,10 +1911,8 @@ public BlockLocation[] getFileBlockLocations(final Path p, final long offset,
   }
 
   private boolean isGetFileBlockLocationsException(RemoteException e) {
-    return e.getMessage() != null
-        && e.getMessage().contains("Invalid value for webhdfs parameter")
-        && e.getMessage()
-        .contains(GetOpParam.Op.GETFILEBLOCKLOCATIONS.toString());
+    return e.getMessage() != null && e.getMessage().contains("Invalid value for webhdfs parameter")
+        && e.getMessage().contains(GetOpParam.Op.GETFILEBLOCKLOCATIONS.toString());
   }
 
   private BlockLocation[] getFileBlockLocations(GetOpParam.Op operation,
@@ -1932,8 +1926,7 @@ BlockLocation[] decodeResponse(Map<?, ?> json) throws IOException {
         case GETFILEBLOCKLOCATIONS:
           return JsonUtilClient.toBlockLocationArray(json);
         case GET_BLOCK_LOCATIONS:
-          return DFSUtilClient
-              .locatedBlocks2Locations(JsonUtilClient.toLocatedBlocks(json));
+          return DFSUtilClient.locatedBlocks2Locations(JsonUtilClient.toLocatedBlocks(json));
         default:
           throw new IOException("Unknown operation " + operation.name());
         }

diff --git a/...oject/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java b/...oject/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
@@ -2203,9 +2203,9 @@ public Void execute(FileSystem fs) throws IOException {
   @SuppressWarnings("rawtypes")
   public static class FSFileBlockLocations
       implements FileSystemAccess.FileSystemExecutor<Map> {
-    private Path path;
-    private long offsetValue;
-    private long lengthValue;
+    final private Path path;
+    final private long offsetValue;
+    final private long lengthValue;
 
     /**
      * Creates a file-block-locations executor.
@@ -2214,8 +2214,7 @@ public static class FSFileBlockLocations
      * @param offsetValue offset into the given file
      * @param lengthValue length for which to get locations for
      */
-    public FSFileBlockLocations(String path, long offsetValue,
-        long lengthValue) {
+    public FSFileBlockLocations(String path, long offsetValue, long lengthValue) {
       this.path = new Path(path);
       this.offsetValue = offsetValue;
       this.lengthValue = lengthValue;
@@ -2238,9 +2237,9 @@ public Map execute(FileSystem fs) throws IOException {
   @SuppressWarnings("rawtypes")
   public static class FSFileBlockLocationsLegacy
       implements FileSystemAccess.FileSystemExecutor<Map> {
-    private Path path;
-    private long offsetValue;
-    private long lengthValue;
+    final private Path path;
+    final private long offsetValue;
+    final private long lengthValue;
 
     /**
      * Creates a file-block-locations executor.
@@ -2249,8 +2248,7 @@ public static class FSFileBlockLocationsLegacy
      * @param offsetValue offset into the given file
      * @param lengthValue length for which to get locations for
      */
-    public FSFileBlockLocationsLegacy(String path, long offsetValue,
-        long lengthValue) {
+    public FSFileBlockLocationsLegacy(String path, long offsetValue, long lengthValue) {
       this.path = new Path(path);
       this.offsetValue = offsetValue;
       this.lengthValue = lengthValue;

diff --git a/...-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSParametersProvider.java b/...-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSParametersProvider.java
@@ -60,7 +60,6 @@ public class HttpFSParametersProvider extends ParametersProvider {
     PARAMS_DEF.put(Operation.GETQUOTAUSAGE, new Class[]{});
     PARAMS_DEF.put(Operation.GETFILECHECKSUM,
         new Class[]{NoRedirectParam.class});
-    PARAMS_DEF.put(Operation.GETFILEBLOCKLOCATIONS, new Class[]{});
     PARAMS_DEF.put(Operation.GETACLSTATUS, new Class[]{});
     PARAMS_DEF.put(Operation.GETTRASHROOT, new Class[]{});
     PARAMS_DEF.put(Operation.INSTRUMENTATION, new Class[]{});

diff --git a/...oject/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java b/...oject/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java
@@ -374,13 +374,15 @@ public InputStream run() throws Exception {
       long len = Long.MAX_VALUE;
       Long offsetParam = params.get(OffsetParam.NAME, OffsetParam.class);
       Long lenParam = params.get(LenParam.NAME, LenParam.class);
-      AUDIT_LOG.info("[{}] offset [{}] len [{}]",
-          new Object[] {path, offsetParam, lenParam });
+      AUDIT_LOG.info("[{}] offset [{}] len [{}]", path, offsetParam, lenParam);
       if (offsetParam != null && offsetParam.longValue() > 0) {
         offset = offsetParam.longValue();
       }
-      if (lenParam != null && lenParam.longValue() > 0) {
-        len = lenParam.longValue();
+      if (offsetParam != null && offsetParam > 0) {
+        offset = offsetParam;
+      }
+      if (lenParam != null && lenParam > 0) {
+        len = lenParam;
       }
       FSOperations.FSFileBlockLocations command =
           new FSOperations.FSFileBlockLocations(path, offset, len);

diff --git a/...hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java b/...hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java
@@ -18,9 +18,34 @@
 
 package org.apache.hadoop.fs.http.client;
 
-import okhttp3.mockwebserver.MockResponse;
-import okhttp3.mockwebserver.MockWebServer;
-import okhttp3.mockwebserver.RecordedRequest;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.net.URI;
+import java.net.URL;
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.webapp.WebAppContext;
+import org.json.simple.JSONObject;
+import org.json.simple.parser.ContainerFactory;
+import org.json.simple.parser.JSONParser;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.BlockLocation;
 import org.apache.hadoop.fs.BlockStoragePolicySpi;
@@ -44,7 +69,6 @@
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdfs.AppendTestUtil;
-import org.apache.hadoop.hdfs.DFSClientAdapter;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.DFSTestUtil;
 import org.apache.hadoop.hdfs.DFSUtil;
@@ -56,12 +80,11 @@
 import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocol.HdfsFileStatus;
-import org.apache.hadoop.hdfs.protocol.LocatedBlocks;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
 import org.apache.hadoop.hdfs.protocol.SnapshotDiffReportListing;
 import org.apache.hadoop.hdfs.protocol.SnapshotException;
-import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus;
 import org.apache.hadoop.hdfs.protocol.SnapshotStatus;
+import org.apache.hadoop.hdfs.protocol.SnapshottableDirectoryStatus;
 import org.apache.hadoop.hdfs.protocol.SystemErasureCodingPolicies;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants;
 import org.apache.hadoop.hdfs.web.JsonUtil;
@@ -79,35 +102,6 @@
 import org.apache.hadoop.test.TestJetty;
 import org.apache.hadoop.test.TestJettyHelper;
 import org.apache.hadoop.util.Lists;
-import org.json.simple.JSONObject;
-import org.json.simple.parser.ContainerFactory;
-import org.json.simple.parser.JSONParser;
-import org.junit.Assert;
-import org.junit.Assume;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.webapp.WebAppContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.io.Writer;
-import java.net.URI;
-import java.net.URL;
-import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.regex.Pattern;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -117,8 +111,6 @@
 
 @RunWith(value = Parameterized.class)
 public abstract class BaseTestHttpFSWith extends HFSTestCase {
-  public static final Logger LOG = LoggerFactory
-      .getLogger(BaseTestHttpFSWith.class);
   protected abstract Path getProxiedFSTestDir();
 
   protected abstract String getProxiedFSURI();

diff --git a/...t/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java b/...t/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java
@@ -2026,14 +2026,12 @@ public void testGetFileBlockLocations() throws Exception {
     HttpURLConnection conn = sendRequestToHttpFSServer(file1,
         "GETFILEBLOCKLOCATIONS", "length=10&offset10");
     Assert.assertEquals(HttpURLConnection.HTTP_OK, conn.getResponseCode());
-    BlockLocation[] locations1 =
-        dfs.getFileBlockLocations(new Path(file1), 0, 1);
+    BlockLocation[] locations1 = dfs.getFileBlockLocations(new Path(file1), 0, 1);
     Assert.assertNotNull(locations1);
 
     Map<?, ?> jsonMap = JsonSerialization.mapReader().readValue(conn.getInputStream());
 
-    BlockLocation[] httpfsBlockLocations =
-        JsonUtilClient.toBlockLocationArray(jsonMap);
+    BlockLocation[] httpfsBlockLocations = JsonUtilClient.toBlockLocationArray(jsonMap);
 
     assertEquals(locations1.length, httpfsBlockLocations.length);
     for (int i = 0; i < locations1.length; i++) {

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java
@@ -709,15 +709,7 @@ static Map<String, Object> toJsonMap(
 
   public static String toJsonString(BlockLocation[] locations)
       throws IOException {
-    if (locations == null) {
-      return null;
-    }
-    final Map<String, Object> m = new HashMap<>();
-    Object[] blockLocations = new Object[locations.length];
-    for(int i=0; i<locations.length; i++) {
-      blockLocations[i] = toJsonMap(locations[i]);
-    }
-    m.put(BlockLocation.class.getSimpleName(), blockLocations);
+    final Map<String, Object> m = toJsonMap(locations);
     return toJsonString("BlockLocations", m);
   }
 

diff --git a/...hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestJsonUtilClient.java b/...hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestJsonUtilClient.java
@@ -32,9 +32,8 @@
 
 public class TestJsonUtilClient {
   @Test
-  public void testToStringArray() throws Exception {
-    List<String> strList = new ArrayList<String>(
-        Arrays.asList("aaa", "bbb", "ccc"));
+  public void testToStringArray() {
+    List<String> strList = new ArrayList<String>(Arrays.asList("aaa", "bbb", "ccc"));
 
     String[] strArr = JsonUtilClient.toStringArray(strList);
     assertEquals("Expected 3 items in the array", 3, strArr.length);