zyv
commented
7 years ago Proposed patch:
diff --git a/plugin/src/main/scala/de/zalando/play/generator/routes/RuleGenerator.scala b/plugin/src/main/scala/de/zalando/play/generator/routes/RuleGenerator.scala
index 2d63658..2acd78d 100644
--- a/plugin/src/main/scala/de/zalando/play/generator/routes/RuleGenerator.scala
+++ b/plugin/src/main/scala/de/zalando/play/generator/routes/RuleGenerator.scala
@@ -75,7 +75,7 @@ object RuleGenerator {
val name = Path.strip(part)
val (constraint, encode) = model.findParameter(Reference(name)) match {
case Some(param) => (param.constraint, param.encode)
- case None => ("""[^/]+""", true)
+ case None => ("""[^/]*""", true)
}
DynamicPart(name, constraint, encode)
} else {
diff --git a/plugin/src/test/scala/de/zalando/play/generator/routes/RuleGeneratorTest.scala b/plugin/src/test/scala/de/zalando/play/generator/routes/RuleGeneratorTest.scala
index 8a34183..c9c9e2b 100644
--- a/plugin/src/test/scala/de/zalando/play/generator/routes/RuleGeneratorTest.scala
+++ b/plugin/src/test/scala/de/zalando/play/generator/routes/RuleGeneratorTest.scala
@@ -14,7 +14,7 @@ class RuleGeneratorTest extends FunSpec with MustMatchers {
implicit val model = StrictModel(Nil, Map.empty, Map.empty, Map.empty, "/base/", None, Map.empty, Map.empty)
val routes = Map(
- "/" -> Nil, "/a/b/c/d" -> List(StaticPart("a/b/c/d")), "/a/b/c/d/" -> List(StaticPart("a/b/c/d/")), "/a/{b}/c/{d}" -> List(StaticPart("a/"), DynamicPart("b", """[^/]+""", true), StaticPart("/c/"), DynamicPart("d", """[^/]+""", true)), "/{a}/{b}/{c}/{d}/" -> List(DynamicPart("a", """[^/]+""", true), StaticPart("/"), DynamicPart("b", """[^/]+""", true), StaticPart("/"), DynamicPart("c", """[^/]+""", true), StaticPart("/"), DynamicPart("d", """[^/]+""", true), StaticPart("/")), "/{a}/b/{c}/d/" -> List(DynamicPart("a", """[^/]+""", true), StaticPart("/b/"), DynamicPart("c", """[^/]+""", true), StaticPart("/d/"))
+ "/" -> Nil, "/a/b/c/d" -> List(StaticPart("a/b/c/d")), "/a/b/c/d/" -> List(StaticPart("a/b/c/d/")), "/a/{b}/c/{d}" -> List(StaticPart("a/"), DynamicPart("b", """[^/]*""", true), StaticPart("/c/"), DynamicPart("d", """[^/]*""", true)), "/{a}/{b}/{c}/{d}/" -> List(DynamicPart("a", """[^/]*""", true), StaticPart("/"), DynamicPart("b", """[^/]*""", true), StaticPart("/"), DynamicPart("c", """[^/]*""", true), StaticPart("/"), DynamicPart("d", """[^/]*""", true), StaticPart("/")), "/{a}/b/{c}/d/" -> List(DynamicPart("a", """[^/]*""", true), StaticPart("/b/"), DynamicPart("c", """[^/]*""", true), StaticPart("/d/"))
)
describe("RuleGeneratorTest") {New error message showing that at least routing seems now to work correctly:
[info] GET /v1/welcome/{name}
[info] - should discard invalid data *** FAILED ***
[info] Expected true but got false
[info] Contains error: error.pattern in "Bad Request: Illegal character in path at index 1: /\n"
[info] at least one validation failing
[info] given 'Content-Type' [application/json], 'Accept' header [application/json] and URL: [/v1/welcome/\n?] given args: '(\n,None)' did not equal (xxx:290)
LappleApple
slavaschmidt
Hi,
Consider the following hello world schema:
I would expect at least requests to
/v1/welcome/to be routed togetWelcomeByNameand fail validation, because non-empty name is required and result in a BadRequest (400). The auto-generated tests seem to agree with me:In reality, this doesn't happen, because the pattern for parameters expects at least one character. I've patched it to require zero characters or more, and the test for the response code seems to pass, but then fails with a different problem.
I'm not sure that this is the right way to solve the problem and what additional consequences it might have otherwise, therefore I'm not making a pull request, but I will attach my patch to this issue.
Also, I would expect that requests to
/v1/welcomeget redirected to/v1/welcome/if there is no handler explicitly defined to/v1/welcome(my understanding is that this is how Django routing works, for instance). Any opinions on that?