Partially fix #203 #281
Partially fix #203 #281
Conversation
Add tests for partially working shapeless coproducts and classes parametrised by type parameter of an outer type.
| "Build a model for a class using type parameters of an outer class" in { | ||
| val ms = modelOf[OuterString.Inner] | ||
| ms.size must_!== 0 // At least try to return some model. | ||
| // It won't be a fully correct model since TypeBuilder will fail to resolve the type param T to a concrete type. |
There was a problem hiding this comment.
Can you provide an example of why the model is not 'fully correct'?
I tried generating a similar class case class Foo(t: String) to check its output against OutString.Inner and this is what I noted:
Foo:
ModelImpl(model.Foo ,Foo ,Some(Foo) ,Some(object),None,List(),Map(t -> AbstractProperty(string,None,true,None,None ,None)),false,None,None,None,None)
OuterString.Inner:
ModelImpl(model.Outer.Inner,Inner,Some(Inner),Some(object),None,List(),Map(t -> AbstractProperty(string,None,true,None,Some(model.Outer.T),None)),false,None,None,None,None)
The only difference I can see is the description of the t val inside the Inner has Some(model.Outer.T) which I don't think is a bad thing.
There was a problem hiding this comment.
You're right. For Outer[String], the model is accidentally correct, the description is not a problem. I could have done better job choosing the test data. If we go with Outer[Int] the problem will be more apparent:
case class FooInt(t: Int)
object OuterInt extends Outer[Int]
Set(ModelImpl(org.http4s.rho.swagger.model.FooInt, FooInt,Some(FooInt),Some(object),None,List(),Map(t -> AbstractProperty(integer,None,true,None,None, Some(int32))),false,None,None,None,None))
Set(ModelImpl(org.http4s.rho.swagger.model.Outer.Inner,Inner, Some(Inner), Some(object),None,List(),Map(t -> AbstractProperty(string, None,true,None,Some(org.http4s.rho.swagger.model.Outer.T),None)), false,None,None,None,None))
Now the property t which was supposed to be an integer/int32 is a string and that is a bad thing.
I'll adapt the test case to show more clearly where the problem it.
|
@RafalSumislawski Http4s 0.20.0-M5 just landed and I'd like to push this PR into Rho 0.19.0-M5, so I'll hold off for a day or two for your replies. |
|
@RafalSumislawski I just pushed a commit that I think fixes the support for that fully. Let me know if you have any issues with it. |
|
Looks very good 👍 |
|
Appears to fail the scala 2.11 build... looking into why now.... |
There was a problem hiding this comment.
@RafalSumislawski Build for 2.11 is fixed. I had to add an extra check, seems some behavior changed between scala 2.11 and 2.12 around that typeSignatureIn function. Let me know your thoughts. If you are good with the changes, I'll merge it and cut a release.
|
Looks fine 👍 |
Add missing sym.isClass check in isObjectEnum
Add tests for partially working shapeless coproducts and classes parametrised by type parameter of an outer type.