Skip to content

Commit

Permalink
Upgrade wix-accord to 0.7.1 (d2iq-archive#5553)
Browse files Browse the repository at this point in the history
Summary:
This is a pre-requisite to moving to Scala 2.12

A common source of frustration when testing validations has been to
figure out the right matcher format. To help with that aim, we
introduce the shouldViolate and shouldSucceed test helpers which have
much easier-to-grok output

Remove needless implicits, add doc strings, address deprecation warnings,
normalize some code.

Replace shouldViolate, shouldNotViolate, shouldSucceed etc with Matchers.

Remove wix accord test helpers.

JIRA Issues: MARATHON-1779

Also-By: Matthias Veit <[email protected]>
  • Loading branch information
timcharper authored and jeschkies committed Oct 4, 2017
1 parent bc8ffc4 commit 470096f
Show file tree
Hide file tree
Showing 42 changed files with 685 additions and 775 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ object RunSpecValidator {
override def apply(seq: Iterable[T]): Result = {

val violations = seq.view.map(item => (item, validator(item))).zipWithIndex.collect {
case ((item, f: Failure), pos: Int) => GroupViolation(item, "not valid", Some(s"($pos)"), f.violations)
case ((item, f: Failure), pos: Int) => GroupViolation(item, "not valid", f.violations, Descriptions.Indexed(pos.toLong))
}

if (violations.isEmpty) Success
else Failure(Set(GroupViolation(seq, "Seq contains elements, which are not valid.", None, violations.toSet)))
else Failure(Set(GroupViolation(seq, "Seq contains elements, which are not valid.", violations.toSet)))
}
}
}
Expand All @@ -33,7 +33,7 @@ object RunSpecValidator {
// into a shared validations subproject.
import ViolationBuilder._
override def apply(value: T): Result = {
if (test(value)) Success else RuleViolation(value, constraint(value), None)
if (test(value)) Success else RuleViolation(value, constraint(value))
}
}
}
4 changes: 1 addition & 3 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ object Dependencies {
Test.akkaHttpTestKit % "test",
Test.junit % "test",
Test.scalacheck % "test",
Test.wixAccordScalatest % "test",
Test.curatorTest % "test"
) ++ Kamon.all).map(
_.excludeAll(excludeSlf4jLog4j12)
Expand Down Expand Up @@ -108,7 +107,7 @@ object Dependency {
val MarathonApiConsole = "3.0.8-accept"
val Logback = "1.1.3"
val Logstash = "4.9"
val WixAccord = "0.5"
val WixAccord = "0.7.1"
val Curator = "2.11.1"
val Java8Compat = "0.8.0"
val ScalaLogging = "3.5.0"
Expand Down Expand Up @@ -192,7 +191,6 @@ object Dependency {
val diffson = "org.gnieh" %% "diffson" % V.Diffson
val junit = "junit" % "junit" % V.JUnit
val scalacheck = "org.scalacheck" %% "scalacheck" % V.ScalaCheck
val wixAccordScalatest = "com.wix" %% "accord-scalatest" % V.WixAccord
val curatorTest = "org.apache.curator" % "curator-test" % V.Curator
}
}
5 changes: 2 additions & 3 deletions src/main/scala/mesosphere/marathon/api/RestResource.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,15 @@ trait RestResource {
* See [[assumeValid]], which is preferred to this.
*
* @param t object to validate
* @param description optional description which might be injected into the failure message
* @param fn function to execute after successful validation
* @param validator validator to use
* @tparam T type of object
* @return returns a 422 response if there is a failure due to validation. Executes fn function if successful.
*/
protected def withValid[T](t: T, description: Option[String] = None)(fn: T => Response)(implicit validator: Validator[T]): Response = {
protected def withValid[T](t: T)(fn: T => Response)(implicit validator: Validator[T]): Response = {
validator(t) match {
case f: Failure =>
val entity = Json.toJson(description.map(f.withDescription).getOrElse(f)).toString
val entity = Json.toJson(f).toString
Response.status(StatusCodes.UnprocessableEntity.intValue).entity(entity).build()
case Success => fn(t)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import akka.http.scaladsl.model.StatusCodes
import akka.http.scaladsl.server.{ Rejection, RejectionError, Route }
import akka.http.scaladsl.unmarshalling.{ FromEntityUnmarshaller, Unmarshaller }
import akka.util.ByteString
import com.wix.accord.Descriptions.{ Generic, Path }
import com.wix.accord.{ Failure, RuleViolation, Success, Validator }
import kamon.metric.SubscriptionsDispatcher.TickMetricSnapshot
import mesosphere.marathon.api.v2.Validation
Expand All @@ -16,6 +17,8 @@ import mesosphere.marathon.core.plugin.PluginDefinitions
import mesosphere.marathon.state.AppDefinition
import play.api.libs.json._

import scala.collection.breakOut

object EntityMarshallers {
import Directives.complete
import mesosphere.marathon.api.v2.json.Formats._
Expand All @@ -32,6 +35,18 @@ object EntityMarshallers {
private val jsonStringMarshaller =
Marshaller.stringMarshaller(`application/json`)

def jsErrorToFailure(error: JsError): Failure = Failure(
error.errors.flatMap {
case (path, validationErrors) =>
validationErrors.map { validationError =>
RuleViolation(
validationError.args.mkString(", "),
validationError.message,
path = Path(path.toString.split("/").filter(_ != "").map(Generic(_)): _*))
}
}(breakOut)
)

/**
* HTTP entity => `A`
*
Expand All @@ -47,17 +62,9 @@ object EntityMarshallers {
reads
.reads(json)
.recoverTotal {
case JsError(errors) =>
val violations = errors.flatMap {
case (path, validationErrors) =>
validationErrors.map { validationError =>
RuleViolation(
validationError.args.mkString(", "),
validationError.message,
Some(path.toString()))
}
}
throw RejectionError(ValidationFailed(Failure(violations.toSet)))
case error: JsError =>
throw RejectionError(
ValidationFailed(jsErrorToFailure(error)))
}
jsonStringUnmarshaller.map(data => read(Json.parse(data)))
}
Expand Down
Loading

0 comments on commit 470096f

Please sign in to comment.