Skip to content

[fix] Add Produces annotation for binary alias return types#131

Merged
bulldozer-bot[bot] merged 6 commits into
palantir:developfrom
pkoenig10:streaming
Jan 9, 2019
Merged

[fix] Add Produces annotation for binary alias return types#131
bulldozer-bot[bot] merged 6 commits into
palantir:developfrom
pkoenig10:streaming

Conversation

@pkoenig10

@pkoenig10 pkoenig10 commented Dec 5, 2018

Copy link
Copy Markdown
Member

Before this PR

For jersey service endpoints returning a binary alias type, the return type was resolved correctly (to StreamingOutput or Response) but the correct @Produces annotation was not applied.

After this PR

The @Produces annotation is applied to endpoints returning binary alias types.

See discussion about resolving binary types in #128.

@pkoenig10 pkoenig10 requested a review from a team as a code owner December 5, 2018 22:27
@pkoenig10 pkoenig10 changed the title Add Produces annotation for binary alias return types [fix] Add Produces annotation for binary alias return types Dec 5, 2018

private static final ClassName NOT_NULL = ClassName.get("javax.validation.constraints", "NotNull");

private static final ClassName BINARY_METHOD_TYPE = ClassName.get(InputStream.class);

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 call this BINARY_ARGUMENT_TYPE instead, and likewise rename MethodTypeClassNameVisitor to ArgumentTypeClassNameVisitor?
Currently it seems a bit confusing since there's no such thing as a method type.

@iamdanfox iamdanfox left a comment

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.

LGTM, I've pushed a couple more commits to continue the refactor and kill the 'Factory' indirection which seemed a bit redundant now. Hope it looks OK!

TypeDefinition def = types.get(type);
if (def.accept(TypeDefinitionVisitor.IS_ALIAS)) {
Type aliasType = def.accept(TypeDefinitionVisitor.ALIAS).getAlias();
TypeName aliasTypeName = aliasType.accept(this);

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.

Long term I'd like to completely get rid of this class - it's kinda doing some 'dealiasing' here, but it's pretty convoluted with the binary specialization we have going on above.

@bulldozer-bot bulldozer-bot Bot merged commit 85148f9 into palantir:develop Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants