Addressing comments

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

@@ -1899,7 +1899,7 @@ public BlockLocation[] getFileBlockLocations(final Path p, final long offset,
       // 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: " +
+                "Fallback to the old GET_BLOCK_LOCATIONS. Exception: {}",
             e.getMessage());
         isServerHCFSCompatible = false;
         locations = getFileBlockLocations(GetOpParam.Op.GET_BLOCK_LOCATIONS, p, offset, length);
@@ -1915,10 +1915,9 @@ private boolean isGetFileBlockLocationsException(RemoteException e) {
         && e.getMessage().contains(GetOpParam.Op.GETFILEBLOCKLOCATIONS.toString());
   }
 
-  private BlockLocation[] getFileBlockLocations(GetOpParam.Op operation,
+  private BlockLocation[] getFileBlockLocations(final GetOpParam.Op operation,
       final Path p, final long offset, final long length) throws IOException {
-    final HttpOpParam.Op op = operation;
-    return new FsPathResponseRunner<BlockLocation[]>(op, p,
+    return new FsPathResponseRunner<BlockLocation[]>(operation, p,
         new OffsetParam(offset), new LengthParam(length)) {
       @Override
       BlockLocation[] decodeResponse(Map<?, ?> json) throws IOException {

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java

@@ -1752,8 +1752,7 @@ static BlockLocation[] toBlockLocations(JSONObject json) throws IOException {
 
     Map<String, Map<String, BlockLocation[]>> jsonMap =
         mapper.readValue(json.toJSONString(), rootType);
-    Map<String, BlockLocation[]> locationMap =
-        jsonMap.get(BLOCK_LOCATIONS_JSON);
+    Map<String, BlockLocation[]> locationMap = jsonMap.get(BLOCK_LOCATIONS_JSON);
     return locationMap.get(BlockLocation.class.getSimpleName());
   }
 }

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java

@@ -2201,8 +2201,7 @@ public Void execute(FileSystem fs) throws IOException {
 
   @InterfaceAudience.Private
   @SuppressWarnings("rawtypes")
-  public static class FSFileBlockLocations
-      implements FileSystemAccess.FileSystemExecutor<Map> {
+  public static class FSFileBlockLocations implements FileSystemAccess.FileSystemExecutor<Map> {
     final private Path path;
     final private long offsetValue;
     final private long lengthValue;

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSParametersProvider.java

@@ -60,6 +60,8 @@ 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[] {OffsetParam.class, LenParam.class});
     PARAMS_DEF.put(Operation.GETACLSTATUS, new Class[]{});
     PARAMS_DEF.put(Operation.GETTRASHROOT, new Class[]{});
     PARAMS_DEF.put(Operation.INSTRUMENTATION, new Class[]{});
@@ -126,10 +128,7 @@ public class HttpFSParametersProvider extends ParametersProvider {
     PARAMS_DEF.put(Operation.GETECPOLICY, new Class[] {});
     PARAMS_DEF.put(Operation.UNSETECPOLICY, new Class[] {});
     PARAMS_DEF.put(Operation.SATISFYSTORAGEPOLICY, new Class[] {});
-    PARAMS_DEF.put(Operation.GETFILEBLOCKLOCATIONS,
-        new Class[] {OffsetParam.class, LenParam.class});
-    PARAMS_DEF.put(Operation.GET_BLOCK_LOCATIONS,
-        new Class[] {OffsetParam.class, LenParam.class});
+    PARAMS_DEF.put(Operation.GET_BLOCK_LOCATIONS, new Class[] {OffsetParam.class, LenParam.class});
   }
 
   public HttpFSParametersProvider() {

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java

@@ -375,9 +375,6 @@ public InputStream run() throws Exception {
       Long offsetParam = params.get(OffsetParam.NAME, OffsetParam.class);
       Long lenParam = params.get(LenParam.NAME, LenParam.class);
       AUDIT_LOG.info("[{}] offset [{}] len [{}]", path, offsetParam, lenParam);
-      if (offsetParam != null && offsetParam.longValue() > 0) {
-        offset = offsetParam.longValue();
-      }
       if (offsetParam != null && offsetParam > 0) {
         offset = offsetParam;
       }

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java

@@ -1969,11 +1969,11 @@ public void testStoragePolicySatisfier() throws Exception {
 
   private void testGetFileBlockLocations() throws Exception {
     BlockLocation[] blockLocations;
-    Path testFile = null;
+    Path testFile;
     if (!this.isLocalFS()) {
       FileSystem fs = this.getHttpFSFileSystem();
       testFile = new Path(getProxiedFSTestDir(), "singleBlock.txt");
-      DFSTestUtil.createFile(fs, testFile, (long) 1, (short) 1, 0L);
+      DFSTestUtil.createFile(fs, testFile, 1, (short) 1, 0L);
       if (fs instanceof HttpFSFileSystem) {
         HttpFSFileSystem httpFS = (HttpFSFileSystem) fs;
         blockLocations = httpFS.getFileBlockLocations(testFile, 0, 1);

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java

@@ -2035,7 +2035,7 @@ public void testGetFileBlockLocations() throws Exception {
 
     assertEquals(locations1.length, httpfsBlockLocations.length);
     for (int i = 0; i < locations1.length; i++) {
-      assertEquals(locations1.toString(), httpfsBlockLocations.toString());
+      assertEquals(locations1[i].toString(), httpfsBlockLocations[i].toString());
     }
 
     conn.getInputStream().close();

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

@@ -709,24 +709,15 @@ 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);
-    return toJsonString("BlockLocations", m);
+    return toJsonString("BlockLocations", JsonUtil.toJsonString(locations));
   }
 
   public static Map<String, Object> toJsonMap(BlockLocation[] locations)
       throws IOException {
     if (locations == null) {
       return null;
     }
-    final Map<String, Object> m = new TreeMap<>();
+    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]);