Skip to content

Conversation

@SamTheisens
Copy link
Contributor

This PR attempts to address the issue that optional and iterable Java primitive types e.g. Option[Int], Seq[Boolean] are lost in translation as described in #117.

Note that this implementation depends on scala reflection and therefore does not work in Scala 3. There is a stub method ErasureHelper.erasedOptionalPrimitives(cls: Class[_]): Map[String, Class[_]] that would need to be implemented using macros(?).
I may have a stab at that later, but I'm completely new to Scala 3 or macros for that matter. So if someone else know how to do this, help with that would be much appreciated!

but leave all other annotation properties in tact

branch:
by introducing a Scala 3 stub implementation of
`erasedOptionalPrimitives`. Tests still fail
@codecov
Copy link

codecov bot commented Aug 6, 2022

Codecov Report

Merging #166 (c5137b5) into develop (f78b18d) will increase coverage by 3.88%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #166      +/-   ##
===========================================
+ Coverage    78.57%   82.45%   +3.88%     
===========================================
  Files            1        2       +1     
  Lines          140      171      +31     
  Branches        12       20       +8     
===========================================
+ Hits           110      141      +31     
  Misses          30       30              
Impacted Files Coverage Δ
...github/swagger/scala/converter/ErasureHelper.scala 100.00% <100.00%> (ø)
...r/scala/converter/SwaggerScalaModelConverter.scala 81.36% <100.00%> (+2.79%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pjfanning
Copy link
Contributor

Thanks - I'll merge this but I'll probably need to do something to get the tests to run for the scala3 build, even if they will have different assertion expectations. Scala3 version of this lib is not yet widely used.

@pjfanning pjfanning merged commit 076b8ca into swagger-akka-http:develop Aug 6, 2022
@pjfanning pjfanning changed the title Save primitives from erasure Save primitives from erasure (Scala2 only) Aug 6, 2022
@pjfanning
Copy link
Contributor

@SamTheisens this change doesn't work as expected with swagger-akka-http. I have updated the sample - https://github.com/pjfanning/swagger-akka-http-sample - to use the swagger-scala-module v2.7.0.
This upgrade breaks the addOption support. The sample has relied on adding a Schema annotation to the Option[Int] param. With and without this annotation, the swagger doc is now being generated without the int typing. I haven't spent much time debugging it but I've tried it a few times and it definitely seems to be broken.

Swagger-akka-http may need to be changed to help the scala-reflect stuff to work. I think the current code passes around Java class instances and this possibly stops the scala-reflect code getting proper access to do its thing.

@SamTheisens
Copy link
Contributor Author

@pjfanning that's too bad. I'll have a look as well.

@pjfanning
Copy link
Contributor

@SamTheisens I fixed the issue with the swagger annotations being ignored in #168 and did a v2.7.1 release. The new erasure code still does not work in swagger-akka-http-sample but at least the sample now works again when the swagger annotations are used.

@SamTheisens
Copy link
Contributor Author

@SamTheisens I fixed the issue with the swagger annotations being ignored in #168 and did a v2.7.1 release. The new erasure code still does not work in swagger-akka-http-sample but at least the sample now works again when the swagger annotations are used.

I've found out that the reflection doesn't work for case classes that are defined nested inside an object. Trying to figure out if and how that can be fixed.

@pjfanning
Copy link
Contributor

I've reproduced the issue with inner classes. It would be great to support inner classes but if necessary, we can document that the reflection support only works with classes that are not inner classes.

On the Scala 3 side of things, I've used this lib before - https://github.com/gzoller/scala-reflection - it might be an option here.

@SamTheisens
Copy link
Contributor Author

I've reproduced the issue with inner classes. It would be great to support inner classes but if necessary, we can document that the reflection support only works with classes that are not inner classes.

On the Scala 3 side of things, I've used this lib before - https://github.com/gzoller/scala-reflection - it might be an option here.

I think I've found a solution for the inner classes #169
It feels hacky, but I guess that's the nature of reflection.

@SamTheisens SamTheisens deleted the optional-primitives-via-reflection branch February 1, 2025 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants