microsoft / thrifty

Thrift for Android that saves you methods

Thrifty

Thrifty is an implementation of the Apache Thrift software stack, which uses 1/4 of the method count taken by the Apache Thrift compiler, which makes it especially appealing for use on Android.

Thrift is a widely-used cross-language service-definition software stack, with a nifty interface definition language from which to generate types and RPC implementations. Unfortunately for Android devs, the canonical implementation generates very verbose and method-heavy Java code, in a manner that is not very Proguard-friendly.

Like Square's Wire project for Protocol Buffers, Thrifty does away with getters and setters (and is-setters and set-is-setters) in favor of public final fields. It maintains some core abstractions like Transport and Protocol, but saves on methods by dispensing with Factories, omitting server implementations by default and only generating code for the protocols you actually need.

Thrifty was born in the Outlook for Android codebase; before Thrifty, generated thrift classes consumed 20,000 methods. After Thrifty, the thrift method count dropped to 5,000.

Usage

Add the runtime to your project

In build.gradle:

repositories {
  mavenCentral()

  // For snapshot builds
  maven { url 'https://oss.sonatype.org/content/repositories/snapshots' }
}

dependencies {
  implementation 'com.microsoft.thrifty:thrifty-runtime-jvm:3.1.0'
}

Generate code from your thrift files

On the command line:

java -jar thrifty-compiler.jar --out=path/to/output file_one.thrift file_two.thrift file_n.thrift

Or, with the Gradle plugin:

buildscript {
  dependencies {
    classpath 'com.microsoft.thrifty:thrifty-gradle-plugin:3.1.0'
  }
}

apply plugin: 'com.microsoft.thrifty'

thrifty {
  // Optionally configure things, see thrifty-gradle-plugin/README.md
  // for all the details
}

Building

./gradlew build

Testing

./gradlew check

Contributing

We welcome contributions at all levels. Contributions could be as simple as bug reports and feature suggestions, typo fixes, additional tests, bugfixes, even new features. If you wish to contribute code, please be sure to read our Contributing Guide.

Differences with Apache Thrift

Thrifty structs and clients are 100% compatible with Apache Thrift services.

The major differences are:

• Thrifty structs are immutable.
• Thrifty structs are always valid, once built.
• Fields that are neither required nor optional (i.e., "default") are treated as optional; a struct with an unset default field may still be serialized.
• TupleProtocol is unsupported at present.
• Server-specific features are only supported for Kotlin code generation and currently considered experimental

Guide To Thrifty

Thrift is a language-agnostic remote-procedure-call (RPC) definition toolkit. Services, along with a rich set of structured data, are defined using the Thrift Interface Definition Language (IDL). This IDL is then compiled into one or more target languages (e.g. Java), where it can be used as-is to invoke RPC methods on remote services.

Thrifty is an alternate implementation of Thrift targeted at Android usage. Its benefits over the standard Apache implementation are its greatly reduced method count and its increased type-safety. By generating immutable classes that are validated before construction, consuming code doesn't have to constantly check if required data is set or not.

Interface Definition Language

The Thrift IDL is a simple and standardized way to define data, data structures, and services:

// Let's call this example.thrift

namespace java com.foo.bar

struct Query {
  1: required string text,
  2: optional i64 resultsNewerThan
}

struct SearchResult {
  1: required string url,
  2: required list<string> keywords = [], // A list of keywords related to the result
  3: required i64 lastUpdatedMillis // The time at which the result was last checked, in unix millis
}

service Google {
  list<SearchResult> search(1: Query query)
}

For an authoritative source on Thrift IDL, Thrift: The Missing Guide is an excellent introduction.

Generating Code

Use thrifty-compiler to compile IDL into Java classes:

java -jar thrifty-compiler.jar --kt-file-per-type --out=path/to/output example.thrift

The example file will result in the following files being generated:

path/to/output/
  - com/foo/bar/
    - Google.kt
    - GoogleClient.kt
    - Query.kt
    - SearchResult.kt

The interesting files here are, of course, our domain objects Query and SearchResult.

The latter looks like this:

package com.foo.bar

import com.microsoft.thrifty.Struct
import com.microsoft.thrifty.TType
import com.microsoft.thrifty.ThriftField
import com.microsoft.thrifty.kotlin.Adapter
import com.microsoft.thrifty.protocol.Protocol
import com.microsoft.thrifty.util.ProtocolUtil
import kotlin.Long
import kotlin.String
import kotlin.Unit
import kotlin.jvm.JvmField

