[Freeipa-devel] [PATCH] 241 Fix precallback validators in DNS plugin

[Petr Viktorin]

On 03/21/2012 01:51 PM, Martin Kosek wrote:
> DNS plugin contains several RR type record validators run in
> pre_callback which cannot be used as standard param validator
> as it needs more data and resources that standard validators
> provide. However, the precallback validators are not run for
> DNS records created by new structured options and thus an invalid
> value may slip in.
>
> This patch moves the execution of these precallback validators
> _after_ the processing of structured DNS options. It also cleans
> them up a little and makes them more robust.
>
> https://fedorahosted.org/freeipa/ticket/2550
>

Functionally, the patch works fine.
Since you're cleaning up, I have some nitpicks, but ACK if you disagree.


Consider if instead of:

rtype_cb = '_%s_pre_callback' % rtype
if hasattr(self.obj, rtype_cb):
     getattr(self.obj, rtype_cb)(ldap, dn, entry_attrs, *keys, **options)

this wouldn't be more readable:

rtype_cb = getattr(self.obj, '_%s_pre_callback' % rtype, None)
if rtype_cb:
     rtype_cb(ldap, dn, entry_attrs, *keys, **options)

and since the block appears twice now, make it a method.

Also, since is_ns_rec_resolvable raises an exception rather than 
returning a bool, it should probably be renamed to something like 
check_ns_rec_resolvable.

-- 
Petr³