-
Notifications
You must be signed in to change notification settings - Fork 20
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
Framework-less, generalized, annotation-based RPC #57
Conversation
def some(value: Value): O | ||
def fold[B](opt: O, ifEmpty: => B)(f: Value => B): B | ||
} | ||
trait BaseOptionLike[O, A] extends OptionLike[O] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need both of these traits? If yes, could you put a comment why?
Besides that shouldn't BaseOptionLike
be sealed too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the base trait to do implicit search in macros. This way I avoid using wildcards and the implicit search is simpler. At the same time, I want to avoid the base trait to be extended directly because someone might forget about setting the type member to concrete type. For this reason, OptionLike
is sealed while BaseOptionLike
is not.
def materializeForRpc[M[_], T]: M[T] = macro macros.rpc.RPCMacros.rpcMetadata[M[T], T] | ||
} | ||
|
||
trait TypedMetadata[T] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you describe the role of this trait?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it needs docs.
In short: metadata class for method or parameter must extend TypedMetadata[X]
where X
is either the result type of a real method or the type of real parameter. For example:
class FunctionMetadata[T](...) extends TypedMetadata[Future[T]]
and then:
class RpcMetadata[T](
functions: Map[String, FunctionMetadata[_]]
...
)
The macro will then match Future[_]
against each function actual result type and determine what to substitute for the wildcard.
case class GET() extends RestMethod | ||
case class PUT() extends RestMethod | ||
|
||
@methodTag[RestMethod, RestMethod] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@methodTag[RestMethod, GET/POST/PUT]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RestMethod
(the base trait) as the default means pretty much "no default". This is a test, anyway.
implicit def futureAsRealRawFromGenCodec[T: GenCodec]: AsRealRaw[Future[String], Future[T]] = ??? | ||
} | ||
|
||
@methodTag[RestMethod, RestMethod] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@methodTag[RestMethod, GET/POST/PUT]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
* RPC interfaces may also have non-abstract members - these will be invoked locally. However, they may invoke | ||
* remote members in their implementations. | ||
*/ | ||
class RPC extends StaticAnnotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we deprecate it first and remove in the next release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, do you have a subclass of RPC
in Udash? Can we leave that one instead and deprecate it?
/** | ||
* You can use this annotation on overloaded RPC methods to give them unique identifiers for RPC serialization. | ||
*/ | ||
class RPCName(val name: String) extends StaticAnnotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we deprecate it first and remove in the next release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually renamed it to rpcName
to retain some naming convention. To be honest, I'd like to have a single annotation for both GenCodec
(which currently uses @name
) and RPC. So I will probably introduce some deprecated type aliases (although I'm not sure if they work with annotations).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to get deprecation warning (with migration instructions) instead of class not found error. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made an alias in Udash PR
import scala.annotation.implicitNotFound | ||
|
||
@implicitNotFound("don't know how to encode ${T} as ${R}, appropriate AsRaw instance not found") | ||
trait AsRaw[R, T] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you replace R
and T
with more meaningful names? Maybe Raw
and Real
?
} | ||
|
||
@implicitNotFound("don't know how to encode and decode between ${T} and ${R}, appropriate AsRealRaw instance not found") | ||
trait AsRealRaw[R, T] extends AsReal[R, T] with AsRaw[R, T] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this name is misleading: AsRealRaw[Raw, Real]
. In my opinion we should keep Raw
s and Real
s in the same order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you compare RpcFramework performance before and after these changes? Would be a nice sanity check.
@@ -4,7 +4,7 @@ package rpc.akka.client | |||
import akka.actor.ActorSystem | |||
import akka.pattern.ask | |||
import akka.util.Timeout | |||
import com.avsystem.commons.rpc.akka.AkkaRPCFramework.RawRPC | |||
import com.avsystem.commons.rpc.akka.AkkaRPCFramework.{RawInvocation => _, _} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import com.avsystem.commons.rpc.akka.AkkaRPCFramework.{RawRPC, RawValue}
would suffice
* | ||
* In order to specify aggregated annotations, the class that extends `AnnotationAggregate` must | ||
* redefine the `Implied` dummy type member and apply the aggregated annotations on it. Macro engines | ||
* used in `GenCodec` materialization and RPC framework will automatically pick up these annotations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we start with the fact that other macros won't, or move this to a more GenCodec-specific package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Annotation processing is a part of MacroCommons
and it should work everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-AVSystem macros.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specifically listed GenCodec
macros and RPC macros - what's the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must have misread something 😢
* | ||
* case class SomeMongoEntity(@mongoId id: String, data: String) | ||
* }}} | ||
* | ||
* In the above example, applying `@mongoId` annotation on the `id` field has the same effect as if | ||
* annotations `@name("_id") @outOfOrder` were applied directly on that field. | ||
* | ||
* NOTE: thanks to the fact that aggregated annotations are applied on a type member | ||
* you can pass the arguments of original annotation to aggregated annotations, e.g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need to know why when I ctrl+q for a quick overview? The type member note should be a comment on Implied
(and it is).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IntelliJ cannot into Scaladoc. Among other problems, it removes annotations from code snippets, like in this case. I'm not going to work around it. Just read the sources or API reference.
trait AnnotationAggregate extends StaticAnnotation { | ||
/** | ||
* Dummy type member meant to be redefined in order to have aggregated annotations applied on it. | ||
* These annotations will be automatically picked up by macro engines each time they encounter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macro engines or our macros?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not all macro engines in the world.
* (see [[paramTag]] for more information on param tagging). | ||
* | ||
* Raw parameters marked as [[optional]] must be typed as `Option[T]` (or `Opt[T]`, `OptArg[T]` or whatever type that | ||
* has an instance of `OptionLike`). By default, [[optional]] raw parameters are [[verbatim]] which means that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note where that instance should be provided (imported) when using a custom one (obviously T
companion object works, but sometimes you don't want to clutter it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be obvious to everyone who knows how implicits work in Scala 😎
/** | ||
* @author ghik | ||
*/ | ||
object RpcUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are only used in macro-generated code - maybe we could move this to the macro module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macro module is supposed to contain macro implementations only so that it can be a compile-time only dependency if necessary.
* Information about real parameter flags and modifiers as defined in Scala code. | ||
*/ | ||
@transparent | ||
final case class ParamFlags(rawFlags: Int) extends AnyVal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enumeration
+ ValueSet
would be more object friendly and less off-by-one error-prone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you just suggest using scala.Enumeration
to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather Enumeration
than C-style bit-fiddling. ValueSet is actually quite a nice wrapper around BitSet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refuse to use Enumeration
which uses reflection and is widely disgusted. I'm going to introduce some safety measures to avoid off-by-one errors in refactorings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conscience clause of Scala developers
|
||
import scala.reflect.macros.blackbox | ||
|
||
abstract class RPCMacroCommons(ctx: blackbox.Context) extends AbstractMacroCommons(ctx) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rpc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wiedziałem
import scala.collection.mutable | ||
import scala.collection.mutable.ListBuffer | ||
|
||
trait RPCMappings { this: RPCMacroCommons with RPCSymbols => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rpc
|
||
import scala.annotation.StaticAnnotation | ||
|
||
trait RPCMetadatas { this: RPCMacroCommons with RPCSymbols with RPCMappings => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rpc
# Conflicts: # version.sbt
Generalized RPC/proxy/API manipulation framework.
Example code using stuff described below: NewRawRpc
AsRaw
andAsReal
RpcFramework
is no longer required. For backwards compatibility it's kept as a special case of the new framework.AsReal
andAsRaw
(and a bidirectional conversion,AsRealRaw
) have been introduced. All real-raw conversions are realized through these typeclasses. This includes both RPC trait conversion and parameter and return value serialization and deserialization.AsReal
andAsRaw
for real and raw RPC traits. Implicit macros have been removed - all macro materialized instances must be explicitly declared in companion objects of real RPC traits.AsReal
andAsRaw
for RPC traits do it solely based on how raw RPC trait looks like. In order to guide and customize the materialization, raw RPC, it's methods and parameters may be annotated with various meta annotations, e.g.@multi
,@verbatim
,@tagged
.@multi
, it's supposed to handle multiple real methods. This is the most common situation. Whether a real method matches a raw method is determined solely by their signatures.@multi
then its name must be the same as real method's name (with respect to@rpcName
). If raw method is annotated as@multi
then real method's name doesn't matter. Raw method must take that name as the only parameter in its first parameter list.@verbatim
then return types must be the same. If it's@encoded
then there must be an appropriateAsRaw
orAsReal
conversion between raw and real method's return type. The macro performs an ad-hoc implicit search for that conversion and deems the methods non-matching when it cannot be found. Non-@multi
raw methods are@verbatim
by default while@multi
raw methods are@encoded
by default.@multi
may match multiple real parameters (spanning multiple parameter lists). Its type must then be a collection: either some subclass ofIterable[Raw]
orPartialFunction[String,Raw]
whereRaw
is a type that will be matched against each real parameter type. By default,@multi
raw parameters are@encoded
which means that the macro will serialize and deserialize each real parameter value toRaw
value usingAsRaw
andAsReal
instances for these types. However - contrary to how method return types are treated - if one of these instance is not available, it will not affect macro's decision about whether raw and real method match. Implicit search errors for parameters are deferred until the very end, after whole macro has been expanded.@GET
,@POST
,@PUT
, etc.The design above removes several limitations:
RawValue
with single serialization strategy for all parameters and return types - each raw method and parameter may use its own type for encoded value with dedicated typeclasses for serializationList[List[RawValue]]
.New RPC metadata
RPC metadata materialization has been reworked in largely similar way as raw-real RPC translation.
RPCMetadata
trait. RPC metadata may now be any class. Macro engine reflects on the constructor of that class and based on its parameters determines how it should be materialized. Metadata constructor parameters are annotated in the same way as raw RPC methods and parameters are annotated (i.e. with@multi
,@verbatim
,@encoded
, etc.). This way metadata class mirrors the structure of raw RPC trait. Every raw method is reflected by a constructor parameter in metadata class. That parameter, in turn, holds another metadata class whose constructor parameters mirror raw parameters of that method.@infer
- makes the macro engine search for implicit value of parameter's type. Metadata parameters may simply be declared asimplicit
by themselves and will also be inferred by macro, but with explicit@infer
annotation you can make non-implicit parameters effectively implicit.@reifyAnnot
- makes the macro engine reify some annotation(s) from real RPC trait/method/param. By default, macro expects the type of metadata parameter to be a subtype ofStaticAnnotation
. Metadata parameter with@reifyAnnot
may also be specified as@optional
or@multi
to reify optional or repeated annotations.@hasAnnot[A]
- makes the macro engine reify aBoolean
flag telling if given real RPC trait/method/param is annotated asA
.@reifyName
- makes the macro engine reify aString
containing RPC trait/method/param name (either with or without respect to@rpcName
)@reifyPosition
- makes the macro engine reify an instance ofParamPosition
which contains detailed information about real parameter's index, index of its parameter list, etc.@reifyFlags
- makes the macro engine reify an instance ofParamFlags
which contains a set of flags telling if a real parameter is declared as implicit, by-name, repeated (varargs), has default value or is synthetic (e.g. comes from a context bound).TypedMetadata[T]
whereT
must will be matched against real method result type or real parameter type. This way matching of real methods/params to metadata params can mirror matching of real methods/params to raw methods/params.