shotover / shotover-proxy

L7 data-layer proxy
https://docs.shotover.io
Apache License 2.0
83 stars 16 forks source link

Fix transform lifetimes #1724

Closed rukai closed 1 month ago

rukai commented 1 month ago

Fixes some issues with https://github.com/shotover/shotover-proxy/pull/1724/files

I found that trying to use Wrapper after it was borrowed by Transform::transform was impossible. That is:

let mut wrapper = Wrapper::new();
// this line is fine by itself
transform.transform(&mut wrapper);
// but this line fails to compile due to the above line holding onto the reference forever.
println!("{:?}", wrapper.requests);

This is very strange since after transform.transform(&mut wrapper) the reference to the wrapper should be dropped and we should be able to use wrapper again. However it turns out this is expected behavior. I found this article helpful in explaining why exactly this was occurring. https://tfpk.github.io/lifetimekata/chapter_4.html

The article suggests using different lifetimes but we also have situations where we need the lifetimes to be the same. But I found a solution which I have implemented in this PR:

Adding the 'longer and 'shorter lifetimes makes the function signature more confusing to read, but it actually relaxes the lifetime requirements instead of adding more. Previously the two lifetimes had to be exactly the same. Now they can be exactly the same or 'longer can outlive 'shorter.

I also updated the changelog since that was missed in #1724

TL;DR

This PR should have no impact on functionality or performance but enables the way that we call Transform::transform to be more flexible, unblocking https://github.com/shotover/shotover-proxy/pull/1717

codspeed-hq[bot] commented 1 month ago

CodSpeed Performance Report

Merging #1724 will not alter performance

Comparing rukai:fix_transform_lifetimes (17d02eb) with main (ff09b14)

Summary

✅ 39 untouched benchmarks