dave - automatically returns a 400 when all required params are not f…

…ound

src/main/scala/io/github/daviddenton/fintrospect/FintrospectModule.scala

@@ -1,17 +1,17 @@
 package io.github.daviddenton.fintrospect
 
+import _root_.util.ResponseBuilder._
 import com.twitter.finagle.http.path.{Path, _}
 import com.twitter.finagle.http.service.RoutingService
 import com.twitter.finagle.http.{Request, Response}
-import com.twitter.finagle.{Service, SimpleFilter}
+import com.twitter.finagle.{Filter, Service, SimpleFilter}
 import com.twitter.util.Future
 import io.github.daviddenton.fintrospect.FintrospectModule._
 import io.github.daviddenton.fintrospect.parameters.PathParameter
 import io.github.daviddenton.fintrospect.util.ArgoUtil.pretty
-import org.jboss.netty.buffer.ChannelBuffers._
 import org.jboss.netty.handler.codec.http.HttpMethod
 import org.jboss.netty.handler.codec.http.HttpMethod.GET
-import org.jboss.netty.util.CharsetUtil._
+import org.jboss.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST
 
 object FintrospectModule {
   val IDENTIFY_SVC_HEADER = "descriptionServiceId"
@@ -23,34 +23,37 @@ object FintrospectModule {
   def toService(binding: Binding): Svc = RoutingService.byMethodAndPathObject(binding)
 
   def apply(basePath: Path, renderer: Renderer): FintrospectModule = new FintrospectModule(basePath, renderer, Nil, PartialFunction.empty[(HttpMethod, Path), Svc])
-}
 
-class FintrospectModule private(basePath: Path, renderer: Renderer, moduleRoutes: List[ModuleRoute], private val userRoutes: Binding) {
+  private case class ValidateParams(moduleRoute: ModuleRoute) extends SimpleFilter[Request, Response]() {
+    override def apply(request: Request, service: Service[Request, Response]): Future[Response] = {
+      val missingParams = moduleRoute.description.required.map(p => p.unapply(request).map(_ => None).getOrElse(Some(p.name))).flatten
+      if (missingParams.isEmpty) service(request) else Error(BAD_REQUEST, "Missing required parameters:" + missingParams.mkString(","))
+    }
+  }
 
   private case class Identify(moduleRoute: ModuleRoute) extends SimpleFilter[Request, Response]() {
     override def apply(request: Request, service: Service[Request, Response]): Future[Response] = {
-      val url = if(moduleRoute.toString().length == 0) "/" else moduleRoute.toString()
+      val url = if (moduleRoute.toString().length == 0) "/" else moduleRoute.toString()
       request.headers().set(IDENTIFY_SVC_HEADER, request.getMethod() + "." + url)
       service(request)
     }
   }
 
   private case class RoutesContent(descriptionContent: String) extends Service[Request, Response]() {
-    override def apply(request: Request): Future[Response] = {
-      val response = Response()
-      response.setStatusCode(200)
-      response.setContent(copiedBuffer(descriptionContent, UTF_8))
-      Future.value(response)
-    }
+    override def apply(request: Request): Future[Response] = Ok(descriptionContent)
   }
+}
+
+class FintrospectModule private(basePath: Path, renderer: Renderer, moduleRoutes: List[ModuleRoute], private val userRoutes: Binding) {
 
   private def withDefault() = {
     withRoute(Description("Description route"), On(GET, identity), () => RoutesContent(pretty(renderer(moduleRoutes))))
   }
 
-  private def withDescribedRoute(description: Description, on: On, PP: PP[_]*)(bindFn: Identify => Binding): FintrospectModule = {
+  private def withDescribedRoute(description: Description, on: On, PP: PP[_]*)(bindFn: Filter[Request, Response, Request, Response] => Binding): FintrospectModule = {
     val moduleRoute = new ModuleRoute(description, on, basePath, PP)
-    new FintrospectModule(basePath, renderer, moduleRoute :: moduleRoutes, userRoutes.orElse(bindFn(Identify(moduleRoute))))
+    new FintrospectModule(basePath, renderer, moduleRoute :: moduleRoutes,
+      userRoutes.orElse(bindFn(ValidateParams(moduleRoute).andThen(Identify(moduleRoute)))))
   }
 
   def withRouteSpec(routeSpec: RouteSpec): FintrospectModule = routeSpec.attachTo(this)
@@ -99,4 +102,5 @@ class FintrospectModule private(basePath: Path, renderer: Renderer, moduleRoutes
 
   def routes = withDefault().userRoutes
 
-  def toService = FintrospectModule.toService(routes)}
+  def toService = FintrospectModule.toService(routes)
+}

src/main/scala/io/github/daviddenton/fintrospect/ModuleRoute.scala

@@ -1,11 +1,12 @@
 package io.github.daviddenton.fintrospect
 
 import com.twitter.finagle.http.path.Path
+import io.github.daviddenton.fintrospect.parameters.Requirement._
 import io.github.daviddenton.fintrospect.parameters.{Parameter, PathParameter, Requirement}
 
 class ModuleRoute(val description: Description, val on: On, val basePath: Path, pathParams: Seq[PathParameter[_]]) {
-  val allParams: List[(Requirement, Parameter[_])] = description.optional.map(Requirement.Optional -> _) ++
-    (description.required ++ pathParams).map(Requirement.Mandatory -> _)
+  val allParams: List[(Requirement, Parameter[_])] = description.optional.map(Optional -> _) ++
+    (description.required ++ pathParams).map(Mandatory -> _)
 
   override def toString: String = (on.completeRoutePath(basePath).toString :: pathParams.map(_.toString()).toList).mkString("/")
 }

src/main/scala/io/github/daviddenton/fintrospect/parameters/RequiredRequestParameter.scala

@@ -6,5 +6,6 @@ import scala.reflect.ClassTag
 
 class RequiredRequestParameter[T](name: String, description: Option[String], location: Location, parse: (String => Option[T]))(implicit ct: ClassTag[T])
   extends RequestParameter[T](name, description, location, parse)(ct) {
-  def from(request: Request): T = location.from(name, request).flatMap(parse).get
+  def from(request: Request): T = unapply(request).get
+  def unapply(request: Request): Option[T] = location.from(name, request).flatMap(parse)
 }

src/test/scala/io/github/daviddenton/fintrospect/FintrospectModuleTest.scala

@@ -5,6 +5,7 @@ import com.twitter.finagle.http.path.Root
 import com.twitter.finagle.http.{Request, Response}
 import com.twitter.io.Charsets._
 import com.twitter.util.{Await, Future}
+import io.github.daviddenton.fintrospect.parameters.Header
 import io.github.daviddenton.fintrospect.parameters.Path._
 import io.github.daviddenton.fintrospect.renderers.SimpleJson
 import org.jboss.netty.buffer.ChannelBuffers._
@@ -23,37 +24,90 @@ class FintrospectModuleTest extends FunSpec with ShouldMatchers {
   }
 
   describe("FintrospectModule") {
-    describe("Routes a request") {
+    describe("when a route path can be found") {
+      val m = FintrospectModule(Root, SimpleJson())
+      val d = Description("")
+      val on = On(GET, _ / "svc")
 
+      it("with 0 segment") {
+        assertOkResponse(m.withRoute(d, on, () => AService(Seq())), Seq())
+      }
+      it("with 1 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), (_1: String) => AService(Seq(_1))), Seq("a"))
+      }
+      it("with 2 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), (_1: String, _2: String) => AService(Seq(_1, _2))), Seq("a", "b"))
+      }
+      it("with 3 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), (_1: String, _2: String, _3: String) => AService(Seq(_1, _2, _3))), Seq("a", "b", "c"))
+      }
+      it("with 4 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), (_1: String, _2: String, _3: String, _4: String) => AService(Seq(_1, _2, _3, _4))), Seq("a", "b", "c", "d"))
+      }
+      it("with 5 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), (_1: String, _2: String, _3: String, _4: String, _5: String) => AService(Seq(_1, _2, _3, _4, _5))), Seq("a", "b", "c", "d", "e"))
+      }
+      it("with 6 segments") {
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), string("s6"), (_1: String, _2: String, _3: String, _4: String, _5: String, _6: String) => AService(Seq(_1, _2, _3, _4, _5, _6))), Seq("a", "b", "c", "d", "e", "f"))
+      }
+    }
+
+    describe("when a route path cannot be found") {
+      it("returns a 404") {
+        val result = Await.result(FintrospectModule(Root, SimpleJson()).toService.apply(Request("/svc/noSuchRoute")))
+        result.status.getCode should be === 404
+      }
+    }
+
+    describe("Routes valid requests by path") {
       val m = FintrospectModule(Root, SimpleJson())
       val d = Description("")
       val on = On(GET, _ / "svc")
 
       it("with 0 segment") {
-        assertCorrectResponse(m.withRoute(d, on, () => AService(Seq())), Seq())
+        assertOkResponse(m.withRoute(d, on, () => AService(Seq())), Seq())
       }
       it("with 1 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), (_1: String) => AService(Seq(_1))), Seq("a"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), (_1: String) => AService(Seq(_1))), Seq("a"))
       }
       it("with 2 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), string("s2"), (_1: String, _2: String) => AService(Seq(_1, _2))), Seq("a", "b"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), (_1: String, _2: String) => AService(Seq(_1, _2))), Seq("a", "b"))
       }
       it("with 3 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), (_1: String, _2: String, _3: String) => AService(Seq(_1, _2, _3))), Seq("a", "b", "c"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), (_1: String, _2: String, _3: String) => AService(Seq(_1, _2, _3))), Seq("a", "b", "c"))
       }
       it("with 4 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), (_1: String, _2: String, _3: String, _4: String) => AService(Seq(_1, _2, _3, _4))), Seq("a", "b", "c", "d"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), (_1: String, _2: String, _3: String, _4: String) => AService(Seq(_1, _2, _3, _4))), Seq("a", "b", "c", "d"))
       }
       it("with 5 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), (_1: String, _2: String, _3: String, _4: String, _5: String) => AService(Seq(_1, _2, _3, _4, _5))), Seq("a", "b", "c", "d", "e"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), (_1: String, _2: String, _3: String, _4: String, _5: String) => AService(Seq(_1, _2, _3, _4, _5))), Seq("a", "b", "c", "d", "e"))
       }
       it("with 6 segments") {
-        assertCorrectResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), string("s6"), (_1: String, _2: String, _3: String, _4: String, _5: String, _6: String) => AService(Seq(_1, _2, _3, _4, _5, _6))), Seq("a", "b", "c", "d", "e", "f"))
+        assertOkResponse(m.withRoute(d, on, string("s1"), string("s2"), string("s3"), string("s4"), string("s5"), string("s6"), (_1: String, _2: String, _3: String, _4: String, _5: String, _6: String) => AService(Seq(_1, _2, _3, _4, _5, _6))), Seq("a", "b", "c", "d", "e", "f"))
+      }
+    }
+
+    describe("when a valid path does not contain all required parameters") {
+      val d = Description("").taking(Header.required.int("aNumberHeader"))
+      val on = On(GET, _ / "svc")
+      val m = FintrospectModule(Root, SimpleJson()).withRoute(d, on, ()=> AService(Seq()))
+
+      it("it returns a 400 when the param is missing") {
+        val request = Request("/svc")
+        val result = Await.result(m.toService.apply(request))
+        result.status.getCode should be === 400
+      }
+
+      it("it returns a 400 when the param is not the correct type") {
+        val request = Request("/svc")
+        request.headers().add("aNumberHeader", "notANumber")
+        val result = Await.result(m.toService.apply(request))
+        result.status.getCode should be === 400
       }
     }
   }
 
-  def assertCorrectResponse(module: FintrospectModule, segments: Seq[String]): Unit = {
+  def assertOkResponse(module: FintrospectModule, segments: Seq[String]): Unit = {
     val result = Await.result(module.toService.apply(Request("/svc/" + segments.mkString("/"))))
     result.status.getCode should be === 200
     result.content.toString(Utf8) should be === segments.mkString(",")