Skip to content

Commit 8a3b61d

Browse files
sohaibiftikharsohaibiftikhar
committed
Lots of stuff
— Added UI tests — Added common flatMap in DictUtils.elm — Removed optional type in parameterInfo — Removed isEmpty from InstanceService — change DataTypeParamVal to DataTypeVal in Template.elm — Unit tests for all types for config parsing
1 parent c2db244 commit 8a3b61d

31 files changed

+551
-285
lines changed

‎server/src/main/scala/de/frosner/broccoli/models/ParameterInfo.scala

+4-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ case class ParameterInfo(id: String,
1010
name: Option[String],
1111
default: Option[ParameterValue],
1212
secret: Option[Boolean],
13-
`type`: Option[ParameterType],
13+
`type`: ParameterType,
1414
orderIndex: Option[Int])
1515

1616
object ParameterInfo {
@@ -23,10 +23,10 @@ object ParameterInfo {
2323
val id = (json \ "id").as[String]
2424
val name = (json \ "name").asOpt[String]
2525
val secret = (json \ "secret").asOpt[Boolean]
26-
val `type` = (json \ "type").asOpt[String].map(ParameterType.withName)
26+
val `type` = ParameterType.withName((json \ "type").as[String])
2727
val orderIndex = (json \ "orderIndex").asOpt[Int]
2828
val default = (`type`, (json \ "default").toOption) match {
29-
case (Some(paramType), Some(jsValue)) =>
29+
case (paramType, Some(jsValue)) =>
3030
ParameterValue.fromJsValue(paramType, jsValue)
3131
case _ => None
3232
}
@@ -38,6 +38,7 @@ object ParameterInfo {
3838

3939
}
4040

41+
// TODO: Pick the default from the reference.conf and use it for parameter.`type`
4142
def fromTemplateInfoParameter(id: String, parameter: TemplateConfig.Parameter): ParameterInfo =
4243
ParameterInfo(id, parameter.name, parameter.default, parameter.secret, parameter.`type`, parameter.orderIndex)
4344
}

‎server/src/main/scala/de/frosner/broccoli/models/ParameterValue.scala

+3-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package de.frosner.broccoli.models
22

33
import com.typesafe.config.{Config, ConfigValue, ConfigValueType}
44
import com.typesafe.config.impl.ConfigInt
5-
import de.frosner.broccoli.services.{ParameterValueParsingException, TemplateParameterNotFoundException}
5+
import de.frosner.broccoli.services.{ParameterNotFoundException, ParameterValueParsingException}
66
import org.apache.commons.lang3.StringEscapeUtils
77
import play.api.libs.json._
88

@@ -33,8 +33,8 @@ object ParameterValue {
3333
jsValue: JsValue): Try[ParameterValue] =
3434
Try {
3535
val parameterInfo =
36-
parameterInfos.getOrElse(parameterName, ParameterInfo(parameterName, None, None, None, None, None))
37-
fromJsValue(parameterInfo.`type`.getOrElse(ParameterType.Raw), jsValue) match {
36+
parameterInfos.getOrElse(parameterName, throw ParameterNotFoundException(parameterName, parameterInfos.keySet))
37+
fromJsValue(parameterInfo.`type`, jsValue) match {
3838
case Some(param) => param
3939
case None =>
4040
throw ParameterValueParsingException(parameterName)
@@ -59,7 +59,6 @@ object ParameterValue {
5959
}
6060
}
6161
sealed trait ParameterValue {
62-
def isEmpty: Boolean
6362

6463
/**
6564
* Implemented by each parameter value depending on what it wants
@@ -70,22 +69,18 @@ sealed trait ParameterValue {
7069
}
7170

7271
case class IntParameterValue(value: Int) extends ParameterValue {
73-
override def isEmpty: Boolean = false // can never be empty
7472
override def asJsonString: String = value.toString
7573
override def asJsValue: JsValue = JsNumber(value)
7674
}
7775
case class DecimalParameterValue(value: BigDecimal) extends ParameterValue {
78-
override def isEmpty: Boolean = false // can never be empty
7976
override def asJsonString: String = value.toString
8077
override def asJsValue: JsValue = JsNumber(value)
8178
}
8279
case class StringParameterValue(value: String) extends ParameterValue {
83-
override def isEmpty: Boolean = value.isEmpty
8480
override def asJsonString: String = StringEscapeUtils.escapeJson(value)
8581
override def asJsValue: JsValue = JsString(value)
8682
}
8783
case class RawParameterValue(value: String) extends ParameterValue {
88-
override def isEmpty: Boolean = value.isEmpty
8984
override def asJsonString: String = value
9085
override def asJsValue: JsValue = JsString(value)
9186
}

‎server/src/main/scala/de/frosner/broccoli/services/InstanceService.scala

+5-15
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,7 @@ class InstanceService @Inject()(nomadClient: NomadClient,
226226
}
227227
}
228228
} match {
229-
case Success(params) =>
230-
// Remove params with default values
231-
val parameters = params.filter {
232-
case (paramName, paramValue) => !paramValue.isEmpty
233-
}
229+
case Success(parameters) =>
234230
val maybeNewInstance = Try(Instance(id, template, parameters))
235231
maybeNewInstance.flatMap { newInstance =>
236232
val result = instanceStorage.writeInstance(newInstance)
@@ -270,22 +266,16 @@ class InstanceService @Inject()(nomadClient: NomadClient,
270266
}
271267
} match {
272268
case Success(parsedParamValuesUpdated) =>
273-
val parameterValuesUpdatesWithPossibleDefaults = parsedParamValuesUpdated.map { p =>
274-
p.filter {
275-
case (_, value) => !value.isEmpty
276-
}
277-
}
278-
279269
val instanceWithPotentiallyUpdatedTemplateAndParameterValues: Try[Instance] =
280270
if (templateSelector.isDefined) {
281271
val newTemplateId = templateSelector.get
282272
val newTemplate = templateService.template(newTemplateId)
283273
newTemplate
284274
.map { template =>
285275
// Requested template exists, update the template
286-
if (parameterValuesUpdatesWithPossibleDefaults.isDefined) {
276+
if (parsedParamValuesUpdated.isDefined) {
287277
// New parameter values are specified
288-
val newParameterValues = parameterValuesUpdatesWithPossibleDefaults.get
278+
val newParameterValues = parsedParamValuesUpdated.get
289279
instance.updateTemplate(template, newParameterValues)
290280
} else {
291281
// Just use the old parameter values
@@ -298,9 +288,9 @@ class InstanceService @Inject()(nomadClient: NomadClient,
298288
}
299289
} else {
300290
// No template update required
301-
if (parameterValuesUpdatesWithPossibleDefaults.isDefined) {
291+
if (parsedParamValuesUpdated.isDefined) {
302292
// Just update the parameter values
303-
val newParameterValues = parameterValuesUpdatesWithPossibleDefaults.get
293+
val newParameterValues = parsedParamValuesUpdated.get
304294
instance.updateParameterValues(newParameterValues)
305295
} else {
306296
// Neither template update nor parameter value update required

‎server/src/main/scala/de/frosner/broccoli/services/ParameterValueExceptions.scala

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package de.frosner.broccoli.services
2+
3+
case class ParameterValueParsingException(parameter: String) extends Exception(s"Could not parse parameter $parameter.")
4+
5+
case class ParameterNotFoundException(parameter: String, availParams: Set[String])
6+
extends Exception(s"Parameter '$parameter' not found. Available parameters are ${availParams.toString}")

‎server/src/main/scala/de/frosner/broccoli/services/ParameterValueParsingException.scala

-3
This file was deleted.

‎server/src/main/scala/de/frosner/broccoli/services/TemplateParameterNotFoundException.scala

-6
This file was deleted.

‎server/src/main/scala/de/frosner/broccoli/templates/TemplateConfig.scala

+5-5
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ object TemplateConfig {
2222
val paramValueObj = paramConfig.asInstanceOf[ConfigObject].toConfig
2323
val maybeName = Try(paramValueObj.getString("name")).toOption
2424
val maybeSecret = Try(paramValueObj.getBoolean("secret")).toOption
25-
// Don't wrap the last call to withName as we want it to fail in case the wrong type is supplied
26-
val maybeParamType = Try(paramValueObj.getString("type")).toOption.map(ParameterType.withName)
25+
// Don't wrap the call as we want it to fail in case the wrong type or no type is supplied
26+
val paramType = ParameterType.withName(paramValueObj.getString("type"))
2727
val maybeOrderIndex = Try(paramValueObj.getInt("order-index")).toOption
2828
val maybeDefault = Try(paramValueObj.getValue("default")).toOption.map { paramValueConf =>
2929
ParameterValue.fromConfigValue(
30-
maybeParamType.getOrElse(ParameterType.Raw),
30+
paramType,
3131
paramValueConf
3232
) match {
3333
case Success(paramDefault) => paramDefault
3434
case Failure(ex) => throw ex
3535
}
3636
}
37-
(paramName, Parameter(maybeName, maybeDefault, maybeSecret, maybeParamType, maybeOrderIndex))
37+
(paramName, Parameter(maybeName, maybeDefault, maybeSecret, paramType, maybeOrderIndex))
3838
}.toMap
3939
}
4040
TemplateInfo(description, parameters)
@@ -51,7 +51,7 @@ object TemplateConfig {
5151
final case class Parameter(name: Option[String],
5252
default: Option[ParameterValue],
5353
secret: Option[Boolean],
54-
`type`: Option[ParameterType],
54+
`type`: ParameterType,
5555
orderIndex: Option[Int])
5656

5757
}

‎server/src/test/resources/de/frosner/broccoli/templates/curl-without-decription/template.conf

+2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
parameters = {
22
"URL" = {
33
default = "localhost:8000"
4+
type = raw
45
}
56
"enabled" = {
67
default = true
8+
type = raw
79
}
810
}

‎server/src/test/resources/de/frosner/broccoli/templates/curl/template.conf

+2
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ parameters = {
44
"URL" = {
55
default = "localhost:8000",
66
order-index = 1
7+
type = raw
78
}
89
"enabled" = {
910
default = true,
1011
order-index = 2
12+
type = raw
1113
}
1214
}

‎server/src/test/scala/de/frosner/broccoli/controllers/InstanceControllerSpec.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class InstanceControllerSpec
7171
name = Some("secret"),
7272
default = Some(StringParameterValue("value")),
7373
secret = Some(true),
74-
`type` = None,
74+
`type` = ParameterType.String,
7575
orderIndex = None
7676
)
7777
)

‎server/src/test/scala/de/frosner/broccoli/controllers/TemplateControllerSpec.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package de.frosner.broccoli.controllers
22

3-
import de.frosner.broccoli.models.{ParameterInfo, StringParameterValue, Template}
3+
import de.frosner.broccoli.models.{ParameterInfo, ParameterType, StringParameterValue, Template}
44
import de.frosner.broccoli.services.{SecurityService, TemplateService}
55
import play.api.test.{FakeRequest, PlaySpecification, WithApplication}
66
import org.mockito.Mockito._
@@ -23,7 +23,7 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {
2323
name = Some("myname"),
2424
default = Some(StringParameterValue("myid")),
2525
secret = Some(false),
26-
`type` = None,
26+
`type` = ParameterType.String,
2727
orderIndex = None)
2828
)
2929
)

‎server/src/test/scala/de/frosner/broccoli/controllers/WebSocketControllerSpec.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class WebSocketControllerSpec
4141
name = None,
4242
default = Some(StringParameterValue("value")),
4343
secret = Some(true),
44-
`type` = None,
44+
`type` = ParameterType.String,
4545
orderIndex = None
4646
)
4747
)

‎server/src/test/scala/de/frosner/broccoli/instances/storage/InstanceStorageSpec.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class InstanceStorageSpec extends Specification {
6262
None,
6363
None,
6464
secret = Some(false),
65-
`type` = Some(ParameterType.Integer),
65+
`type` = ParameterType.Integer,
6666
orderIndex = None))
6767
),
6868
parameterValues = Map("id" -> StringParameterValue("Frank"), "age" -> IntParameterValue(50))
@@ -80,7 +80,7 @@ class InstanceStorageSpec extends Specification {
8080
template = "\"{{id}} {{age}}\"",
8181
description = "desc",
8282
parameterInfos =
83-
Map("age" -> ParameterInfo("age", None, None, secret = Some(false), `type` = None, orderIndex = None))
83+
Map("age" -> ParameterInfo("age", None, None, secret = Some(false), `type` = ParameterType.Integer, orderIndex = None))
8484
),
8585
parameterValues = Map("id" -> StringParameterValue("Frank"), "age" -> IntParameterValue(50))
8686
)

‎server/src/test/scala/de/frosner/broccoli/models/InstanceSpec.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class InstanceSpec extends Specification with ScalaCheck with ModelArbitraries w
4545
template = "\"{{id}} {{age}}\"",
4646
description = "desc",
4747
parameterInfos =
48-
Map("age" -> ParameterInfo("age", None, None, secret = Some(false), `type` = None, orderIndex = None))
48+
Map("age" -> ParameterInfo("age", None, None, secret = Some(false), `type` = ParameterType.Integer, orderIndex = None))
4949
),
5050
parameterValues = Map("id" -> StringParameterValue("Frank"))
5151
) must throwA[IllegalArgumentException]

‎server/src/test/scala/de/frosner/broccoli/models/InstanceWithStatusSpec.scala

+6-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class InstanceWithStatusSpec extends Specification with ScalaCheck with ModelArb
2525
name = None,
2626
default = None,
2727
secret = Some(true),
28-
`type` = Some(ParameterType.String),
28+
`type` = ParameterType.String,
2929
orderIndex = None
3030
)
3131
)
@@ -52,7 +52,7 @@ class InstanceWithStatusSpec extends Specification with ScalaCheck with ModelArb
5252
name = None,
5353
default = None,
5454
secret = Some(true),
55-
`type` = Some(ParameterType.String),
55+
`type` = ParameterType.String,
5656
orderIndex = None
5757
)
5858
)
@@ -83,15 +83,15 @@ class InstanceWithStatusSpec extends Specification with ScalaCheck with ModelArb
8383
name = None,
8484
default = None,
8585
secret = Some(true),
86-
`type` = Some(ParameterType.String),
86+
`type` = ParameterType.String,
8787
orderIndex = None
8888
),
8989
"id" -> ParameterInfo(
9090
id = "secret id",
9191
name = None,
9292
default = None,
9393
secret = Some(true),
94-
`type` = Some(ParameterType.String),
94+
`type` = ParameterType.String,
9595
orderIndex = None
9696
)
9797
)
@@ -118,15 +118,15 @@ class InstanceWithStatusSpec extends Specification with ScalaCheck with ModelArb
118118
name = None,
119119
default = None,
120120
secret = Some(true),
121-
`type` = Some(ParameterType.String),
121+
`type` = ParameterType.String,
122122
orderIndex = None
123123
),
124124
"id" -> ParameterInfo(
125125
id = "secret id",
126126
name = None,
127127
default = None,
128128
secret = Some(true),
129-
`type` = Some(ParameterType.String),
129+
`type` = ParameterType.String,
130130
orderIndex = None
131131
)
132132
)

‎server/src/test/scala/de/frosner/broccoli/models/ModelArbitraries.scala

+4-4
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ trait ModelArbitraries {
4747
id <- Gen.identifier.label("id")
4848
name <- Gen.option(Gen.identifier.label("name"))
4949
secret <- Gen.option(Gen.oneOf(true, false)).label("secret")
50-
type_ <- Gen.option(Gen.oneOf(ParameterType.values)).label("type")
51-
default <- Gen.option(genForParamValue(type_.getOrElse(ParameterType.Raw))).label("default")
50+
type_ <- Gen.oneOf(ParameterType.values).label("type")
51+
default <- Gen.option(genForParamValue(type_)).label("default")
5252
} yield
5353
ParameterInfo(
5454
id = id,
@@ -68,7 +68,7 @@ trait ModelArbitraries {
6868
templateDescription <- Gen.identifier.label("description")
6969
templateParameters <- Gen.listOf(arbParameterInfo.arbitrary).label("parameterInfos")
7070
} yield {
71-
val idParameter = ParameterInfo(id = "id", None, None, None, None, None)
71+
val idParameter = ParameterInfo(id = "id", None, None, None, ParameterType.String, None)
7272
val template = templateParameters.map(i => s"{{${i.id}}}").mkString(" ")
7373
Template(
7474
id = templateId,
@@ -87,7 +87,7 @@ trait ModelArbitraries {
8787
values <- Gen.sequence[Map[String, ParameterValue], (String, ParameterValue)](
8888
template.parameterInfos.map {
8989
case (paramName, info) =>
90-
genForParamValue(info.`type`.getOrElse(ParameterType.Raw))
90+
genForParamValue(info.`type`)
9191
.label("value")
9292
.map(paramName -> _)
9393
}

‎server/src/test/scala/de/frosner/broccoli/models/ParameterInfoSpec.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class ParameterInfoSpec extends Specification {
1313
name = None,
1414
default = Some(StringParameterValue("d")),
1515
secret = Some(false),
16-
`type` = Some(ParameterType.String),
16+
`type` = ParameterType.String,
1717
orderIndex = None)
1818
Json.fromJson(Json.toJson(info)).get === info
1919
}

‎server/src/test/scala/de/frosner/broccoli/models/TemplateSpec.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class TemplateSpec extends Specification {
6161
template = "template JSON {{id}}",
6262
description = "desc",
6363
parameterInfos = Map(
64-
"id" -> ParameterInfo("id", None, None, secret = Some(false), `type` = None, orderIndex = None)
64+
"id" -> ParameterInfo("id", None, None, secret = Some(false), `type` = ParameterType.String, orderIndex = None)
6565
)
6666
).version
6767
}

‎server/src/test/scala/de/frosner/broccoli/templates/DirectoryTemplateSourceSpec.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package de.frosner.broccoli.templates
22

33
import java.nio.file.{FileSystems, Files, Path}
44

5-
import de.frosner.broccoli.models.{ParameterInfo, RawParameterValue, StringParameterValue, Template}
5+
import de.frosner.broccoli.models._
66
import org.specs2.mutable.Specification
77

88
import scala.io.Source
@@ -28,8 +28,8 @@ class DirectoryTemplateSourceSpec extends Specification with TemporaryTemplatesC
2828
Source.fromFile(templatesDirectory.resolve("curl/template.json").toFile).mkString,
2929
"A periodic job that sends an HTTP GET request to a specified address every minute.",
3030
Map(
31-
"URL" -> ParameterInfo("URL", None, Some(RawParameterValue("localhost:8000")), None, None, Some(1)),
32-
"enabled" -> ParameterInfo("enabled", None, Some(RawParameterValue("true")), None, None, Some(2))
31+
"URL" -> ParameterInfo("URL", None, Some(RawParameterValue("localhost:8000")), None, ParameterType.Raw, Some(1)),
32+
"enabled" -> ParameterInfo("enabled", None, Some(RawParameterValue("true")), None, ParameterType.Raw, Some(2))
3333
)
3434
))).exactly(1)
3535
}

0 commit comments

Comments
 (0)