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: remove unnecessary Arc in SessionAllocator #38

Merged
merged 6 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions linkup-cli/src/local_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ async fn linkup_config_handler(
string_store: web::Data<MemoryStringStore>,
req_body: web::Bytes,
) -> impl Responder {
let sessions = SessionAllocator::new(string_store.into_inner());

let input_json_conf = match String::from_utf8(req_body.to_vec()) {
Ok(input_json_conf) => input_json_conf,
Err(_) => {
Expand All @@ -35,6 +33,7 @@ async fn linkup_config_handler(

match update_session_req_from_json(input_json_conf) {
Ok((desired_name, server_conf)) => {
let sessions = SessionAllocator::new(string_store.as_ref());
let session_name = sessions
.store_session(server_conf, NameKind::Animal, desired_name)
.await;
Expand All @@ -59,7 +58,7 @@ async fn linkup_ws_request_handler(
req: HttpRequest,
req_stream: web::Payload,
) -> impl Responder {
let sessions = SessionAllocator::new(string_store.into_inner());
let sessions = SessionAllocator::new(string_store.as_ref());

let url = format!("http://localhost:{}{}", LINKUP_LOCALSERVER_PORT, req.uri());
let mut headers = LinkupHeaderMap::from_actix_request(&req);
Expand Down Expand Up @@ -161,7 +160,7 @@ async fn linkup_request_handler(
req: HttpRequest,
req_body: web::Bytes,
) -> impl Responder {
let sessions = SessionAllocator::new(string_store.into_inner());
let sessions = SessionAllocator::new(string_store.as_ref());

let url = format!("http://localhost:{}{}", LINKUP_LOCALSERVER_PORT, req.uri());
let mut headers = LinkupHeaderMap::from_actix_request(&req);
Expand Down
11 changes: 6 additions & 5 deletions linkup/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,6 @@ fn extrace_tracestate(tracestate: &str, linkup_key: String) -> String {

#[cfg(test)]
mod tests {
use std::sync::Arc;

use super::*;

const CONF_STR: &str = r#"
Expand Down Expand Up @@ -325,7 +323,8 @@ mod tests {

#[tokio::test]
async fn test_get_request_session_by_subdomain() {
let sessions = SessionAllocator::new(Arc::new(MemoryStringStore::new()));
let string_store = MemoryStringStore::new();
let sessions = SessionAllocator::new(&string_store);

let config_value: serde_json::Value = serde_json::from_str(CONF_STR).unwrap();
let config: Session = config_value.try_into().unwrap();
Expand Down Expand Up @@ -455,7 +454,8 @@ mod tests {

#[tokio::test]
async fn test_get_target_url() {
let sessions = SessionAllocator::new(Arc::new(MemoryStringStore::new()));
let string_store = MemoryStringStore::new();
let sessions = SessionAllocator::new(&string_store);

let input_config_value: serde_json::Value = serde_json::from_str(CONF_STR).unwrap();
let input_config: Session = input_config_value.try_into().unwrap();
Expand Down Expand Up @@ -544,7 +544,8 @@ mod tests {

#[tokio::test]
async fn test_repeatable_rewritten_routes() {
let sessions = SessionAllocator::new(Arc::new(MemoryStringStore::new()));
let string_store = MemoryStringStore::new();
let sessions = SessionAllocator::new(&string_store);

let input_config_value: serde_json::Value = serde_json::from_str(CONF_STR).unwrap();
let input_config: Session = input_config_value.try_into().unwrap();
Expand Down
10 changes: 4 additions & 6 deletions linkup/src/session_allocator.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use std::sync::Arc;

use crate::{
extract_tracestate_session, first_subdomain, headers::HeaderName, random_animal,
random_six_char, session_to_json, ConfigError, HeaderMap, NameKind, Session, SessionError,
StringStore,
};

pub struct SessionAllocator {
store: Arc<dyn StringStore>,
pub struct SessionAllocator<'a> {
store: &'a dyn StringStore,
}

impl SessionAllocator {
pub fn new(store: Arc<dyn StringStore>) -> Self {
impl<'a> SessionAllocator<'a> {
pub fn new(store: &'a dyn StringStore) -> Self {
Self { store }
}

Expand Down
24 changes: 12 additions & 12 deletions worker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
// TODO(augustoccesar)[2023-10-16]: We need to revisit the session allocator and how we are using Arc.
// It seems that `CfWorkerStringStore` is not really Send or Sync, which is making so that clippy warn about this.
// For more info check: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
#![allow(clippy::arc_with_non_send_sync)]

use http_util::*;
use kv_store::CfWorkerStringStore;
use linkup::{HeaderMap as LinkupHeaderMap, *};
use regex::Regex;
use std::sync::Arc;
use worker::*;
use ws::linkup_ws_handler;

Expand All @@ -16,7 +10,10 @@ mod kv_store;
mod utils;
mod ws;

async fn linkup_session_handler(mut req: Request, sessions: SessionAllocator) -> Result<Response> {
async fn linkup_session_handler<'a>(
mut req: Request,
sessions: &'a SessionAllocator<'a>,
) -> Result<Response> {
let body_bytes = match req.bytes().await {
Ok(bytes) => bytes,
Err(_) => return plaintext_error("Bad or missing request body", 400),
Expand Down Expand Up @@ -84,7 +81,10 @@ async fn set_cached_req(
Ok(resp)
}

async fn linkup_request_handler(mut req: Request, sessions: SessionAllocator) -> Result<Response> {
async fn linkup_request_handler<'a>(
mut req: Request,
sessions: &'a SessionAllocator<'a>,
) -> Result<Response> {
let url = match req.url() {
Ok(url) => url.to_string(),
Err(_) => return plaintext_error("Bad or missing request url", 400),
Expand Down Expand Up @@ -154,17 +154,17 @@ pub async fn main(req: Request, env: Env, _ctx: worker::Context) -> Result<Respo

let string_store = CfWorkerStringStore::new(kv);

let sessions = SessionAllocator::new(Arc::new(string_store));
let sessions = SessionAllocator::new(&string_store);

if let Ok(Some(upgrade)) = req.headers().get("upgrade") {
if upgrade == "websocket" {
return linkup_ws_handler(req, sessions).await;
return linkup_ws_handler(req, &sessions).await;
}
}

if req.method() == Method::Post && req.path() == "/linkup" {
return linkup_session_handler(req, sessions).await;
return linkup_session_handler(req, &sessions).await;
}

linkup_request_handler(req, sessions).await
linkup_request_handler(req, &sessions).await
}
5 changes: 4 additions & 1 deletion worker/src/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use futures::{

use crate::http_util::plaintext_error;

pub async fn linkup_ws_handler(req: Request, sessions: SessionAllocator) -> Result<Response> {
pub async fn linkup_ws_handler<'a>(
req: Request,
sessions: &'a SessionAllocator<'a>,
) -> Result<Response> {
let url = match req.url() {
Ok(url) => url.to_string(),
Err(_) => return plaintext_error("Bad or missing request url", 400),
Expand Down
Loading