public data class SearchResult(
  @JvmField
  @ThriftField(fieldId = 1, isRequired = true)
  public val url: String,

  @JvmField
  @ThriftField(fieldId = 2, isRequired = true)
  public val keywords: List<String>,

  @JvmField
  @ThriftField(fieldId = 3, isRequired = true)
  public val lastUpdatedMillis: Long
) : Struct {
  public override fun write(protocol: Protocol): Unit {
    ADAPTER.write(protocol, this)
  }

  private class SearchResultAdapter : Adapter<SearchResult> {
    // Uninteresting but important serialization code
  }

  public companion object {
    @JvmField
    public val ADAPTER: Adapter<SearchResult> = SearchResultAdapter()
  }
}

The struct itself is immutable and has a minimal number of methods. It can be constructed only with all required fields (all of them, in this example). An Adapter implementation (whose body we omit here because it is long and mechanical) handles reading and writing SearchResult structs to and from Protocols.

Finally and separately, note Google and GoogleClient - the former is an interface, and the latter is an autogenerated implementation.

You may notice the similarity to protobuf classes generated by Wire - this is intentional! The design principles codified there - immutable data, build-time validation, preferring fields over methods, separating data representation from serialization logic - lead to better, safer code, and more breathing room for Android applications.

Using Generated Code

Given the example above, the code to invoke Google.search() might be:

// Transports define how bytes move to and from their destination
val transport = SocketTransport.Builder("thrift.google.com", 80).build().apply { connect() }

// Protocols define the mapping between structs and bytes
val protocol = BinaryProtocol(transport)

// Generated clients do the plumbing
val client = GoogleClient(protocol, object : AsyncClientBase.Listener {
    override fun onTransportClosed() {

    }

    override fun onError(throwable: Throwable) {
        throw AssertionError(throwable)
    }
})

val query = Query(text = "thrift vs protocol buffers")

// RPC clients are asynchronous and callback-based
client.search(query, object : ServiceMethodCallback<List<SearchResult>> {
    override fun onSuccess(response: List<SearchResult>) {
        // yay
    }

    override fun onError(throwable: Throwable) {
        Log.e("GoogleClient", "Search error: $throwable")
    }
})

// ...unless coroutine clients were generated:
val results = async { client.search(query) }.await()

Extensibility

Every project has its own requirements, and no one style of boilerplate can fill them all. Thrifty offers a small but powerful plugin model that you can implement, using the standard Java SPI mechanism, which will allow one to customize each generated class before it is written out to disk. Read more about it in the thrifty-compiler-plugins README. You can see a worked example in thrifty-example-postprocessor.

Hiding PII with Redaction and Obfuscation

Personally-Identifiable Information (PII) is an inevitability in most systems, and often there are legal consequences if it is not handled carefully. Thrifty allows you to avoid logging PII contained in generated classes by supporting both total redaction and obfuscation. It is as simple as adding annotations to your Thrift IDL:

struct User {
  1: required string email (obfuscated)
  2: required string ssn (redacted)
}

The difference between redaction and obfuscation is small but important. In .toString(), redacted fields are totally replaced with the string "<REDACTED>" - no information survives. This meets the goal of not leaking PII, but has the consequence that sometimes debugging can be difficult. obfuscated fields, on the other hand, are treated differently. Their values are hashed, and this hash is printed. This allows one to distinguish between unique values in log files, without compromising user privacy.

The Thrift annotations (thrifty.redacted) and (thrifty.obfuscated) are also accepted by the compiler.

The Thrift example above leads to code similar to the following:

public data class User(
    @JvmField
    @ThriftField(fieldId = 1, required = true)
    @Obfuscated
    public val email: String,

    @JvmField
    @ThriftField(fieldId = 2, required = true)
    @Redacted
    public val ssn: String
) : Struct {
  public override fun toString() =
      "User(email=${ObfuscationUtil.hash(email)}, ssn=<REDACTED>)"

  // more code
}

Obfuscated fields that are collections are not hashed; instead, their type is printed, along with the collection size, e.g. map<i32, string>(size=5).

Close readers will note that the compiler will also respond to @redacted and @obfuscated in field documentation; this is currently valid but not supported and subject to change in future releases. It is a legacy from the time before Thrifty implemented Thrift annotations.

Java Support

Thrifty generates Kotlin code by default, but if needed it can also produce Java. Generated Java code has very slightly more method references than Kotlin. Instead of data classes, Java structs are final classes with public final fields, and are constructable only with dedicated Builder types.

