Fix inconsistencies in filtering devices inside lots

ereuse_devicehub/resources/device/views.py

@@ -5,7 +5,6 @@ from flask import current_app as app, render_template, request
 from flask.json import jsonify
 from flask_sqlalchemy import Pagination
 from marshmallow import fields, fields as f, validate as v
-from sqlalchemy.orm import aliased
 from teal import query
 from teal.cache import cache
 from teal.resource import View
@@ -46,12 +45,10 @@ class LotQ(query.Query):
 
 
 class Filters(query.Query):
-    _parent = aliased(Computer)
+    _parent = Computer.__table__.alias()
     _device_inside_lot = (Device.id == LotDevice.device_id) & (Lot.id == LotDevice.lot_id)
-    _component_inside_lot_through_parent = (Device.id == Component.id) \
+    _parent_device_in_lot = (Device.id == Component.id) & (Component.parent_id == _parent.c.id) \
-                                           & (Component.parent_id == _parent.id) \
+                            & (_parent.c.id == LotDevice.device_id) & (Lot.id == LotDevice.lot_id)
-                                           & (_parent.id == LotDevice.device_id) \
-                                           & (Lot.id == LotDevice.lot_id)
 
     type = query.Or(OfType(Device.type))
     model = query.ILike(Device.model)
@@ -59,7 +56,10 @@ class Filters(query.Query):
     serialNumber = query.ILike(Device.serial_number)
     rating = query.Join(Device.id == Rate.device_id, RateQ)
     tag = query.Join(Device.id == Tag.device_id, TagQ)
-    lot = query.Join(_device_inside_lot | _component_inside_lot_through_parent, LotQ)
+    # todo This part of the query is really slow
+    # And forces usage of distinct, as it returns many rows
+    # due to having multiple paths to the same
+    lot = query.Join(_device_inside_lot | _parent_device_in_lot, LotQ)
 
 
 class Sorting(query.Sort):
@@ -71,7 +71,7 @@ class DeviceView(View):
     class FindArgs(marshmallow.Schema):
         search = f.Raw()
         filter = f.Nested(Filters, missing=[])
-        sort = f.Nested(Sorting, missing=[])
+        sort = f.Nested(Sorting, missing=[Device.id.asc()])
         page = f.Integer(validate=v.Range(min=1), missing=1)
 
     def get(self, id):
@@ -123,7 +123,7 @@ class DeviceView(View):
     def find(self, args: dict):
         """Gets many devices."""
         search_p = args.get('search', None)
-        query = Device.query
+        query = Device.query.distinct()  # todo we should not force to do this if the query is ok
         if search_p:
             properties = DeviceSearch.properties
             tags = DeviceSearch.tags

ereuse_devicehub/resources/lot/models.py

@@ -101,14 +101,14 @@ class Lot(Thing):
             Path.path.lquery(exp.cast('*{{1}}.{}.*'.format(id), LQUERY))
         )
 
-    def __contains__(self, child: 'Lot'):
-        return Path.has_lot(self.id, child.id)
-
     @classmethod
     def roots(cls):
         """Gets the lots that are not under any other lot."""
         return cls.query.join(cls.paths).filter(db.func.nlevel(Path.path) == 1)
 
+    def __contains__(self, child: 'Lot'):
+        return Path.has_lot(self.id, child.id)
+
     def __repr__(self) -> str:
         return '<Lot {0.name} devices={0.devices!r}>'.format(self)
 

tests/test_device_find.py

@@ -4,7 +4,7 @@ from teal.utils import compiled
 from ereuse_devicehub.client import UserClient
 from ereuse_devicehub.db import db
 from ereuse_devicehub.devicehub import Devicehub
-from ereuse_devicehub.resources.device.models import Desktop, Device, Laptop, Processor, \
+from ereuse_devicehub.resources.device.models import Desktop, Device, GraphicCard, Laptop, Server, \
     SolidStateDrive
 from ereuse_devicehub.resources.device.search import DeviceSearch
 from ereuse_devicehub.resources.device.views import Filters, Sorting
@@ -56,51 +56,70 @@ def test_device_sort():
 
 @pytest.fixture()
 def device_query_dummy(app: Devicehub):
+    """
+    3 computers, where:
+
+    1. s1 Desktop with a Processor
+    2. s2 Desktop with an SSD
+    3. s3 Laptop
+    4. s4 Server with another SSD
+
+    :param app:
+    :return:
+    """
     with app.app_context():
         devices = (  # The order matters ;-)
-            Desktop(serial_number='s1',
+            Desktop(serial_number='1',
                     model='ml1',
                     manufacturer='mr1',
                     chassis=ComputerChassis.Tower),
-            Laptop(serial_number='s3',
+            Desktop(serial_number='2',
-                   model='ml3',
-                   manufacturer='mr3',
-                   chassis=ComputerChassis.Detachable),
-            Desktop(serial_number='s2',
                     model='ml2',
                     manufacturer='mr2',
                     chassis=ComputerChassis.Microtower),
-            SolidStateDrive(serial_number='s4', model='ml4', manufacturer='mr4')
+            Laptop(serial_number='3',
+                   model='ml3',
+                   manufacturer='mr3',
+                   chassis=ComputerChassis.Detachable),
+            Server(serial_number='4',
+                   model='ml4',
+                   manufacturer='mr4',
+                   chassis=ComputerChassis.Tower),
+        )
+        devices[0].components.add(
+            GraphicCard(serial_number='1-gc', model='s1ml', manufacturer='s1mr')
+        )
+        devices[1].components.add(
+            SolidStateDrive(serial_number='2-ssd', model='s2ml', manufacturer='s2mr')
+        )
+        devices[-1].components.add(
+            SolidStateDrive(serial_number='4-ssd', model='s4ml', manufacturer='s4mr')
         )
-        devices[-1].parent = devices[0]  # s4 in s1
         db.session.add_all(devices)
-
-        devices[0].components.add(Processor(model='ml5', manufacturer='mr5'))
-
         db.session.commit()
 
 
 @pytest.mark.usefixtures(device_query_dummy.__name__)
 def test_device_query_no_filters(user: UserClient):
     i, _ = user.get(res=Device)
-    assert tuple(d['type'] for d in i['items']) == (
+    assert ('1', '2', '3', '4', '1-gc', '2-ssd', '4-ssd') == tuple(
-        'Desktop', 'Laptop', 'Desktop', 'SolidStateDrive', 'Processor'
+        d['serialNumber'] for d in i['items']
     )
 
 
 @pytest.mark.usefixtures(device_query_dummy.__name__)
 def test_device_query_filter_type(user: UserClient):
     i, _ = user.get(res=Device, query=[('filter', {'type': ['Desktop', 'Laptop']})])
-    assert tuple(d['type'] for d in i['items']) == ('Desktop', 'Laptop', 'Desktop')
+    assert ('1', '2', '3') == tuple(d['serialNumber'] for d in i['items'])
 
 
 @pytest.mark.usefixtures(device_query_dummy.__name__)
 def test_device_query_filter_sort(user: UserClient):
     i, _ = user.get(res=Device, query=[
-        ('sort', {'created': Sorting.ASCENDING}),
+        ('sort', {'created': Sorting.DESCENDING}),
         ('filter', {'type': ['Computer']})
     ])
-    assert tuple(d['type'] for d in i['items']) == ('Desktop', 'Laptop', 'Desktop')
+    assert ('4', '3', '2', '1') == tuple(d['serialNumber'] for d in i['items'])
 
 
 @pytest.mark.usefixtures(device_query_dummy.__name__)
@@ -111,7 +130,7 @@ def test_device_query_filter_lots(user: UserClient):
     i, _ = user.get(res=Device, query=[
         ('filter', {'lot': {'id': [parent['id']]}})
     ])
-    assert len(i['items']) == 0, 'No devices in lot'
+    assert not i['items'], 'No devices in lot'
 
     parent, _ = user.post({},
                           res=Lot,
@@ -120,42 +139,37 @@ def test_device_query_filter_lots(user: UserClient):
     i, _ = user.get(res=Device, query=[
         ('filter', {'type': ['Computer']})
     ])
-    lot, _ = user.post({},
+    assert ('1', '2', '3', '4') == tuple(d['serialNumber'] for d in i['items'])
+    parent, _ = user.post({},
                           res=Lot,
                           item='{}/devices'.format(parent['id']),
-                       query=[('id', d['id']) for d in i['items'][:-1]])
+                          query=[('id', d['id']) for d in i['items'][:2]])
-    lot, _ = user.post({},
+    child, _ = user.post({},
                          res=Lot,
                          item='{}/devices'.format(child['id']),
-                       query=[('id', i['items'][-1]['id'])])
+                         query=[('id', d['id']) for d in i['items'][2:]])
     i, _ = user.get(res=Device, query=[
-        ('filter', {'lot': {'id': [parent['id']]}}),
+        ('filter', {'lot': {'id': [parent['id']]}})
-        ('sort', {'id': Sorting.ASCENDING})
     ])
-    assert tuple(x['id'] for x in i['items']) == (1, 2, 3, 4, 5), \
+    assert ('1', '2', '3', '4', '1-gc', '2-ssd', '4-ssd') == tuple(
-        'The parent lot contains 2 items plus indirectly the third one, and 1st device the HDD.'
+        x['serialNumber'] for x in i['items']
+    ), 'The parent lot contains 2 items plus indirectly the other ' \
+       '2 from the child lot, with all their 2 components'
 
     i, _ = user.get(res=Device, query=[
         ('filter', {'type': ['Computer'], 'lot': {'id': [parent['id']]}}),
-        ('sort', {'id': Sorting.ASCENDING})
     ])
-    assert tuple(x['id'] for x in i['items']) == (1, 2, 3)
+    assert ('1', '2', '3', '4') == tuple(x['serialNumber'] for x in i['items'])
-
     s, _ = user.get(res=Device, query=[
         ('filter', {'lot': {'id': [child['id']]}})
     ])
-    assert len(s['items']) == 1
+    assert ('3', '4', '4-ssd') == tuple(x['serialNumber'] for x in s['items'])
-    assert s['items'][0]['chassis'] == 'Microtower', 'The child lot only contains the last device.'
     s, _ = user.get(res=Device, query=[
         ('filter', {'lot': {'id': [child['id'], parent['id']]}})
     ])
-    assert all(x['id'] == id for x, id in zip(i['items'], (1, 2, 3, 4))), \
+    assert ('1', '2', '3', '4', '1-gc', '2-ssd', '4-ssd') == tuple(
-        'Adding both lots is redundant in this case and we have the 4 elements.'
+        x['serialNumber'] for x in s['items']
-    i, _ = user.get(res=Device, query=[
+    ), 'Adding both lots is redundant in this case and we have the 4 elements.'
-        ('filter', {'lot': {'id': [parent['id']]}, 'type': ['Computer']}),
-        ('sort', {'id': Sorting.ASCENDING})
-    ])
-    assert tuple(x['id'] for x in i['items']) == (1, 2, 3), 'Only computers now'
 
 
 def test_device_query(user: UserClient):

tests/test_lot.py

@@ -32,30 +32,37 @@ def test_lot_modify_patch_endpoint(user: UserClient):
     assert l_after['name'] == 'bar'
 
 
-@pytest.mark.xfail(reason='Components are not added to lots!')
+@pytest.mark.xfail(reason='the IN comparison does not work for device')
 @pytest.mark.usefixtures(conftest.auth_app_context.__name__)
 def test_lot_device_relationship():
     device = Desktop(serial_number='foo',
                      model='bar',
                      manufacturer='foobar',
                      chassis=ComputerChassis.Lunchbox)
-    lot = Lot('lot1')
+    child = Lot('child')
-    lot.devices.add(device)
+    child.devices.add(device)
-    db.session.add(lot)
+    db.session.add(child)
     db.session.flush()
 
     lot_device = LotDevice.query.one()  # type: LotDevice
     assert lot_device.device_id == device.id
-    assert lot_device.lot_id == lot.id
+    assert lot_device.lot_id == child.id
     assert lot_device.created
     assert lot_device.author_id == g.user.id
-    assert device.lots == {lot}
+    assert device.lots == {child}
-    assert device in lot
+    # todo Device IN LOT does not work
+    assert device in child
 
     graphic = GraphicCard(serial_number='foo', model='bar')
     device.components.add(graphic)
     db.session.flush()
-    assert graphic in lot
+    assert graphic in child
+
+    parent = Lot('parent')
+    db.session.add(parent)
+    db.session.flush()
+    parent.add_child(child)
+    assert child in parent
 
 
 @pytest.mark.usefixtures(conftest.auth_app_context.__name__)