From 324c3c9c3f807da12f7e681e6d69b679100f83d1 Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Mon, 19 Oct 2020 18:29:53 +0800 Subject: [PATCH] config test improve --- .../apache/kyuubi/config/ConfigBuilder.scala | 6 +- .../apache/kyuubi/config/ConfigEntry.scala | 78 +++++++++++++------ .../apache/kyuubi/config/ConfigHelpers.scala | 9 --- .../org/apache/kyuubi/config/KyuubiConf.scala | 2 +- .../kyuubi/config/ConfigEntrySuite.scala | 73 +++++++++++++++++ .../kyuubi/config/ConfigProviderSuite.scala | 35 +++++++++ 6 files changed, 167 insertions(+), 36 deletions(-) create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigProviderSuite.scala diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala index f7bfa29b5..41e85c58b 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala @@ -133,7 +133,7 @@ private[kyuubi] case class TypedConfigBuilder[T]( } def createOptional: OptionalConfigEntry[T] = { - val entry = OptionalConfigEntry(parent.key, fromStr, toStr, parent._doc, parent._version) + val entry = new OptionalConfigEntry(parent.key, fromStr, toStr, parent._doc, parent._version) parent._onCreate.foreach(_(entry)) entry } @@ -143,13 +143,13 @@ private[kyuubi] case class TypedConfigBuilder[T]( case _ => val d = fromStr(toStr(default)) val entry = - ConfigEntryWithDefault(parent.key, d, fromStr, toStr, parent._doc, parent._version) + new ConfigEntryWithDefault(parent.key, d, fromStr, toStr, parent._doc, parent._version) parent._onCreate.foreach(_(entry)) entry } def createWithDefaultString(default: String): ConfigEntryWithDefaultString[T] = { - val entry = ConfigEntryWithDefaultString( + val entry = new ConfigEntryWithDefaultString( parent.key, default, fromStr, toStr, parent._doc, parent._version) parent._onCreate.foreach(_(entry)) entry diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala index 92fdb5dab..2b4f642d4 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala @@ -25,9 +25,9 @@ trait ConfigEntry[T] { def version: String def defaultValStr: String - def defaultVal: Option[T] = None + def defaultVal: Option[T] - final override def toString: String = { + override def toString: String = { s"ConfigEntry(key=$key, defaultValue=$defaultValStr, doc=$doc, version=$version)" } @@ -40,30 +40,44 @@ trait ConfigEntry[T] { ConfigEntry.registerEntry(this) } -case class OptionalConfigEntry[T]( - key: String, +class OptionalConfigEntry[T]( + _key: String, rawValueConverter: String => T, rawStrConverter: T => String, - doc: String, - version: String) extends ConfigEntry[Option[T]] { - override def valueConverter: String => Option[T] = s => Option(rawValueConverter(s)) + _doc: String, + _version: String) extends ConfigEntry[Option[T]] { + override def valueConverter: String => Option[T] = { + s => Option(rawValueConverter(s)) + } - override def strConverter: Option[T] => String = v => v.map(rawStrConverter).orNull + override def strConverter: Option[T] => String = { + v => v.map(rawStrConverter).orNull + } - override def defaultValStr: String = ConfigEntry.UNDEFINED + override def defaultValStr: String = { + ConfigEntry.UNDEFINED + } override def readFrom(conf: ConfigProvider): Option[T] = { readString(conf).map(rawValueConverter) } + + override def defaultVal: Option[Option[T]] = None + + override def key: String = _key + + override def doc: String = _doc + + override def version: String = _version } -case class ConfigEntryWithDefault[T]( - key: String, +class ConfigEntryWithDefault[T]( + _key: String, _defaultVal: T, - valueConverter: String => T, - strConverter: T => String, - doc: String, - version: String) extends ConfigEntry[T] { + _valueConverter: String => T, + _strConverter: T => String, + _doc: String, + _version: String) extends ConfigEntry[T] { override def defaultValStr: String = strConverter(_defaultVal) override def defaultVal: Option[T] = Option(_defaultVal) @@ -71,15 +85,25 @@ case class ConfigEntryWithDefault[T]( override def readFrom(conf: ConfigProvider): T = { readString(conf).map(valueConverter).getOrElse(_defaultVal) } + + override def key: String = _key + + override def valueConverter: String => T = _valueConverter + + override def strConverter: T => String = _strConverter + + override def doc: String = _doc + + override def version: String = _version } -case class ConfigEntryWithDefaultString[T]( - key: String, +class ConfigEntryWithDefaultString[T]( + _key: String, _defaultVal: String, - valueConverter: String => T, - strConverter: T => String, - doc: String, - version: String) extends ConfigEntry[T] { + _valueConverter: String => T, + _strConverter: T => String, + _doc: String, + _version: String) extends ConfigEntry[T] { override def defaultValStr: String = _defaultVal override def defaultVal: Option[T] = Some(valueConverter(_defaultVal)) @@ -88,6 +112,16 @@ case class ConfigEntryWithDefaultString[T]( val value = readString(conf).getOrElse(_defaultVal) valueConverter(value) } + + override def key: String = _key + + override def valueConverter: String => T = _valueConverter + + override def strConverter: T => String = _strConverter + + override def doc: String = _doc + + override def version: String = _version } object ConfigEntry { @@ -99,6 +133,4 @@ object ConfigEntry { val existing = knownConfigs.putIfAbsent(entry.key, entry) require(existing == null, s"Config entry ${entry.key} already registered!") } - - def findEntry(key: String): ConfigEntry[_] = knownConfigs.get(key) } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigHelpers.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigHelpers.scala index c5bedba97..3538e1a36 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigHelpers.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigHelpers.scala @@ -28,13 +28,4 @@ object ConfigHelpers { def seqToStr[T](v: Seq[T], stringConverter: T => String): String = { v.map(stringConverter).mkString(",") } - - def toNumber[T](s: String, converter: String => T, key: String, configType: String): T = { - try { - converter(s.trim) - } catch { - case _: NumberFormatException => - throw new IllegalArgumentException(s"$key should be $configType, but was $s") - } - } } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala index 005c1e235..b27cb1cbb 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala @@ -51,7 +51,7 @@ case class KyuubiConf(loadSysDefault: Boolean = true) extends Logging { } def set[T](entry: OptionalConfigEntry[T], value: T): KyuubiConf = { - set(entry.key, entry.rawStrConverter(value)) + set(entry.key, entry.strConverter(Option(value))) this } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala new file mode 100644 index 000000000..7078c0119 --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala @@ -0,0 +1,73 @@ +/* + * 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.kyuubi.config + +import org.apache.kyuubi.KyuubiFunSuite + +class ConfigEntrySuite extends KyuubiFunSuite { + + test("optional config entry") { + val e1 = new OptionalConfigEntry[Int]( + "kyuubi.int.spark", + s => s.toInt + 1, + v => (v - 1).toString, + "this is dummy documentation", + "") + + val conf = KyuubiConf() + assert(conf.get(e1).isEmpty) + val e = intercept[IllegalArgumentException](new OptionalConfigEntry[Int]( + "kyuubi.int.spark", + s => s.toInt + 1, + v => (v - 1).toString, + "this is dummy documentation", + "")) + assert(e.getMessage === + "requirement failed: Config entry kyuubi.int.spark already registered!") + conf.set(e1.key, "2") + assert(conf.get(e1).get === 3) + + } + + test("config entry with default") { + val e1 = new ConfigEntryWithDefault[Long]("kyuubi.long.spark", + 2, + s => s.toLong + 1, + v => (v - 1).toString, + "", + "") + val conf = KyuubiConf() + assert(conf.get(e1) === 2) + conf.set(e1.key, "5") + assert(conf.get(e1) === 6) + } + + test("config entry with default string") { + val e1 = new ConfigEntryWithDefaultString[Double]( + "kyuubi.double.spark", + "3.0", s => java.lang.Double.valueOf(s), + v => v.toString, + "", + "") + val conf = KyuubiConf() + assert(conf.get(e1) === 3.0) + conf.set(e1.asInstanceOf[ConfigEntry[AnyVal]], 5.0) + assert(conf.get(e1) === 5.0) + } + +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigProviderSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigProviderSuite.scala new file mode 100644 index 000000000..30593591a --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigProviderSuite.scala @@ -0,0 +1,35 @@ +/* + * 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.kyuubi.config + +import scala.collection.JavaConverters._ + +import org.apache.kyuubi.KyuubiFunSuite + +class ConfigProviderSuite extends KyuubiFunSuite { + + test("config provider") { + val conf = Map("kyuubi.abc" -> "1", "kyuubi.xyz" -> "2", "spark.abc" -> "kyuubi") + val provider = new ConfigProvider(conf.asJava) + assert(provider.get("kyuubi.abc").get === "1") + assert(provider.get("kyuubi.xyz").get === "2") + assert(provider.get("spark.abc") === None) + + } + +}