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

moved RequestHandler creation #78

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Conversation

mickvandijke
Copy link
Member

No description provided.

@josecelano
Copy link
Member

josecelano commented Sep 8, 2022

@WarmBeer look good to me. Thank you.

@josecelano josecelano merged commit 36e34ec into protocol-tests Sep 8, 2022
@mickvandijke
Copy link
Member Author

@josecelano maybe this can be further refactored if you think the PacketHandler is too big now. I just quickly moved your code/logic to the right places and didn't change much further than that.

@mickvandijke mickvandijke deleted the mick/protocol-tests branch September 8, 2022 11:07
@josecelano
Copy link
Member

@josecelano maybe this can be further refactored if you think the PacketHandler is too big now. I just quickly moved your code/logic to the right places and didn't change much further than that.

I think it's ok. I think there are like two levels: packets and requests. I see them as two different OSI levels:

Levels
REQUESTS
PACKETS

The low level handles the UPD packages, and the higher level handles the BitTorrent requests.

In the future, we could move some functions around to create those two levels. For example:

    async fn send_response(socket: Arc<UdpSocket>, remote_addr: SocketAddr, response: Response) {
        debug!("sending response to: {:?}", &remote_addr);

        let buffer = vec![0u8; MAX_PACKET_SIZE];
        let mut cursor = Cursor::new(buffer);

        match response.write(&mut cursor) {
            Ok(_) => {
                let position = cursor.position() as usize;
                let inner = cursor.get_ref();

                debug!("{:?}", &inner[..position]);
                UdpServer::send_packet(socket, &remote_addr, &inner[..position]).await;
            }
            Err(_) => { debug!("could not write response to bytes."); }
        }
    }

That function belongs to the server. It converts a Response (higher level) into a UDP Package (lower level). We could move the "packaging" logic to a new mod. I think it makes more sense to use RequestHandler instead of PacketHandler because most of the time, you are dealing with requests and responses. Maybe the server itself would be the PacketHandler because it's the higher service that directly speaks to the UDP port. A UDP server is basically a UPD packets handler.

But since there is not much code there and it's easy to refactor, we can do it when we agree on that view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants