Add read-only mode that doesn't modify entries

[langesven]

This will also make hasattr on LDAPEntry more predictable in situations
where we do not want an empty set returned to add values but rather want
to know whether or not the entry has that specific attribute or not.

[tinloaf]

I kinda want to know the story behind this feature. 😏

[langesven]

It's actually not as bad as you're probably imagining!
We're migrating a lot of older scripts from an old ldapom version to the latest one.
These scripts make heavy use of hasattr to check for attribute existence.
Well, this ldapom version has a nifty feature that'll return you an empty set on __getattr__ so you can start creating non-existing entries. The downside of that is, it doesn't return an AttributeError and as such hasattr thinks the attribute exists, which messes with the logic in our current scripts.
So I figured I'd just add a read-only parameter, that prevents any sort of modification and on top of that brings back the old behaviour of hasattr because in read-only returning an empty set so you can modify an attribute doesn't make a whole lot of sense.

[xhochy]

Makes sense to me to have this feature. Left two small inline comments.

[xhochy]

This is probably better suited as a decorator, e.g. @check_for_ro.

[xhochy]

As only the base error uses LDAPom, I would skip the om in the class name here.

[leonhandreke]

This behavior is only for multi-value attributes. I think back then we were unsure about how to represent "zero entries in this multi-value attribute" and we chose the empty set (rather than AttributeError) because it made sense to us in this context. An empty list is not written back to the server either (see connection.py).

Instead of using hasattr, would it be possible to update these scripts to rely on the truthiness of sets instead? So instead of if hasattr(entry.ipAddresses), do if entry.ipAddresses?

[langesven]

Sure in theory I could change it in the scripts, but correct me if I'm wrong, but the way I see it I would then need to pay attention to which attributes I'm checking.
If it's not an multi-value attribute the if entry.attribute will run into the AttributeError which means I need to add exception handling.
Or I need to explicitly use hasattr(entry, 'attribute') on the attributes of which I know they're single-value and use if entry.attribute on the ones that are multi-value, which is rather error-prone imo.

[leonhandreke]

Hm, I agree, that sucks.

I'm not against a read only mode, it might be useful in some situations as a protection against programming mistakes modifying things, but changing how attribute value retrieval works in a subtle way in just this mode sounds weird to me. Changing the behavior in the general case would be a bigger task and would also result in weird behavior. Remove a bunch of values from a list, poof, AttributeError instead of an empty list.

Maybe we could add a util method to the attribute class? Maybe is_present that returns bool(self._values)? I'm actually surprised we don't have something like this already because it sounds like a real problem. What do you think?