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

[rust-server] Enhance middleware support (+ perf fix) #8114

Merged
merged 37 commits into from
May 3, 2018

Conversation

bjgill
Copy link
Contributor

@bjgill bjgill commented May 1, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Stop re-creating hyper clients

To workaround a bug in pre-async hyper, we had to re-create the HTTP client for each client call. This bug has since been fixed, so we can remove the workaround for an unquantified performance benefit.

Context enhancement

Updates rust-server to use v0.12.1 of the swagger-rs crate. This replaces the old Context struct with a generic type parameter, using trait bounds to specify what each hyper service expects the context to contain. This allows hyper middleware layers to exchange data without intermediate layers needing to know about it.

The traits for manipulating contexts are Has<T> (for accessing a field of type T in the context), Push<T> (for adding a field of type T to the context) and Pop<T> (for removing a field of type T from the context).

The following changes will be neccessary in the main.rs file of a service using rust-server:

  • Replace uses of the NoAuthentication struct from swagger-rs with AddContext from the same crate. This is a simple renaming.
  • Do one of the following:
    • Import the EmptyContext type from swagger-rs. Do this if you are not using any hyper services other than AddContext and those generated by swagger-codegen.
    • Use the new_context_type! macro from swagger-rs to define a new context builder and empty context type, passing the macro a list of types that will be stored in the context. Do this if you use a hyper middleware layer that adds anything else to the context.
  • Add EmptyContext (or the new empty context type you defined with new_context_type!) as an explicit type parameter to the AddContext struct that you instantiate.

For example, the following code

use swagger::NoAuthentication;

fn main() {

    // The struct implementing the Api trait generated by swagger-codegen
    let my_api_impl = MyApiImpl::new();

    // The hyper NewService generated by swagger-codegen
    let my_api_new_service = my_api::server::NewService::new(my_api_impl.clone());

    // The wrapped NewService
    let my_new_service = NoAuthentication(my_api_new_service);

    // ...
}

would be replaced with

use swagger::{AddContext, EmptyContext};

fn main() {
    let my_api_impl = MyApiImpl::new();
    let my_api_new_service = my_api::server::NewService::new(my_api_impl.clone());
    let my_new_service = AddContext<_, EmptyContext>(my_api_new_service);
    // ...
}

See the rust-server petstore sample code, and the doc comments and test module in swagger-rs/src/context.rs for more details and examples.

(with thanks to @tca-ms, @alango and @NigelThorpe)

alango and others added 30 commits May 1, 2018 10:42
@bjgill
Copy link
Contributor Author

bjgill commented May 1, 2018

@technical_committee and relevant person: @frol @farcaller @wing328

The description above should give a good idea of the motivation for these changes.

@bjgill bjgill changed the title Context changes [rust-server] Enhance middleware support (+ perf fix) May 1, 2018
@frol
Copy link
Contributor

frol commented May 1, 2018

LGTM

@wing328 wing328 added this to the v2.4.0 milestone May 3, 2018
@wing328 wing328 merged commit 415edb8 into swagger-api:master May 3, 2018
@bjgill bjgill deleted the context-changes branch May 3, 2018 12:36
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.

6 participants