4
\$\begingroup\$

I've got a item inventory that each inventory item is tagged with multiple tags. What I've got is an API that takes a query parameter with multiple comma separated query items and returns a list of all items that match all of the tags.

The following code is working, however this isn't really using the ManyToMany attributes in the Django models and I'm sure there is a better way to do this.

Schema ---

class InventoryTag(models.Model):
    class Meta:
        unique_together = ('key', 'value')

    key   = models.CharField(max_length=240, db_index=True)
    value = models.CharField(max_length=240, db_index=True)

class InventoryItem(models.Model):
    instance_id = models.CharField(max_length=255)

    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

    tags = models.ManyToManyField(InventoryTag, through='InventoryItemTag', through_fields=('item', 'tag'))

    def as_json(self):
        return serializers.serialize('json', [self])[0]

class InventoryItemTag(models.Model):
    class Meta:
        unique_together = ('item', 'tag')

    item = models.ForeignKey(InventoryItem, db_index=True, related_name='invitem')
    tag  = models.ForeignKey(InventoryTag, db_index=True, related_name='invtag')

    created_at = models.DateTimeField(auto_now_add=True)

Get Function --

def get(self, request, *args, **kwargs):
    query = request.GET.get('q', '').strip()
    start = int(request.GET.get('start', 0))
    count = int(request.GET.get('count', 50))

    query_items = [q.strip() for q in query.split(',') if q.strip()]

    items = InventoryItem.objects
    if query_items:
        for q in query_items:
            tags = InventoryTag.objects
            q = q.lstrip(':')
            if ':' in q:
                (k, v) = q.split(':', 1)
                tags = tags.filter(Q(key = k) & Q(value__startswith = v))
            else:
                tags = tags.filter(value__startswith = q)

            tags_pks = InventoryItemTag.objects.filter(Q(tag__in = tags.values_list('id')))
            items = items.filter(pk__in=tags_pks.values_list('item_id'))

    items = items.all()

How this is used:

  • InventoryItem -- For practical purpose this is an AWS instance with an instance_id
  • InventoryTag -- Other attributes of that instance (e.g. Name) or IP address. This is unique for any key/value pair of the tag.

You might have a set of instances like:

Item(id=1) -- tags:  name=dev.test zone=us-east-1a
Item(id=2) -- tags:  name=dev.vpc  zone=us-east-1a
Item(id=3) -- tags:  name=dev.cron zone=us-east-1a
Item(id=4) -- tags:  name=dev.test zone=us-east-1c
Item(id=5) -- tags:  name=dev.test zone=us-west-2a

So the goal is that you should be able to do a search where you ask for all name=dev.test and zone=us-east-1a machines and get item #1 and #4.

\$\endgroup\$
2
\$\begingroup\$

You are, more or less, manually performing in Python what can be achieved in SQL using Q objects. Basically, you can chain Q objects to create an SQL query on the InventoryItems with one WHERE clause for each part of your HTTP query.

Also note that, on queryset operations that will be converted to a WHERE clause (like get, filter...) as well as Q objects, the keywords arguments are ANDed together. So there is no need in doing tags.filter(Q(key = k) & Q(value__startswith = v)) since tags.filter(key=k, value__startswith=v) performs the same query and is more readable.

Building the tags research query should already include the lookup that span relationship and can be something like:

relevant_tags = Q()  # Fallback, if the query is empty will select everything
for tag_searched in query_items:
    tag_searched = tag_searched.lstrip(':')
    try:
        tag_name, tag_value = tag_searched.split(':', 1)
    except ValueError:
        # tag_searched did not contain any :
        query = Q(tags__value__startswith=tag_searched)
    else:
        query = Q(tags__key=tag_name, tags__value__startswith=tag_value)

    relevant_tags = relevant_tags & query

And from there, we can retrieve objects we want in a single query:

items = InventoryItems.objects.filter(relevant_tags)

A few things to note:

  • if iterable just before for element in iterable is pointless as the for loop will be a no-op anyway if the iterable is empty;
  • you can use EAFP rather than LBYL, especially if you expect one path to happen more frequently than the other. I made a wild guess that the user will use the name:value syntax of the query more often than the simple value one. It is also to show an alternate syntax.
  • you can clean your query better in the list-comprehension to avoid an extra lstrip in the computation. Or you can avoid the list-comp and perform the whole cleanup in the for loop:

    for tag_searched in query.split(','):
        tag_searched = tag_searched.strip().lstrip(':')
        if not tag_searched:
            continue
        ...
    
  • you never use start or count, you should remove them.

\$\endgroup\$
  • \$\begingroup\$ If I read this code this affects an OR on the Tags rather than an AND on the tag items. So a query of "us-west-1a" "dev.test" would return 5 items (from the example) rather than 2. Now I could be reading the code wrong, but lacking any AND to cause an intersection on the result sets I'm concerned. \$\endgroup\$ – koblas Dec 6 '16 at 13:34
  • \$\begingroup\$ @koblas Let's base our reflexion on tag ids rather than values and names. Let's say that "dev.test" is 1 and "us-west-1a" is 3. So InventoryTag.objects.filter(relevant_tags) would roughtly translate to SELECT * FROM django_inventory_tag WHERE id=1 OR id=3, yielding the two tags we are interested in. We then filter InventoryItemTag and/or InventoryItems for elements that contains both ids. The AND you are looking for is hiden in tags.values_list('id'). \$\endgroup\$ – Mathias Ettinger Dec 6 '16 at 13:41
  • \$\begingroup\$ As long as there is a OR on the InventoryTag you're not getting the correct result. I have tested it. You get all items that have "us-west-1" OR "dev.test" you do not get the AND intersection of the two sets of inventory items that have both tags. \$\endgroup\$ – koblas Dec 9 '16 at 13:19
  • \$\begingroup\$ @koblas Ok, my bad, even if the tags.values_list('id') is an AND, the tag__in or tags__id__in is still a OR. Will rewrite the answer shortly \$\endgroup\$ – Mathias Ettinger Dec 9 '16 at 13:36
  • \$\begingroup\$ @koblas Done, it should be better now \$\endgroup\$ – Mathias Ettinger Dec 9 '16 at 13:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.