YARN-5659. getPathFromYarnURL should use standard methods. Contributed by Sergey Shelukhin.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/URL.java

@@ -18,11 +18,15 @@
 
 package org.apache.hadoop.yarn.api.records;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import java.net.URI;
 import java.net.URISyntaxException;
 
 import org.apache.hadoop.classification.InterfaceAudience.Public;
+import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceStability.Stable;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.yarn.factory.providers.RecordFactoryProvider;
 import org.apache.hadoop.yarn.util.Records;
@@ -52,7 +56,7 @@ public abstract class URL {
   @Public
   @Stable
   public abstract String getScheme();
-  
+
   /**
    * Set the scheme of the URL
    * @param scheme scheme of the URL
@@ -68,7 +72,7 @@ public abstract class URL {
   @Public
   @Stable
   public abstract String getUserInfo();
-  
+
   /**
    * Set the user info of the URL.
    * @param userInfo user info of the URL
@@ -84,7 +88,7 @@ public abstract class URL {
   @Public
   @Stable
   public abstract String getHost();
-  
+
   /**
    * Set the host of the URL.
    * @param host host of the URL
@@ -100,7 +104,7 @@ public abstract class URL {
   @Public
   @Stable
   public abstract int getPort();
-  
+
   /**
    * Set the port of the URL
    * @param port port of the URL
@@ -116,7 +120,7 @@ public abstract class URL {
   @Public
   @Stable
   public abstract String getFile();
-  
+
   /**
    * Set the file of the URL.
    * @param file file of the URL
@@ -124,32 +128,20 @@ public abstract class URL {
   @Public
   @Stable
   public abstract void setFile(String file);
-  
+
   @Public
   @Stable
   public Path toPath() throws URISyntaxException {
-    String scheme = getScheme() == null ? "" : getScheme();
-    
-    String authority = "";
-    if (getHost() != null) {
-      authority = getHost();
-      if (getUserInfo() != null) {
-        authority = getUserInfo() + "@" + authority;
-      }
-      if (getPort() > 0) {
-        authority += ":" + getPort();
-      }
-    }
-    
-    return new Path(
-        (new URI(scheme, authority, getFile(), null, null)).normalize());
+    return new Path(new URI(getScheme(), getUserInfo(),
+      getHost(), getPort(), getFile(), null, null));
   }
-  
-  @Public
-  @Stable
-  public static URL fromURI(URI uri) {
+
+
+  @Private
+  @VisibleForTesting
+  public static URL fromURI(URI uri, Configuration conf) {
     URL url =
-        RecordFactoryProvider.getRecordFactory(null).newRecordInstance(
+        RecordFactoryProvider.getRecordFactory(conf).newRecordInstance(
             URL.class);
     if (uri.getHost() != null) {
       url.setHost(uri.getHost());
@@ -162,7 +154,19 @@ public abstract class URL {
     url.setFile(uri.getPath());
     return url;
   }
-  
+
+  @Public
+  @Stable
+  public static URL fromURI(URI uri) {
+    return fromURI(uri, null);
+  }
+
+  @Private
+  @VisibleForTesting
+  public static URL fromPath(Path path, Configuration conf) {
+    return fromURI(path.toUri(), conf);
+  }
+
   @Public
   @Stable
   public static URL fromPath(Path path) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestURL.java

@@ -0,0 +1,99 @@
+/**
+ * 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.yarn.api.records;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.apache.hadoop.yarn.factories.RecordFactory;
+import org.junit.Test;
+
+/** Test for the URL class. */
+public class TestURL {
+
+  @Test
+  public void testConversion() throws Exception {
+    Configuration conf = new Configuration();
+    conf.set(YarnConfiguration.IPC_RECORD_FACTORY_CLASS,
+        RecordFactoryForTest.class.getName());
+    String[] pathStrs = new String[] {"/", ".", "foo/bar", "foo",
+        "/foo/bar/baz", "moo://bar/baz", "moo://bar:123/baz", "moo:///foo",
+        "moo://foo@bar:123/baz/foo", "moo://foo@bar/baz/foo", "moo://foo@bar",
+        "moo://foo:123"};
+    for (String s : pathStrs) {
+      Path path = new Path(s);
+      assertEquals(path, URL.fromPath(path, conf).toPath());
+    }
+
+    Path p = new Path("/foo/bar#baz");
+    assertEquals(p, URL.fromPath(p, conf).toPath());
+  }
+
+  /** Record factory that instantiates URLs for this test. */
+  public static class RecordFactoryForTest implements RecordFactory {
+    private static final RecordFactoryForTest SELF =
+        new RecordFactoryForTest();
+    @SuppressWarnings("unchecked")
+    @Override
+    public <T> T newRecordInstance(Class<T> clazz) {
+      return (T) new URLForTest();
+    }
+    public static RecordFactory get() {
+      return SELF;
+    }
+  }
+
+  /** URL fake for this test; sidesteps proto-URL dependency. */
+  public static class URLForTest extends URL {
+    private String scheme, userInfo, host, file;
+    private int port;
+    public String getScheme() {
+      return scheme;
+    }
+    public void setScheme(String scheme) {
+      this.scheme = scheme;
+    }
+    public String getUserInfo() {
+      return userInfo;
+    }
+    public void setUserInfo(String userInfo) {
+      this.userInfo = userInfo;
+    }
+    public String getHost() {
+      return host;
+    }
+    public void setHost(String host) {
+      this.host = host;
+    }
+    public String getFile() {
+      return file;
+    }
+    public void setFile(String file) {
+      this.file = file;
+    }
+    public int getPort() {
+      return port;
+    }
+    public void setPort(int port) {
+      this.port = port;
+    }
+  }
+
+}