add OctetsFrom implementations for common octet types

[vavrusa]

Hi! 👋 This implements OctetsFrom trait for most common octet type conversions instead of providing a blanket implementation as a followup for #89

[partim]

Thanks for the PR!

Looks like we are having MSRV issues in dependencies again. Could you merge main so the checks go green, both here and in #127?

[vavrusa]

This is actually a blanket implementation from the previous PR. But it also converts ParsedDname -> Dname. I think it can either work like this, or or something like to_dname::<SrcOctets>()?.octets_into().

[partim]

Isn’t just to_dname enough here? If this is about the error type – not sure why that is PushError in ToDname::to_dname. The type implementing ToDname should always have a valid domain name, so the only error is a ShortBuf. Unless I am missing something, I happily change the type in the trait.

[vavrusa]

I think this is so you can convert Record<ParsedDname<Src>, Src> to Record<Dname<Src>, Src> using octets conversion instead of rebuilding the record (which is useful when storing a record from message for example), but let me try to rebase without these changes and see if we need it.

[partim]

Feels like there should be a better way to do this: if the name isn’t compressed, this can be done very cheaply for certain types. Maybe have a method flatten on ParseDname<_> and Record<ParseDname<_>, _>?

[vavrusa]

I can do that too! So how is flatten different from ToDname? It should be possible to optimize ToDname the same way (conversion is cheaper when as_flat_slice is some or octets can be shallow copied/referenced).

Should flatten be something like Dname::as_octets but return a an Option? (so for example ParsedDname<&Bytes> should return Option<&Bytes> which could be cheap to convert into Dname or Dname<&Bytes> as ParsedDname guarantees that the octets are a valid Dname). Is that what you mean? Or flatten would convert both type (ParsedDname -> Dname) and octets type (like this method)?

For me this is just a shortcut for building records from &[u8] and I rarely want to shallow copy Bytes backed records, so I'm also happy to drop this particular change, as it's just a convenience.

[partim]

The idea would be that if the parsed name is not compressed, flatten can simply return with self.parser.parse_octets(...) while otherwise it constructs the name via Dname::to_dname. I think this should be possible with slightly extravagant trait bounds – you’d need <Ref as OctetsRef>::Range to be FromBuilder plus some more things to tie it together.

Edit: I guess that wouldn’t actually work for your case, since you still need an octets conversion. Perhaps flatten_into instead with a type argument for a target octets type? I imagine it is cheaper to just copy the octets on an uncompressed name than constructing it label by label.

[partim]

How do you want to proceed? I want to merge #130 for other work I am doing but there’s some overlap, so I would rather merge this PR first to avoid things getting too messy.

[vavrusa]

I haven't been able to get back to this yet but let me decline it for now. I think I can make do without the nicer interface (or add extension traits to my library for record building with octets conversion) for now and will resubmit this if needed.

[partim]

Thanks! Feel free to propose or request something at any time! I might implement the two proposed flatten methods anyway – that seems like a useful thing to have.

[partim]

For my own understanding: Is this PR because Bytes doesn’t provide all necessary conversions via the From trait?