CodeSOD: Observable Queries

Rachel is doing some Python/Django work on an application that, among other things, handles a pile of Internet addresses, a mix of IP addresses and domain names. Since each of those has a very different query path (domains could be filtered on the TLD or the name portion of the domain, for example), Rachel implemented some helper objects built on Django observables, the IPAddressObservable and the DomainObservable.

This way, when someone wanted to search the database for an address, some code like this could run:

OBSERVABLE_TYPES = {"ipaddress": IPAddressObservable, "domain", DomainObservable}# ... snip ...def search(self, query): q = Observable.objects.none() for klass in OBSERVABLE_TYPES.values(): q |= klass.get_search_query(query) return self.get_queryset().filter(q)

As you can see, this code was planned to be somewhat extensible. The actual observable types are stored in a dictionary. The search method starts with an empty query, and then for every observable in OBSERVABLE_TYPES, we get a query from that and append it to the empty query, and then we can filter our query set with the resulting query.

One can have some minor quibbles with the style, or even the framework's choice to overload the | operator to do this concatenation, but if you understand how Django handles Observables and queries, this all makes sense.

The problem arose months later, when another developer came through to extend this code. Rachel's code searches by both IP and domain name. Someone wanted to be able to pick. Now, as Rachel implemented it, this would be easy:

def search(self, query, query_type=None): if query_type and query_type in OBSERVABLE_TYPES: return self.get_queryset().filter( OBSERVABLE_TYPES[query_type] ) else: q = Observable.objects.none() for klass in OBSERVABLE_TYPES.values(): q |= klass.get_search_query(query) return self.get_queryset().filter(q)

There are, obviously, smarter solutions to cut down on the conditionals, the key point here is that the new path isn't hard to implement, if one thinks about it for five seconds.

If, on the other hand, you either don't think, or presumably think about it for many hours, you instead might get the solution that was actually used:

def search(self, query, query_type=None): q = Observable.objects.none() OBSERVABLE_TYPES_FILTERED = [v for k, v in OBSERVABLE_TYPES.items() if k == query_type] if query_type and len(OBSERVABLE_TYPES_FILTERED): for klass in OBSERVABLE_TYPES_FILTERED: q |= klass.get_search_query(query) else: for klass in OBSERVABLE_TYPES.values(): q |= klass.get_search_query(query) return self.get_queryset().filter(q)def exact_search(self, query, query_type=None): q = Observable.objects.none() OBSERVABLE_TYPES_FILTERED = [v for k, v in OBSERVABLE_TYPES.items() if k == query_type] if query_type and len(OBSERVABLE_TYPES_FILTERED): for klass in OBSERVABLE_TYPES_FILTERED: q |= klass.get_search_query(query) else: for klass in OBSERVABLE_TYPES.values(): q |= klass.get_exact_search_query(query) return self.get_queryset().filter(q)

I'm not sure why exact_search is a duplicate of search. I really appreciate that they filter whether or not a query_type was supplied. And of course, the real magic here is the choice to filter instead of just to treat the dictionary as a dictionary. You have key access, you don't need to filter, you can just... use the dictionary.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!