From d84ca4f01394f077ef4d3fd1c130fc2aa8d8b0b9 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Fri, 1 Jul 2022 15:11:43 +0200 Subject: remodel storage of tags Using a weird custom preprocessor and saving the tags as a space separated list in the database was not a good decision. A secondary table is more in line with how databases work, will probably help us with faster search later if we implement it (by creating an index for it). Additionally, this opens the possibility to have things like user-styled tags (custom color), as we can save additional information in the tag table. --- fietsboek/models/__init__.py | 2 +- fietsboek/models/track.py | 88 ++++++++++++++++++++++++++++++++------ fietsboek/templates/details.jinja2 | 2 +- fietsboek/templates/edit.jinja2 | 2 +- fietsboek/views/edit.py | 3 +- fietsboek/views/upload.py | 3 +- 6 files changed, 81 insertions(+), 19 deletions(-) diff --git a/fietsboek/models/__init__.py b/fietsboek/models/__init__.py index fb24f39..723cca9 100644 --- a/fietsboek/models/__init__.py +++ b/fietsboek/models/__init__.py @@ -12,7 +12,7 @@ import zope.sqlalchemy # ``Base.metadata`` prior to any initialization routines. from .user import User, FriendRequest, Token # flake8: noqa from .badge import Badge # flake8: noqa -from .track import Track, TrackCache, Upload # flake8: noqa +from .track import Tag, Track, TrackCache, Upload # flake8: noqa # Run ``configure_mappers`` after defining all of the models to ensure # all relationships can be setup. diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py index 16c6ff0..e9a6da5 100644 --- a/fietsboek/models/track.py +++ b/fietsboek/models/track.py @@ -38,20 +38,22 @@ from .meta import Base from .. import util -class TagBag(TypeDecorator): - """A custom type that represents a set of tags.""" - # pylint: disable=abstract-method - impl = Text - python_type = set +class Tag(Base): + """A tag is a single keyword associated with a track. - def process_bind_param(self, value, dialect): - tags = list(set(v.lower() for v in value)) - tags.sort() - return ' '.join(tags) + :ivar track_id: ID of the track that this tag belongs to. + :vartype track_id: int + :ivar tag: Actual text of the tag. + :vartype tag: str + :ivar track: The track object that this tag belongs to. + :vartype track: Track + """ + # pylint: disable=too-few-public-methods + __tablename__ = 'tags' + track_id = Column(Integer, ForeignKey("tracks.id"), primary_key=True) + tag = Column(Text, primary_key=True) - def process_result_value(self, value, dialect): - tags = set(value.split(' ')) - return tags + track = relationship('Track', back_populates='tags') class Visibility(enum.Enum): @@ -109,7 +111,7 @@ class Track(Base): :ivar visibility: Visibility of the track. :vartype visibility: Visibility :ivar tags: Tags of the track. - :vartype tags: set + :vartype tags: list[Tag] :ivar link_secret: The secret string for the share link. :vartype link_secret: str :ivar owner: Owner of the track. @@ -129,7 +131,6 @@ class Track(Base): date = Column(DateTime) gpx = Column(LargeBinary) visibility = Column(Enum(Visibility)) - tags = Column(TagBag) link_secret = Column(Text) owner = relationship('User', back_populates='tracks') @@ -137,6 +138,7 @@ class Track(Base): tagged_people = relationship('User', secondary=track_people_assoc, back_populates='tagged_tracks') badges = relationship('Badge', secondary=track_badge_assoc, back_populates='tracks') + tags = relationship('Tag', back_populates='track', cascade="all, delete-orphan") # GPX files are XML files with a lot of repeated property names. Really, it # is quite inefficient to store a whole ton of GPS points in big XML @@ -195,6 +197,64 @@ class Track(Base): self.cache.start_time = meta["start_time"] self.cache.end_time = meta["end_time"] + def text_tags(self): + """Returns a set of textual tags. + + :return: The tags of the track, as a set of strings. + :rtype: set[str] + """ + return {tag.tag for tag in self.tags} + + def add_tag(self, text): + """Add a tag with the given text. + + If a tag with the text already exists, does nothing. + + :param text: The text of the tag. + :type text: str + """ + for tag in self.tags: + if tag.tag.lower() == text.lower(): + return + self.tags.append(Tag(tag=text)) + + def remove_tag(self, text): + """Remove the tag with the given text. + + :param text: The text of the tag to remove. + :type text: str + """ + for i, tag in enumerate(self.tags): + if tag.tag.lower() == text.lower(): + break + else: + return + del self.tags[i] + + def sync_tags(self, tags): + """Syncs the track's tags with a given set of wanted tags. + + This adds and removes tags from the track as needed. + + :param tags: The wanted tags. + :type tags: set[str] + """ + my_tags = {tag.tag.lower() for tag in self.tags} + lower_tags = {tag.lower() for tag in tags} + # We cannot use the set difference methods because we want to keep the + # capitalization of the original tags when adding them, but use the + # lower-case version when comparing them. + for to_add in tags: + if to_add.lower() not in my_tags: + self.tags.append(Tag(tag=to_add)) + + to_delete = [] + for (i, tag) in enumerate(self.tags): + if tag.tag.lower() not in lower_tags: + to_delete.append(i) + for i in to_delete[::-1]: + del self.tags[i] + @property def length(self): """Returns the length of the track.. diff --git a/fietsboek/templates/details.jinja2 b/fietsboek/templates/details.jinja2 index ae07fef..2d6d55a 100644 --- a/fietsboek/templates/details.jinja2 +++ b/fietsboek/templates/details.jinja2 @@ -41,7 +41,7 @@

- {{ _("page.details.tags") }}: {% for tag in track.tags %}{{ tag }} {% endfor %} + {{ _("page.details.tags") }}: {% for tag in track.tags %}{{ tag.tag }} {% endfor %}

{% if 'secret' in request.GET %} diff --git a/fietsboek/templates/edit.jinja2 b/fietsboek/templates/edit.jinja2 index 2a77e4b..33d18f5 100644 --- a/fietsboek/templates/edit.jinja2 +++ b/fietsboek/templates/edit.jinja2 @@ -9,7 +9,7 @@