To generate Java classes, pass --lang=java to the compiler (if using the command-line compiler), or provide a java {} block within the thrifty Gradle plugin configuration block.

Language-specific command-line options

Kotlin-specific command-line options

There are a few new command-line options to control Kotlin code generation:

java -jar thrifty-compiler.jar \
    --lang=kotlin \
    --service-type=coroutine \
    --kt-file-per-type \
    --omit-file-comments \
    --kt-struct-builders \
    --experimental-kt-generate-server \
    ...

By default, generated service clients are callback-based:

public interface Google {
  public fun search(query: Query, callback: ServiceMethodCallback<List<SearchResult>>)
}

If, instead, you wish to have a coroutine-based client, specify --service-type=coroutine:

public interface Google {
  public suspend fun search(query: Query): List<SearchResult>
}

Builders are unnecessary, and are not included by default. For compatibility with older code, you can use the --kt-struct-builders flag, which will result in Java-style classes with Builders.

By default, Thrifty generates one Kotlin file per JVM package. For larger thrift files, this can be a little hard on the Kotlin compiler. If you find build times or IDE performance suffering, the --kt-file-per-type flag can help. Outlook Mobile's single, large, Kotlin file took up to one minute just to typecheck, using Kotlin 1.2.51! For these cases, --kt-file-per-type will tell Thrifty to generate one single file per top-level class - just like the Java code.

--experimental-kt-generate-server enabled code generation for the server portion of a thrift service. You can use this to implement a thrift server with the same benefits as the kotlin client: no runtime surprises thanks to structs being always valid by having nullability guarantees and unions represented as sealed classes. See Server Support.

Server Support

Support for generating a Kotlin server implementation was only added very recently, and while it passes the 'official' Java client integration test, you should consider this code experimental.

Thrifty generates a Processor implementation that you pass an input Protocol, an output Protocol and a service handler and the code will take care of reading the request, passing it to the handler and returning the correct response to the output.

If you want to use it, you need to wrap an appropriate communication layer around it, e.g. an HTTP server. You can have a look at the integration tests for a basic example.

Java-specific command-line options

Thrifty can be made to add various kinds of nullability annotations to Java types with the --nullability-annotation-type flag. Valid options are none (the default), android-support, and androidx. Specifying android-support will cause generated code to use @Nullable and @NonNull from the android.support.annotation package. Similarly, specifying androidx will use analogous annotations from androidx.annotation.

Thanks

Thrifty owes an enormous debt to Square and the Wire team; without them, this project would not exist. Thanks! An equal debt is owed to Facebook and Apache for developing and opening Thrift to the world.

---

Copyright © Microsoft Corporation

thrifty's Issues

Broken code generation for services that extends other services

Description

Let's say we define a service Foo:

service Foo {
  void foo()
}

and another service Bar which extends Foo:

include 'Foo.thrift'

service Bar extends Foo {
  void bar()
}

The generated code for the BarClient should provide an implementation of the foo method in order for the generated code to compile but currently only an implementation for bar is generated.

Can't disambiguate between similar-enough imported enum constants

A pathological-but-believable scenario might be:

http_api.thrift

enum StatusCode {
  NO_ERROR = 1,
  OK = 2,
  // etc
}

other_api.thrift

enum StatusCode {
  NO_ERROR = 1,
  SUCCESS = 2,
  // etc
}

client.thrift

struct Response {
  1: required http_api.StatusCode status = StatusCode.NO_ERROR;
}

There is certainly enough context in the IDL to know that the correct StatusCode is http_api.StatusCode, but presently Constant validation doesn't have enough info completely resolve the identifier StatusCode.NO_ERROR. Current behavior has the type resolved non-deterministically - that is, the first matching type to be linked is chosen - and soon we will force a compilation error suggesting that the identifier be fully-qualified.

Apache Thrift gets this right, incidentally.

Clean up thrifty-java-codegen

The exercise of implementing int enums, and realizing just how far-flung were the changes required, led to a realization that the codegen code is not very maintainable. The thing works, but is a jumble of hard-to-understand spaghetti.

General idea: try to encapsulate code generation within a model of the final Java output. That is, map from StructType -> StructClass, exposing TypeSpecs, FieldSpecs, etc. Hypothetically, we may be able to use these to clean up and centralize logic that has just exploded everywhere.

No idea if this is feasible, or if it represents an improvement in maintainability, but we ought to attempt something here.

Simplify ClientBase

