7. Refactoring Galore

Hellooooo! We’re back, and today I want to reorganize and refactor some of the methods, especially on the server side. All the changes we made in the last log was a bit messy 😬. So let’s dive in!

Refactoring

Right now, all our function are scattered around, some in struct Command, and others like handle_command, cmd_list, cmd_reqs etc. are just floating around. We could refactor all this into struct Commands, like so. This looks a whole lot cleaner doesn’t it? I’m still conflicted on keeping the Command enum itself, because if we’re going to execute and consume the enum variant as soon as we parse the command, where do we need to use this enum? But I have a feeling it might come useful in the future maybe. Let’s keep it for now.

After some more trivial changes, like adding command line arguments support for the client, and opening up the connection to the local network in the server, we’re ready to move on!

Actual file transferring!

Finally! We’re at the big finale, getting file transferring working. Here’s how our program flow is going to look like,

clientA and clientB connect to the server
clientA sends a request to clientB, blocking clientA (on the client side) from performing other actions, until clientB responds.
- clientB responds with an OK signal, and the file is send chunk by chunk from clientA to clientB
- OR clientB responds with a NO signal, and the request is removed, clientA is informed, everything is done.

Sounds good? Great, let’s get to it!

Detour: Major Blunder

Remember our implementation of the glide command?

utils/commands.rs

    async fn cmd_glide(&self, state: &SharedState, username: &str) -> String {
        let (path, to) = match self {
            Command::Glide { path, to } => (path, to),
            _ => unreachable!(),
        };
 
        // Check if file exists
        if !Path::new(path).exists() && fs::metadata(&path).unwrap().is_file() {
            return format!("Path '{}' is invalid. File does not exist", path);
        }
 
        // Check if user exists
        let mut clients = state.lock().await;
        if !clients.contains_key(to) && username != to {
            return format!("User @{} does not exist", to);
        }
 
        let file_size = fs::metadata(&path).unwrap().size();
 
        // Add request
        clients
            .get_mut(to)
            .unwrap()
            .incoming_requests
            .push(Request {
                from_username: username.to_string(),
                filename: path.to_string(),
                size: file_size,
            });
 
        format!("Successfully sent share request to @{} for {}", to, path)
    }

Spot the problem? Why are we checking if the file exists in the server!? Shouldn’t we be doing those checks in the client before sending the damn command over?? 🤦🏾‍♂️ I knew things were going too smoothly. Let’s get on top of this horrible mishap. And this would be the perfect chance to move the commands into a separate crate called utils, because then we can parse the command at the client itself and check the validity of the commands.

glide-client/src/main.rs 13

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
	// -- snip --
 
    // Command loop
    let stdin = io::stdin();
    let mut input = String::new();
 
    println!("Type 'help' to see available commands.");
 
    loop {
        // Get user input
        input.clear();
        print!("glide> ");
        io::stdout().flush()?;
        stdin.lock().read_line(&mut input)?;
 
        let input = input.trim();
        if input == "exit" {
            println!("Thank you for using Glide. Goodbye!");
            break;
        }
 
        // Parse the command
        let command = Command::parse(input);
 
        // Validate glide command
        if let Command::Glide { path, to } = command {
            // Check if file exists
            if Path::new(&path).try_exists().is_err() || !Path::new(&path).is_file() {
                println!("Path '{}' is invalid. File does not exist", path);
                continue;
            }
 
        }
 
        // Send command to the server
        stream.write_all(command.as_bytes()).await?;
 
        // Await server response
        let mut response = vec![0; CHUNK_SIZE];
        let bytes_read = stream.read(&mut response).await?;
        if bytes_read == 0 {
            println!("Server disconnected.");
            break;
        }
 
        // Print server response
        let response_str = String::from_utf8_lossy(&response[..bytes_read]);
        println!("{}", response_str);
    }
 
}

And everything seems to be working as expected. Now a lot of my previous oversights are showing up. First of all, I find it pretty annoying that the server is sending whole messages instead of codes or tiny messages like OK or just a list of usernames instead of whole formatted text to be outputted by the client. This will hinder our process when we’re trying to develop and alternative client. So let’s do a clean sweep of all those nasty messages, package them into enums, and use these enums to send and receive these messages. First let’s classify the types of messages the server sends.

Invalid username
Username taken
Username accepted
Unknown command
Unknown user
Connected users list
Incoming requests list
Share request successful
and the help command, which I don’t know why its not purely a client side implementation but here we are. These are all the things the server sends to the user as plain text, as of now. Let’s get some enums going!

After adding the enums (code changes were too big to show here, check out the repo here), and integrating them to the client code, let’s get to transferring the file. There’s going to be a few changes as to how this is going to work, though. For the sake of simplicity, I’m going to store the file at the server as soon as the command is successfully sent, and then the receiving client can download it from the server as long as the sender client is still connected to the server. With this, we don’t have to deal with blocking the sender client and all that. When I get to implementing the TUI client, we can make things better, but with this REPL style interface, this is the best solution I can think of right now. It would be a problem if a lot of people are trying to transfer files but, we’re not going to worry about that right now.

Back on track!

Let’s just implement the same file transferring logic we used in logs 2 and 3. It isn’t much work, just replicate the same logic with some minor changes, check out the commit. Let’s also not forget to delete the user’s folder when they disconnect.

glide-server/src/main.rs 145:1

async fn remove_client(username: &str, state: &SharedState) {
	// -- snip --
 
    // Remove folder under user
    fs::remove_dir_all(username);
 
    println!("Client @{} disconnected", username);
}

Now let’s implement the ok command. Firstly we need to verify if the ok is valid. We can do this super easy by checking incoming requests and seeing if the entry we need is there.

utils/commands.rs 177:1

    async fn cmd_ok(&self, state: &SharedState, username: &str) -> ServerResponse {
        // When the Ok command is sent, we check if the Ok is valid, and let the handler
        // do the rest
 
        let Command::Ok(from) = self else {
            unreachable!()
        };
 
        let clients = state.lock().await;
        for Request {
            from_username,
            filename: _,
        } in clients.get(username).unwrap().incoming_requests.iter()
        {
            if from_username == from {
                return ServerResponse::OkSuccess;
            }
        }
 
        ServerResponse::OkFailed
    }

Using the same logic used in the client to send the file, we can send our file to the desired client. We can also use the same logic used to receive files on the server, in the client side to download the shared file. But before that I want to implement the no command and get it over with.

utils/commands.rs

async fn cmd_no(&self, state: &SharedState, username: &str) -> ServerResponse {
	let Command::No(from) = self else {
		unreachable!()
	};
 
	let mut clients = state.lock().await;
 
	if let Some(client) = clients.get_mut(username) {
		if let Some(pos) = client
			.incoming_requests
			.iter()
			.position(|req| &req.from_username == from)
		{
			let request = client.incoming_requests.remove(pos);
			let file_path = format!("{}/{}/{}", from, username, request.filename);
			let _ = tokio::fs::remove_file(file_path).await; // ignore errors
		}
	}
 
	ServerResponse::NoSuccess
}

And now after implementing the file receiving logic in the client, let’s test things out shall we? Fingers crossed 🤞🏾!

Not all sunshine and rainbows

Well, guess what, things actually worked out when both clients are on the same device, but as soon as I start testing with transfer between devices, I get hit with errors. Specifically, the file content is being sent along with the metadata, making parsing the metadata a problem. What do we do here? Well, upon research, I found out that it COULD be the network delay, or it could be that TCP optimizes and send multiple writes together instead of sending the content only after the metadata is read. This is the perfect place to introduce protocols.

What are protocols? Basically, in this context, protocols are rules or structures for sending data across the network. TCP doesn’t enforce any protocols on its data transfers, so lets do it ourselves. We already did a pretty good job by sending the metadata in filename\0size format, we’re sending the size in ASCII, instead of good old numbers, which they are. Let’s reserve 4 bytes for the file size, which should let us transfer files as big as $2^{32}$ bytes, which is approximately 4 GB. This will be an upper bound to the file size transferrable, but this is something we can easily increase in the future.

We’re still running into problems where we send multiple pieces of data across! The server response gets jumbled up with the file metadata. We could just read the response and parse the rest of the data, but we don’t know how long the data is going to be, so let’s also parse the server responses into a single byte instead of text, we could get things done easier. In fact we need to design a whole new protocol, ground up, sticking to fixed size reads. We will discuss this in detail on the next one!

Reflection

This one’s been LOOONG overdue. Things coming up and life getting in the way, and collecting my messy thoughts and ideas on how to make this project better, dragged this one out way too long. I’ve learnt a lot from this project, all of which I’ll compile into a document and post it at the end of v1 of this project. Seeya on the next one!

🧠 BrainSpace

Recent Writing

8. Wrapping it Up