From 397adc9773c9a497c2d3d86c3d03481d04c6616d Mon Sep 17 00:00:00 2001 From: Masatake Iwasaki Date: Wed, 4 Dec 2019 20:08:36 +0900 Subject: [PATCH 1/4] HDFS-14522. Allow compact property description in xml in httpfs. --- .../hadoop/lib/util/ConfigurationUtils.java | 68 +------------------ .../lib/util/TestConfigurationUtils.java | 27 +++++++- 2 files changed, 25 insertions(+), 70 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/util/ConfigurationUtils.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/util/ConfigurationUtils.java index 6611dd22fefc9..baecce4d8569d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/util/ConfigurationUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/util/ConfigurationUtils.java @@ -20,17 +20,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; -import org.w3c.dom.DOMException; -import org.w3c.dom.Document; -import org.w3c.dom.Element; -import org.w3c.dom.Node; -import org.w3c.dom.NodeList; -import org.w3c.dom.Text; -import org.xml.sax.SAXException; -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import java.io.IOException; import java.io.InputStream; import java.util.Map; @@ -98,62 +88,6 @@ public static Configuration resolve(Configuration conf) { * @throws IOException thrown if the configuration could not be read. */ public static void load(Configuration conf, InputStream is) throws IOException { - try { - DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); - // ignore all comments inside the xml file - docBuilderFactory.setIgnoringComments(true); - DocumentBuilder builder = docBuilderFactory.newDocumentBuilder(); - Document doc = builder.parse(is); - parseDocument(conf, doc); - } catch (SAXException e) { - throw new IOException(e); - } catch (ParserConfigurationException e) { - throw new IOException(e); - } - } - - // Canibalized from FileSystemAccess Configuration.loadResource(). - private static void parseDocument(Configuration conf, Document doc) throws IOException { - try { - Element root = doc.getDocumentElement(); - if (!"configuration".equals(root.getTagName())) { - throw new IOException("bad conf file: top-level element not "); - } - NodeList props = root.getChildNodes(); - for (int i = 0; i < props.getLength(); i++) { - Node propNode = props.item(i); - if (!(propNode instanceof Element)) { - continue; - } - Element prop = (Element) propNode; - if (!"property".equals(prop.getTagName())) { - throw new IOException("bad conf file: element not "); - } - NodeList fields = prop.getChildNodes(); - String attr = null; - String value = null; - for (int j = 0; j < fields.getLength(); j++) { - Node fieldNode = fields.item(j); - if (!(fieldNode instanceof Element)) { - continue; - } - Element field = (Element) fieldNode; - if ("name".equals(field.getTagName()) && field.hasChildNodes()) { - attr = ((Text) field.getFirstChild()).getData().trim(); - } - if ("value".equals(field.getTagName()) && field.hasChildNodes()) { - value = ((Text) field.getFirstChild()).getData(); - } - } - - if (attr != null && value != null) { - conf.set(attr, value); - } - } - - } catch (DOMException e) { - throw new IOException(e); - } + conf.addResource(is); } - } diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java index 925edc5408420..15344a34751ce 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java @@ -21,9 +21,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.StringWriter; import org.apache.hadoop.conf.Configuration; import org.junit.Test; @@ -44,11 +46,12 @@ public void constructors() throws Exception { } - @Test(expected = IOException.class) - public void constructorsFail3() throws Exception { - InputStream is = new ByteArrayInputStream("".getBytes()); + @Test + public void constructors3() throws Exception { + InputStream is = new ByteArrayInputStream("".getBytes()); Configuration conf = new Configuration(false); ConfigurationUtils.load(conf, is); + assertEquals(conf.get("key1"), "val1"); } @Test @@ -124,4 +127,22 @@ public void testVarResolutionAndSysProps() { assertEquals(conf.get("user.name"), "foo"); } + @Test + public void testCompactFormatProperty() throws IOException { + Configuration conf = new Configuration(false); + assertEquals(conf.size(), 0); + StringWriter writer = new StringWriter(); + BufferedWriter out = new BufferedWriter(writer); + out.write("\n"); + out.write("\n"); + out.write(""); + out.write("\n"); + out.flush(); + out.close(); + InputStream is = new ByteArrayInputStream(writer.toString().getBytes()); + conf = new Configuration(false); + ConfigurationUtils.load(conf, is); + assertEquals(conf.size(), 1); + assertEquals(conf.get("key1"), "val1"); + } } From ec0d8f0e9e8bce0efbc953d6e7a112f1f395c613 Mon Sep 17 00:00:00 2001 From: Masatake Iwasaki Date: Fri, 6 Dec 2019 14:01:24 +0900 Subject: [PATCH 2/4] moved test configuraiton to test resource file. --- .../lib/util/TestConfigurationUtils.java | 22 ++++++------------- .../test-compact-format-property.xml | 18 +++++++++++++++ 2 files changed, 25 insertions(+), 15 deletions(-) create mode 100644 hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/resources/test-compact-format-property.xml diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java index 15344a34751ce..baff8959cafee 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java @@ -21,11 +21,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.io.StringWriter; import org.apache.hadoop.conf.Configuration; import org.junit.Test; @@ -129,20 +127,14 @@ public void testVarResolutionAndSysProps() { @Test public void testCompactFormatProperty() throws IOException { + final String TESTFILE = "test-compact-format-property.xml"; Configuration conf = new Configuration(false); assertEquals(conf.size(), 0); - StringWriter writer = new StringWriter(); - BufferedWriter out = new BufferedWriter(writer); - out.write("\n"); - out.write("\n"); - out.write(""); - out.write("\n"); - out.flush(); - out.close(); - InputStream is = new ByteArrayInputStream(writer.toString().getBytes()); - conf = new Configuration(false); - ConfigurationUtils.load(conf, is); - assertEquals(conf.size(), 1); - assertEquals(conf.get("key1"), "val1"); + ConfigurationUtils.load(conf, + Thread.currentThread() + .getContextClassLoader().getResource(TESTFILE).openStream()); + assertEquals(conf.size(), 2); + assertEquals("val1", conf.get("key.1")); + assertEquals("val2", conf.get("key.2")); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/resources/test-compact-format-property.xml b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/resources/test-compact-format-property.xml new file mode 100644 index 0000000000000..f216c4c5ca85b --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/resources/test-compact-format-property.xml @@ -0,0 +1,18 @@ + + + + + + From 6c71324aae6c8532f1130ebbc15e8e3ef4ae5054 Mon Sep 17 00:00:00 2001 From: Masatake Iwasaki Date: Mon, 9 Dec 2019 09:44:39 +0900 Subject: [PATCH 3/4] fixed the order of arguments of assertEquals as expected value comes first. --- .../org/apache/hadoop/lib/util/TestConfigurationUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java index baff8959cafee..3f3f98d2ec1cd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java @@ -49,7 +49,7 @@ public void constructors3() throws Exception { InputStream is = new ByteArrayInputStream("".getBytes()); Configuration conf = new Configuration(false); ConfigurationUtils.load(conf, is); - assertEquals(conf.get("key1"), "val1"); + assertEquals("val1", conf.get("key1")); } @Test @@ -129,11 +129,11 @@ public void testVarResolutionAndSysProps() { public void testCompactFormatProperty() throws IOException { final String TESTFILE = "test-compact-format-property.xml"; Configuration conf = new Configuration(false); - assertEquals(conf.size(), 0); + assertEquals(0, conf.size()); ConfigurationUtils.load(conf, Thread.currentThread() .getContextClassLoader().getResource(TESTFILE).openStream()); - assertEquals(conf.size(), 2); + assertEquals(2, conf.size()); assertEquals("val1", conf.get("key.1")); assertEquals("val2", conf.get("key.2")); } From 3a7962218f9e8cfca6a4ba627c04090bcb721447 Mon Sep 17 00:00:00 2001 From: Masatake Iwasaki Date: Mon, 9 Dec 2019 09:59:44 +0900 Subject: [PATCH 4/4] addressed checkstyle warnings. --- .../org/apache/hadoop/lib/util/TestConfigurationUtils.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java index 3f3f98d2ec1cd..b868d0b3a2b21 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/util/TestConfigurationUtils.java @@ -46,7 +46,8 @@ public void constructors() throws Exception { @Test public void constructors3() throws Exception { - InputStream is = new ByteArrayInputStream("".getBytes()); + InputStream is = new ByteArrayInputStream( + "".getBytes()); Configuration conf = new Configuration(false); ConfigurationUtils.load(conf, is); assertEquals("val1", conf.get("key1")); @@ -127,12 +128,12 @@ public void testVarResolutionAndSysProps() { @Test public void testCompactFormatProperty() throws IOException { - final String TESTFILE = "test-compact-format-property.xml"; + final String testfile = "test-compact-format-property.xml"; Configuration conf = new Configuration(false); assertEquals(0, conf.size()); ConfigurationUtils.load(conf, Thread.currentThread() - .getContextClassLoader().getResource(TESTFILE).openStream()); + .getContextClassLoader().getResource(testfile).openStream()); assertEquals(2, conf.size()); assertEquals("val1", conf.get("key.1")); assertEquals("val2", conf.get("key.2"));