Its design is adapted from a duplex-transport architecture, where both ends of a transport are peers that can initiate a message. While this works here, it is inappropriate for a basic RPC construct like that defined by Thrift.

We can get away with fewer resources, eliminate the complicated locking, and likely acheive better performance by replacing Reader and Writer threads with one single worker.

RPC method timeouts

We need them.

Proposed api for generated methods:

Thrift:

service FooService {
  i32 bar(1: string arg)
}

corresponding Java:

public interface FooService {
  int bar(String arg);
  int bar(String arg, long timeout, TimeUnit unit);
}

Timeouts would be communicated via ServiceMethodCallback#onError(Throwable) - likely via a java.util.concurrent.TimeoutException.

The downside being, of course, that the number of service methods doubles. On the other hand, such methods are typically a small portion of the overall number of generated methods, so it may not be so impactful.

Add simple JSON protocol

We want it, if for no other reason than to log messages received.

Exception when generating string constant

After upgrading to 0.3.0 I'm getting this exception when running the compiler:

java.lang.UnsupportedOperationException: cannot unbox java.lang.String
        at com.squareup.javapoet.TypeName.unbox(TypeName.java:160)
        at com.microsoft.thrifty.gen.ThriftyCodeGenerator.buildConst(ThriftyCodeGenerator.java:926)
        at com.microsoft.thrifty.gen.ThriftyCodeGenerator.generateTypes(ThriftyCodeGenerator.java:231)
        at com.microsoft.thrifty.gen.ThriftyCodeGenerator.generate(ThriftyCodeGenerator.java:262)
        at com.microsoft.thrifty.gen.ThriftyCodeGenerator.generate(ThriftyCodeGenerator.java:167)
        at com.microsoft.thrifty.compiler.ThriftyCompiler.compile(ThriftyCompiler.java:217)
        at com.microsoft.thrifty.compiler.ThriftyCompiler.main(ThriftyCompiler.java:96)

This thrift file reproduces the issue

namespace java com.example.test

const string TEST = "test";

Don't depend on reference equality for comparing `ThriftType` instances

Right now during linking we resolve TypeElement to ThriftType by collapsing multiple instances of the former into a single instance of the latter. This means that the annotations on single instances of TypeElement are not carried through to ThriftType.

A workaround for the problem of lost annotations can be seen in #43, but this is not ideal; it is fragile and not resilient to model changes.

A better solution would be to allow ThriftType to carry annotations. The first step is to eliminate the dependency on each unique thrift type having a single canonical ThriftType instance.

i64 consts larger than Integer.MAX_VALUE are emitted as invalid Java

Update JavaPoet to 1.8.0

Help running Unit test

Hi, whenever I attempt to run a unit test (ex.ThriftyCodeGeneratorTest), I get the following error message on my console (see screenshot). I am using Eclipse IDE. Any idea how I can fix this?

Use Optional instead of returning nulls

Re-investigate ANTLR parsing

Now that we've had a production parser for months, let's investigate re-writing it all!

Motivations:

• Manual parsing is manual
• ANTLR would, in theory, do lots of the peekChar-type things for free (i.e., whitespace no longer a fiddly concern)
• With a parser generator, we could turn around bugs much more quickly

Questions:

• With a hand-rolled parser we have ultimate flexibility - case in point, trailing documentation. How might we do that in ANTLR?
• We have a public parser API; can we maintain it with ANTLR?

Update schema, compiler, etc. to Java 8

Time passes, and bits rot. Lots of Guava things we use have standard-library equivalents in JDK 8, and newer versions of that library explicitly deprecate those things.

We should probably git er done before 1.0.0.

Proposal: Make generated structs implement an interface

My use case involves serializing any struct without knowing the specific class. In order to do this, they need to implement a common interface. In this case, I imagine the interface will be something like the following:

public interface Struct<T, B extends StructBuilder<T>> {
    Adapter<T, B> getAdapter();
}

Then a given generated would look like the following:

public final class Foo implements Struct<Foo, Foo.Builder> {
  public static final Adapter<Foo, Builder> ADAPTER = new FooAdapter();

  ...

  @Override
  public Adapter<Foo, Builder> getAdapter() {
    return ADAPTER;
  }

  ...
}

and you'd be able to serialize a generic struct via something like:

<T extends Struct<T, B>, B extends StructBuilder<T>> void serialize(T struct) {
  Buffer buffer = new Buffer();
  Transport transport = new BufferTransport(buffer);
  Protocol protocol = new BinaryProtocol(transport);

  struct.getAdapter().write(protocol, struct)
}

