Skip to content
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

Ensure in compile time that the specified serializer is used for every message, event and persistent state #5

Closed
PawelLipski opened this issue Apr 12, 2021 · 4 comments · Fixed by #19
Assignees
Labels
HIGH PRIORITY Should be picked up first soundness Relates to guaranteeing run-time safety at compile-time

Comments

@PawelLipski
Copy link
Collaborator

PawelLipski commented Apr 12, 2021

Since methods like ask/? and tell/! do not accept any implicit codec param, instead resolving the (de)serializer for messages at runtime ://

Thus it's still perfectly possible that even if Borer serializer is present in config, it still doesn't cover all classes that are used as messages/events/state. For some of them, either Java serializer may be used as a fallback, or, if that's disabled, just an exception thrown.

Currently, using ArchUnit, we're checking whether type params of all Behaviors, ReplyEffects etc. present in method return types are extending CborSerializable... but that's pretty awkward. A more proper solution should check that in compile time rather than in tests using reflection :/

@PawelLipski PawelLipski added the soundness Relates to guaranteeing run-time safety at compile-time label Apr 27, 2021
@PawelLipski
Copy link
Collaborator Author

@MarconZet I think you can take this as the next priority, given that we got discouraged from delving into #7 🤔

@PawelLipski
Copy link
Collaborator Author

Not sure how this can be achieved tho... compiler plugin maybe? 🤔

@PawelLipski
Copy link
Collaborator Author

Pls start with runtime-reflection-based solution, similar to what's currently in ArchUnit (but without using ArchUnit as a dependency):

  // The below tests (roughly) check that the classes used as messages/events/state in Akka
  // are always marked as CborSerializable, to ensure that Jackson CBOR and not legacy Java serialization
  // is used for their serialization.
  // For Akka to ensure that condition statically, a major redesign would be necessary -
  // all methods like `ask`, `tell`, `Effect.persist` etc. would need to require an implicit `Codec` (?) parameter.

  // The below tests do NOT ensure that the Jackson serialization of messages/events/state will actually succeed in the runtime.

  "Messages, events and entity state classes" should {
    "implement CborSerializable" in {
      classes
        .should(new ArchCondition[JavaClass]("only use CborSerializable message/event/state types") {
          override def check(clazz: JavaClass, events: ConditionEvents): Unit = {

            clazz.getAllMethods.asScala.foreach { method =>
              def checkType(tpe: Type, category: String, failsWhen: String): Unit = {
                tpe match {
                  case clazz: Class[_] if clazz.getPackageName.startsWith("akka") =>
                  // OK, acceptable

                  case clazz: Class[_] if clazz == classOf[scala.Nothing] =>
                  // OK, acceptable

                  case clazz: Class[_] if !classOf[CborSerializable].isAssignableFrom(clazz) =>
                    val message =
                      s"Type ${clazz.getName} is used as Akka $category (as observed in the return type of method ${method.getFullName}), " +
                      s"but does NOT extend CborSerializable; this will fail in the runtime $failsWhen"
                    events.add(SimpleConditionEvent.violated(clazz, message))

                  case _ =>
                }
              }

              val returnType = method.getRawReturnType
              val genericReturnType = method.reflect.getGenericReturnType

              if (returnType.isEquivalentTo(classOf[akka.persistence.typed.scaladsl.ReplyEffect[_, _]])) {
                genericReturnType match {
                  case parameterizedType: ParameterizedType =>
                    val Array(eventType, stateType) = parameterizedType.getActualTypeArguments
                    checkType(eventType, "event", "when saving to the journal")
                    checkType(stateType, "persistent state", "when doing a snapshot")
                  case _ =>
                }
              } else if (returnType.isEquivalentTo(classOf[akka.projection.eventsourced.EventEnvelope[_]])) {
                genericReturnType match {
                  case parameterizedType: ParameterizedType =>
                    val Array(eventType) = parameterizedType.getActualTypeArguments
                    checkType(eventType, "event", "when saving to the journal")
                  case _ =>
                }
              } else if (returnType.isEquivalentTo(classOf[akka.actor.typed.Behavior[_]])) {
                genericReturnType match {
                  case parameterizedType: ParameterizedType =>
                    val Array(messageType) = parameterizedType.getActualTypeArguments
                    checkType(messageType, "message", "when sending a message outside of the current JVM")
                  case _ =>
                }
              }
            }
          }
        })
        .check(importedProductionClasses)
    }
  }

@PawelLipski
Copy link
Collaborator Author

^ This is very suboptimal since e.g. it can't ever check the type of objects passed to tell/ask though :/

@PawelLipski PawelLipski added the HIGH PRIORITY Should be picked up first label Apr 30, 2021
@PawelLipski PawelLipski changed the title Ensure in compile time that Borer serializer is used for every message, event and persistent state Ensure in compile time that the specified serializer is used for every message, event and persistent state May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HIGH PRIORITY Should be picked up first soundness Relates to guaranteeing run-time safety at compile-time
Projects
None yet
2 participants