inejge / ldap3

A pure-Rust LDAP library using the Tokio stack
Apache License 2.0
220 stars 38 forks source link

Potential alternative `SearchEntry` structure #89

Closed agraven closed 1 year ago

agraven commented 2 years ago

The current structure for SearchEntry can be a bit awkward to use, since the library consumer needs to check both attrs and bin_attrs when anticipating a potential binary attribute value. An alternative approach that could rectify this somewhat would be to define an AttributeValue enum like

pub enum AttributeValue {
    String(String),
    Binary(Vec<u8>),
}

and then redefining SearchResult as

pub struct SearchEntry {
    pub dn: String,
    pub attrs: HashMap<String, Vec<AttributeValue>>,
}
inejge commented 2 years ago

I tried mocking attribute retrieval with the proposed enum in the usual (String) case, and I'm not happy with the result. An enum variant is not a type and can't be destructured with a simple let (being a refutable pattern), so it needs if let or match, which makes it very verbose. Binary attributes are probably a single-digit percent of all cases, if that, and I didn't want to optimize their retrieval at the cost of making string-valued attributes less ergonomic. (It's annoying if you hit that low-percent use case regularly, I know.)

My long-term plan is to provide some form of entry deserialization through proc macros, which would, among other things, let one tag the destination struct members with a "binary" attribute and avoid the whole attrs/bin_attrs dance. But the accent is on long.

agraven commented 1 year ago

I can definitely see the problems with that, optimizing for the most common case definitely feels like the most reasonable approach