lambda_http: Make extension methods available for http::request::Parts

[dcormier]

It would be nice to be able to use Request::into_parts() and still have access to the helper methods for getting AWS-specific data. With that in mind, I would like to propose breaking RequestExt into two traits. RequestExt and ExtensionsExt (if someone has a better name, please suggest it).

The existing trait and its impl resemble this (as of #602):

pub trait RequestExt {
    fn raw_http_path(&self) -> String;
    fn raw_http_path_str(&self) -> &str;
    fn with_raw_http_path(self, path: &str) -> Self;

    fn query_string_parameters(&self) -> QueryMap;
    fn query_string_parameters_ref(&self) -> Option<&QueryMap>;
    fn with_query_string_parameters<Q>(self, parameters: Q) -> Self
    where
        Q: Into<QueryMap>;

    fn path_parameters(&self) -> QueryMap;
    fn path_parameters_ref(&self) -> Option<&QueryMap>;
    fn with_path_parameters<P>(self, parameters: P) -> Self
    where
        P: Into<QueryMap>;

    fn stage_variables(&self) -> QueryMap;
    fn stage_variables_ref(&self) -> Option<&QueryMap>;
    #[cfg(test)]
    fn with_stage_variables<V>(self, variables: V) -> Self
    where
        V: Into<QueryMap>;

    fn request_context(&self) -> RequestContext;
    fn request_context_ref(&self) -> Option<&RequestContext>;
    fn with_request_context(self, context: RequestContext) -> Self;

    fn payload<D>(&self) -> Result<Option<D>, PayloadError>
    where
        for<'de> D: Deserialize<'de>;

    fn lambda_context(&self) -> Context;
    fn lambda_context_ref(&self) -> Option<&Context>;
    fn with_lambda_context(self, context: Context) -> Self;
}

impl RequestExt for lambda_http::Request { ... }

What I am proposing resembles this:

pub trait ExtensionsExt {
    fn raw_http_path(&self) -> &str;
    fn with_raw_http_path<S>(self, path: S) -> Self
    where
        S: Into<String>;

    fn query_string_parameters(&self) -> Option<&QueryMap>;
    fn with_query_string_parameters<Q>(self, parameters: Q) -> Self
    where
        Q: Into<QueryMap>;

    fn path_parameters(&self) -> Option<&QueryMap>;
    fn with_path_parameters<P>(self, parameters: P) -> Self
    where
        P: Into<QueryMap>;

    fn stage_variables(&self) -> Option<&QueryMap>;
    #[cfg(test)]
    fn with_stage_variables<V>(self, variables: V) -> Self
    where
        V: Into<QueryMap>;

    fn request_context(&self) -> Option<&RequestContext>;
    fn with_request_context(self, context: RequestContext) -> Self;

    fn lambda_context(&self) -> Option<&Context>;
    fn with_lambda_context(self, context: Context) -> Self;
}

impl ExtensionsExt for http::Extensions { ... }
impl ExtensionsExt for http::Parts { ... }
impl<B> ExtensionsExt for http::Request<B> { ... }

pub trait RequestExt {
    fn payload<D>(&self) -> Result<Option<D>, PayloadError>
    where
        for<'de> D: Deserialize<'de>;
}

impl RequestExt for lambda_http::Request { ... }

There are multiple breaking changes in this proposal.

Not only would users wanting extension methods other than payload() need to import a new trait (ExtensionsExt), the methods on that trait have changed from what existed in RequestExt. Specifically, the *_ref(&self) -> Option<&T> methods introduced in #602 have been renamed without the _ref suffix, replacing the methods that existed prior to #602. A benefit is that it removes methods that silently clone, and may panic.

The method signatures that change are:

• fn raw_http_path(&self) -> String becomes
  • fn raw_http_path(&self) -> &str
• fn with_raw_http_path(self, path: &str) -> Self becomes
  • fn with_raw_http_path<S: Into<String>>(self, path: S) -> Self
• fn query_string_parameters(&self) -> QueryMap becomes
  • fn query_string_parameters(&self) -> Option<&QueryMap>
• fn path_parameters(&self) -> QueryMap becomes
  • fn path_parameters(&self) -> Option<&QueryMap>
• fn stage_variables(&self) -> QueryMap becomes
  • fn stage_variables(&self) -> Option<&QueryMap>
• fn request_context(&self) -> RequestContext becomes
  • fn request_context(&self) -> Option<&RequestContext>
• fn lambda_context(&self) -> Context becomes
  • fn lambda_context(&self) -> Option<&Context>

Thoughts?

[dcormier]

After discussion on the PR, the method changes have been reduced to:

• fn raw_http_path(&self) -> String becomes
  • fn raw_http_path(&self) -> &str
• fn with_raw_http_path(self, path: &str) -> Self becomes
  • fn with_raw_http_path<S: Into<String>>(self, path: S) -> Self

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for the maintainers of this repository to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.