Skip to content
This repository has been archived by the owner on Apr 10, 2019. It is now read-only.

Changing a post param to required causes generation error in model #52

Closed
okthatsneat opened this issue Dec 16, 2015 · 9 comments
Closed
Assignees
Labels
Milestone

Comments

@okthatsneat
Copy link

steps to reproduce:

  • run a mint activator template
  • load the swagger ui at localhost:9000
  • add required: true to the post param name
  • reload the page

expected: required constraint rendered in swagger ui

got instead:

[info] - play.api.Play - Application started (Dev)
[info] Compiling 15 Scala sources and 1 Java source to /Users/phofmann/code/Fresh/target/scala-2.11/classes...
[warn] /Users/phofmann/code/Fresh/target/scala-2.11/routes/main/controllers_base/echo.yaml.scala:32: Adaptation of argument list by inserting () has been deprecated: this is unlikely to be what you want.
[warn]         signature: EchoYamlBase.processValidgetRequest(f: EchoYamlBase.this.getActionType)(request: EchoYamlBase.this.getActionRequestType): play.api.mvc.Result
[warn]   given arguments: <none>
[warn]  after adaptation: EchoYamlBase.processValidgetRequest((): Unit)
[warn]             val result = processValidgetRequest(f)()
[warn]                                                   ^
[error] /Users/phofmann/code/Fresh/app/controllers/echo.yaml.scala:26: type mismatch;
[error]  found   : String
[error]  required: echo.yaml.PostYear
[error]     (which expands to)  Option[String]
[error]                                                                                     Success(Some(PostResponses200Opt(name, year)))
[error]                                                                                                                      ^
[warn] one warning found
[error] one error found
[error] (compile:compileIncremental) Compilation failed
[error] - application -

! @6oeikbk6d - Internal server error, for (GET) [/] ->

play.sbt.PlayExceptions$CompilationException: Compilation error[type mismatch;
 found   : String
 required: echo.yaml.PostYear
    (which expands to)  Option[String]]
    at play.sbt.PlayExceptions$CompilationException$.apply(PlayExceptions.scala:27) ~[na:na]
    at play.sbt.PlayExceptions$CompilationException$.apply(PlayExceptions.scala:27) ~[na:na]
    at scala.Option.map(Option.scala:145) ~[scala-library-2.11.7.jar:na]
    at play.sbt.run.PlayReload$$anonfun$taskFailureHandler$1.apply(PlayReload.scala:49) ~[na:na]
    at play.sbt.run.PlayReload$$anonfun$taskFailureHandler$1.apply(PlayReload.scala:44) ~[na:na]
    at scala.Option.map(Option.scala:145) ~[scala-library-2.11.7.jar:na]
    at play.sbt.run.PlayReload$.taskFailureHandler(PlayReload.scala:44) ~[na:na]
    at play.sbt.run.PlayReload$.compileFailure(PlayReload.scala:40) ~[na:na]
    at play.sbt.run.PlayReload$$anonfun$compile$1.apply(PlayReload.scala:17) ~[na:na]
    at play.sbt.run.PlayReload$$anonfun$compile$1.apply(PlayReload.scala:17) ~[na:na]
    at scala.util.Either$LeftProjection.map(Either.scala:377) ~[scala-library-2.11.7.jar:na]
    at play.sbt.run.PlayReload$.compile(PlayReload.scala:17) ~[na:na]
    at play.sbt.run.PlayRun$$anonfun$playRunTask$1$$anonfun$apply$2$$anonfun$apply$3$$anonfun$2.apply(PlayRun.scala:61) ~[na:na]
    at play.sbt.run.PlayRun$$anonfun$playRunTask$1$$anonfun$apply$2$$anonfun$apply$3$$anonfun$2.apply(PlayRun.scala:61) ~[na:na]
    at play.runsupport.Reloader$$anonfun$reload$1.apply(Reloader.scala:323) ~[na:na]
    at play.runsupport.Reloader$$anon$3.run(Reloader.scala:43) ~[na:na]
    at java.security.AccessController.doPrivileged(Native Method) ~[na:1.8.0_45]
    at play.runsupport.Reloader$.play$runsupport$Reloader$$withReloaderContextClassLoader(Reloader.scala:39) ~[na:na]
    at play.runsupport.Reloader.reload(Reloader.scala:321) ~[na:na]
    at play.core.server.DevServerStart$$anonfun$mainDev$1$$anon$1$$anonfun$get$1.apply(DevServerStart.scala:113) ~[play-server_2.11-2.4.3.jar:2.4.3]
    at play.core.server.DevServerStart$$anonfun$mainDev$1$$anon$1$$anonfun$get$1.apply(DevServerStart.scala:111) ~[play-server_2.11-2.4.3.jar:2.4.3]
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.liftedTree1$1(Future.scala:24) ~[scala-library-2.11.7.jar:na]
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.run(Future.scala:24) ~[scala-library-2.11.7.jar:na]
    at akka.dispatch.TaskInvocation.run(AbstractDispatcher.scala:40) ~[akka-actor_2.11-2.3.13.jar:na]
    at akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinTask.exec(AbstractDispatcher.scala:397) ~[akka-actor_2.11-2.3.13.jar:na]
    at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260) ~[scala-library-2.11.7.jar:na]
    at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339) ~[scala-library-2.11.7.jar:na]
    at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979) ~[scala-library-2.11.7.jar:na]
    at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107) ~[scala-library-2.11.7.jar:na]

generated file at fault:
target/scala-2.11/routes/main/model/echo.yaml.scala

package echo
package object yaml {
import java.util.Date
import java.io.File


    type PostName = String

    type PostYear = Option[String]

    type PostResponses200 = Option[PostResponses200Opt]

    case class PostResponses200Opt(name: PostYear, 
year: PostYear
) 



}
@okthatsneat
Copy link
Author

is this the intended behavior? as PostName was correctly updated by the generator, I assume PostResponse200Opt should also update the type of the name param?

@slavaschmidt
Copy link
Contributor

This is indeed the expected behaviour. The snippet Success(Some(PostResponses200Opt(name, year)))is what programmer created, it's not part of the generated code. And because of that it's not consistent with the changes introduced to the generated code by the change in the specification.
Hence, compile error. Does it make sense?

Do you think we should provide marker comments for the beginning and for the end of the code which won't be changed by the plugin despite changes in the specification?

@slavaschmidt slavaschmidt self-assigned this Dec 16, 2015
@okthatsneat
Copy link
Author

@slavaschmidt doesn't make sense to me yet; the snippet Success(Some(PostResponses200Opt(name, year))) was actually generated, also there is no way for the programmer to correct it in this file as the case class itself requires parameters of the now wrong type. What would be the fix in the controller?

@okthatsneat
Copy link
Author

I guess I can wrap the name parameter that is no longer optional into an Option before passing it to the case class; however, that feels like a hack; Is that what was intended?

@okthatsneat
Copy link
Author

and yes, definitely. Giving clearly documented / marked entry points to the developers will help with keeping play-swagger errors apart from developer errors.

@slavaschmidt
Copy link
Contributor

@okthatsneat Success(Some(PostResponses200Opt(name, year))) was not generated. You can check what's generated by removing the controller file. The plugin will generate a new skeleton for you. Than you can compare what we provide as a convenience implementation for new users and what is actually generated.

Yes, you can wrap the parameter into the Option. The code will compile then.

And actually, this would not be a hack in a real project. The action method expect YOU as a programmer to provide a logic with a return type Try[Option[PostResponses200Opt]]. So please do that. To play with a plugin it's enough to return `Success(Some(PostResponses200Opt(name, year)))``, but this won't be the case in the real project, right?

@okthatsneat
Copy link
Author

I was able to get the new controller skeleton generated: for this the play application needs to be aborted, and the artefacts cleaned.

The generated controller now reads

import play.api.mvc.{Action, Controller}
import play.api.data.validation.Constraint
import de.zalando.play.controllers._
import PlayBodyParsing._
import PlayValidations._
package echo.yaml {

    class EchoYaml extends EchoYamlBase {
    val get = getAction {
            ???

        } //////// EOF ////////  getAction
    val post = postAction {
            input: (PostName, PostName) =>
            val (name, year) = input

            ???

        } //////// EOF ////////  postAction
    val getTest_pathById = getTest_pathByIdAction {
            (id: String) =>
???

        } //////// EOF ////////  getTest_pathByIdAction
    }
}

and the generated model

package echo
package object yaml {
import java.util.Date
import java.io.File


    type PostName = Option[String]

    type `Test-pathIdGetId` = String

    type PostResponses200 = Option[PostResponses200Opt]

    case class PostResponses200Opt(name: PostName, 
year: PostName
) 



}

if I now add to the name post param to required: true and reload in the browser, and implement all controller actions with dummy Success(Some("") implementations, I get this error:

type mismatch;
 found   : String("")
 required: echo.yaml.PostResponses200Opt

which points me to the generated model case class that I have no control over, which still is implemented as

package echo
package object yaml {
import java.util.Date
import java.io.File


    type PostName = Option[String]

    type `Test-pathIdGetId` = String

    type PostResponses200 = Option[PostResponses200Opt]

    case class PostResponses200Opt(name: PostName, 
year: PostName
) 



}

I would have expected it to re-generate with a correct type and signature for the exptected return type with regards to the now no longer optional parameter name.

@slavaschmidt slavaschmidt added this to the 0.1.5 milestone Dec 19, 2015
@slavaschmidt
Copy link
Contributor

It seems for me, that you have a return type for Postdefined like an objectwith two properties, name and year, right?

If yes, then you expected to return something of that type from your controller, not a string

It is your responsibility as a programmer to return correct result from your business logic, and in this case it should be of type Try[PostResponses200Opt] if I can guess your specification details correctly.

@slavaschmidt
Copy link
Contributor

Won't fix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants