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

Serialization not working with generic type parameter in events #43

Open
toggm opened this issue Feb 18, 2016 · 7 comments
Open

Serialization not working with generic type parameter in events #43

toggm opened this issue Feb 18, 2016 · 7 comments

Comments

@toggm
Copy link

toggm commented Feb 18, 2016

When you declare a generic event as an envelop containing some payload declared by a generic type parameter, the canPersist method of the Persister returns true for all Events even if the payload differs.
i.e.

case class Event[P](payload:P)
case class Payload1(msg:String)
case class Payload2(value:Int)

implicit val payload1Format = jsonFormat1(Payload1)
implicit val payload2Format = jsonFormat1(Payload2)

implicit val eventPayload1Format = jsonFormat(Event[Payload1])
implicit val eventPayload2Format = jsonFormat(Event[Payload2])

payload1Persister = persister[Event[Payload1]]("payload1")
payload2Persister = persister[Event[Payload2]]("payload2")

At compile time everything seams to be ok but at runtime the vm will raise a ClassCastException when you try to persist an event with payload2. This due to the fact that the Persister itself makes a type check of the Root element only:

private[stamina] def convertToT(any: AnyRef): Option[T] = any match {
    case t: T ⇒ Some(t)
    case _    ⇒ None
  }
@raboof
Copy link
Member

raboof commented Feb 19, 2016

Thanks for your report! This is indeed a situation we haven't sufficiently considered.

The general approach here is probably to have one Persister[Event[_]] (which somehow may be composed), but how exactly we'll fit this in and safely include it in the DSL will need some further thought.

Good catch!

@raboof raboof self-assigned this Feb 19, 2016
raboof added a commit to raboof/stamina that referenced this issue Feb 19, 2016
raboof added a commit to raboof/stamina that referenced this issue Feb 19, 2016
This makes sure the situation described in scalapenos#43 is at least detected at
initialization, instead of blowing up when an event is actually persisted.

In the future we might want to actually support having multiple persisters for
types with the same root class, but that will require some more changes.
@agemooij
Copy link
Contributor

Hi @toggm I've been looking into this in the hope of finding a usable solution but my Scala type system skills are no match for this problem I'm afraid.

The problem is that the Akka Serializer trait demands us to take a raw AnyRef and persist it. In order to find the right persister for this raw value, we will have to match it up using type information. That's where that convertToT method comes in, which depends on the available ClassTag to be able to match against an AnyRef.

I tried using TypeTag instead of ClassTag but the APIs are completely different and I could not get that to compile in any useful way. Pattern matching doesn't seem to work at all in that context.

In other words, I'm stumped as to how to make this possible 😢
Do you perhaps have any ideas yourself?

@toggm
Copy link
Author

toggm commented Feb 21, 2016

I think there is no way getting around type erasure if you only get anyref from the Akka Serializer trait. The best way would be to be able to create somehow smart composed persister objects. Best way would be to have some compile time binding through implicits.
Thats the same for the current implementation. There is currently no compile time guarantee that all persistet classes have a matching persister available in the context. Binding them somehow to implicits could solve this problem.

@toggm
Copy link
Author

toggm commented Feb 21, 2016

I had some further thoughts about this problem.
Maybe adding another marker interface (i.e.TypeTagged) would help to deal with this situation.
I've made a small example demonstrating have you could distinguish those persisters based on nested type tag. Sure, this would users force to mark their nested messages accordingly.

import scala.reflect.runtime.universe._

object TestTypeTag extends App {
        trait TypeTagged[X] {
            val typeTag: TypeTag[X]
        }

        class Persister[T: TypeTag] {

            val tt = typeOf[T]

            def matches(any:AnyRef) = any match  {
                    case x if classOf[TypeTagged[_]].isAssignableFrom(x.getClass) =>  {
                        val typeTag = x.asInstanceOf[TypeTagged[_]].typeTag                     
                        typeTag.tpe match {
                            case `tt` =>
                                Some(x)
                            case _ =>
                                None
                        }
                    }
                    case _ => None
                }
        }


        class Event[E](implicit val typeTag:TypeTag[Event[E]]) extends TypeTagged[Event[E]]
        case class Payload1(txt:String)
        case class Payload2(value:Int)

        val persister1 = new Persister[Event[Payload1]]
        val persister2 = new Persister[Event[Payload2]]

        println(persister1.tt)
        println(persister2.tt)

        val obj1 = (new Event[Payload1]).asInstanceOf[AnyRef]
        val obj2 = (new Event[Payload2]).asInstanceOf[AnyRef]


        println(persister1.matches(obj1))
        println(persister1.matches(obj2))

        println(persister2.matches(obj1))
        println(persister2.matches(obj2))
}

@toggm
Copy link
Author

toggm commented Feb 22, 2016

Or even shorter...

class Persister[T: TypeTag] {

            val tt = typeOf[T]

            def matches(any:AnyRef) = any match  {
              case x:TypeTagged[_] => 
                        val typeTag = x.typeTag                     
                        typeTag.tpe match {
                            case `tt` =>
                                Some(x)
                            case _ =>
                                None
                        }                   
                    case _ => None
                }
        }

@agemooij
Copy link
Contributor

Thanks! We're looking into this but please feel free to turn this into a pull request if you have the time for that.

The TypeTagged marker trait would have to be optional for cases such as yours so we don't force users to tag their domain unless this is the only way to get their events properly persisted. For "normal cases" the current ClassTag approach would probably still be required.

toggm added a commit to toggm/stamina that referenced this issue Feb 24, 2016
@toggm
Copy link
Author

toggm commented Feb 24, 2016

created pull request #46

@raboof raboof removed their assignment Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants