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

feat: add setter methods for Message struct to improve configurability #2147

Merged

Conversation

situ2001
Copy link
Contributor

I've added several set_**_count methods to the Message struct to simplify the process of setting the count fields within the Header of a mutable Message instance before encoding it into binary format. Previously, to achieve this, I had to:

  1. Get a cloned header from the Message.
  2. Set the count on the cloned header.
  3. Set the modified header back as the Message's header.

Here is how I do in my production code.

let mut msg = Message::new();

// Ignored code part: init and add `Query` to `Message`
msg.add_query(q);

let mut header = msg.header().clone();
header.set_query_count(1);
msg.set_header(header);

@bluejekyll
Copy link
Member

Can you help explain how this is being used? The goal of not exposing that directly was to ensure that the query count matches the number of associated queries in the Message when it's serialized.

@situ2001
Copy link
Contributor Author

Can you help explain how this is being used? The goal of not exposing that directly was to ensure that the query count matches the number of associated queries in the Message when it's serialized.

Sure. The code above is my program's test code. There is another example: I have a DNS response cache directly storing the Message struct. Before storing the response into the cache, I will deserialize the Message struct, modify the answer information according to the rules (such as removing some answers) and update the the answer count in header.

@bluejekyll
Copy link
Member

Ok, I think that's reasonable. Could you do me a favor, I just double checked and these values are ignored during serialization, could you add something like this to your doc comments on the new methods: this count will be ignored during serialization, where the length of the associated records will be used instead

@situ2001
Copy link
Contributor Author

Ok, I think that's reasonable. Could you do me a favor, I just double checked and these values are ignored during serialization, could you add something like this to your doc comments on the new methods: this count will be ignored during serialization, where the length of the associated records will be used instead

I have added these comments and written a related test.

@bluejekyll
Copy link
Member

Thanks for this PR!

@bluejekyll bluejekyll merged commit 966bc27 into hickory-dns:main Mar 2, 2024
17 of 18 checks passed
@situ2001 situ2001 deleted the feat/add-setter-for-message-struct branch March 2, 2024 23:23
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

3 participants