Maintain route file ordering in SwaggerSpecGenerator

[tculshaw]

Test passes, but isn't particularly pleasing to the eye;)

Uses the student routes as the test case - hope that's good enough.

[coveralls]

Coverage increased (+0.06%) to 91.398% when pulling 75d3939 on tculshaw:master into 4962ace on iheartradio:master.

[kailuowang]

Thanks very much! See my trivial comment above.

[kailuowang]

Can we add some comment here to indicate the purpose of these 4 lines is to maintain the order?

[kailuowang]

This could be written as

val result = List(first, second, third, fourth)
result.forall(_.isDefined) === true
result.orderBy(_.get) === result

Right?

[tculshaw]

Comment added in code. Test left as is because need difference of exactly 1 to prevent other routes being inserted inbetween. R/e test, In a perfect world I would probably add the forall and then reduce it checking result is all '1's? But I reckon it's "clunky" readable as is:) Tony Culshaw MA Cantab Director Mentation Limited - Custom Software Development

…

On 5 April 2017 at 23:34, Kai(luo) Wang ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks very much! See my trivial comment above. ------------------------------ In core/src/main/scala/com/iheart/playSwagger/SwaggerSpecGenerator.scala <#152 (comment)> : > @@ -276,9 +276,14 @@ final case class SwaggerSpecGenerator( private def paths(routes: Seq[Route], prefix: String, tag: Option[Tag]): JsObject = { JsObject { - routes.flatMap(endPointEntry(_, prefix, tag)) - .groupBy(_._1) // Routes grouped by path - .mapValues(_.map(_._2).reduce(_ deepMerge _)) + val endPointEntries = routes.flatMap(route ⇒ endPointEntry(route, prefix, tag)) + + val zgbp = endPointEntries.zipWithIndex.groupBy(_._1._1) + import collection.mutable.LinkedHashMap + val lhm = LinkedHashMap(zgbp.toSeq sortBy (_._2.head._2): _*) + val gbp2 = lhm mapValues (_ map (_._1)) toSeq Can we add some comment here to indicate the purpose of these 4 lines is to maintain the order? ------------------------------ In core/src/test/scala/com/iheart/playSwagger/ SwaggerSpecGeneratorSpec.scala <#152 (comment)> : > + val fieldsWithIndexMap = fieldKeys.zipWithIndex.toMap + + val first = fieldsWithIndexMap.get("/api/students/{name}") + val second = fieldsWithIndexMap.get("/api/students/defaultValueParam") + val third = fieldsWithIndexMap.get("/api/students/defaultValueParamString") + val fourth = fieldsWithIndexMap.get("/api/students/defaultValueParamString3") + + // all paths must be retrieved + first must beSome[Int] + second must beSome[Int] + third must beSome[Int] + fourth must beSome[Int] + + // must be in exact order with nothing inbetween + second.get - first.get === 1 + third.get - second.get === 1 This could be written as val result = List(first, second, third, fourth) result.forall(_.isDefined) === true result.orderBy(_.get) === result Right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#152 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAianexWVl3e6GLbGDAGPgtPpPCpq0nbks5rs3xQgaJpZM4MzwIq> .

[coveralls]

Coverage increased (+0.06%) to 91.398% when pulling 5147f99 on tculshaw:master into 4962ace on iheartradio:master.

[kailuowang]

I see. Makes sense. Thanks again!