Since generated structs don't currently extend from or implement any interfaces, I don't imagine this would cause any breakage.

I'm happy to submit a PR that accomplishes this, I believe it would be pretty straight forward.

Validate service definitions at link-time

Linker has had a stub for this forever. Currently, Thrifty will happily accept all manner of nonsensical service definitions, like oneway functions that return data. It will be relatively simple to add validation - just a matter of adding validation on Service and ServiceMethod. Validation should verify that:

For services:

• A service either has no base type, or if it does, that base type is also a service

For service methods:

• A service method has a non-void return type and is not marked one-way or it is marked one-way and has a void return type
• All exceptionTypes are, in fact, exceptions; this can be verified by using Linker#lookupSymbol(ThriftType) and checking the resulting StructType.

Most of the other Named subtypes (e.g. StructType, Constant) have validate(Linker) methods that can serve as an example.

Throw informative error for non-null but unrecognized enum values

For example, say we have:

enum Foo { FOO = 1, BAR = 2 }
struct Baz {
  1: required Foo foo = Foo.FOO;
}

Let's say a client has this definition, but the server adds a new enum field QUX. Right now, clients receiving a Baz with the new enum value will crash. This is because the generated Foo.findByValue method returns null, and the adapter will attempt to set the field with that null value.

It seems wrong to attempt to overwrite a default value with null. Conversely, it also seems wrong to silently ignore incomprehensible data.

What to do?

Parser fails when throws keyword is on the next line

The following fails during parsing:

GetStuffResponse getStuff(
  1: required GetStuffRequest request;
) 
throws (
  1: NotAuthorizedException notAuthorized;
  2: NotFoundException notFound;
  3: InvalidRequestException invalidRequest;
)

but this passes:

GetStuffResponse getStuff(
  1: required GetStuffRequest request;
) throws (
  1: NotAuthorizedException notAuthorized;
  2: NotFoundException notFound;
  3: InvalidRequestException invalidRequest;
)

Specifically, it tries to read throws as an identifier rather than recognize it as a keyword continuing from the previous block. Not sure if this is a parser issue or if thrift specs require throws keywords to be on the same line following the previous block.

Implement warnings

We only have hard failures in parsing/codegen at this point, but there are constructs in Thrift that are specified as warnings. For example, adding a namespace for an unrecognized language is specified to still parse, but with a warning:

namespace not.a.language com.foo.bar // This should be ignored and a warning should be shown

This involves:

1. Collecting warnings at parse time
2. Collecting warnings at load/link time
3. Emitting warnings in a sensible way.

Thinking further, a non-ad-hoc approach for emitting errors would be useful here.

Compilation breaks for IDLs with use includes with relative paths

Observed Behaviour

The thrifty compiler fails when a Thrift IDL file declares includes using relative paths. So for example: there is a file Foo.thrift in the foo folder that depends on another file Bar.thrift in the bar folder and it declares the dependency like this: include ../bar/Bar.thrift. When running java -jar thrifty-compiler-0.2.0.jar ./foo/Foo.thrift ./bar/Bar.thrift the compiler will fail with the following exception:

Exception in thread "main" java.lang.AssertionError: All includes should have been resolved by now: /some/absolute/path/foo/../bar/Bar.thrift
    at com.microsoft.thrifty.schema.Loader.getAndCheck(Loader.java:274)
    at com.microsoft.thrifty.schema.Loader.resolveIncludedProgram(Loader.java:232)
    at com.microsoft.thrifty.schema.Program.loadIncludedPrograms(Program.java:232)
    at com.microsoft.thrifty.schema.Loader.loadFromDisk(Loader.java:145)
    at com.microsoft.thrifty.schema.Loader.load(Loader.java:107)
    at com.microsoft.thrifty.compiler.ThriftyCompiler.compile(ThriftyCompiler.java:178)
    at com.microsoft.thrifty.compiler.ThriftyCompiler.main(ThriftyCompiler.java:93)

Cause

The loadFromDisk method in the Loader class stores Programs using their canonical paths but getAndCheck is invoked using the absolute path built by the findFirstExisting method, which creates a file using parent and file paths when resolving paths according to the current location or include paths rules.

Fix

Invoking the getAndCheck using canonical paths instead of absolute ones.

ServiceMethod#returnType() should not return an Optional<ThriftType>

After linking (i.e. before a user of the parser API ever sees an instance of ServiceMethod), the returnType field is definitively set either to ThriftType.VOID or an actual type; it is never null and so we don't actually get any benefit from exposing it as an optional type.

This is a breaking change, but a good one to make.

Emit ints defined in hex as hex literals?

Right now, we unconditionally write out int constants in base 10, so the following transformation happens:

const i32 FOO = 0xF

becomes

public static final int FOO = 16;

Should we preserve the hex-ness of integer constants? It would require threading extra state through from the parser to the code generator - a lot of work, for something that is at best a nice-to-have.

Field default values from constants don't work

Example:

namespace java test;
const bool YES = true
struct Test {
  1: required bool isYes = YES;
}

The default value for the generated field should be test.Constants.YES, but is false.

Nested empty generic collection default values in Builders are broken

Given:

struct Foo {
  1: list<list<i32>> ints = [[]]
}

Thrifty generates:

class Builder implements StructBuilder<Foo> {
  // code code code

  public Builder() {
    this.ints = new ArrayList<List<List<Integer>>>();
    List<List<Integer>> list = new ArrayList<List<Integer>>();
    list.add(Collections.emptyList());  // BROKEN
    this.ints.add(list);
  }
}

This won't compile; the call to Collections.emptyList() needs type parameters.

Detect circular inheritence chain during Service validation

It's hard to imagine anyone actually trying this, but it wouldn't hurt to have a check for this.

Update Guava to 21.0

Prepare for Java 9

What, if anything, does Jigsaw mean for us? We should figure that out sometime before July.

Add Parcelable generation

We should support a compiler flag which, when set, causes generated structs to implement android.os.Parcelable

Use java.nio.file.Path instead of strings in thrifty-schema

Some of the File/String things we do are kind of shaky, and would be better accomplished with the newer Path API.

Thrift Server Support

Nice work overall on the project! One thing I noticed missing in the library is support for thrift server, similar to TSimpleServer in the apache library. Are there plans to add those or are you assuming most thrift servers will be hosted on actual server and not on the Android client.

A specific use-case for this is when communicating with hardware from the Android device. The Android device may want to act as the server to retrieve data

i64 consts larger than Integer.MAX_VALUE fail to parse

Proposal: support alternate compiler/codegen/servicebuilder setups

The project's actually well-suited for this, all I had to do was make them and put them in the same package space where the normal ones would go. This would allow developers to take advantage of the wonderful parsing support you all have already written, while being able to generate their own models/services that fit their needs.

Rules around comment parsing are unclear

The following struct fails to parse:

struct SomeRequest {
    /** Here's a comment. */
    1: required UUID clientUuid

    /**
     * Here's a longer comment.
     * One two lines.
     */
    2: optional string someOtherField

    /**
     * Here's another longer comment.
     * Also on two lines.
     */
    3: optional UUID someField

    /** Here's another single line that works fine. */
    4: optional UUID adminUuid
}

This fails with the following error:

trailing comment must be closed on the same line

Could you explain where the issue is in these comments? It seems like the formatting is fine, and the error message isn't really clear since I see several tests with multiline comments like this.

Add tests for serializing collections of enums

#73, on top of everything else, fixed a codegen issue wherein collections of enums were serialized incorrectly - single enums correctly got the typecode TType.I32, but list, set, and map metadata had TType.ENUM, which despite its name is incorrect.

This was a subtle and long-lived bug because not all protocol implementations have a problem with this - we noticed only because of Python's TBinaryProtocolAccelerated, a C module that does actually enforce correct typecodes.

We need tests to ensure that this doesn't regress.

A good approach would be to write a test protocol that records which Protocol methods are invoked and in what order, and uses it to test against code that is generated e.g. in thrifty-integration-tests.

Enum validation fails when enum is defined in separate thrift file

We noticed an error like this when running code gen:

E: 'One' is not a member of enum type Foo: members=[One]

with testing_foo.thrift defined as:

namespace java testing 

enum Foo {
  One = 1
}

and testing_bar.thrift defined as:

namespace java testing

include 'testing_foo.thrift'

struct Bar {
  1: required testing_foo.Foo foo = Foo.One;
}

(This happens with required testing_foo.Foo foo = Foo.One and required testing_foo.Foo foo = One)

For reference required testing_foo.Foo foo = 1 seems to work fine.

It seems like the linking happens correctly, just the validation fails.

Suppress number-equality warnings in generated `equals` methods

We use the general form this.x == that.x || (this.x != null && this.x.equals(that.x)), which Android Studio dislikes. We should add @SupressWarnings("NumberEquality") to generated equals methods that use boxed Number types.

How do I import and build this in Eclipse?

I am a beginner to the open source community. I want to know how to import this project into Eclipse and build it. Thank you:).

Show "null" for unset @redacted fields

When marking a field as redacted, that field is always shown as hidden when the structure is printed. It would be useful if "null" was printed for cases where the value is unset. This allows you to at least differentiate between cases where the field was set or not when logging structures.

Make integration tests non-janky

Currently, integration tests are unreliable on Travis CI.

The integration suite will, for each run, open a (local) server socket and a Thrifty client will connect and make RPC calls. The client will time out after 2 seconds, which happens on Travis for reasons unknown. The server doesn't do any real work, so it's likelier to be a timing issue (good) or something intrinsic to the CI environment (bad).

If the former, then it's just a matter of fixing the race. If the latter, then we need a different approach to integration testing.

[QUESTION] Use enumerated annotations instead of enums

Given how inefficient enums are on Android, would it be possible to leverage enumerated annotations instead of generating plain Java enums?

Merge with Apache Thrift

I am deeply concerned about the road that Thrift took during the past years:

• Facebook donated Thrift to the Apache Foundation
• Facebook forked its own incompatible branch for Linux only, adding new features like float support
• Microsoft optimized the Apache Thrift JAVA code

What I fear is that we end up with 4 or 5 different versions of one of the best IPC libraries of the world and that Thrift wil simply die, die, die due to this.

Could you please explain what hinders you to merge Thrifty back into Apache Thrift ? Is the Apache developer team not responsive enough? Are there technical issues? Are there incompatibilities?

Add decorating protocol

It would be great if Thrifty could provide something to decorate a Protocol. My usecase is using a multiplexed protocol like TMultiplexedProtocol.

An alternative to adding the implementation below would be to add an accessor method to the transport inside the decorated protocol (so that something equivalent to super(protocol.transport) could be achieved in other packages).

package com.microsoft.thrifty.protocol;

import okio.ByteString;
import java.io.IOException;

public abstract class DecoratingProtocol extends Protocol {

    private final Protocol concreteProtocol;

    public DecoratingProtocol(Protocol protocol) {
        super(protocol.transport);
        concreteProtocol = protocol;
    }

    @Override
    public void writeMessageBegin(String name, byte typeId, int seqId) throws IOException {
        concreteProtocol.writeMessageBegin(name, typeId, seqId);
    }

    @Override
    public void writeMessageEnd() throws IOException {
        concreteProtocol.writeMessageEnd();
    }

    @Override
    public void writeStructBegin(String structName) throws IOException {
        concreteProtocol.writeStructBegin(structName);
    }

    @Override
    public void writeStructEnd() throws IOException {
        concreteProtocol.writeStructEnd();
    }

    @Override
    public void writeFieldBegin(String fieldName, int fieldId, byte typeId) throws IOException {
        concreteProtocol.writeFieldBegin(fieldName, fieldId, typeId);
    }

    @Override
    public void writeFieldEnd() throws IOException {
        concreteProtocol.writeFieldEnd();
    }

    @Override
    public void writeFieldStop() throws IOException {
        concreteProtocol.writeFieldStop();
    }

    @Override
    public void writeMapBegin(byte keyTypeId, byte valueTypeId, int mapSize) throws IOException {
        concreteProtocol.writeMapBegin(keyTypeId, valueTypeId, mapSize);
    }

    @Override
    public void writeMapEnd() throws IOException {
        concreteProtocol.writeMapEnd();
    }

    @Override
    public void writeListBegin(byte elementTypeId, int listSize) throws IOException {
        concreteProtocol.writeListBegin(elementTypeId, listSize);
    }

    @Override
    public void writeListEnd() throws IOException {
        concreteProtocol.writeListEnd();
    }

    @Override
    public void writeSetBegin(byte elementTypeId, int setSize) throws IOException {
        concreteProtocol.writeSetBegin(elementTypeId, setSize);
    }

    @Override
    public void writeSetEnd() throws IOException {
        concreteProtocol.writeSetEnd();
    }

    @Override
    public void writeBool(boolean b) throws IOException {
        concreteProtocol.writeBool(b);
    }

    @Override
    public void writeByte(byte b) throws IOException {
        concreteProtocol.writeByte(b);
    }

    @Override
    public void writeI16(short i16) throws IOException {
        concreteProtocol.writeI16(i16);
    }

    @Override
    public void writeI32(int i32) throws IOException {
        concreteProtocol.writeI32(i32);
    }

    @Override
    public void writeI64(long i64) throws IOException {
        concreteProtocol.writeI64(i64);
    }

    @Override
    public void writeDouble(double dub) throws IOException {
        concreteProtocol.writeDouble(dub);
    }

    @Override
    public void writeString(String str) throws IOException {
        concreteProtocol.writeString(str);
    }

    @Override
    public void writeBinary(ByteString buf) throws IOException {
        concreteProtocol.writeBinary(buf);
    }

    @Override
    public MessageMetadata readMessageBegin() throws IOException {
        return concreteProtocol.readMessageBegin();
    }

    @Override
    public void readMessageEnd() throws IOException {
        concreteProtocol.readMessageEnd();
    }

    @Override
    public StructMetadata readStructBegin() throws IOException {
        return concreteProtocol.readStructBegin();
    }

    @Override
    public void readStructEnd() throws IOException {
        concreteProtocol.readStructEnd();
    }

    @Override
    public FieldMetadata readFieldBegin() throws IOException {
        return concreteProtocol.readFieldBegin();
    }

    @Override
    public void readFieldEnd() throws IOException {
        concreteProtocol.readFieldEnd();
    }

    @Override
    public MapMetadata readMapBegin() throws IOException {
        return concreteProtocol.readMapBegin();
    }

    @Override
    public void readMapEnd() throws IOException {
        concreteProtocol.readMapEnd();
    }

    @Override
    public ListMetadata readListBegin() throws IOException {
        return concreteProtocol.readListBegin();
    }

    @Override
    public void readListEnd() throws IOException {
        concreteProtocol.readListEnd();
    }

    @Override
    public SetMetadata readSetBegin() throws IOException {
        return concreteProtocol.readSetBegin();
    }

    @Override
    public void readSetEnd() throws IOException {
        concreteProtocol.readSetEnd();
    }

    @Override
    public boolean readBool() throws IOException {
        return concreteProtocol.readBool();
    }

    @Override
    public byte readByte() throws IOException {
        return concreteProtocol.readByte();
    }

    @Override
    public short readI16() throws IOException {
        return concreteProtocol.readI16();
    }

    @Override
    public int readI32() throws IOException {
        return concreteProtocol.readI32();
    }

    @Override
    public long readI64() throws IOException {
        return concreteProtocol.readI64();
    }

    @Override
    public double readDouble() throws IOException {
        return concreteProtocol.readDouble();
    }

    @Override
    public String readString() throws IOException {
        return concreteProtocol.readString();
    }

    @Override
    public ByteString readBinary() throws IOException {
        return concreteProtocol.readBinary();
    }

    @Override
    public void flush() throws IOException {
        concreteProtocol.flush();
    }

    @Override
    public void reset() {
        concreteProtocol.reset();
    }

    @Override
    public void close() throws IOException {
        concreteProtocol.close();
    }
}

Upgrade Gradle to 3.3

Apply user-specified `FieldNamingPolicy` to RPC method parameters

We currently allow users to customize the generated names of fields with FieldNamingPolicy. It's already available in Program, where we convert the raw parse tree data, we just aren't yet threading it through to Service and ServiceMethod.

We should just add constructor parameters to those classes and pass the user-provided policy through.

Provide access to struct/union/enum/exception namespaces from ThriftType

Investigate Okio upgrade

Some new additions to the Okio API (like Pipe) present in newer versions offer interesting possibilities. Can we take advantage without breaking consumers? Are there important fixes or improvements to be had?

typedef annotations are not parsed in schema if the annotation precedes the newName

The following typedef reports no annotations when I try to parse it. This appears to be valid thrift ordering in our other language parsers.

typedef i64 (js.type = 'Date') Date

Referring to included constants is wonky

a.thrift:

namespace java com.foo.bar

const i32 foo = 21

b.thrift:

namespace java com.foo.bar

include 'a.thrift'

const i32 bar = a.foo

Loading this fails with Unrecognized identifier: foo

Unify Named with ThriftType

As time passes, the distinction seems less and less useful.

ThriftType.UserType should subsume Named, and ThriftType in general should become enriched. This would fix all the fiddly Constant validation bugs and simplify lots of requested use cases from @hzsweers and friends.

Investigate Google's compile-testing for thrifty-java-codegen

Google's truth testing library is a solid 7/10, not a huge win over Hamcrest. The reason to consider it now, though, is their compile-testing companion library that can assert on generated Java code. It would be interesting to see about using it.