Skip to content

Update serde interfaces to differentiate in/out #305

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

Merged

Conversation

JordonPhillips
Copy link
Contributor

This updates the interfaces for rest services to differentiate when an operation's input vs output vs error is being serialized / deserialized. This is necessary because there can be subtly different rules in each case.

Right now this is pretty ugly so I put it up as a draft. The major alternative I am considering is to instead have explicit methods for each case, e.g:

protected abstract void serializeInputDocumentBody(
    GenerationContext context,
    OperationShape operation,
    List<HttpBinding> documentBindings
);

protected abstract void serializeOutputDocumentBody(
    GenerationContext context,
    OperationShape operation,
    List<HttpBinding> documentBindings
);

protected abstract void serializeErrorDocumentBody(
    GenerationContext context,
    OperationShape operation,
    StructureShape error,
    List<HttpBinding> documentBindings
);

Implementers could then make it generic on their end if necessary as apposed to having to split things up like is the case now.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@JordonPhillips JordonPhillips marked this pull request as ready for review April 8, 2021 15:21
Copy link
Contributor

@adamthom-amzn adamthom-amzn left a comment

Choose a reason for hiding this comment

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

I don't like adding a boolean flag to methods in general, but looking at the structure of methods like writeContentTypeHeader I think you may end up having a private method with the flag added anyway.

@JordonPhillips JordonPhillips force-pushed the differentiate-server-serde branch 2 times, most recently from 6ccd617 to c923de9 Compare April 29, 2021 16:08
@JordonPhillips JordonPhillips changed the base branch from ssdk to main April 29, 2021 16:09
@JordonPhillips JordonPhillips requested a review from kstich May 6, 2021 15:27
@JordonPhillips JordonPhillips force-pushed the differentiate-server-serde branch from c923de9 to 57b8400 Compare May 6, 2021 15:29
@JordonPhillips JordonPhillips force-pushed the differentiate-server-serde branch from 57b8400 to b863c13 Compare May 11, 2021 14:51
This updates the interfaces for rest services to differentiate when an
operation's input vs output vs error is being serialized / deserialized.
This is necessary because there can be subtly different rules in each
case.
This adds a protected method that allows http binding protocols to
customize when a default body should be set. This is needed for
restJson1 which has some complicated rules around that.
@JordonPhillips JordonPhillips force-pushed the differentiate-server-serde branch from b863c13 to 26b52b6 Compare May 12, 2021 11:11
@JordonPhillips
Copy link
Contributor Author

rebased so tests can be run one more time

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.

3 participants