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

refactor: use trait bound accept fn to reduce duplicate logic #24

Merged
merged 14 commits into from
Dec 6, 2023

Conversation

jdockerty
Copy link
Owner

@jdockerty jdockerty commented Dec 5, 2023

This PR is two fold in factoring:

First off, at the moment both TcpProxy and HttpProxy both start, regardless of whether there is anything configured against those listeners. This isn't a huge deal, but it's an easy win to quickly check the provided configuration on startup and simply gate the start of either proxy based on whether that Protocol type actually exists.

Second of it more around general software design. When I initially introduced the Proxy trait in #15 I was thinking about it in the wrong way, whereby I still had the accept function defined within the trait, meaning that there was a tonne of duplicated logic and the same changes being done, just in a different place. Instead, I've now done what I previously hoped and pulled out common functionality, i.e. the accept function has become a trait-bound function, requiring only the implementation-specific details of actually proxying connections to be left to to be filled in by those which satisfy the trait - the reasoning here is that implementations may differ, such as HTTP being concerned with L7 (HTTP method, request path etc.), but the initial acceptance logic should remain the same.

@jdockerty jdockerty self-assigned this Dec 5, 2023
@jdockerty jdockerty changed the title refactor: proxying refactor: use trait bound accept fn to reduce duplicate logic Dec 6, 2023
@jdockerty jdockerty marked this pull request as ready for review December 6, 2023 17:32
@jdockerty jdockerty merged commit 7686eaf into main Dec 6, 2023
1 check passed
@jdockerty jdockerty deleted the refactor/proxying branch December 6, 2023 17:44
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.

None yet

1 participant