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

EntityShape#operations should be ordered #591

Closed
Baccata opened this issue Oct 5, 2020 · 4 comments · Fixed by #602
Closed

EntityShape#operations should be ordered #591

Baccata opened this issue Oct 5, 2020 · 4 comments · Fixed by #602
Labels
feature-request A feature should be added or improved.

Comments

@Baccata
Copy link

Baccata commented Oct 5, 2020

Currently, operations (and resources) are exposed from an EntityShape as Sets, backed by HasSets This loses the ordering from the spec, which happens to be useful information to construct routing logic in REST-like protocols.

https://github.com/awslabs/smithy/blob/acc7c95a2052b0597c1ea1eab2a3ed9273092bc5/smithy-model/src/main/java/software/amazon/smithy/model/shapes/EntityShape.java#L28-L29

I'd personally like for the data structure to retain ordering, and for the ordering guarantee to be reflected in the exposed types.

@mtdowling
Copy link
Member

Why does ordering of these matter for REST protocols? I think in AWS, we typically alphabetically order operations when doing codegen and generating documentation.

@Baccata
Copy link
Author

Baccata commented Oct 13, 2020

Why does ordering of these matter for REST protocols

Typically, pure REST protocols do not send the operation name in the headers like you guys do at AWS, so the URI is used to discriminate between operations, meaning that typically URIs are pattern-matched against in a sequential fashion.

Smithy is a good citizen in the sense that it checks that URIs do not collide within a given service, but as a smithy user, I'd rather have control over this to potentially optimise the order of the pattern-matching, to ensure that the endpoint that gets called the most is first in line.

Another scenario is that as a smithy user, I'd like to control the ordering in which the operations are reflected in generated interfaces, to ensure the ones that are typically called more often appear at the top.

@mtdowling
Copy link
Member

mtdowling commented Oct 15, 2020

Ah, I see. In AWS with the restJson protocol, we do perform operation matching based on HTTP methods, paths, and query strings. Since our pattern conflict detection is so strict, we don't have a need for ordering. In the case of conflicts between routes (for example, a static path segment that conflicts with a label), we prefer the most concrete route.

I think I could see there being the possibility that other protocols might need to use the order in which operations are defined in the model as significant though. And I think generating documentation based on the order of operations could make sense too.

Note though that resources do have lifecycle operations, and I don't think we really want to provide an ordering guarantee for them since we load Smithy models from both the IDL and JSON (and JSON objects technically have no ordering guarantees).

@mtdowling mtdowling added the feature-request A feature should be added or improved. label Oct 15, 2020
mtdowling added a commit that referenced this issue Oct 15, 2020
This commit updates service and operation shapes to maintain the order
in which operations and resources are defined in a Smithy model. Note
that resource lifecycle operation order is not maintained since those
are named operations that don't have any particular order in a model
like a list of operations, collectionOperations, or resources do.

To achieve this, I added a couple new helper methods to make copies of
Maps and Sets that maintains order.

Closes #591
@Baccata
Copy link
Author

Baccata commented Oct 15, 2020

Note though that resources do have lifecycle operations, and I don't think we really want to provide an ordering guarantee for them since we load Smithy models from both the IDL and JSON (and JSON objects technically have no ordering guarantees).

I think that is absolutely fine

mtdowling added a commit that referenced this issue Oct 19, 2020
This commit updates service and operation shapes to maintain the order
in which operations and resources are defined in a Smithy model. Note
that resource lifecycle operation order is not maintained since those
are named operations that don't have any particular order in a model
like a list of operations, collectionOperations, or resources do.

To achieve this, I added a couple new helper methods to make copies of
Maps and Sets that maintains order.

Closes #591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants