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

Feature Suggestion: Custom type adapters #342

Open
tnn opened this issue Jan 21, 2021 · 1 comment
Open

Feature Suggestion: Custom type adapters #342

tnn opened this issue Jan 21, 2021 · 1 comment

Comments

@tnn
Copy link

tnn commented Jan 21, 2021

When using the service defintion of the generated code, there is a mismatch between the high level types and the underlying thrift binary serialization strategy. The canonical example for Scala/Java is UUID or ZonedDateTime, leading to implementors either polluting their service traits with Int and Long's or using Strings when the customer of those APIs often would like to work in higher level types such as UUID, ZonedDateTime - or their own custom such as UserID.

There is multiple ways this could be implemented, however there seems to be an chicken-and-egg problem by the thrift definitions not yet being compiled and generated yet, when loading any custom adapter, therefore the trait for an adapter likely have to be at a lower level.

struct UUID {
    1: i64 mostSigBits;
    2: i64 leastSigBits;
} (com.twitter.scrooge.scala.customadapter = "com.example.thrift.UUIDAdapter")

struct UserID {
    1: UUID id;
} (com.twitter.scrooge.scala.customadapter = "com.example.thrift.UserIDAdapter")
class UserIDAdapter extends CustomAdapter[UserID] {
  override def fromThrift(struct: MagicMetaThrift): UserID {
    UserID.from(UUID(struct._1, struct._2))
  }
  override def toThrift(userID: UserID): MagicMetaThrift) {
    // magic serialization
  }
} 

Please let me know if this is completely off the rails or if something like this is already possible today. I would also like to reference the Java / XML solution, XmlAdapter

@bonniee
Copy link
Contributor

bonniee commented Jan 26, 2021

Hi @tnn ,

Thanks for this suggestion. If you open a pull request, we'd be happy to review a patch for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants