Skip to content

[break] alias of binary return types is the same as binary return types#50

Merged
iamdanfox merged 4 commits into
developfrom
qchen/AliasedBinary
Jul 26, 2018
Merged

[break] alias of binary return types is the same as binary return types#50
iamdanfox merged 4 commits into
developfrom
qchen/AliasedBinary

Conversation

@qinfchen
Copy link
Copy Markdown
Contributor

@qinfchen qinfchen commented Jul 25, 2018

Before this PR

alias of binary return type was treated differently as binary return type

After this PR

alias of binary return type is now treated the same as binary return type

@qinfchen qinfchen requested a review from iamdanfox July 25, 2018 16:54
@iamdanfox
Copy link
Copy Markdown
Contributor

Cool, looks like this achieves the short-term goal of enabling client and server-side streaming when a conjure endpoint returns an alias of binary! Kinda sad that we lose the alias name though - for a longer term solution, could we set up some kind of MessageBodyWriter somehow?

return delegate.visitSet(type);
}

private TypeName resolveReferenceType(com.palantir.conjure.spec.TypeName type) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we dedupe this with the jersey version?

Qinfeng Chen added 2 commits July 26, 2018 11:09
io.dropwizard:dropwizard-* = 1.3.1
@qinfchen
Copy link
Copy Markdown
Contributor Author

qinfchen commented Jul 26, 2018

@iamdanfox, what does the alias name of return binary give us? Wouldn't that just introduce some ambiguity around what the return type of the generated interfaces should behave, both the client and the server side? In the end, we want alias of binary return type to be always considered as StreamingOutput, so is it desirable to expose alias name that wraps around ByteBuffer to the end users and then have a MessageBodyWriter to handle the response as StreamingOutput?

@qinfchen qinfchen changed the title alias of binary return types is the same as binary return types [break] alias of binary return types is the same as binary return types Jul 26, 2018
public static TypeName resolveReturnReferenceType(
Map<com.palantir.conjure.spec.TypeName, TypeDefinition> types,
com.palantir.conjure.spec.TypeName type,
Supplier<TypeName> binaryTypeSupplier) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this just be a plain TypeName instead of a supplier?

@iamdanfox iamdanfox merged commit ad8af33 into develop Jul 26, 2018
@iamdanfox iamdanfox deleted the qchen/AliasedBinary branch July 26, 2018 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants