Fix #119 - Add ability to use set queries on list subcommands (#133)

The use-case here is described by @jathanism in nsot#221, where single
objects should be able to be referenced via set query for the various
subcommands on `list`, such as `next_network` on `Network` objects.
Since these subcommands operate on a single object, we set `unique=True`
in the request so that the API will return an error if the set query
returns != 1 object.

The implementation for this is done in the `list` subcommand callback,
where we were already translating from a natural key into an ID to be
given to the subcommand request. The heavy lifting is done by a new
method on `pynsot.App`.

While I was in there, I refactored the old `set_query` method to have a
better name. If this is expected to create issues with people
programatically consuming pynsot, I can undo that refactor.

pynsot/app.py

@@ -568,23 +568,40 @@ def detail(self, data, resource):
         except HTTP_ERRORS as err:
             self.handle_error('detail', data, err)
 
-    def set_query(self, data, delimited=False):
+    def set_query(self, data, resource=None):
         """
-        Run a set query and return the results.
+        Get objects based on a set query for this resource.
 
         :param data:
             Dict of query parameters
 
-        :param delimited:
-            Whether to display the results as comma or newline delimited
+        :param resource:
+            (Optional) API resource object
         """
+
         self.rebase(data)
+
+        if resource is None:
+            resource = self.resource
+
         try:
-            results = self.resource.query.get(**data)
+            result = resource.query.get(**data)
         except HTTP_ERRORS as err:
             self.handle_error('list', data, err)
 
-        objects = get_result(results)
+        return get_result(result)
+
+    def natural_keys_by_query(self, data, delimited=False):
+        """
+        Run a set query and return the natural keys of the results.
+
+        :param data:
+            Dict of query parameters
+
+        :param delimited:
+            Whether to display the results as comma or newline delimited
+        """
+        objects = self.set_query(data)
         delimiter = ',' if delimited else '\n'
         self.print_by_natural_key(objects, delimiter)
 

pynsot/commands/callbacks.py

