-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@command and @query annotations should be provided by tapiro #220
Comments
I agree in principle, although it's a bit strange to have a library dependency for two lines of code. @calippo what do you think? |
Another idea we can explore is not to require annotations, but use comments instead, as in trait MyController[F[_], AuthToken] {
// tapiro:query
def myGet(): F[Error, String]
// tapiro:command
def myPost(): F[Error, String]
} |
Need to think more about this. I don't love comments. I think we can write a small library for this, it might include also a tapiro auth token with deserialization import sttp.tapir._
import sttp.tapir.Codec._
case class CustomAuth(token: String)
def decodeAuth(s: String): DecodeResult[CustomAuth] = {
val TokenPattern = "Token token=(.+)".r
s match {
case TokenPattern(token) => DecodeResult.Value(CustomAuth(token))
case _ => DecodeResult.Error(s, new Exception("token not found"))
}
}
def encodeAuth(auth: CustomAuth): String = auth.token
implicit val authCodec: PlainCodec[CustomAuth] = Codec.stringPlainCodecUtf8
.mapDecode(decodeAuth)(encodeAuth) |
Partially related: should tapiro define an |
I don't think there's a way to completely decouple the authentication from an HTTP library. I think it's fine to embrace it to a certain extent. To me it is ok having a lightweight "token handling module" in tapiro. |
Asking users to implement them via
means they have to define them in random files of their package. They can't put the definitions at the top of each
trait Controller
because they'll clash as soon as the package has >= 2 controllers.Why not defining them in a library such that users can import them as many times they need?
The text was updated successfully, but these errors were encountered: