Add secondary id to tags

docs/tags.rst

@@ -19,11 +19,9 @@ A device can have many tags but a tag can only be linked to one device.
 As for the actual implementation, you cannot unlink them.
 
 Devicehub users can design, generate and print tags, manually setting
-an ID and an tag provider. Future Devicehub versions can allow
-parametrizing an ID generator.
-
-Note that these virtual tags don't have to forcefully be printed or
-have a physical representation (this is not imposed at system level).
+an ID and a tag provider. Note though that these virtual tags don't have
+to forcefully be printed or have a physical representation
+(this is not imposed at system level).
 
 eTags
 *****
@@ -33,12 +31,16 @@ by tag providers that comply with the eReuse.org requisites.
 
 The eTags are designed to empower device exchange between
 organizations and identification efficiency. They are built with durable
-plastic and have a QR code, NFC chip and a written ID.
+plastic and have a QR code, a NFC chip and a written ID.
 
 These tags live in separate databases from Devicehubs, empowered by
-the `eReuse.org Tag <https://github.com/ereuse/tag>`_. By using this
-software, eReuse.org certified tag providers can create and manage
-the tags, and send them to Devicehubs of their choice.
+the `eReuse.org Tag <https://github.com/ereuse/tag>`_ software.
+By using this software, eReuse.org certified tag providers
+can create and manage the tags, and send them to Devicehubs of their
+choice.
+
+The section *Use-case with eTags* shows the use-case of these
+eTags.
 
 Tag ID design
 =============
@@ -95,16 +97,13 @@ Getting a device through its tag
 When performing ``GET /tags/<tag-id>/device`` you will get directly the
 device of such tag, as long as there are not two tags with the same
 tag-id. In such case you should use ``GET /tags/<ngo>/<tag-id>/device``
-to inequivocally get the correct device (to develop).
+to unequivocally get the correct device (feature to develop).
 
 Tags and migrations
 *******************
 Tags travel with the devices they are linked when migrating them. Future
 implementations can parameterize this.
 
-http://t.devicetag.io/TG-1234567890
-
-
 Use-case with eTags
 *******************
 We explain the use-case of tagging a device with an :ref:`tags:eTags`,

ereuse_devicehub/dummy/dummy.py

@@ -37,7 +37,8 @@ class Dummy:
         with click_spinner.spinner():
             self.app.init_db(erase=True)
             user = self.user_client('user@dhub.com', '1234')
-            user.post(res=Tag, query=[('ids', i) for i in self.TAGS], data={})
+            for id in self.TAGS:
+                user.post({'id': id}, res=Tag)
         files = tuple(Path(__file__).parent.joinpath('files').iterdir())
         print('done.')
         with click.progressbar(files, label='Creating devices...'.ljust(28)) as bar:

ereuse_devicehub/resources/agent/models.py

@@ -6,11 +6,12 @@ from flask import current_app as app, g
 from sqlalchemy import Column, Enum as DBEnum, ForeignKey, Unicode, UniqueConstraint
 from sqlalchemy.dialects.postgresql import UUID
 from sqlalchemy.ext.declarative import declared_attr
-from sqlalchemy.orm import backref, relationship
+from sqlalchemy.orm import backref, relationship, validates
 from sqlalchemy_utils import EmailType, PhoneNumberType
 from teal import enums
 from teal.db import INHERIT_COND, POLYMORPHIC_ID, \
     POLYMORPHIC_ON
+from teal.marshmallow import ValidationError
 
 from ereuse_devicehub.resources.models import STR_SIZE, STR_SM_SIZE, Thing
 from ereuse_devicehub.resources.user.models import User
@@ -72,6 +73,12 @@ class Agent(Thing):
         # todo test
         return sorted(chain(self.events_agent, self.events_to), key=attrgetter('created'))
 
+    @validates('name')
+    def does_not_contain_slash(self, _, value: str):
+        if '/' in value:
+            raise ValidationError('Name cannot contain slash \'')
+        return value
+
     def __repr__(self) -> str:
         return '<{0.t} {0.name}>'.format(self)
 

ereuse_devicehub/resources/device/sync.py

@@ -155,7 +155,7 @@ class Sync:
             with suppress(ResourceNotFound):
                 db_device = Device.query.filter_by(hid=device.hid).one()
         try:
-            tags = {Tag.query.filter_by(id=tag.id).one() for tag in device.tags}  # type: Set[Tag]
+            tags = {Tag.from_an_id(tag.id).one() for tag in device.tags}  # type: Set[Tag]
         except ResourceNotFound:
             raise ResourceNotFound('tag you are linking to device {}'.format(device))
         linked_tags = {tag for tag in tags if tag.device_id}  # type: Set[Tag]

ereuse_devicehub/resources/lot/views.py

@@ -89,7 +89,8 @@ class LotDeviceView(LotBaseChildrenView):
     """
 
     def _post(self, lot: Lot, ids: Set[uuid.UUID]):
-        lot.devices |= self.get_ids()
+        # todo this works?
+        lot.devices |= ids
 
     def _delete(self, lot: Lot, ids: Set[uuid.UUID]):
-        lot.devices -= self.get_ids()
+        lot.devices -= ids

ereuse_devicehub/resources/tag/__init__.py

@@ -1,6 +1,8 @@
-from typing import Tuple
+import csv
+import pathlib
 
 from click import argument, option
+from ereuse_utils import cli
 from teal.resource import Resource
 from teal.teal import Teal
 
@@ -14,12 +16,19 @@ class TagDef(Resource):
     SCHEMA = schema.Tag
     VIEW = TagView
 
+    ORG_H = 'The name of an existing organization in the DB. '
+    'By default the organization operating this Devicehub.'
+    PROV_H = 'The Base URL of the provider. '
+    'By default set to the actual Devicehub.'
+    CLI_SCHEMA = schema.Tag(only=('id', 'provider', 'org', 'secondary'))
+
     def __init__(self, app: Teal, import_name=__package__, static_folder=None,
                  static_url_path=None,
                  template_folder=None, url_prefix=None, subdomain=None, url_defaults=None,
                  root_path=None):
         cli_commands = (
-            (self.create_tags, 'create-tags'),
+            (self.create_tag, 'create-tag'),
+            (self.create_tags_csv, 'create-tags-csv')
         )
         super().__init__(app, import_name, static_folder, static_url_path, template_folder,
                          url_prefix, subdomain, url_defaults, root_path, cli_commands)
@@ -27,20 +36,37 @@ class TagDef(Resource):
         self.add_url_rule('/<{}:{}>/device'.format(self.ID_CONVERTER.value, self.ID_NAME),
                           view_func=_get_device_from_tag,
                           methods={'GET'})
+        self.tag_schema = schema.Tag
 
-    @option('--org',
-            help='The name of an existing organization in the DB. '
-                 'By default the organization operating this Devicehub.')
-    @option('--provider',
-            help='The Base URL of the provider. '
-                 'By default set to the actual Devicehub.')
-    @argument('ids', nargs=-1, required=True)
-    def create_tags(self, ids: Tuple[str], org: str = None, provider: str = None):
+    @option('-o', '--org', help=ORG_H)
+    @option('-p', '--provider', help=PROV_H)
+    @option('-s', '--sec', help=Tag.secondary.comment)
+    @argument('id')
+    def create_tag(self,
+                   id: str,
+                   org: str = None,
+                   sec: str = None,
+                   provider: str = None):
         """Create TAGS and associates them to a specific PROVIDER."""
-        tag_schema = schema.Tag(only=('id', 'provider', 'org'))
-
-        db.session.add_all(
-            Tag(**tag_schema.load({'id': tag_id, 'provider': provider, 'org': org}))
-            for tag_id in ids
-        )
+        db.session.add(Tag(**self.schema.load(
+            dict(id=id, org=org, secondary=sec, provider=provider)
+        )))
+        db.session.commit()
+
+    @option('--org', help=ORG_H)
+    @option('--provider', help=PROV_H)
+    @argument('path', type=cli.Path(writable=True))
+    def create_tags_csv(self, path: pathlib.Path, org: str, provider: str):
+        """Creates tags by reading CSV from ereuse-tag.
+
+        CSV must have the following columns:
+
+        1. ID tag
+        2. Secondary id tag (or empty)
+        """
+        with path.open() as f:
+            for id, sec in csv.reader(f):
+                db.session.add(Tag(**self.schema.load(
+                    dict(id=id, org=org, secondary=sec, provider=provider)
+                )))
         db.session.commit()

ereuse_devicehub/resources/tag/model.py

@@ -1,7 +1,9 @@
+from contextlib import suppress
+
 from sqlalchemy import BigInteger, Column, ForeignKey, Unicode, UniqueConstraint
 from sqlalchemy.dialects.postgresql import UUID
 from sqlalchemy.orm import backref, relationship, validates
-from teal.db import DB_CASCADE_SET_NULL, URL
+from teal.db import DB_CASCADE_SET_NULL, Query, URL
 from teal.marshmallow import ValidationError
 
 from ereuse_devicehub.resources.agent.models import Organization
@@ -11,6 +13,7 @@ from ereuse_devicehub.resources.models import Thing
 
 class Tag(Thing):
     id = Column(Unicode(), primary_key=True)
+    id.comment = """The ID of the tag."""
     org_id = Column(UUID(as_uuid=True),
                     ForeignKey(Organization.id),
                     primary_key=True,
@@ -22,23 +25,52 @@ class Tag(Thing):
                        backref=backref('tags', lazy=True),
                        primaryjoin=Organization.id == org_id,
                        collection_class=set)
+    """The organization that issued the tag."""
     provider = Column(URL(),
                       comment='The tag provider URL. If None, the provider is this Devicehub.')
+    provider.comment = """The provider URL."""
     device_id = Column(BigInteger,
                        # We don't want to delete the tag on device deletion, only set to null
                        ForeignKey(Device.id, ondelete=DB_CASCADE_SET_NULL))
     device = relationship(Device,
                           backref=backref('tags', lazy=True, collection_class=set),
                           primaryjoin=Device.id == device_id)
+    """The device linked to this tag."""
+    secondary = Column(Unicode())
+    secondary.comment = """
+        A secondary identifier for this tag. It has the same
+        constraints as the main one.
+        
+        Only needed in special cases.
+    """
 
-    @validates('id')
+    def __init__(self, id: str, **kwargs) -> None:
+        super().__init__(id=id, **kwargs)
+
+    def like_etag(self):
+        """Checks if the tag conforms to the `eTag spec <http:
+        //devicehub.ereuse.org/tags.html#etags>`_.
+        """
+        with suppress(ValueError):
+            provider, id = self.id.split('-')
+            if len(provider) == 2 and 5 <= len(id) <= 10:
+                return True
+        return False
+
+    @classmethod
+    def from_an_id(cls, id: str) -> Query:
+        """Query to look for a tag from a possible identifier."""
+        return cls.query.filter((cls.id == id) | (cls.secondary == id))
+
+    @validates('id', 'secondary')
     def does_not_contain_slash(self, _, value: str):
         if '/' in value:
             raise ValidationError('Tags cannot contain slashes (/).')
         return value
 
     __table_args__ = (
-        UniqueConstraint(device_id, org_id, name='one_tag_per_organization'),
+        UniqueConstraint(device_id, org_id, name='one_tag_per_org'),
+        UniqueConstraint(secondary, org_id, name='one_secondary_per_org')
     )
 
     def __repr__(self) -> str:

ereuse_devicehub/resources/tag/model.pyi

@@ -3,7 +3,9 @@ from uuid import UUID
 from boltons.urlutils import URL
 from sqlalchemy import Column
 from sqlalchemy.orm import relationship
+from teal.db import Query
 
+from ereuse_devicehub.resources.agent.models import Organization
 from ereuse_devicehub.resources.device.models import Device
 from ereuse_devicehub.resources.models import Thing
 
@@ -15,11 +17,25 @@ class Tag(Thing):
     provider = ...  # type: Column
     device_id = ...  # type: Column
     device = ...  # type: relationship
+    secondary = ...  # type: Column
 
-    def __init__(self, **kwargs) -> None:
-        super().__init__(**kwargs)
+    def __init__(self, id: str,
+                 org: Organization = None,
+                 secondary: str = None,
+                 provider: URL = None,
+                 device: Device = None) -> None:
+        super().__init__()
         self.id = ...  # type: str
         self.org_id = ...  # type: UUID
+        self.org = ...  # type: Organization
         self.provider = ...  # type: URL
         self.device_id = ...  # type: int
         self.device = ...  # type: Device
+        self.secondary = ...  # type: str
+
+    @classmethod
+    def from_an_id(cls, id: str) -> Query:
+        pass
+
+    def like_etag(self) -> bool:
+        pass

ereuse_devicehub/resources/tag/schema.py

@@ -4,12 +4,20 @@ from teal.marshmallow import URL
 from ereuse_devicehub.marshmallow import NestedOn
 from ereuse_devicehub.resources.device.schemas import Device
 from ereuse_devicehub.resources.schemas import Thing
+from ereuse_devicehub.resources.tag import model as m
+
+
+def without_slash(x: str) -> bool:
+    """Returns true if x does not contain a slash."""
+    return '/' not in x
 
 
 class Tag(Thing):
-    id = String(description='The ID of the tag.',
-                validator=lambda x: '/' not in x,
+    id = String(description=m.Tag.id.comment,
+                validator=without_slash,
                 required=True)
-    provider = URL(description='The provider URL.')
-    device = NestedOn(Device, description='The device linked to this tag.')
-    org = String(description='The organization that issued the tag.')
+    provider = URL(description=m.Tag.provider.comment,
+                   validator=without_slash)
+    device = NestedOn(Device, dump_only=True)
+    org = String()
+    secondary = String(description=m.Tag.secondary.comment)

ereuse_devicehub/resources/tag/view.py

@@ -1,44 +1,22 @@
 from flask import Response, current_app as app, request
-from marshmallow.fields import List, String, URL
 from teal.marshmallow import ValidationError
-from teal.resource import Schema, View
-from webargs.flaskparser import parser
+from teal.resource import View
 
+from ereuse_devicehub.db import db
 from ereuse_devicehub.resources.device.models import Device
 from ereuse_devicehub.resources.tag import Tag
 
 
 class TagView(View):
-    class PostArgs(Schema):
-        ids = List(String(), required=True, description='A list of tags identifiers.')
-        org = String(description='The name of an existing organization in the DB. '
-                                 'If not set, the default organization is used.')
-        provider = URL(description='The Base URL of the provider. By default is this Devicehub.')
-
-    post_args = PostArgs()
 
     def post(self):
-        """
-        Creates tags.
-
-        ---
-        parameters:
-          - name: tags
-            in: path
-            description: Number of tags to create.
-        """
-        args = parser.parse(self.post_args, request, locations={'querystring'})
-        # Ensure user is not POSTing an eReuse.org tag
-        # For now only allow them to be created through command-line
-        for id in args['ids']:
-            try:
-                provider, _id = id.split('-')
-            except ValueError:
-                pass
-            else:
-                if len(provider) == 2 and 5 <= len(_id) <= 10:
-                    raise CannotCreateETag(id)
-        self.resource_def.create_tags(**args)
+        """Creates a tag."""
+        t = request.get_json()
+        tag = Tag(**t)
+        if tag.like_etag():
+            raise CannotCreateETag(tag.id)
+        db.session.add(tag)
+        db.session.commit()
         return Response(status=201)
 
 
@@ -58,13 +36,13 @@ def get_device_from_tag(id: str):
     return app.resources[Device.t].schema.jsonify(device)
 
 
-class CannotCreateETag(ValidationError):
-    def __init__(self, id: str):
-        message = 'Only sysadmin can create an eReuse.org Tag ({})'.format(id)
-        super().__init__(message)
-
-
 class TagNotLinked(ValidationError):
     def __init__(self, id):
         message = 'The tag {} is not linked to a device.'.format(id)
         super().__init__(message, field_names=['device'])
+
+
+class CannotCreateETag(ValidationError):
+    def __init__(self, id: str):
+        message = 'Only sysadmin can create an eReuse.org Tag ({})'.format(id)
+        super().__init__(message)

tests/files/tags-cli.csv

@@ -0,0 +1,4 @@
+id1,sec1
+id2,sec2
+id3,sec3
+id4,sec4

tests/test_agent.py

@@ -1,6 +1,7 @@
 from uuid import UUID
 
 import pytest
+from marshmallow import ValidationError
 from sqlalchemy_utils import PhoneNumber
 from teal.db import UniqueViolation
 from teal.enums import Country
@@ -127,3 +128,9 @@ def test_create_organization_main_method(app: Devicehub):
     assert org.name == o['name'] == 'ACME'
     assert org.tax_id == o['taxId'] == 'FOO'
     assert org.country.name == o['country'] == 'ES'
+
+
+@pytest.mark.usefixtures(app_context.__name__)
+def test_organization_no_slash_name():
+    with pytest.raises(ValidationError):
+        Organization(name='/')

tests/test_device.py

@@ -3,8 +3,10 @@ from uuid import UUID
 
 import pytest
 from colour import Color
+from ereuse_utils.naming import Naming
 from pytest import raises
 from sqlalchemy.util import OrderedSet
+from teal.db import ResourceNotFound
 
 from ereuse_devicehub.client import UserClient
 from ereuse_devicehub.db import db
@@ -20,8 +22,6 @@ from ereuse_devicehub.resources.enums import ComputerChassis, DisplayTech
 from ereuse_devicehub.resources.event.models import Remove, Test
 from ereuse_devicehub.resources.tag.model import Tag
 from ereuse_devicehub.resources.user import User
-from ereuse_utils.naming import Naming
-from teal.db import ResourceNotFound
 from tests import conftest
 
 
@@ -217,7 +217,8 @@ def test_sync_execute_register_Desktop_existing_no_tag():
     db.session.add(pc)
     db.session.commit()
 
-    pc = Desktop(**conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
+    pc = Desktop(
+        **conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
     # 1: device exists on DB
     db_pc = Sync().execute_register(pc)
     assert pc.physical_properties == db_pc.physical_properties
@@ -287,7 +288,7 @@ def test_sync_execute_register_tag_does_not_exist():
 
     Tags have to be created before trying to link them through a Snapshot.
     """
-    pc = Desktop(**conftest.file('pc-components.db')['device'], tags=OrderedSet([Tag()]))
+    pc = Desktop(**conftest.file('pc-components.db')['device'], tags=OrderedSet([Tag('foo')]))
     with raises(ResourceNotFound):
         Sync().execute_register(pc)
 
@@ -304,7 +305,8 @@ def test_sync_execute_register_tag_linked_same_device():
     db.session.add(Tag(id='foo', device=orig_pc))
     db.session.commit()
 
-    pc = Desktop(**conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
+    pc = Desktop(
+        **conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
     pc.tags.add(Tag(id='foo'))
     db_pc = Sync().execute_register(pc)
     assert db_pc.id == orig_pc.id
@@ -326,7 +328,8 @@ def test_sync_execute_register_tag_linked_other_device_mismatch_between_tags():
     db.session.add(Tag(id='foo-2', device=pc2))
     db.session.commit()
 
-    pc1 = Desktop(**conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
+    pc1 = Desktop(
+        **conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
     pc1.tags.add(Tag(id='foo-1'))
     pc1.tags.add(Tag(id='foo-2'))
     with raises(MismatchBetweenTags):
@@ -349,7 +352,8 @@ def test_sync_execute_register_mismatch_between_tags_and_hid():
     db.session.add(Tag(id='foo-2', device=pc2))
     db.session.commit()
 
-    pc1 = Desktop(**conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
+    pc1 = Desktop(
+        **conftest.file('pc-components.db')['device'])  # Create a new transient non-db object
     pc1.tags.add(Tag(id='foo-2'))
     with raises(MismatchBetweenTagsAndHid):
         Sync().execute_register(pc1)

tests/test_tag.py

@@ -1,3 +1,5 @@
+import pathlib
+
 import pytest
 from pytest import raises
 from teal.db import MultipleResourcesFound, ResourceNotFound, UniqueViolation
@@ -40,7 +42,10 @@ def test_create_tag_default_org():
 def test_create_tag_no_slash():
     """Checks that no tags can be created that contain a slash."""
     with raises(ValidationError):
-        Tag(id='/')
+        Tag('/')
+
+    with raises(ValidationError):
+        Tag('bar', secondary='/')
 
 
 @pytest.mark.usefixtures(conftest.app_context.__name__)
@@ -60,7 +65,7 @@ def test_create_two_same_tags():
 
 def test_tag_post(app: Devicehub, user: UserClient):
     """Checks the POST method of creating a tag."""
-    user.post(res=Tag, query=[('ids', 'foo')], data={})
+    user.post({'id': 'foo'}, res=Tag)
     with app.app_context():
         assert Tag.query.filter_by(id='foo').one()
 
@@ -70,16 +75,14 @@ def test_tag_post_etag(user: UserClient):
     Ensures users cannot create eReuse.org tags through POST;
     only terminal.
     """
-    user.post(res=Tag, query=[('ids', 'FO-123456')], data={}, status=CannotCreateETag)
+    user.post({'id': 'FO-123456'}, res=Tag, status=CannotCreateETag)
     # Although similar, these are not eTags and should pass
-    user.post(res=Tag, query=[
-        ('ids', 'FO-0123-45'),
-        ('ids', 'FOO012345678910'),
-        ('ids', 'FO'),
-        ('ids', 'FO-'),
-        ('ids', 'FO-123'),
-        ('ids', 'FOO-123456')
-    ], data={})
+    user.post({'id': 'FO-0123-45'}, res=Tag)
+    user.post({'id': 'FOO012345678910'}, res=Tag)
+    user.post({'id': 'FO'}, res=Tag)
+    user.post({'id': 'FO-'}, res=Tag)
+    user.post({'id': 'FO-123'}, res=Tag)
+    user.post({'id': 'FOO-123456'}, res=Tag)
 
 
 def test_tag_get_device_from_tag_endpoint(app: Devicehub, user: UserClient):
@@ -125,8 +128,38 @@ def test_tag_get_device_from_tag_endpoint_multiple_tags(app: Devicehub, user: Us
 def test_tag_create_tags_cli(app: Devicehub, user: UserClient):
     """Checks creating tags with the CLI endpoint."""
     runner = app.test_cli_runner()
-    runner.invoke(args=['create-tags', 'id1'], catch_exceptions=False)
+    runner.invoke(args=['create-tag', 'id1'], catch_exceptions=False)
     with app.app_context():
         tag = Tag.query.one()  # type: Tag
         assert tag.id == 'id1'
         assert tag.org.id == Organization.get_default_org_id()
+
+
+@pytest.mark.usefixtures(conftest.app_context.__name__)
+def test_tag_secondary():
+    """Creates and consumes tags with a secondary id."""
+    t = Tag('foo', secondary='bar')
+    db.session.add(t)
+    db.session.flush()
+    assert Tag.from_an_id('bar').one() == t
+    assert Tag.from_an_id('foo').one() == t
+    with pytest.raises(ResourceNotFound):
+        Tag.from_an_id('nope').one()
+
+
+def test_tag_create_tags_cli_csv(app: Devicehub, user: UserClient):
+    """Checks creating tags with the CLI endpoint using a CSV."""
+    csv = pathlib.Path(__file__).parent / 'files' / 'tags-cli.csv'
+    runner = app.test_cli_runner()
+    runner.invoke(args=['create-tags-csv', str(csv)],
+                  catch_exceptions=False)
+    with app.app_context():
+        t1 = Tag.from_an_id('id1').one()
+        t2 = Tag.from_an_id('sec1').one()
+        assert t1 == t2
+
+
+def test_tag_multiple_secondary_org(user: UserClient):
+    """Ensures two secondary ids cannot be part of the same Org."""
+    user.post({'id': 'foo', 'secondary': 'bar'}, res=Tag)
+    user.post({'id': 'foo1', 'secondary': 'bar'}, res=Tag, status=UniqueViolation)

tests/test_workbench.py

@@ -29,7 +29,9 @@ def test_workbench_server_condensed(user: UserClient):
     ))
     s['components'][5]['events'] = [file('workbench-server-3.erase')]
     # Create tags
-    user.post(res=Tag, query=[('ids', t['id']) for t in s['device']['tags']], data={})
+    for t in s['device']['tags']:
+        user.post({'id': t['id']}, res=Tag)
+
     snapshot, _ = user.post(res=em.Snapshot, data=s)
     events = snapshot['events']
     assert {(event['type'], event['device']) for event in events} == {