@@ -287,9 +287,18 @@ def list_subcommand(ctx, display_fields=None, my_name=None, grep_name=None,
     # because we want to maintain dynamism across resource types.
     if with_parent:
         if parent_resource_id is None:
-            parent_resource_id = get_resource_by_natural_key(
-                ctx, data, parent_resource_name, parent_resource
-            )
+            if data.get('query'):
+                # Enforce that we get a single item back from the query, the
+                # server will return a non-2xx response on != 1 items returned,
+                # which we surface as an error to the user
+                data['unique'] = True
+
+                parent_resource_id = ctx.obj.set_query(
+                        data, resource=parent_resource)[0]['id']
+            else:
+                parent_resource_id = get_resource_by_natural_key(
+                    ctx, data, parent_resource_name, parent_resource
+                )
 
         # e.g. /api/sites/1/networks/5/supernets/
         my_resource = getattr(parent_resource(parent_resource_id), my_name)

pynsot/commands/cmd_devices.py

@@ -190,7 +190,7 @@ def list(ctx, attributes, delimited, grep, hostname, id, limit, natural_key,
 
     if ctx.invoked_subcommand is None:
         if query is not None:
-            ctx.obj.set_query(data, delimited)
+            ctx.obj.natural_keys_by_query(data, delimited)
         else:
             ctx.obj.list(data, display_fields=DISPLAY_FIELDS)
 

pynsot/commands/cmd_interfaces.py

@@ -326,7 +326,7 @@ def list(ctx, attributes, delimited, device, description, grep, id, limit,
     # fallback to default behavior.
     if ctx.invoked_subcommand is None:
         if query is not None:
-            ctx.obj.set_query(data, delimited)
+            ctx.obj.natural_keys_by_query(data, delimited)
         else:
             ctx.obj.list(
                 data, display_fields=display_fields,

pynsot/commands/cmd_networks.py

@@ -290,7 +290,7 @@ def list(ctx, attributes, cidr, delimited, grep, id, include_ips,
     # fallback to default behavior.
     if ctx.invoked_subcommand is None:
         if query is not None:
-            ctx.obj.set_query(data, delimited)
+            ctx.obj.natural_keys_by_query(data, delimited)
         else:
             ctx.obj.list(data, display_fields=DISPLAY_FIELDS)
 

tests/app/test_circuits.py

@@ -7,8 +7,8 @@
 from __future__ import absolute_import, unicode_literals
 import logging
 
-from tests.fixtures import (attribute, client, config, device, interface,
-                            network, runner, site, site_client)
+from tests.fixtures import (attribute, attributes, client, config, device,
+                            interface, network, runner, site, site_client)
 from tests.fixtures.circuits import (circuit, circuit_attributes, device_a,
                                      device_z, interface_a, interface_z)
 from tests.util import assert_output, assert_outputs
@@ -180,6 +180,14 @@ def test_circuits_list_interfaces(runner, circuit, interface_a, interface_z):
         )
 
 
+def test_circuits_subcommand_query(runner, circuit):
+    """ Make sure we can run a subcommand given a unique set query """
+
+    with runner.isolated_filesystem():
+        result = runner.run('circuits list -q owner=alice interfaces')
+        assert result.exit_code == 0
+
+
 def test_circuits_remove(runner, circuit):
     """ Make sure we can remove an existing circuit """
 

tests/fixtures/__init__.py

@@ -111,29 +111,66 @@ def attribute(site_client):
 
 
 @pytest.fixture
-def device(site_client):
+def attributes(site_client):
+    """ A bunch of attributes for each resource type """
+
+    results = []
+    resources = (
+        'Circuit',
+        'Device',
+        'Interface',
+        'Network',
+    )
+
+    for r in resources:
+        attr = site_client.sites(site_client.default_site).attributes.post(
+            {'name': 'foo', 'resource_name': r}
+        )
+
+        results.append(attr)
+
+    return results
+
+
+@pytest.fixture
+def device(site_client, attributes):
     """Return a Device object."""
     return site_client.sites(site_client.default_site).devices.post(
-        {'hostname': 'foo-bar1'}
+        {
+            'hostname': 'foo-bar1',
+            'attributes': {
+                'foo': 'test_device'
+            },
+        }
     )
 
 
 @pytest.fixture
-def network(site_client):
+def network(site_client, attributes):
     """Return a Network object."""
     return site_client.sites(site_client.default_site).networks.post(
-        {'cidr': '10.20.30.0/24'}
+        {
+            'cidr': '10.20.30.0/24',
+            'attributes': {
+                'foo': 'test_network'
+            }
+        }
     )
 
 
 @pytest.fixture
-def interface(site_client, device, network):
+def interface(site_client, attributes, device, network):
     """
     Return an Interface object.
 
     Interface is bound to ``device`` with an address assigned from ``network``.
     """
     device_id = device['id']
     return site_client.sites(site_client.default_site).interfaces.post(
-        {'name': 'eth0', 'addresses': ['10.20.30.1/32'], 'device': device_id}
+        {
+            'name': 'eth0',
+            'addresses': ['10.20.30.1/32'],
+            'device': device_id,
+            'attributes': {'foo': 'test_interface'},
+        }
     )

tests/test_app.py

@@ -9,8 +9,8 @@
 
 import pytest
 
-from .fixtures import (attribute, client, config, device, network, interface,
-                       site, site_client)
+from .fixtures import (attribute, attributes, client, config, device, network,
+                       interface, site, site_client)
 from .util import CliRunner, assert_output
 
 
@@ -356,6 +356,12 @@ def test_devices_subcommands(site_client, device):
         for e in expected:
             assert e in result.output
 
+        # Lookup by set query
+        result = runner.run('devices list -q foo=test_device interfaces')
+        assert result.exit_code == 0
+        for e in expected:
+            assert e in result.output
+
 
 def test_devices_update(site_client):
     """Test ``nsot devices update``."""
@@ -644,7 +650,7 @@ def test_networks_list(site_client):
         assert 'No closing quotation' in result.output
 
 
-def test_networks_subcommands(site_client):
+def test_networks_subcommands(site_client, network):
     """Test ``nsot networks list ... <subcommand>``."""
     runner = CliRunner(site_client.config)
     with runner.isolated_filesystem():
@@ -722,6 +728,14 @@ def test_networks_subcommands(site_client):
         result = runner.run('networks list -c 1.2.3.4/32 closest_parent')
         assert_output(result, ['No such Network found'], exit_code=1)
 
+        # Test that subcommands work when given a query
+        result = runner.run('networks list -q foo=test_network parent')
+        assert result.exit_code == 0
+
+        # Queries that return >1 Network should cause an error
+        result = runner.run('networks list -q owner=jathan parent')
+        assert result.exit_code == 1
+
 
 def test_networks_allocation(site_client, device, network, interface):
     """Test network allocation-related subcommands."""
@@ -753,20 +767,23 @@ def test_networks_allocation(site_client, device, network, interface):
         assert_output(result, ['10.20.30.4', '32'])
         assert_output(result, ['10.20.30.5', '32'])
 
-        #Test strict allocations
+        # Test strict allocations
         runner.run('networks add -c 10.2.1.0/24')
         runner.run('networks add -c 10.2.1.0/25')
-        result = runner.run('networks list -c 10.2.1.0/24 next_network -p 28 -n 3 -s')
+        result = runner.run(
+            'networks list -c 10.2.1.0/24 next_network -p 28 -n 3 -s')
         assert_output(result, ['10.2.1.128', '28'])
         assert_output(result, ['10.2.1.144', '28'])
         assert_output(result, ['10.2.1.160', '28'])
 
-        #Test strict allocations for next_address
-        result = runner.run('networks list -c 10.2.1.0/24 next_address -n 3 -s')
+        # Test strict allocations for next_address
+        result = runner.run(
+            'networks list -c 10.2.1.0/24 next_address -n 3 -s')
         assert_output(result, ['10.2.1.128', '32'])
         assert_output(result, ['10.2.1.129', '32'])
         assert_output(result, ['10.2.1.130', '32'])
 
+
 def test_networks_update(site_client):
     """Test ``nsot networks update``."""
     runner = CliRunner(site_client.config)
@@ -1024,7 +1041,8 @@ def test_interfaces_subcommands(site_client, device):
         # Test addresses
         cmds = [
             'interfaces list -D %s -n eth0 -N addresses' % device_id,
-            'interfaces list -i %s:eth0 -N addresses' % device_hostname
+            'interfaces list -i %s:eth0 -N addresses' % device_hostname,
+            'interfaces list -q vlan=100 -N addresses'
         ]
 
         for cmd in cmds:
@@ -1035,7 +1053,8 @@ def test_interfaces_subcommands(site_client, device):
         # Test networks
         cmds = [
             'interfaces list -D %s -n eth0 -N networks' % device_id,
-            'interfaces list -i %s:eth0 -N networks' % device_hostname
+            'interfaces list -i %s:eth0 -N networks' % device_hostname,
+            'interfaces list -q vlan=100 -N networks'
         ]
 
         for cmd in cmds:
@@ -1046,7 +1065,8 @@ def test_interfaces_subcommands(site_client, device):
         # Test assignments
         cmds = [
             'interfaces list -D %s -n eth0 -N assignments' % device_id,
-            'interfaces list -i %s:eth0 -N assignments' % device_hostname
+            'interfaces list -i %s:eth0 -N assignments' % device_hostname,
+            'interfaces list -q vlan=100 -N assignments'
         ]
         for cmd in cmds:
             result = runner.run(cmd)