From e2cdee30e6720cfad4710e1b390b2831964518e7 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 16 Aug 2021 23:00:19 +0200 Subject: [PATCH 01/39] Add faceted custom tags foundation with JSON serialization --- CMakeLists.txt | 4 + src/library/tags/customtags.cpp | 452 ++++++++++++++++++++++++++++++++ src/library/tags/customtags.h | 180 +++++++++++++ src/library/tags/tag.cpp | 140 ++++++++++ src/library/tags/tag.h | 310 ++++++++++++++++++++++ src/mixxxapplication.cpp | 8 + src/test/customtags_test.cpp | 248 ++++++++++++++++++ src/test/librarytags_test.cpp | 205 +++++++++++++++ 8 files changed, 1547 insertions(+) create mode 100644 src/library/tags/customtags.cpp create mode 100644 src/library/tags/customtags.h create mode 100644 src/library/tags/tag.cpp create mode 100644 src/library/tags/tag.h create mode 100644 src/test/customtags_test.cpp create mode 100644 src/test/librarytags_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 26df0e64727..6d76b39242b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -852,6 +852,8 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyfactory.cpp src/track/keys.cpp src/track/keyutils.cpp + src/library/tags/customtags.cpp + src/library/tags/tag.cpp src/track/playcounter.cpp src/track/replaygain.cpp src/track/serato/beatgrid.cpp @@ -1570,6 +1572,7 @@ add_executable(mixxx-test src/test/cratestorage_test.cpp src/test/cue_test.cpp src/test/cuecontrol_test.cpp + src/test/customtags_test.cpp src/test/dbconnectionpool_test.cpp src/test/dbidtest.cpp src/test/directorydaotest.cpp @@ -1584,6 +1587,7 @@ add_executable(mixxx-test src/test/enginemastertest.cpp src/test/enginemicrophonetest.cpp src/test/enginesynctest.cpp + src/test/librarytags_test.cpp src/test/fileinfo_test.cpp src/test/frametest.cpp src/test/globaltrackcache_test.cpp diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp new file mode 100644 index 00000000000..cd681d10c6c --- /dev/null +++ b/src/library/tags/customtags.cpp @@ -0,0 +1,452 @@ +#include "library/tags/customtags.h" + +#include +#include +#include +#include + +namespace { + +Q_LOGGING_CATEGORY(kLogger, "mixxx.library.tags.CustomTags") + +} + +namespace mixxx { + +namespace library { + +namespace tags { + +bool CustomTags::isEmpty() const { + for (auto i = getFacetedTags().begin(); + i != getFacetedTags().end(); + ++i) { + if (!i.value().isEmpty()) { + return false; + } + } + return true; +} + +void CustomTags::compact() { + auto i = refFacetedTags().begin(); + while (i != refFacetedTags().end()) { + if (i.value().isEmpty()) { + i = refFacetedTags().erase(i); + } else { + ++i; + } + } +} + +bool CustomTags::validate() const { + for (auto i = getFacetedTags().begin(); + i != getFacetedTags().end(); + ++i) { + if (i.key().isEmpty()) { + // plain tags + for (auto j = i.value().begin(); + j != i.value().end(); + ++j) { + if (j.key().isEmpty() || // label must not be empty + !j.key().isValid() || + !j.value().isValid()) { + return false; + } + } + } else { + // faceted tags + if (!i.key().isValid()) { + // invalid facet + return false; + } + for (auto j = i.value().begin(); + j != i.value().end(); + ++j) { + // empty label is allowed + if (!j.key().isValid() || + !j.value().isValid()) { + return false; + } + } + } + } + return true; +} + +bool CustomTags::containsTag( + const TagLabel& label, + const TagFacet& facet) const { + const auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end()) { + return false; + } + return i.value().contains(label); +} + +int CustomTags::countTags( + const TagLabel& label, + const TagFacet& facet) const { + const auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end()) { + return 0; + } + return i.value().count(label); +} + +int CustomTags::countFacetedTags( + const TagFacet& facet) const { + const auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end()) { + return 0; + } + return i.value().size(); +} + +bool CustomTags::addOrReplaceTag( + const Tag& tag, + const TagFacet& facet) { + DEBUG_ASSERT(countTags(tag.getLabel(), facet) <= 1); + auto i = refFacetedTags().find(facet); + if (i == refFacetedTags().end()) { + i = refFacetedTags().insert(facet, TagMap{}); + } + DEBUG_ASSERT(!tag.getLabel().isEmpty() || i.value().size() <= 1); + DEBUG_ASSERT(i.value().count(tag.getLabel()) <= 1); + auto j = i.value().find(tag.getLabel()); + if (j == i.value().end()) { + j = i.value().insert(tag.getLabel(), tag.getScore()); + return true; + } + if (j.value() == tag.getScore()) { + return false; + } + j.value() = tag.getScore(); + return true; +} + +bool CustomTags::removeTag( + const TagLabel& label, + const TagFacet& facet) { + DEBUG_ASSERT(countTags(label, facet) <= 1); + const auto i = refFacetedTags().find(facet); + if (i == refFacetedTags().end()) { + return false; + } + DEBUG_ASSERT(!label.isEmpty() || i.value().size() <= 1); + DEBUG_ASSERT(i.value().count(label) <= 1); + if (i.value().remove(label) > 0) { + if (i.value().isEmpty()) { + // Compact entry for this facet, i.e. remove it entirely + refFacetedTags().erase(i); + } + return true; + } else { + return false; + } +} + +TagVector CustomTags::getTags() const { + TagVector tags; + const auto i = getFacetedTags().find(TagFacet()); + if (i == getFacetedTags().end()) { + return tags; + } + tags.reserve(i.value().size()); + for (auto j = i.value().begin(); + j != i.value().end(); + ++j) { + tags += Tag(j.key(), j.value()); + } + return tags; +} + +FacetTagMap::iterator CustomTags::replaceAllFacetedTags( + const TagFacet& facet, + const Tag& tag) { + DEBUG_ASSERT(!facet.isEmpty()); + DEBUG_ASSERT(countTags(tag.getLabel(), facet) <= 1); + auto i = refFacetedTags().find(facet); + if (i == refFacetedTags().end()) { + i = refFacetedTags().insert(facet, TagMap{}); + } else { + i.value().clear(); + } + i.value().insert(tag.getLabel(), tag.getScore()); + DEBUG_ASSERT(countFacetedTags(facet) == 1); + return i; +} + +int CustomTags::removeAllFacetedTags( + const TagFacet& facet) { + DEBUG_ASSERT(!facet.isEmpty()); + return refFacetedTags().remove(facet); +} + +TagVector CustomTags::getFacetedTagsOrdered( + const TagFacet& facet) const { + DEBUG_ASSERT(!facet.isEmpty()); + TagVector tags; + auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end()) { + return tags; + } + tags.reserve(i.value().size()); + for (auto j = i.value().begin(); j != i.value().end(); ++j) { + tags += Tag(j.key(), j.value()); + } + DEBUG_ASSERT(tags.size() == countFacetedTags(facet)); + std::sort( + tags.begin(), + tags.end(), + [](const Tag& lhs, const Tag& rhs) { + return lhs.getScore() > rhs.getScore(); + }); + return tags; +} + +TagLabel CustomTags::getFacetedTagLabel( + const TagFacet& facet) const { + DEBUG_ASSERT(countFacetedTags(facet) <= 1); + const auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end() || + i.value().isEmpty()) { + return TagLabel(); + } + DEBUG_ASSERT(i.value().size() == 1); + DEBUG_ASSERT(i.value().first() == TagScore()); + return i.value().firstKey(); +} + +std::optional CustomTags::getTagScore( + const TagFacet& facet, + const TagLabel& label) const { + DEBUG_ASSERT(countFacetedTags(facet) <= 1); + const auto i = getFacetedTags().find(facet); + if (i == getFacetedTags().end() || + i.value().isEmpty()) { + return std::nullopt; + } + DEBUG_ASSERT(!label.isEmpty() || i.value().size() <= 1); + DEBUG_ASSERT(i.value().count(label) <= 1); + const auto j = i.value().find(label); + if (j == i.value().end()) { + return std::nullopt; + } + return j.value(); +} + +Tag CustomTags::mergeFacetedTags( + const TagFacet& facet, + AggregateScoring aggregateScoring, + const QString& joinLabelSeparator) const { + DEBUG_ASSERT(!facet.isEmpty()); + const auto tags = getFacetedTagsOrdered(facet); + if (tags.isEmpty()) { + return Tag(); + } + TagScore::value_t scoreValue; + switch (aggregateScoring) { + case AggregateScoring::Maximum: + scoreValue = tags.first().getScore(); + break; + case AggregateScoring::Average: + scoreValue = 0.0; + for (const auto& tag : tags) { + scoreValue += tag.getScore() - TagScore::kMinValue; + } + scoreValue = TagScore::kMinValue + scoreValue / tags.size(); + DEBUG_ASSERT(TagScore::isValidValue(scoreValue)); + break; + default: + DEBUG_ASSERT(!"unreachable"); + scoreValue = TagScore(); + } + QStringList labels; + labels.reserve(tags.size()); + for (const auto& tag : tags) { + labels += tag.getLabel(); + } + auto labelValue = + TagLabel::clampValue(labels.join(joinLabelSeparator)); + return Tag( + TagLabel(std::move(labelValue)), + TagScore(scoreValue)); +} + +bool CustomTags::addOrReplaceAllTags( + const CustomTags& tags) { + bool modified = false; + VERIFY_OR_DEBUG_ASSERT(this != &tags) { + return false; + } + for (auto i = tags.getFacetedTags().begin(); + i != tags.getFacetedTags().end(); + ++i) { + for (auto j = i.value().begin(); + j != i.value().end(); + ++j) { + modified |= addOrReplaceTag( + Tag(j.key(), j.value()), + i.key()); + } + } + return modified; +} + +//static +std::optional CustomTags::fromJsonObject( + const QJsonObject& jsonObject, + bool strict) { + CustomTags customTags; + for (auto i = jsonObject.begin(); + i != jsonObject.end(); + ++i) { + auto facetValue = TagFacet::filterEmptyValue(i.key()); + if (!TagFacet::isValidValue(facetValue)) { + VERIFY_OR_DEBUG_ASSERT(!strict) { + qCWarning(kLogger) + << "Invalid facet" + << facetValue + << "from JSON"; + return std::nullopt; + } + qCWarning(kLogger) + << "Skipping invalid facet" + << facetValue + << "from JSON"; + continue; + } + const auto facet = TagFacet(std::move(facetValue)); + DEBUG_ASSERT(i.value().isArray()); + const auto jsonArray = i.value().toArray(); + if (jsonArray.isEmpty()) { + // Add an empty placeholder slot just for the facet. + // This is behavior intended and needed, i.e. when + // loading a set of predefined tags from a JSON file. + // Some entries may only contains the name of the facet, + // but no predefined tags/labels. The facet will be + // displayed in the UI with the option to add custom + // labels. + customTags.touchFacet(facet); + continue; + } + for (const auto& jsonValue : jsonArray) { + const auto tag = Tag::fromJsonValue(jsonValue); + if (!tag || + (*tag != Tag() && !tag->isValid())) { + VERIFY_OR_DEBUG_ASSERT(!strict) { + qCWarning(kLogger) + << "Invalid tag" + << jsonValue + << "from JSON for facet" + << facet.value(); + return std::nullopt; + } + qCWarning(kLogger) + << "Skipping invalid tag" + << jsonValue + << "from JSON for facet" + << facet.value(); + continue; + } + customTags.addOrReplaceTag( + std::move(*tag), + facet); + } + } + return customTags; +} + +QJsonObject CustomTags::toJsonObject( + ToJsonMode mode) const { + QJsonObject jsonObject; + for (auto i = getFacetedTags().begin(); + i != getFacetedTags().end(); + ++i) { + QJsonArray jsonArray; + for (auto j = i.value().begin(); + j != i.value().end(); + ++j) { + jsonArray += Tag(j.key(), j.value()).toJsonValue(); + } + if (jsonArray.isEmpty() && + mode == ToJsonMode::Compact) { + // Skip empty plain tags in compact mode + continue; + } + jsonObject.insert(i.key(), jsonArray); + } + return jsonObject; +} + +//static +std::optional CustomTags::parseJsonData( + const QByteArray& jsonData) { + if (jsonData.isEmpty()) { + qCWarning(kLogger) + << "Failed to parse empty JSON data"; + return std::nullopt; + } + QJsonParseError parseError; + const auto jsonDoc = QJsonDocument::fromJson(jsonData, &parseError); + // QJsonDocument::fromJson() returns a non-null document + // if parsing succeeds and otherwise null on error. The + // parse error must only be evaluated if the returned + // document is null! + if (jsonDoc.isNull() && + parseError.error != QJsonParseError::NoError) { + qCWarning(kLogger) + << "Failed to parse JSON data:" + << parseError.errorString() + << "at offset" + << parseError.offset; + return std::nullopt; + } + if (!jsonDoc.isObject()) { + qCWarning(kLogger) + << "Invalid JSON document" + << jsonDoc; + return std::nullopt; + } + const auto jsonObject = jsonDoc.object(); + auto customTags = CustomTags::fromJsonObject(jsonObject); + if (!customTags) { + qCWarning(kLogger) + << "Invalid JSON object" + << jsonObject; + return std::nullopt; + } + if (!customTags->validate()) { + qCWarning(kLogger) + << "Validation failed" + << *customTags; + return std::nullopt; + } + return customTags; +} + +QByteArray CustomTags::dumpJsonData() const { + return QJsonDocument(toJsonObject()).toJson(QJsonDocument::Compact); +} + +bool operator==( + const CustomTags& lhs, + const CustomTags& rhs) { + return lhs.getFacetedTags() == rhs.getFacetedTags(); +} + +QDebug operator<<( + QDebug dbg, + const CustomTags& arg) { + dbg << "CustomTags{"; + arg.dbgFacetedTags(dbg); + dbg << '}'; + return dbg; +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h new file mode 100644 index 00000000000..db22fab3b85 --- /dev/null +++ b/src/library/tags/customtags.h @@ -0,0 +1,180 @@ +#pragma once + +#include + +#include "library/tags/tag.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +typedef QMap TagMap; +typedef QMap FacetTagMap; + +/// Custom tags +/// +/// Each custom tag is represented by a triple: a facet, a label, and a score: +/// +/// - The *facet* is a lowercase, non-empty ASCII string without whitespace +/// - The *label* is non-empty free text +/// - The *score* is a floating-point value in the interval [0.0, 1.0] +/// +/// Both *facet* and *label* are optional (= nullable), but at least one of them must be present. +/// The *score* is mandatory and is supposed to be interpreted as a *weight*, *degree of cohesion*, +/// or *normalized level* with a default value of 1.0. +/// +/// The following combinations are allowed, missing values are indicated by `-`: +/// +/// - `(-, label, score)`, e.g. `label` = "Top 40", `score` = 1.0 for tagging a track with +/// the label "Top 40" and full (= no particular, i.e. binary) score. Instead of using +/// a score of 0.0 you would simply remove the entire tag then. +/// - `(facet, -, score)`, e.g. `facet` = "energy", `score` = 0.8 for tagging a track with +/// a fairly high energy level. +/// - `(facet, label, score)`, e.g. `facet` = "genre", `label` = "Hip Hop/Rap", `score` = 0.3 +/// for tagging a track that resembles the genre "Hip Hop/Rap" to a fairly low amount and +/// might be a candidate during a genre change. The "genre" tag with the highest score will +/// represent the main genre and the score defines an ordering between multiple tags of the +/// same facet. +/// +/// The tuple (`facet`, `label`) defines a uniqueness constraint among all custom tags, i.e. within +/// an instance of `CustomTags` which is basically two nested maps. The 1st key is `facet` (nullable) +/// and the 2nd key is `label` (nullable). +class CustomTags final { + MIXXX_DECL_PROPERTY(FacetTagMap, facetedTags, FacetedTags) + + public: + CustomTags() = default; + CustomTags(CustomTags&&) = default; + CustomTags(const CustomTags&) = default; + CustomTags& operator=(CustomTags&&) = default; + CustomTags& operator=(const CustomTags&) = default; + + /// Purge all empty and unused map entries + void compact(); + + /// Check for consistency + bool validate() const; + + bool isEmpty() const; + + bool containsFacet( + const TagFacet& facet) const { + return getFacetedTags().contains(facet); + } + /// Add an (empty) entry for the given facet if it does not exist yet. + /// + /// Existing entries are not affected. + void touchFacet( + const TagFacet& facet) { + refFacetedTags()[facet]; + DEBUG_ASSERT(containsFacet(facet)); + } + + bool containsTag( + const TagLabel& label, + const TagFacet& facet = TagFacet()) const; + int countTags( + const TagLabel& label, + const TagFacet& facet = TagFacet()) const; + + int countTags() const { + return countFacetedTags(TagFacet()); + } + TagMap& refTags() { + return refFacetedTags()[TagFacet()]; + } + + int countFacetedTags( + const TagFacet& facet) const; + TagMap& refFacetedTags( + const TagFacet& facet) { + return refFacetedTags()[facet]; + } + + bool addOrReplaceTag( + const Tag& tag, + const TagFacet& facet = TagFacet()); + bool removeTag( + const TagLabel& label, + const TagFacet& facet = TagFacet()); + + bool addOrReplaceAllTags( + const CustomTags& tags); + + // Get all plain tags as a list (in no particular order) + TagVector getTags() const; + + // Replace all existing tags of this facet with a single + // faceted tag or insert a new faceted tag. + FacetTagMap::iterator replaceAllFacetedTags( + const TagFacet& facet, + const Tag& tag); + int removeAllFacetedTags( + const TagFacet& facet); + // Get all tags of a given facet sorted by score in descending order + TagVector getFacetedTagsOrdered( + const TagFacet& facet) const; + + // Get the label of a single faceted tag, i.e. that + // occurs at most once and has no custom score. If the + // facet is unknown/absent an empty label is returned. + TagLabel getFacetedTagLabel( + const TagFacet& facet) const; + + // Get the score of a tag if present. + std::optional getTagScore( + const TagFacet& facet, + const TagLabel& label = TagLabel()) const; + + enum class AggregateScoring { + Maximum, + Average, + }; + + // Merge the score and label of all faceted tags into + // a single plain tag. The strings of the labels are joined + // with a separator in between and the scores are aggregated. + Tag mergeFacetedTags( + const TagFacet& facet, + AggregateScoring aggregateScoring, + const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const; + TagLabel joinFacetedTagsLabel( + const TagFacet& facet, + const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const { + return mergeFacetedTags( + facet, + AggregateScoring::Maximum, + joinLabelSeparator) + .getLabel(); + } + + static std::optional fromJsonObject( + const QJsonObject& jsonObject, + bool strict = false); + enum class ToJsonMode { + Plain, + Compact, + }; + QJsonObject toJsonObject( + ToJsonMode mode = ToJsonMode::Compact) const; + + static std::optional parseJsonData( + const QByteArray& jsonData); + QByteArray dumpJsonData() const; +}; + +bool operator==(const CustomTags& lhs, const CustomTags& rhs); + +inline bool operator!=(const CustomTags& lhs, const CustomTags& rhs) { + return !(lhs == rhs); +} + +QDebug operator<<(QDebug dbg, const CustomTags& arg); + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/library/tags/tag.cpp b/src/library/tags/tag.cpp new file mode 100644 index 00000000000..0118865b86e --- /dev/null +++ b/src/library/tags/tag.cpp @@ -0,0 +1,140 @@ +#include "library/tags/tag.h" + +#include +#include + +namespace { + +const QRegularExpression kLowercaseAsciiNotEmpty( + QStringLiteral("^[\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); +const QRegularExpression kInverseLowercaseAsciiNotEmpty( + QStringLiteral("[^\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); + +} // anonymous namespace + +namespace mixxx { + +namespace library { + +namespace tags { + +//static +bool TagFacet::isValidValue( + const value_t& value) { + if (value.isNull()) { + return true; + } + if (value.isEmpty()) { + // for disambiguation with null + return false; + } + const auto match = kLowercaseAsciiNotEmpty.match(value); + DEBUG_ASSERT(match.isValid()); + DEBUG_ASSERT(value.length() > 0); + // match = exact match + return match.capturedLength() == value.length(); +} + +//static +TagFacet::value_t TagFacet::clampValue( + const value_t& value) { + auto clampedValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); + DEBUG_ASSERT(isValidValue(clampedValue)); + return clampedValue; +} + +//static +bool TagLabel::isValidValue( + const value_t& value) { + if (value.isNull()) { + return true; + } + if (value.isEmpty()) { + // for disambiguation with null + return false; + } + return value.trimmed() == value; +} + +//static +TagLabel::value_t TagLabel::clampValue( + const value_t& value) { + auto clampedValue = filterEmptyValue(value.trimmed()); + DEBUG_ASSERT(isValidValue(clampedValue)); + return clampedValue; +} + +bool operator==( + const Tag& lhs, + const Tag& rhs) { + return lhs.getLabel() == rhs.getLabel() && + lhs.getScore() == rhs.getScore(); +} + +QDebug operator<<( + QDebug dbg, + const Tag& arg) { + dbg << "Tag{"; + arg.dbgLabel(dbg); + arg.dbgScore(dbg); + dbg << '}'; + return dbg; +} + +std::optional Tag::fromJsonValue( + const QJsonValue& jsonValue) { + if (jsonValue.isString()) { + // label: string + auto labelValue = TagLabel::filterEmptyValue(jsonValue.toString()); + if (TagLabel::isValidValue(labelValue)) { + return Tag(TagLabel(std::move(labelValue))); + } + } else if (jsonValue.isDouble()) { + // score: number + auto scoreValue = jsonValue.toDouble(); + if (TagScore::isValidValue(scoreValue)) { + return Tag(TagScore(scoreValue)); + } + } else if (jsonValue.isArray()) { + // [label: string, score: number] + auto jsonArray = jsonValue.toArray(); + if (jsonArray.size() == 2 && + jsonArray.at(0).isString() && + jsonArray.at(1).isDouble()) { + auto labelValue = TagLabel::filterEmptyValue(jsonArray.at(0).toString()); + auto scoreValue = jsonArray.at(1).toDouble(); + if (TagLabel::isValidValue(labelValue) && + TagScore::isValidValue(scoreValue)) { + return Tag( + TagLabel(std::move(labelValue)), + TagScore(scoreValue)); + } + } + } + return std::nullopt; +} + +QJsonValue Tag::toJsonValue() const { + if (hasLabel()) { + // Regular plain tag + DEBUG_ASSERT(isValid()); + if (getScore() != TagScore()) { + return QJsonArray{ + QJsonValue{getLabel()}, + QJsonValue{getScore()}}; + } else { + // Omit the default score + return QJsonValue{getLabel()}; + } + } else { + // Empty label, may only happen when part of a faceted tag + DEBUG_ASSERT(getLabel().isEmpty()); + return QJsonValue{getScore()}; + } +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h new file mode 100644 index 00000000000..94ca8db997d --- /dev/null +++ b/src/library/tags/tag.h @@ -0,0 +1,310 @@ +#pragma once + +#include +#include + +#include "util/assert.h" +#include "util/macros.h" +#include "util/math.h" +#include "util/optional.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +/// A category for tags. +/// +/// Constraints: Lowercase ASCII, no whitespace characters +class TagFacet final { + public: + typedef QString value_t; + + static bool isValidValue( + const value_t& value); + + /// Converts the given string into lowercase and then + /// removes all whitespace and non-ASCII characters. + static value_t clampValue( + const value_t& value); + + /// Ensure that empty values are always null + static value_t filterEmptyValue( + const value_t& value) { + return value.isEmpty() ? value_t() : value; + } + + explicit TagFacet( + const value_t& value = value_t()) + : m_value(value) { + // Full validation not possible due to static constants with + // regular expressions that are inaccessible when creating + // static constants of this type! + DEBUG_ASSERT(filterEmptyValue(m_value) == m_value); + } + TagFacet(const TagFacet&) = default; + TagFacet(TagFacet&&) = default; + + TagFacet& operator=(const TagFacet&) = default; + TagFacet& operator=(TagFacet&&) = default; + + bool isValid() const { + return isValidValue(m_value); + } + + bool isEmpty() const { + DEBUG_ASSERT(isValid()); + return m_value.isEmpty(); + } + + const value_t& value() const { + DEBUG_ASSERT(isValid()); + return m_value; + } + operator const value_t&() const { + return value(); + } + + private: + value_t m_value; +}; + +inline bool operator==( + const TagFacet& lhs, + const TagFacet& rhs) { + return lhs.value() == rhs.value(); +} + +inline bool operator!=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs == rhs); +} + +inline bool operator<( + const TagFacet& lhs, + const TagFacet& rhs) { + return lhs.value() < rhs.value(); +} + +inline bool operator>( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs < rhs); +} + +inline bool operator<=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs > rhs); +} + +inline bool operator>=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs < rhs); +} + +inline uint qHash( + const TagFacet& facet, + uint seed = 0) { + return qHash(facet.value(), seed); +} + +/// The name/title of a tag. +/// +/// Constraints: No leading/trailing whitespace +class TagLabel final { + public: + typedef QString value_t; + + static bool isValidValue( + const value_t& value); + + static value_t clampValue( + const value_t& value); + + /// Ensure that empty values are always null + static value_t filterEmptyValue( + const value_t& value) { + return value.isEmpty() ? value_t() : value; + } + + explicit TagLabel( + const value_t& value = value_t()) + : m_value(value) { + DEBUG_ASSERT(isValid()); + } + TagLabel(const TagLabel&) = default; + TagLabel(TagLabel&&) = default; + + TagLabel& operator=(const TagLabel&) = default; + TagLabel& operator=(TagLabel&&) = default; + + bool isValid() const { + return isValidValue(m_value); + } + + bool isEmpty() const { + return m_value.isEmpty(); + } + + constexpr const value_t& value() const { + return m_value; + } + constexpr operator const value_t&() const { + return value(); + } + + private: + value_t m_value; +}; + +inline bool operator==( + const TagLabel& lhs, + const TagLabel& rhs) { + return lhs.value() == rhs.value(); +} + +inline bool operator!=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs == rhs); +} + +inline bool operator<( + const TagLabel& lhs, + const TagLabel& rhs) { + return lhs.value() < rhs.value(); +} + +inline bool operator>( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs < rhs); +} + +inline bool operator<=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs > rhs); +} + +inline bool operator>=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs < rhs); +} + +inline uint qHash( + const TagLabel& label, + uint seed = 0) { + return qHash(label.value(), seed); +} + +/// The score or weight of a tag relationship, defaulting to 1.0. +/// +/// Constraints: Normalized to the unit interval [0.0, 1.0] +class TagScore final { + public: + typedef double value_t; + + static constexpr value_t kMinValue = 0.0; + static constexpr value_t kMaxValue = 1.0; + static constexpr value_t kDefaultValue = kMaxValue; + + static constexpr bool isValidValue( + value_t value) { + return value >= kMinValue && value <= kMaxValue; + } + + static value_t clampValue( + value_t value) { + return math_min(kMaxValue, math_max(kMinValue, value)); + } + + explicit TagScore( + value_t value = kDefaultValue) + : m_value(value) { + DEBUG_ASSERT(isValid()); + } + + constexpr bool isValid() const { + return isValidValue(m_value); + } + + constexpr value_t value() const { + return m_value; + } + constexpr operator value_t() const { + return value(); + } + + private: + value_t m_value; +}; + +// A plain tag with label and score. +class Tag final { + // Properties + MIXXX_DECL_PROPERTY(TagLabel, label, Label) + MIXXX_DECL_PROPERTY(TagScore, score, Score) + + public: + explicit Tag( + const TagLabel& label, + TagScore score = TagScore()) + : m_label(label), + m_score(score) { + } + explicit Tag( + TagScore score = TagScore()) + : m_score(score) { + } + Tag(const Tag&) = default; + Tag(Tag&&) = default; + + Tag& operator=(Tag&&) = default; + Tag& operator=(const Tag&) = default; + + bool isValid() const { + return getLabel().isValid() && + getScore().isValid(); + } + + bool hasLabel() const { + return !m_label.isEmpty(); + } + + static std::optional fromJsonValue( + const QJsonValue& jsonValue); + + QJsonValue toJsonValue() const; +}; + +bool operator==( + const Tag& lhs, + const Tag& rhs); + +inline bool operator!=( + const Tag& lhs, + const Tag& rhs) { + return !(lhs == rhs); +} + +QDebug operator<<(QDebug dbg, const Tag& arg); + +typedef QVector TagVector; + +} // namespace tags + +} // namespace library + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::library::tags::TagFacet) +Q_DECLARE_METATYPE(mixxx::library::tags::TagLabel) +Q_DECLARE_METATYPE(mixxx::library::tags::TagScore) +Q_DECLARE_METATYPE(mixxx::library::tags::Tag) +Q_DECLARE_METATYPE(mixxx::library::tags::TagVector) diff --git a/src/mixxxapplication.cpp b/src/mixxxapplication.cpp index 4e3c014dc4c..d6d8758a923 100644 --- a/src/mixxxapplication.cpp +++ b/src/mixxxapplication.cpp @@ -7,6 +7,7 @@ #include "audio/frame.h" #include "audio/types.h" #include "control/controlproxy.h" +#include "library/tags/tag.h" #include "library/trackset/crate/crateid.h" #include "moc_mixxxapplication.cpp" #include "soundio/soundmanagerutil.h" @@ -94,6 +95,13 @@ void MixxxApplication::registerMetaTypes() { qRegisterMetaType(); QMetaType::registerComparators(); + // Library: Tags + qRegisterMetaType("mixxx::library::tags::TagFacet"); + qRegisterMetaType("mixxx::library::tags::TagLabel"); + qRegisterMetaType("mixxx::library::tags::TagScore"); + qRegisterMetaType("mixxx::library::tags::Tag"); + qRegisterMetaType("mixxx::library::tags::TagVector"); + // Various custom data types qRegisterMetaType("mixxx::ReplayGain"); qRegisterMetaType("mixxx::cache_key_t"); diff --git a/src/test/customtags_test.cpp b/src/test/customtags_test.cpp new file mode 100644 index 00000000000..66e31714ad2 --- /dev/null +++ b/src/test/customtags_test.cpp @@ -0,0 +1,248 @@ +#include "library/tags/customtags.h" + +#include + +#include +#include +#include + +namespace mixxx { + +namespace library { + +namespace tags { + +class CustomTagsTest : public testing::Test { +}; + +TEST_F(CustomTagsTest, isValidFacetValue) { + EXPECT_TRUE(TagFacet::isValidValue(QString())); + EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1"))); + EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("with\\backslash"))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("Uppercase"))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1_and_whitespace "))); + EXPECT_FALSE(TagFacet::isValidValue(QLatin1String(""))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral(" "))); +} + +TEST_F(CustomTagsTest, clampFacetValue) { + EXPECT_EQ(QString(), TagFacet::clampValue(QString())); + EXPECT_EQ( + QString("lower-case_with_digit_1_and_whitespaces"), + TagFacet::clampValue(QStringLiteral(" lower -\tcase_with _digit_1_and_whitespaces "))); + EXPECT_EQ(QString(), TagFacet::clampValue(QStringLiteral(" \n \t \r"))); + EXPECT_EQ(QString("uppercase"), TagFacet::clampValue(QStringLiteral("Uppercase"))); + EXPECT_EQ(QString("with\\back/slash"), + TagFacet::clampValue(QStringLiteral("with \\ back/\tslash"))); +} + +TEST_F(CustomTagsTest, isValidLabelValue) { + EXPECT_TRUE(TagLabel::isValidValue(QString())); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("lower-case_with_digit_1"))); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("With\\backslash"))); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("Uppercase with\twhitespace"))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" With leading space"))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral("With trailing space\n"))); + EXPECT_FALSE(TagLabel::isValidValue(QLatin1String(""))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" "))); +} + +TEST_F(CustomTagsTest, isEmptyAndCount) { + const auto facet = TagFacet(QStringLiteral("facet")); + const auto label = TagLabel(QStringLiteral("Label")); + + CustomTags customTags; + EXPECT_TRUE(customTags.isEmpty()); + EXPECT_EQ(0, customTags.countTags(label)); + EXPECT_EQ(0, customTags.countTags(label, TagFacet())); + EXPECT_EQ(0, customTags.countTags(label, facet)); + EXPECT_EQ(0, customTags.countFacetedTags(facet)); + + customTags.addOrReplaceTag(Tag(label)); + EXPECT_FALSE(customTags.isEmpty()); + EXPECT_EQ(1, customTags.countTags(label)); + EXPECT_EQ(1, customTags.countTags(label, TagFacet())); + EXPECT_EQ(0, customTags.countTags(label, facet)); + EXPECT_EQ(0, customTags.countFacetedTags(facet)); + + customTags.removeTag(label); + EXPECT_TRUE(customTags.isEmpty()); + EXPECT_EQ(0, customTags.countTags(label)); + EXPECT_EQ(0, customTags.countTags(label, TagFacet())); + EXPECT_EQ(0, customTags.countTags(label, facet)); + EXPECT_EQ(0, customTags.countFacetedTags(facet)); + + customTags.addOrReplaceTag(Tag(label), facet); + EXPECT_FALSE(customTags.isEmpty()); + EXPECT_EQ(0, customTags.countTags(label)); + EXPECT_EQ(0, customTags.countTags(label, TagFacet())); + EXPECT_EQ(1, customTags.countTags(label, facet)); + EXPECT_EQ(1, customTags.countFacetedTags(facet)); + + customTags.removeTag(label, facet); + EXPECT_TRUE(customTags.isEmpty()); + EXPECT_EQ(0, customTags.countTags(label)); + EXPECT_EQ(0, customTags.countTags(label, TagFacet())); + EXPECT_EQ(0, customTags.countTags(label, facet)); + EXPECT_EQ(0, customTags.countFacetedTags(facet)); +} + +TEST_F(CustomTagsTest, isEmptyWithEmptyFacetedSlot) { + const auto facet = TagFacet(QStringLiteral("facet")); + + CustomTags customTags; + ASSERT_EQ(0, customTags.getFacetedTags().size()); + ASSERT_TRUE(customTags.isEmpty()); + + // Add an empty placeholder slot just for the facet + customTags.refFacetedTags(facet); + + // Verify that an empty slot for faceted tags does + // not affect the emptiness check. + EXPECT_EQ(1, customTags.getFacetedTags().size()); + EXPECT_TRUE(customTags.isEmpty()); + + // Add an empty placeholder slot with an empty facet + customTags.refFacetedTags(TagFacet()); + + // Verify that an empty slot for an empty facet does + // not affect the emptiness check. + EXPECT_EQ(2, customTags.getFacetedTags().size()); + EXPECT_TRUE(customTags.isEmpty()); +} + +TEST_F(CustomTagsTest, encodeDecodeJsonRoundtripEmpty) { + // Encode default/empty JSON + EXPECT_EQ(QJsonObject{}, + CustomTags().toJsonObject(CustomTags::ToJsonMode::Plain)); + EXPECT_EQ(QJsonObject{}, + CustomTags().toJsonObject(CustomTags::ToJsonMode::Compact)); + EXPECT_EQ(QJsonObject{}, + CustomTags().toJsonObject(/*default*/)); + + // JSON with empty arrays + const auto customTagsJson = QJsonObject{ + { + "", + QJsonArray{}, + }, + { + "facet", + QJsonArray{}, + }, + }; + + // Decode JSON with empty arrays + const QByteArray jsonData = + QJsonDocument{customTagsJson}.toJson(QJsonDocument::Compact); + const auto jsonDoc = + QJsonDocument::fromJson(jsonData); + std::optional decoded = + CustomTags::fromJsonObject(jsonDoc.object()); + ASSERT_TRUE(static_cast(decoded)); + ASSERT_TRUE(decoded->validate()); + // Empty arrays shall be preserved by decoding + EXPECT_EQ(2, decoded->getFacetedTags().size()); + + // Re-encode JSON with empty arrays + EXPECT_EQ(customTagsJson, + decoded->toJsonObject(CustomTags::ToJsonMode::Plain)); + EXPECT_EQ(QJsonObject{}, + decoded->toJsonObject(CustomTags::ToJsonMode::Compact)); + EXPECT_EQ(QJsonObject{}, + decoded->toJsonObject(/*default*/)); + + // Re-encode JSON with empty arrays after compacting + decoded->compact(); + EXPECT_EQ(QJsonObject{}, + decoded->toJsonObject(CustomTags::ToJsonMode::Plain)); +} + +TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { + const auto customTagsJson = QJsonObject{ + { + "facet1", + QJsonArray{ + QJsonValue{0.2}, // empty label + QJsonValue{"Label 1"}, // score = 1.0 + }, + }, + { + "facet2", + QJsonArray{ + QJsonValue{1.0}, + QJsonArray{QJsonValue{"Label 2"}, QJsonValue{0.4}}, + }, + }, + { + "facet3", + QJsonArray{}, + }, + { + "", + QJsonArray{ + QJsonValue{"Label 1"}, + QJsonArray{QJsonValue{"Label 2"}, QJsonValue{0.5}}, + }, + }, + }; + + const auto facet1 = TagFacet(QStringLiteral("facet1")); + const auto facet2 = TagFacet(QStringLiteral("facet2")); + const auto facet3 = TagFacet(QStringLiteral("facet3")); + + const auto label1 = TagLabel(QStringLiteral("Label 1")); + const auto label2 = TagLabel(QStringLiteral("Label 2")); + + // Decode JSON + const QByteArray jsonData = + QJsonDocument{customTagsJson}.toJson(QJsonDocument::Compact); + const auto jsonDoc = + QJsonDocument::fromJson(jsonData); + std::optional decoded = + CustomTags::fromJsonObject(jsonDoc.object()); + ASSERT_TRUE(static_cast(decoded)); + ASSERT_TRUE(decoded->validate()); + + // All tags (plain + 2 facets with tags + 1 facet with an empty slot) + EXPECT_EQ(4, decoded->getFacetedTags().size()); + + // Plain tags + EXPECT_EQ(2, decoded->countTags()); + EXPECT_EQ(1, decoded->countTags(label1)); + EXPECT_EQ(1, decoded->countTags(label2)); + EXPECT_EQ( + TagScore(), + decoded->getFacetedTags().value(TagFacet()).value(label1)); + EXPECT_EQ( + TagScore(0.5), + decoded->getFacetedTags().value(TagFacet()).value(label2)); + + // Faceted tags + EXPECT_EQ(2, decoded->countFacetedTags(facet1)); + EXPECT_EQ(2, decoded->countFacetedTags(facet2)); + EXPECT_EQ(0, decoded->countFacetedTags(facet3)); + const auto facet1TagsOrdered = TagVector{ + Tag(label1), + Tag(TagScore(0.2))}; + EXPECT_EQ( + facet1TagsOrdered, + decoded->getFacetedTagsOrdered(facet1)); + const auto facet2TagsOrdered = TagVector{ + Tag(), + Tag(label2, TagScore(0.4))}; + EXPECT_EQ( + facet2TagsOrdered, + decoded->getFacetedTagsOrdered(facet2)); + + // Compaction + decoded->compact(); + // facet3 has disappeared + EXPECT_EQ(3, decoded->getFacetedTags().size()); +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp new file mode 100644 index 00000000000..342949ac3ce --- /dev/null +++ b/src/test/librarytags_test.cpp @@ -0,0 +1,205 @@ +#include + +#include +#include + +#include "library/tags/tag.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +class TagFacetTest : public testing::Test { +}; + +TEST_F(TagFacetTest, isEmpty) { + ASSERT_TRUE(TagFacet().isEmpty()); + // Null string + EXPECT_EQ( + TagFacet(), + TagFacet(TagFacet::value_t())); + // Empty string + EXPECT_EQ( + TagFacet(), + TagFacet(TagFacet::filterEmptyValue(""))); + // Non-empty string + EXPECT_FALSE(TagFacet("x").isEmpty()); +} + +TEST_F(TagFacetTest, filterEmptyValue) { + EXPECT_EQ( + TagFacet::value_t(), + TagFacet::filterEmptyValue(TagFacet::value_t())); + EXPECT_EQ( + TagFacet::value_t(), + TagFacet::filterEmptyValue("")); + EXPECT_EQ( + TagFacet::value_t("x"), + TagFacet::filterEmptyValue("x")); +} + +TEST_F(TagFacetTest, clampValue) { + // Clamp empty string + EXPECT_EQ( + TagFacet(), + TagFacet(TagFacet::clampValue(""))); + // Clamped whitespace string + EXPECT_EQ( + TagFacet(), + TagFacet(TagFacet::clampValue(" \t\n "))); + // Lowercase + EXPECT_EQ( + TagFacet("x"), + TagFacet(TagFacet::clampValue(" \tX\n "))); + // Whitespace replacement + EXPECT_EQ( + TagFacet("xy_{+}"), + TagFacet(TagFacet::clampValue("X y _\n{+} "))); + // Lowercase ASCII alphabet without whitespace + EXPECT_EQ( + TagFacet("!\"#$%&'()*+,-./0123456789:;<=>?" + "@abcdefghijklmnopqrstuvwxyz[\\]^_" + "`abcdefghijklmnopqrstuvwxyz{|}~"), + TagFacet(TagFacet::clampValue( + " !\"#$%&'()*+,-./0123456789:;<=>?" + "@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_" + "`abcdefghijklmnopqrstuvwxyz{|}~"))); +} + +class TagLabelTest : public testing::Test { +}; + +TEST_F(TagLabelTest, isEmpty) { + ASSERT_TRUE(TagLabel().isEmpty()); + // Null string + EXPECT_EQ( + TagLabel(), + TagLabel(TagLabel::value_t())); + // Empty string + EXPECT_EQ( + TagLabel(), + TagLabel(TagLabel::filterEmptyValue(""))); + // Non-empty string + EXPECT_FALSE(TagLabel("X").isEmpty()); +} + +TEST_F(TagLabelTest, clampEmptyValueIsValid) { + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::clampValue(TagLabel::value_t()))); + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::clampValue(""))); + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::clampValue(" "))); +} + +TEST_F(TagLabelTest, filterEmptyValue) { + EXPECT_EQ( + TagLabel::value_t(), + TagLabel::filterEmptyValue(TagLabel::value_t())); + EXPECT_EQ( + TagLabel::value_t(), + TagLabel::filterEmptyValue("")); + EXPECT_EQ( + TagLabel::value_t("X"), + TagLabel::filterEmptyValue("X")); +} + +TEST_F(TagLabelTest, clampValue) { + // Clamp empty string + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::clampValue(""))); + // Clamped whitespace string + EXPECT_EQ( + TagLabel(), + TagLabel(TagLabel::clampValue(" \t\n "))); + // Trimmed + EXPECT_EQ( + TagLabel("X y"), + TagLabel(TagLabel::clampValue(" \tX y\n "))); +} + +class TagTest : public testing::Test { +}; + +TEST_F(TagTest, fromJsonValue) { + const char* labelText = "Label"; + const auto label = TagLabel(labelText); + const TagScore::value_t scoreValue = 0.1357; + const auto score = TagScore(scoreValue); + // Empty array + EXPECT_EQ( + std::nullopt, + Tag::fromJsonValue( + QJsonArray{})); + // Empty label + EXPECT_EQ( + std::make_optional(Tag()), + Tag::fromJsonValue( + "")); + // Default score + EXPECT_EQ( + std::make_optional(Tag()), + Tag::fromJsonValue( + TagScore::kDefaultValue)); + // Label + EXPECT_EQ( + std::make_optional(Tag(label)), + Tag::fromJsonValue( + labelText)); + // Label as 1-element array + EXPECT_EQ( + std::nullopt, + Tag::fromJsonValue( + QJsonArray{labelText})); + // Score + EXPECT_EQ( + std::make_optional(Tag(score)), + Tag::fromJsonValue( + scoreValue)); + // Score as 1-element array + EXPECT_EQ( + std::nullopt, + Tag::fromJsonValue( + QJsonArray{scoreValue})); + // Both label and score as array + EXPECT_EQ( + std::make_optional(Tag(label, score)), + Tag::fromJsonValue( + QJsonArray{labelText, scoreValue})); + // 3-element array + EXPECT_EQ( + std::nullopt, + Tag::fromJsonValue( + QJsonArray{labelText, scoreValue, scoreValue})); +} + +TEST_F(TagTest, toJsonValue) { + const char* labelText = "Label"; + const auto label = TagLabel(labelText); + const TagScore::value_t scoreValue = 0.1357; + const auto score = TagScore(scoreValue); + // Only label, no score + EXPECT_EQ( + QJsonValue{labelText}, + Tag(label).toJsonValue()); + // Only score, no label + EXPECT_EQ( + QJsonValue{TagScore::kDefaultValue}, + Tag().toJsonValue()); + EXPECT_EQ( + QJsonValue{scoreValue}, + Tag(score).toJsonValue()); + // Both label and score + const auto expectedJsonArray = QJsonArray{labelText, scoreValue}; + EXPECT_EQ( + QJsonValue{expectedJsonArray}, + Tag(label, score).toJsonValue()); +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx From 7ec7af924b4bbf94ce03ca314136525f4d013db0 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 28 Aug 2021 11:53:40 +0200 Subject: [PATCH 02/39] Extract utility functions for parsing JSON data --- CMakeLists.txt | 1 + src/library/tags/customtags.cpp | 32 ++++------------- src/library/tags/customtags.h | 2 ++ src/network/jsonwebtask.cpp | 20 +++-------- src/util/json.cpp | 64 +++++++++++++++++++++++++++++++++ src/util/json.h | 28 +++++++++++++++ 6 files changed, 105 insertions(+), 42 deletions(-) create mode 100644 src/util/json.cpp create mode 100644 src/util/json.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 6d76b39242b..51bdadc31c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -901,6 +901,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/util/fileinfo.cpp src/util/imageutils.cpp src/util/indexrange.cpp + src/util/json.cpp src/util/logger.cpp src/util/logging.cpp src/util/mac.cpp diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index cd681d10c6c..499b1e3fab5 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -1,10 +1,10 @@ #include "library/tags/customtags.h" #include -#include -#include #include +#include "util/json.h" + namespace { Q_LOGGING_CATEGORY(kLogger, "mixxx.library.tags.CustomTags") @@ -383,33 +383,13 @@ QJsonObject CustomTags::toJsonObject( //static std::optional CustomTags::parseJsonData( const QByteArray& jsonData) { - if (jsonData.isEmpty()) { - qCWarning(kLogger) - << "Failed to parse empty JSON data"; - return std::nullopt; - } - QJsonParseError parseError; - const auto jsonDoc = QJsonDocument::fromJson(jsonData, &parseError); - // QJsonDocument::fromJson() returns a non-null document - // if parsing succeeds and otherwise null on error. The - // parse error must only be evaluated if the returned - // document is null! - if (jsonDoc.isNull() && - parseError.error != QJsonParseError::NoError) { - qCWarning(kLogger) - << "Failed to parse JSON data:" - << parseError.errorString() - << "at offset" - << parseError.offset; - return std::nullopt; - } - if (!jsonDoc.isObject()) { + const auto [jsonObject, parseError] = json::parseObject(jsonData); + if (parseError != QJsonParseError::NoError) { qCWarning(kLogger) - << "Invalid JSON document" - << jsonDoc; + << "Failed to parse JSON data" + << parseError; return std::nullopt; } - const auto jsonObject = jsonDoc.object(); auto customTags = CustomTags::fromJsonObject(jsonObject); if (!customTags) { qCWarning(kLogger) diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index db22fab3b85..0370ab27ff8 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include "library/tags/tag.h" diff --git a/src/network/jsonwebtask.cpp b/src/network/jsonwebtask.cpp index 961d3a05430..7449c2e1c75 100644 --- a/src/network/jsonwebtask.cpp +++ b/src/network/jsonwebtask.cpp @@ -8,6 +8,7 @@ #include "moc_jsonwebtask.cpp" #include "util/counter.h" +#include "util/json.h" #include "util/logger.h" #include "util/thread_affinity.h" @@ -55,21 +56,8 @@ std::optional readJsonContent( kLogger.debug() << "Empty JSON network reply"; return QJsonDocument{}; } - QJsonParseError parseError; - auto jsonDocument = QJsonDocument::fromJson( - *optContentData, - &parseError); - // QJsonDocument::fromJson() returns a non-null document - // if parsing succeeds and otherwise null on error. The - // parse error must only be evaluated if the returned - // document is null! - if (jsonDocument.isNull() && - parseError.error != QJsonParseError::NoError) { - kLogger.warning() - << "Failed to parse JSON data:" - << parseError.errorString() - << "at offset" - << parseError.offset; + const auto [jsonDoc, parseError] = json::parseDocument(*optContentData); + if (parseError != QJsonParseError::NoError) { if (pInvalidResponseContent) { *pInvalidResponseContent = std::make_pair( std::move(contentType), @@ -77,7 +65,7 @@ std::optional readJsonContent( } return std::nullopt; } - return jsonDocument; + return jsonDoc; } // TODO: Allow to customize headers and attributes? diff --git a/src/util/json.cpp b/src/util/json.cpp new file mode 100644 index 00000000000..4b54c2cb41a --- /dev/null +++ b/src/util/json.cpp @@ -0,0 +1,64 @@ +#include "util/json.h" + +namespace mixxx { + +namespace json { + +Q_LOGGING_CATEGORY(kLogger, "mixxx.util.json") + +std::pair parseDocument( + const QByteArray& jsonData) { + if (jsonData.isEmpty()) { + return std::make_pair(QJsonDocument{}, QJsonParseError::NoError); + } + QJsonParseError parseError; + const auto doc = QJsonDocument::fromJson(jsonData, &parseError); + // QJsonDocument::fromJson() returns a non-null document + // if parsing succeeds and otherwise null on error. The + // parse error must only be evaluated if the returned + // document is null, otherwise it might not be initialized! + if (doc.isNull() && + parseError.error != QJsonParseError::NoError) { + qCWarning(kLogger) + << "Failed to parse JSON data:" + << parseError.errorString() + << "at offset" + << parseError.offset; + return std::make_pair(doc, parseError.error); + } + return std::make_pair(doc, QJsonParseError::NoError); +} + +std::pair parseObject( + const QByteArray& jsonData) { + const auto [jsonDoc, parseError] = parseDocument(jsonData); + if (jsonDoc.isNull()) { + return std::make_pair(QJsonObject{}, parseError); + } + if (!jsonDoc.isObject()) { + qCWarning(kLogger) + << "JSON document does not contain an object:" + << jsonDoc; + return std::make_pair(QJsonObject{}, QJsonParseError::MissingObject); + } + return std::make_pair(jsonDoc.object(), parseError); +} + +std::pair parseArray( + const QByteArray& jsonData) { + const auto [jsonDoc, parseError] = parseDocument(jsonData); + if (jsonDoc.isNull()) { + return std::make_pair(QJsonArray{}, parseError); + } + if (!jsonDoc.isArray()) { + qCWarning(kLogger) + << "JSON document does not contain an array" + << jsonDoc; + return std::make_pair(QJsonArray{}, QJsonParseError::IllegalValue); + } + return std::make_pair(jsonDoc.array(), parseError); +} + +} // namespace json + +} // namespace mixxx diff --git a/src/util/json.h b/src/util/json.h new file mode 100644 index 00000000000..a6298373ad5 --- /dev/null +++ b/src/util/json.h @@ -0,0 +1,28 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +namespace mixxx { + +namespace json { + +Q_DECLARE_LOGGING_CATEGORY(kLogger) + +std::pair parseDocument( + const QByteArray& jsonData); + +std::pair parseObject( + const QByteArray& jsonData); + +std::pair parseArray( + const QByteArray& jsonData); + +} // namespace json + +} // namespace mixxx From c5462fe744d004eb1785a48f173d42f3f4062322 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 28 Aug 2021 18:22:01 +0200 Subject: [PATCH 03/39] CustomTags: Log and report parsing errors --- src/library/tags/customtags.cpp | 29 ++++++++++++----------------- src/library/tags/customtags.h | 20 +++++++++++++++++--- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index 499b1e3fab5..6cd6b691e7d 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -297,14 +297,14 @@ bool CustomTags::addOrReplaceAllTags( //static std::optional CustomTags::fromJsonObject( const QJsonObject& jsonObject, - bool strict) { + FromJsonMode mode) { CustomTags customTags; for (auto i = jsonObject.begin(); i != jsonObject.end(); ++i) { auto facetValue = TagFacet::filterEmptyValue(i.key()); if (!TagFacet::isValidValue(facetValue)) { - VERIFY_OR_DEBUG_ASSERT(!strict) { + VERIFY_OR_DEBUG_ASSERT(mode == FromJsonMode::Lenient) { qCWarning(kLogger) << "Invalid facet" << facetValue @@ -335,7 +335,7 @@ std::optional CustomTags::fromJsonObject( const auto tag = Tag::fromJsonValue(jsonValue); if (!tag || (*tag != Tag() && !tag->isValid())) { - VERIFY_OR_DEBUG_ASSERT(!strict) { + VERIFY_OR_DEBUG_ASSERT(mode == FromJsonMode::Lenient) { qCWarning(kLogger) << "Invalid tag" << jsonValue @@ -381,29 +381,24 @@ QJsonObject CustomTags::toJsonObject( } //static -std::optional CustomTags::parseJsonData( - const QByteArray& jsonData) { +std::pair, CustomTags::ParseJsonDataResult> CustomTags::parseJsonData( + const QByteArray& jsonData, + FromJsonMode mode) { const auto [jsonObject, parseError] = json::parseObject(jsonData); if (parseError != QJsonParseError::NoError) { qCWarning(kLogger) - << "Failed to parse JSON data" + << "Failed to deserialize JSON data" << parseError; - return std::nullopt; + return std::make_pair(std::nullopt, ParseJsonDataResult::DeserializationError); } - auto customTags = CustomTags::fromJsonObject(jsonObject); + auto customTags = CustomTags::fromJsonObject(jsonObject, mode); if (!customTags) { qCWarning(kLogger) - << "Invalid JSON object" + << "Failed to transform JSON object" << jsonObject; - return std::nullopt; - } - if (!customTags->validate()) { - qCWarning(kLogger) - << "Validation failed" - << *customTags; - return std::nullopt; + return std::make_pair(std::nullopt, ParseJsonDataResult::TransformationError); } - return customTags; + return std::make_pair(customTags, ParseJsonDataResult::Ok); } QByteArray CustomTags::dumpJsonData() const { diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index 0370ab27ff8..5cedc640169 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -152,9 +152,13 @@ class CustomTags final { .getLabel(); } + enum class FromJsonMode { + Lenient, + Strict, + }; static std::optional fromJsonObject( const QJsonObject& jsonObject, - bool strict = false); + FromJsonMode mode = FromJsonMode::Lenient); enum class ToJsonMode { Plain, Compact, @@ -162,8 +166,18 @@ class CustomTags final { QJsonObject toJsonObject( ToJsonMode mode = ToJsonMode::Compact) const; - static std::optional parseJsonData( - const QByteArray& jsonData); + enum class ParseJsonDataResult { + Ok, + DeserializationError, + TransformationError, + }; + /// Create a new instance from JSON data + /// + /// Returns a new instance if no error occurred. The resulting + /// instance should be validated for consistency using `validate()`. + static std::pair, ParseJsonDataResult> parseJsonData( + const QByteArray& jsonData, + FromJsonMode mode = FromJsonMode::Lenient); QByteArray dumpJsonData() const; }; From 1d02a87b781582386e86ff04c03752480e214e1e Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 11:46:16 +0200 Subject: [PATCH 04/39] Custom tags: Refine and extend documentation --- src/library/tags/customtags.h | 23 ++++++++++++++--------- src/library/tags/tag.h | 33 +++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index 5cedc640169..a37120c3c39 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -17,7 +17,7 @@ typedef QMap FacetTagMap; /// Custom tags /// -/// Each custom tag is represented by a triple: a facet, a label, and a score: +/// Each custom tag is represented by a 3-tuple (triple): a facet, a label, and a score: /// /// - The *facet* is a lowercase, non-empty ASCII string without whitespace /// - The *label* is non-empty free text @@ -105,27 +105,32 @@ class CustomTags final { bool addOrReplaceAllTags( const CustomTags& tags); - // Get all plain tags as a list (in no particular order) + /// Get all plain tags as a list (in no particular order) TagVector getTags() const; - // Replace all existing tags of this facet with a single - // faceted tag or insert a new faceted tag. + /// Replace all existing tags of this facet with a single + /// faceted tag or insert a new faceted tag. FacetTagMap::iterator replaceAllFacetedTags( const TagFacet& facet, const Tag& tag); + int removeAllFacetedTags( const TagFacet& facet); - // Get all tags of a given facet sorted by score in descending order + + /// Get all tags of a given facet sorted by score in descending order TagVector getFacetedTagsOrdered( const TagFacet& facet) const; - // Get the label of a single faceted tag, i.e. that - // occurs at most once and has no custom score. If the - // facet is unknown/absent an empty label is returned. + /// Get the label of a single faceted tag, i.e. that + /// occurs at most once and has no custom score. If the + /// facet is unknown/absent an empty label is returned. TagLabel getFacetedTagLabel( const TagFacet& facet) const; - // Get the score of a tag if present. + /// Get the score of a tag if present. + /// + /// Returns `std::nullopt` if the corresponding (facet, label) + /// combination that serves as the key does not exist. std::optional getTagScore( const TagFacet& facet, const TagLabel& label = TagLabel()) const; diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h index 94ca8db997d..a5efc3eac10 100644 --- a/src/library/tags/tag.h +++ b/src/library/tags/tag.h @@ -16,7 +16,17 @@ namespace tags { /// A category for tags. /// -/// Constraints: Lowercase ASCII, no whitespace characters +/// Facets are used for grouping/categorizing and providing context or meaning. +/// Their value serves as a symbolic, internal identifier that is not intended +/// to be displayed literally in the UI. This is also the reasons for the +/// restrictions on naming them. +/// +/// Value constraints: +/// - lowercase ASCII +/// - and no whitespace +/// +/// References: +/// - https://en.wikipedia.org/wiki/Faceted_classification class TagFacet final { public: typedef QString value_t; @@ -112,9 +122,13 @@ inline uint qHash( return qHash(facet.value(), seed); } -/// The name/title of a tag. +/// The displayable name or title of a tag. +/// +/// The value contains arbitrary Unicode text that is supposed to +/// be displayed to the user. /// -/// Constraints: No leading/trailing whitespace +/// Value constraints: +/// - no leading/trailing whitespace class TagLabel final { public: typedef QString value_t; @@ -245,7 +259,15 @@ class TagScore final { value_t m_value; }; -// A plain tag with label and score. +/// A plain tag with an optional label and a score. +/// +/// Plain tags are not faceted. The facet is usually provided +/// by the outer context, e.g. the key of a map that stores +/// plain tags as values. +/// +/// Mixxx uses these kind of tags as `CustomTags` to store extended +/// metadata. They should not be confused with file tags (ID3, MP4, +/// VorbisComment) for storing metadata in media files. class Tag final { // Properties MIXXX_DECL_PROPERTY(TagLabel, label, Label) @@ -273,6 +295,9 @@ class Tag final { getScore().isValid(); } + /// Check if a label is present. + /// + /// Empty labels are considered as missing. bool hasLabel() const { return !m_label.isEmpty(); } From 8929eb3d8014ecf710b51e29aa306eb4f152ca79 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 11:46:48 +0200 Subject: [PATCH 05/39] Custom tags: Rename member function --- src/library/tags/customtags.cpp | 2 +- src/library/tags/customtags.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index 6cd6b691e7d..563bc366b46 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -328,7 +328,7 @@ std::optional CustomTags::fromJsonObject( // but no predefined tags/labels. The facet will be // displayed in the UI with the option to add custom // labels. - customTags.touchFacet(facet); + customTags.addOrIgnoreFacet(facet); continue; } for (const auto& jsonValue : jsonArray) { diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index a37120c3c39..84196b1fa74 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -65,10 +65,11 @@ class CustomTags final { const TagFacet& facet) const { return getFacetedTags().contains(facet); } + /// Add an (empty) entry for the given facet if it does not exist yet. /// /// Existing entries are not affected. - void touchFacet( + void addOrIgnoreFacet( const TagFacet& facet) { refFacetedTags()[facet]; DEBUG_ASSERT(containsFacet(facet)); From 7607dcce9578d052ecde3386edfb0e6fe57402d6 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 12:20:35 +0200 Subject: [PATCH 06/39] Tag facet/label/score: clampValue() -> convertIntoValidValue() --- src/library/tags/customtags.cpp | 2 +- src/library/tags/tag.cpp | 16 ++++++++-------- src/library/tags/tag.h | 10 ++++++---- src/test/customtags_test.cpp | 14 +++++++------- src/test/librarytags_test.cpp | 26 +++++++++++++------------- 5 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index 563bc366b46..f9b30a281ca 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -268,7 +268,7 @@ Tag CustomTags::mergeFacetedTags( labels += tag.getLabel(); } auto labelValue = - TagLabel::clampValue(labels.join(joinLabelSeparator)); + TagLabel::convertIntoValidValue(labels.join(joinLabelSeparator)); return Tag( TagLabel(std::move(labelValue)), TagScore(scoreValue)); diff --git a/src/library/tags/tag.cpp b/src/library/tags/tag.cpp index 0118865b86e..dc7473254e9 100644 --- a/src/library/tags/tag.cpp +++ b/src/library/tags/tag.cpp @@ -36,11 +36,11 @@ bool TagFacet::isValidValue( } //static -TagFacet::value_t TagFacet::clampValue( +TagFacet::value_t TagFacet::convertIntoValidValue( const value_t& value) { - auto clampedValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); - DEBUG_ASSERT(isValidValue(clampedValue)); - return clampedValue; + auto validValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); + DEBUG_ASSERT(isValidValue(validValue)); + return validValue; } //static @@ -57,11 +57,11 @@ bool TagLabel::isValidValue( } //static -TagLabel::value_t TagLabel::clampValue( +TagLabel::value_t TagLabel::convertIntoValidValue( const value_t& value) { - auto clampedValue = filterEmptyValue(value.trimmed()); - DEBUG_ASSERT(isValidValue(clampedValue)); - return clampedValue; + auto validValue = filterEmptyValue(value.trimmed()); + DEBUG_ASSERT(isValidValue(validValue)); + return validValue; } bool operator==( diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h index a5efc3eac10..750ad527a54 100644 --- a/src/library/tags/tag.h +++ b/src/library/tags/tag.h @@ -34,9 +34,9 @@ class TagFacet final { static bool isValidValue( const value_t& value); - /// Converts the given string into lowercase and then + /// Convert the given string into lowercase and then /// removes all whitespace and non-ASCII characters. - static value_t clampValue( + static value_t convertIntoValidValue( const value_t& value); /// Ensure that empty values are always null @@ -136,7 +136,8 @@ class TagLabel final { static bool isValidValue( const value_t& value); - static value_t clampValue( + /// Remove leading/trailing whitespace. + static value_t convertIntoValidValue( const value_t& value); /// Ensure that empty values are always null @@ -233,7 +234,8 @@ class TagScore final { return value >= kMinValue && value <= kMaxValue; } - static value_t clampValue( + /// Clamp the value to the valid range [kMinValue, kMaxValue]. + static constexpr value_t convertIntoValidValue( value_t value) { return math_min(kMaxValue, math_max(kMinValue, value)); } diff --git a/src/test/customtags_test.cpp b/src/test/customtags_test.cpp index 66e31714ad2..65a0d24c9c9 100644 --- a/src/test/customtags_test.cpp +++ b/src/test/customtags_test.cpp @@ -26,14 +26,14 @@ TEST_F(CustomTagsTest, isValidFacetValue) { } TEST_F(CustomTagsTest, clampFacetValue) { - EXPECT_EQ(QString(), TagFacet::clampValue(QString())); - EXPECT_EQ( - QString("lower-case_with_digit_1_and_whitespaces"), - TagFacet::clampValue(QStringLiteral(" lower -\tcase_with _digit_1_and_whitespaces "))); - EXPECT_EQ(QString(), TagFacet::clampValue(QStringLiteral(" \n \t \r"))); - EXPECT_EQ(QString("uppercase"), TagFacet::clampValue(QStringLiteral("Uppercase"))); + EXPECT_EQ(QString(), TagFacet::convertIntoValidValue(QString())); + EXPECT_EQ(QString("lower-case_with_digit_1_and_whitespaces"), + TagFacet::convertIntoValidValue(QStringLiteral( + " lower -\tcase_with _digit_1_and_whitespaces "))); + EXPECT_EQ(QString(), TagFacet::convertIntoValidValue(QStringLiteral(" \n \t \r"))); + EXPECT_EQ(QString("uppercase"), TagFacet::convertIntoValidValue(QStringLiteral("Uppercase"))); EXPECT_EQ(QString("with\\back/slash"), - TagFacet::clampValue(QStringLiteral("with \\ back/\tslash"))); + TagFacet::convertIntoValidValue(QStringLiteral("with \\ back/\tslash"))); } TEST_F(CustomTagsTest, isValidLabelValue) { diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp index 342949ac3ce..a44cb6a9d94 100644 --- a/src/test/librarytags_test.cpp +++ b/src/test/librarytags_test.cpp @@ -40,29 +40,29 @@ TEST_F(TagFacetTest, filterEmptyValue) { TagFacet::filterEmptyValue("x")); } -TEST_F(TagFacetTest, clampValue) { +TEST_F(TagFacetTest, convertIntoValidValue) { // Clamp empty string EXPECT_EQ( TagFacet(), - TagFacet(TagFacet::clampValue(""))); + TagFacet(TagFacet::convertIntoValidValue(""))); // Clamped whitespace string EXPECT_EQ( TagFacet(), - TagFacet(TagFacet::clampValue(" \t\n "))); + TagFacet(TagFacet::convertIntoValidValue(" \t\n "))); // Lowercase EXPECT_EQ( TagFacet("x"), - TagFacet(TagFacet::clampValue(" \tX\n "))); + TagFacet(TagFacet::convertIntoValidValue(" \tX\n "))); // Whitespace replacement EXPECT_EQ( TagFacet("xy_{+}"), - TagFacet(TagFacet::clampValue("X y _\n{+} "))); + TagFacet(TagFacet::convertIntoValidValue("X y _\n{+} "))); // Lowercase ASCII alphabet without whitespace EXPECT_EQ( TagFacet("!\"#$%&'()*+,-./0123456789:;<=>?" "@abcdefghijklmnopqrstuvwxyz[\\]^_" "`abcdefghijklmnopqrstuvwxyz{|}~"), - TagFacet(TagFacet::clampValue( + TagFacet(TagFacet::convertIntoValidValue( " !\"#$%&'()*+,-./0123456789:;<=>?" "@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_" "`abcdefghijklmnopqrstuvwxyz{|}~"))); @@ -87,11 +87,11 @@ TEST_F(TagLabelTest, isEmpty) { TEST_F(TagLabelTest, clampEmptyValueIsValid) { EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::clampValue(TagLabel::value_t()))); + TagLabel::isValidValue(TagLabel::convertIntoValidValue(TagLabel::value_t()))); EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::clampValue(""))); + TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::clampValue(" "))); + TagLabel::isValidValue(TagLabel::convertIntoValidValue(" "))); } TEST_F(TagLabelTest, filterEmptyValue) { @@ -106,18 +106,18 @@ TEST_F(TagLabelTest, filterEmptyValue) { TagLabel::filterEmptyValue("X")); } -TEST_F(TagLabelTest, clampValue) { +TEST_F(TagLabelTest, convertIntoValidValue) { // Clamp empty string EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::clampValue(""))); + TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); // Clamped whitespace string EXPECT_EQ( TagLabel(), - TagLabel(TagLabel::clampValue(" \t\n "))); + TagLabel(TagLabel::convertIntoValidValue(" \t\n "))); // Trimmed EXPECT_EQ( TagLabel("X y"), - TagLabel(TagLabel::clampValue(" \tX y\n "))); + TagLabel(TagLabel::convertIntoValidValue(" \tX y\n "))); } class TagTest : public testing::Test { From 3645c1d880ded6f74bed673f3c23c742d25849b9 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 12:29:57 +0200 Subject: [PATCH 07/39] Tag facet/label: Trade constexpr for validation Use debug assertions to validate the string values, even though that precludes constexpr. --- src/library/tags/tag.h | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h index 750ad527a54..0c507a28647 100644 --- a/src/library/tags/tag.h +++ b/src/library/tags/tag.h @@ -31,6 +31,10 @@ class TagFacet final { public: typedef QString value_t; + static value_t defaultValue() { + return value_t{}; + } + static bool isValidValue( const value_t& value); @@ -41,17 +45,15 @@ class TagFacet final { /// Ensure that empty values are always null static value_t filterEmptyValue( - const value_t& value) { - return value.isEmpty() ? value_t() : value; + value_t value) { + DEBUG_ASSERT(defaultValue().isEmpty()); + return value.isEmpty() ? defaultValue() : value; } explicit TagFacet( - const value_t& value = value_t()) - : m_value(value) { - // Full validation not possible due to static constants with - // regular expressions that are inaccessible when creating - // static constants of this type! - DEBUG_ASSERT(filterEmptyValue(m_value) == m_value); + value_t value = defaultValue()) + : m_value(std::move(value)) { + DEBUG_ASSERT(isValid()); } TagFacet(const TagFacet&) = default; TagFacet(TagFacet&&) = default; @@ -133,6 +135,10 @@ class TagLabel final { public: typedef QString value_t; + static const value_t defaultValue() { + return value_t{}; + } + static bool isValidValue( const value_t& value); @@ -142,13 +148,14 @@ class TagLabel final { /// Ensure that empty values are always null static value_t filterEmptyValue( - const value_t& value) { - return value.isEmpty() ? value_t() : value; + value_t value) { + DEBUG_ASSERT(defaultValue().isEmpty()); + return value.isEmpty() ? defaultValue() : value; } explicit TagLabel( - const value_t& value = value_t()) - : m_value(value) { + value_t value = defaultValue()) + : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); } TagLabel(const TagLabel&) = default; @@ -162,13 +169,15 @@ class TagLabel final { } bool isEmpty() const { + DEBUG_ASSERT(isValid()); return m_value.isEmpty(); } - constexpr const value_t& value() const { + const value_t& value() const { + DEBUG_ASSERT(isValid()); return m_value; } - constexpr operator const value_t&() const { + operator const value_t&() const { return value(); } @@ -278,12 +287,12 @@ class Tag final { public: explicit Tag( const TagLabel& label, - TagScore score = TagScore()) + TagScore score = TagScore{}) : m_label(label), m_score(score) { } explicit Tag( - TagScore score = TagScore()) + TagScore score = TagScore{}) : m_score(score) { } Tag(const Tag&) = default; From e7f3c7b357264dd603a3d3bd34f83227b83e6122 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 15:54:20 +0200 Subject: [PATCH 08/39] Custom tags: Split source files --- CMakeLists.txt | 2 + src/library/tags/customtags.h | 1 + src/library/tags/tag.cpp | 63 -------- src/library/tags/tag.h | 276 ++-------------------------------- src/library/tags/tagfacet.cpp | 49 ++++++ src/library/tags/tagfacet.h | 130 ++++++++++++++++ src/library/tags/taglabel.cpp | 34 +++++ src/library/tags/taglabel.h | 123 +++++++++++++++ src/library/tags/tagscore.h | 63 ++++++++ src/mixxxapplication.cpp | 1 + src/test/librarytags_test.cpp | 1 + 11 files changed, 414 insertions(+), 329 deletions(-) create mode 100644 src/library/tags/tagfacet.cpp create mode 100644 src/library/tags/tagfacet.h create mode 100644 src/library/tags/taglabel.cpp create mode 100644 src/library/tags/taglabel.h create mode 100644 src/library/tags/tagscore.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 51bdadc31c6..53062983382 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -854,6 +854,8 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyutils.cpp src/library/tags/customtags.cpp src/library/tags/tag.cpp + src/library/tags/tagfacet.cpp + src/library/tags/taglabel.cpp src/track/playcounter.cpp src/track/replaygain.cpp src/track/serato/beatgrid.cpp diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index 84196b1fa74..4d6f0bfe02e 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -5,6 +5,7 @@ #include #include "library/tags/tag.h" +#include "library/tags/tagfacet.h" namespace mixxx { diff --git a/src/library/tags/tag.cpp b/src/library/tags/tag.cpp index dc7473254e9..cb6c4d1c7d7 100644 --- a/src/library/tags/tag.cpp +++ b/src/library/tags/tag.cpp @@ -1,16 +1,6 @@ #include "library/tags/tag.h" #include -#include - -namespace { - -const QRegularExpression kLowercaseAsciiNotEmpty( - QStringLiteral("^[\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); -const QRegularExpression kInverseLowercaseAsciiNotEmpty( - QStringLiteral("[^\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); - -} // anonymous namespace namespace mixxx { @@ -18,59 +8,6 @@ namespace library { namespace tags { -//static -bool TagFacet::isValidValue( - const value_t& value) { - if (value.isNull()) { - return true; - } - if (value.isEmpty()) { - // for disambiguation with null - return false; - } - const auto match = kLowercaseAsciiNotEmpty.match(value); - DEBUG_ASSERT(match.isValid()); - DEBUG_ASSERT(value.length() > 0); - // match = exact match - return match.capturedLength() == value.length(); -} - -//static -TagFacet::value_t TagFacet::convertIntoValidValue( - const value_t& value) { - auto validValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); - DEBUG_ASSERT(isValidValue(validValue)); - return validValue; -} - -//static -bool TagLabel::isValidValue( - const value_t& value) { - if (value.isNull()) { - return true; - } - if (value.isEmpty()) { - // for disambiguation with null - return false; - } - return value.trimmed() == value; -} - -//static -TagLabel::value_t TagLabel::convertIntoValidValue( - const value_t& value) { - auto validValue = filterEmptyValue(value.trimmed()); - DEBUG_ASSERT(isValidValue(validValue)); - return validValue; -} - -bool operator==( - const Tag& lhs, - const Tag& rhs) { - return lhs.getLabel() == rhs.getLabel() && - lhs.getScore() == rhs.getScore(); -} - QDebug operator<<( QDebug dbg, const Tag& arg) { diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h index 0c507a28647..ba6da5d693c 100644 --- a/src/library/tags/tag.h +++ b/src/library/tags/tag.h @@ -3,9 +3,10 @@ #include #include +#include "library/tags/taglabel.h" +#include "library/tags/tagscore.h" #include "util/assert.h" #include "util/macros.h" -#include "util/math.h" #include "util/optional.h" namespace mixxx { @@ -14,267 +15,10 @@ namespace library { namespace tags { -/// A category for tags. +/// A plain, non-faceted tag with an optional label and a score. /// -/// Facets are used for grouping/categorizing and providing context or meaning. -/// Their value serves as a symbolic, internal identifier that is not intended -/// to be displayed literally in the UI. This is also the reasons for the -/// restrictions on naming them. -/// -/// Value constraints: -/// - lowercase ASCII -/// - and no whitespace -/// -/// References: -/// - https://en.wikipedia.org/wiki/Faceted_classification -class TagFacet final { - public: - typedef QString value_t; - - static value_t defaultValue() { - return value_t{}; - } - - static bool isValidValue( - const value_t& value); - - /// Convert the given string into lowercase and then - /// removes all whitespace and non-ASCII characters. - static value_t convertIntoValidValue( - const value_t& value); - - /// Ensure that empty values are always null - static value_t filterEmptyValue( - value_t value) { - DEBUG_ASSERT(defaultValue().isEmpty()); - return value.isEmpty() ? defaultValue() : value; - } - - explicit TagFacet( - value_t value = defaultValue()) - : m_value(std::move(value)) { - DEBUG_ASSERT(isValid()); - } - TagFacet(const TagFacet&) = default; - TagFacet(TagFacet&&) = default; - - TagFacet& operator=(const TagFacet&) = default; - TagFacet& operator=(TagFacet&&) = default; - - bool isValid() const { - return isValidValue(m_value); - } - - bool isEmpty() const { - DEBUG_ASSERT(isValid()); - return m_value.isEmpty(); - } - - const value_t& value() const { - DEBUG_ASSERT(isValid()); - return m_value; - } - operator const value_t&() const { - return value(); - } - - private: - value_t m_value; -}; - -inline bool operator==( - const TagFacet& lhs, - const TagFacet& rhs) { - return lhs.value() == rhs.value(); -} - -inline bool operator!=( - const TagFacet& lhs, - const TagFacet& rhs) { - return !(lhs == rhs); -} - -inline bool operator<( - const TagFacet& lhs, - const TagFacet& rhs) { - return lhs.value() < rhs.value(); -} - -inline bool operator>( - const TagFacet& lhs, - const TagFacet& rhs) { - return !(lhs < rhs); -} - -inline bool operator<=( - const TagFacet& lhs, - const TagFacet& rhs) { - return !(lhs > rhs); -} - -inline bool operator>=( - const TagFacet& lhs, - const TagFacet& rhs) { - return !(lhs < rhs); -} - -inline uint qHash( - const TagFacet& facet, - uint seed = 0) { - return qHash(facet.value(), seed); -} - -/// The displayable name or title of a tag. -/// -/// The value contains arbitrary Unicode text that is supposed to -/// be displayed to the user. -/// -/// Value constraints: -/// - no leading/trailing whitespace -class TagLabel final { - public: - typedef QString value_t; - - static const value_t defaultValue() { - return value_t{}; - } - - static bool isValidValue( - const value_t& value); - - /// Remove leading/trailing whitespace. - static value_t convertIntoValidValue( - const value_t& value); - - /// Ensure that empty values are always null - static value_t filterEmptyValue( - value_t value) { - DEBUG_ASSERT(defaultValue().isEmpty()); - return value.isEmpty() ? defaultValue() : value; - } - - explicit TagLabel( - value_t value = defaultValue()) - : m_value(std::move(value)) { - DEBUG_ASSERT(isValid()); - } - TagLabel(const TagLabel&) = default; - TagLabel(TagLabel&&) = default; - - TagLabel& operator=(const TagLabel&) = default; - TagLabel& operator=(TagLabel&&) = default; - - bool isValid() const { - return isValidValue(m_value); - } - - bool isEmpty() const { - DEBUG_ASSERT(isValid()); - return m_value.isEmpty(); - } - - const value_t& value() const { - DEBUG_ASSERT(isValid()); - return m_value; - } - operator const value_t&() const { - return value(); - } - - private: - value_t m_value; -}; - -inline bool operator==( - const TagLabel& lhs, - const TagLabel& rhs) { - return lhs.value() == rhs.value(); -} - -inline bool operator!=( - const TagLabel& lhs, - const TagLabel& rhs) { - return !(lhs == rhs); -} - -inline bool operator<( - const TagLabel& lhs, - const TagLabel& rhs) { - return lhs.value() < rhs.value(); -} - -inline bool operator>( - const TagLabel& lhs, - const TagLabel& rhs) { - return !(lhs < rhs); -} - -inline bool operator<=( - const TagLabel& lhs, - const TagLabel& rhs) { - return !(lhs > rhs); -} - -inline bool operator>=( - const TagLabel& lhs, - const TagLabel& rhs) { - return !(lhs < rhs); -} - -inline uint qHash( - const TagLabel& label, - uint seed = 0) { - return qHash(label.value(), seed); -} - -/// The score or weight of a tag relationship, defaulting to 1.0. -/// -/// Constraints: Normalized to the unit interval [0.0, 1.0] -class TagScore final { - public: - typedef double value_t; - - static constexpr value_t kMinValue = 0.0; - static constexpr value_t kMaxValue = 1.0; - static constexpr value_t kDefaultValue = kMaxValue; - - static constexpr bool isValidValue( - value_t value) { - return value >= kMinValue && value <= kMaxValue; - } - - /// Clamp the value to the valid range [kMinValue, kMaxValue]. - static constexpr value_t convertIntoValidValue( - value_t value) { - return math_min(kMaxValue, math_max(kMinValue, value)); - } - - explicit TagScore( - value_t value = kDefaultValue) - : m_value(value) { - DEBUG_ASSERT(isValid()); - } - - constexpr bool isValid() const { - return isValidValue(m_value); - } - - constexpr value_t value() const { - return m_value; - } - constexpr operator value_t() const { - return value(); - } - - private: - value_t m_value; -}; - -/// A plain tag with an optional label and a score. -/// -/// Plain tags are not faceted. The facet is usually provided -/// by the outer context, e.g. the key of a map that stores -/// plain tags as values. +/// The optional facet is usually provided by the outer context, +/// e.g. the key of a map that stores plain tags as values. /// /// Mixxx uses these kind of tags as `CustomTags` to store extended /// metadata. They should not be confused with file tags (ID3, MP4, @@ -319,9 +63,12 @@ class Tag final { QJsonValue toJsonValue() const; }; -bool operator==( +inline bool operator==( const Tag& lhs, - const Tag& rhs); + const Tag& rhs) { + return lhs.getLabel() == rhs.getLabel() && + lhs.getScore() == rhs.getScore(); +} inline bool operator!=( const Tag& lhs, @@ -339,8 +86,5 @@ typedef QVector TagVector; } // namespace mixxx -Q_DECLARE_METATYPE(mixxx::library::tags::TagFacet) -Q_DECLARE_METATYPE(mixxx::library::tags::TagLabel) -Q_DECLARE_METATYPE(mixxx::library::tags::TagScore) Q_DECLARE_METATYPE(mixxx::library::tags::Tag) Q_DECLARE_METATYPE(mixxx::library::tags::TagVector) diff --git a/src/library/tags/tagfacet.cpp b/src/library/tags/tagfacet.cpp new file mode 100644 index 00000000000..a918d7cec6d --- /dev/null +++ b/src/library/tags/tagfacet.cpp @@ -0,0 +1,49 @@ +#include "library/tags/tagfacet.h" + +#include + +namespace { + +const QRegularExpression kLowercaseAsciiNotEmpty( + QStringLiteral("^[\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); +const QRegularExpression kInverseLowercaseAsciiNotEmpty( + QStringLiteral("[^\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); + +} // anonymous namespace + +namespace mixxx { + +namespace library { + +namespace tags { + +//static +bool TagFacet::isValidValue( + const value_t& value) { + if (value.isNull()) { + return true; + } + if (value.isEmpty()) { + // for disambiguation with null + return false; + } + const auto match = kLowercaseAsciiNotEmpty.match(value); + DEBUG_ASSERT(match.isValid()); + DEBUG_ASSERT(value.length() > 0); + // match = exact match + return match.capturedLength() == value.length(); +} + +//static +TagFacet::value_t TagFacet::convertIntoValidValue( + const value_t& value) { + auto validValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); + DEBUG_ASSERT(isValidValue(validValue)); + return validValue; +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h new file mode 100644 index 00000000000..ca5deab5718 --- /dev/null +++ b/src/library/tags/tagfacet.h @@ -0,0 +1,130 @@ +#pragma once + +#include +#include + +#include "util/assert.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +/// A category for tags. +/// +/// Facets are used for grouping/categorizing and providing context or meaning. +/// Their value serves as a symbolic, internal identifier that is not intended +/// to be displayed literally in the UI. This is also the reasons for the +/// restrictions on naming them. +/// +/// Value constraints: +/// - lowercase ASCII +/// - and no whitespace +/// +/// References: +/// - https://en.wikipedia.org/wiki/Faceted_classification +class TagFacet final { + public: + typedef QString value_t; + + static value_t defaultValue() { + return value_t{}; + } + + static bool isValidValue( + const value_t& value); + + /// Convert the given string into lowercase and then + /// removes all whitespace and non-ASCII characters. + static value_t convertIntoValidValue( + const value_t& value); + + /// Ensure that empty values are always null + static value_t filterEmptyValue( + value_t value) { + DEBUG_ASSERT(defaultValue().isEmpty()); + return value.isEmpty() ? defaultValue() : value; + } + + explicit TagFacet( + value_t value = defaultValue()) + : m_value(std::move(value)) { + DEBUG_ASSERT(isValid()); + } + TagFacet(const TagFacet&) = default; + TagFacet(TagFacet&&) = default; + + TagFacet& operator=(const TagFacet&) = default; + TagFacet& operator=(TagFacet&&) = default; + + bool isValid() const { + return isValidValue(m_value); + } + + bool isEmpty() const { + DEBUG_ASSERT(isValid()); + return m_value.isEmpty(); + } + + const value_t& value() const { + DEBUG_ASSERT(isValid()); + return m_value; + } + operator const value_t&() const { + return value(); + } + + private: + value_t m_value; +}; + +inline bool operator==( + const TagFacet& lhs, + const TagFacet& rhs) { + return lhs.value() == rhs.value(); +} + +inline bool operator!=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs == rhs); +} + +inline bool operator<( + const TagFacet& lhs, + const TagFacet& rhs) { + return lhs.value() < rhs.value(); +} + +inline bool operator>( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs < rhs); +} + +inline bool operator<=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs > rhs); +} + +inline bool operator>=( + const TagFacet& lhs, + const TagFacet& rhs) { + return !(lhs < rhs); +} + +inline uint qHash( + const TagFacet& facet, + uint seed = 0) { + return qHash(facet.value(), seed); +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::library::tags::TagFacet) diff --git a/src/library/tags/taglabel.cpp b/src/library/tags/taglabel.cpp new file mode 100644 index 00000000000..8a785167a95 --- /dev/null +++ b/src/library/tags/taglabel.cpp @@ -0,0 +1,34 @@ +#include "library/tags/taglabel.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +//static +bool TagLabel::isValidValue( + const value_t& value) { + if (value.isNull()) { + return true; + } + if (value.isEmpty()) { + // for disambiguation with null + return false; + } + return value.trimmed() == value; +} + +//static +TagLabel::value_t TagLabel::convertIntoValidValue( + const value_t& value) { + auto validValue = filterEmptyValue(value.trimmed()); + DEBUG_ASSERT(isValidValue(validValue)); + return validValue; +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx diff --git a/src/library/tags/taglabel.h b/src/library/tags/taglabel.h new file mode 100644 index 00000000000..8ae5b9cec11 --- /dev/null +++ b/src/library/tags/taglabel.h @@ -0,0 +1,123 @@ +#pragma once + +#include +#include + +#include "util/assert.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +/// The displayable name or title of a tag. +/// +/// The value contains arbitrary Unicode text that is supposed to +/// be displayed to the user. +/// +/// Value constraints: +/// - no leading/trailing whitespace +class TagLabel final { + public: + typedef QString value_t; + + static const value_t defaultValue() { + return value_t{}; + } + + static bool isValidValue( + const value_t& value); + + /// Remove leading/trailing whitespace. + static value_t convertIntoValidValue( + const value_t& value); + + /// Ensure that empty values are always null + static value_t filterEmptyValue( + value_t value) { + DEBUG_ASSERT(defaultValue().isEmpty()); + return value.isEmpty() ? defaultValue() : value; + } + + explicit TagLabel( + value_t value = defaultValue()) + : m_value(std::move(value)) { + DEBUG_ASSERT(isValid()); + } + TagLabel(const TagLabel&) = default; + TagLabel(TagLabel&&) = default; + + TagLabel& operator=(const TagLabel&) = default; + TagLabel& operator=(TagLabel&&) = default; + + bool isValid() const { + return isValidValue(m_value); + } + + bool isEmpty() const { + DEBUG_ASSERT(isValid()); + return m_value.isEmpty(); + } + + const value_t& value() const { + DEBUG_ASSERT(isValid()); + return m_value; + } + operator const value_t&() const { + return value(); + } + + private: + value_t m_value; +}; + +inline bool operator==( + const TagLabel& lhs, + const TagLabel& rhs) { + return lhs.value() == rhs.value(); +} + +inline bool operator!=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs == rhs); +} + +inline bool operator<( + const TagLabel& lhs, + const TagLabel& rhs) { + return lhs.value() < rhs.value(); +} + +inline bool operator>( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs < rhs); +} + +inline bool operator<=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs > rhs); +} + +inline bool operator>=( + const TagLabel& lhs, + const TagLabel& rhs) { + return !(lhs < rhs); +} + +inline uint qHash( + const TagLabel& label, + uint seed = 0) { + return qHash(label.value(), seed); +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::library::tags::TagLabel) diff --git a/src/library/tags/tagscore.h b/src/library/tags/tagscore.h new file mode 100644 index 00000000000..b3c5788305d --- /dev/null +++ b/src/library/tags/tagscore.h @@ -0,0 +1,63 @@ +#pragma once + +#include + +#include "util/assert.h" +#include "util/math.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +/// The score or weight of a tag relationship, defaulting to 1.0. +/// +/// Constraints: Normalized to the unit interval [0.0, 1.0] +class TagScore final { + public: + typedef double value_t; + + static constexpr value_t kMinValue = 0.0; + static constexpr value_t kMaxValue = 1.0; + static constexpr value_t kDefaultValue = kMaxValue; + + static constexpr bool isValidValue( + value_t value) { + return value >= kMinValue && value <= kMaxValue; + } + + /// Clamp the value to the valid range [kMinValue, kMaxValue]. + static constexpr value_t convertIntoValidValue( + value_t value) { + return math_min(kMaxValue, math_max(kMinValue, value)); + } + + explicit TagScore( + value_t value = kDefaultValue) + : m_value(value) { + DEBUG_ASSERT(isValid()); + } + + constexpr bool isValid() const { + return isValidValue(m_value); + } + + constexpr value_t value() const { + return m_value; + } + constexpr operator value_t() const { + return value(); + } + + private: + value_t m_value; +}; + +} // namespace tags + +} // namespace library + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::library::tags::TagScore) diff --git a/src/mixxxapplication.cpp b/src/mixxxapplication.cpp index d6d8758a923..6be256ad211 100644 --- a/src/mixxxapplication.cpp +++ b/src/mixxxapplication.cpp @@ -8,6 +8,7 @@ #include "audio/types.h" #include "control/controlproxy.h" #include "library/tags/tag.h" +#include "library/tags/tagfacet.h" #include "library/trackset/crate/crateid.h" #include "moc_mixxxapplication.cpp" #include "soundio/soundmanagerutil.h" diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp index a44cb6a9d94..1568c505b52 100644 --- a/src/test/librarytags_test.cpp +++ b/src/test/librarytags_test.cpp @@ -4,6 +4,7 @@ #include #include "library/tags/tag.h" +#include "library/tags/tagfacet.h" namespace mixxx { From 70635272b1d1cc80c0e56dd883b8ef4f59425776 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 29 Aug 2021 18:27:18 +0200 Subject: [PATCH 09/39] TagFacet: Enable definition of static, non-validated constants --- src/library/tags/tagfacet.h | 34 +++++++++++++++++++++++++++------- src/library/tags/taglabel.h | 9 ++------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h index ca5deab5718..c61bd658d07 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacet.h @@ -28,10 +28,6 @@ class TagFacet final { public: typedef QString value_t; - static value_t defaultValue() { - return value_t{}; - } - static bool isValidValue( const value_t& value); @@ -43,15 +39,39 @@ class TagFacet final { /// Ensure that empty values are always null static value_t filterEmptyValue( value_t value) { - DEBUG_ASSERT(defaultValue().isEmpty()); - return value.isEmpty() ? defaultValue() : value; + return value.isEmpty() ? value_t{} : value; } + /// Default constructor. + TagFacet() = default; + + /// Create a new instance. + /// + /// This constructor must not be used for static constants! explicit TagFacet( - value_t value = defaultValue()) + value_t value) : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); } + + /// Type-tag for creating non-validated, static constants. + /// + /// The regular expressions required for validation are also + /// static constant defined in this compilation unit. The + /// initialization order between compilation units is undefined! + enum struct StaticCtor {}; + + /// Constructor for creating non-validated, static constants. + TagFacet( + StaticCtor, + value_t value) + : m_value(std::move(value)) { + } + + static TagFacet staticConst(value_t value) { + return TagFacet(StaticCtor{}, value); + } + TagFacet(const TagFacet&) = default; TagFacet(TagFacet&&) = default; diff --git a/src/library/tags/taglabel.h b/src/library/tags/taglabel.h index 8ae5b9cec11..fe00152bfc4 100644 --- a/src/library/tags/taglabel.h +++ b/src/library/tags/taglabel.h @@ -22,10 +22,6 @@ class TagLabel final { public: typedef QString value_t; - static const value_t defaultValue() { - return value_t{}; - } - static bool isValidValue( const value_t& value); @@ -36,12 +32,11 @@ class TagLabel final { /// Ensure that empty values are always null static value_t filterEmptyValue( value_t value) { - DEBUG_ASSERT(defaultValue().isEmpty()); - return value.isEmpty() ? defaultValue() : value; + return value.isEmpty() ? value_t{} : value; } explicit TagLabel( - value_t value = defaultValue()) + value_t value = value_t{}) : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); } From fd640b52837fd1a8baffe0c273f20112514f8f25 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 30 Aug 2021 22:36:40 +0200 Subject: [PATCH 10/39] Explain the purpose of facet identifiers --- src/library/tags/tagfacet.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h index c61bd658d07..ac45511d2af 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacet.h @@ -14,9 +14,13 @@ namespace tags { /// A category for tags. /// /// Facets are used for grouping/categorizing and providing context or meaning. +/// /// Their value serves as a symbolic, internal identifier that is not intended -/// to be displayed literally in the UI. This is also the reasons for the -/// restrictions on naming them. +/// to be displayed literally in the UI. The restrictive nameing constraints +/// ensure that they are not used for storing arbitrary text. Instead facet +/// identifiers should be mapped to translated display strings, e.g. the +/// facet "genre" could be mapped to "Genre" in English and the facet "venue" +/// could be mapped to "Veranstaltungsort" in German. /// /// Value constraints: /// - lowercase ASCII From 726388f55d3048f316a89975effb6c4f386a0791 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 30 Aug 2021 23:46:57 +0200 Subject: [PATCH 11/39] Fix clazy warnings --- src/library/tags/tagfacet.h | 6 ++++-- src/library/tags/taglabel.h | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h index ac45511d2af..ee91b0d1f14 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacet.h @@ -43,7 +43,9 @@ class TagFacet final { /// Ensure that empty values are always null static value_t filterEmptyValue( value_t value) { - return value.isEmpty() ? value_t{} : value; + // std::move() is required despite Return Value Optimization (RVO) + // to avoid clazy warnings! + return value.isEmpty() ? value_t{} : std::move(value); } /// Default constructor. @@ -73,7 +75,7 @@ class TagFacet final { } static TagFacet staticConst(value_t value) { - return TagFacet(StaticCtor{}, value); + return TagFacet(StaticCtor{}, std::move(value)); } TagFacet(const TagFacet&) = default; diff --git a/src/library/tags/taglabel.h b/src/library/tags/taglabel.h index fe00152bfc4..1e7fb22fd19 100644 --- a/src/library/tags/taglabel.h +++ b/src/library/tags/taglabel.h @@ -32,7 +32,9 @@ class TagLabel final { /// Ensure that empty values are always null static value_t filterEmptyValue( value_t value) { - return value.isEmpty() ? value_t{} : value; + // std::move() is required despite Return Value Optimization (RVO) + // to avoid clazy warnings! + return value.isEmpty() ? value_t{} : std::move(value); } explicit TagLabel( From e95d6ff1d795cefe10f5db75d2792717198a60fa Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 31 Aug 2021 02:29:21 +0200 Subject: [PATCH 12/39] Restrict the alphabet of tag facets --- src/library/tags/tagfacet.cpp | 12 ++--- src/library/tags/tagfacet.h | 9 +++- src/library/tags/taglabel.h | 1 + src/test/customtags_test.cpp | 32 ------------ src/test/librarytags_test.cpp | 97 ++++++++++++++++++++++++----------- 5 files changed, 81 insertions(+), 70 deletions(-) diff --git a/src/library/tags/tagfacet.cpp b/src/library/tags/tagfacet.cpp index a918d7cec6d..63020e14cfd 100644 --- a/src/library/tags/tagfacet.cpp +++ b/src/library/tags/tagfacet.cpp @@ -4,10 +4,10 @@ namespace { -const QRegularExpression kLowercaseAsciiNotEmpty( - QStringLiteral("^[\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); -const QRegularExpression kInverseLowercaseAsciiNotEmpty( - QStringLiteral("[^\\x{0021}-\\x{0040}\\x{005B}-\\x{007E}]+")); +const QRegularExpression kValidFacetStringNotEmpty( + QStringLiteral("^[\\+\\-\\./0-9@a-z\\[\\]_]+")); +const QRegularExpression kInversekValidFacetStringNotEmpty( + QStringLiteral("[^\\+\\-\\./0-9@a-z\\[\\]_]+")); } // anonymous namespace @@ -27,7 +27,7 @@ bool TagFacet::isValidValue( // for disambiguation with null return false; } - const auto match = kLowercaseAsciiNotEmpty.match(value); + const auto match = kValidFacetStringNotEmpty.match(value); DEBUG_ASSERT(match.isValid()); DEBUG_ASSERT(value.length() > 0); // match = exact match @@ -37,7 +37,7 @@ bool TagFacet::isValidValue( //static TagFacet::value_t TagFacet::convertIntoValidValue( const value_t& value) { - auto validValue = filterEmptyValue(value.toLower().remove(kInverseLowercaseAsciiNotEmpty)); + auto validValue = filterEmptyValue(value.toLower().remove(kInversekValidFacetStringNotEmpty)); DEBUG_ASSERT(isValidValue(validValue)); return validValue; } diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h index ee91b0d1f14..996001249db 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacet.h @@ -23,8 +23,8 @@ namespace tags { /// could be mapped to "Veranstaltungsort" in German. /// /// Value constraints: -/// - lowercase ASCII -/// - and no whitespace +/// - charset: +-._/0123456789abcdefghijklmnopqrstuvwxyz +/// - no leading/trailing/inner whitespace /// /// References: /// - https://en.wikipedia.org/wiki/Faceted_classification @@ -32,6 +32,11 @@ class TagFacet final { public: typedef QString value_t; + /// The alphabet of facets + /// + /// All valid characters, ordered by their ASCII codes. + static constexpr const char* kAlphabet = "+-./0123456789@[]_abcdefghijklmnopqrstuvwxyz"; + static bool isValidValue( const value_t& value); diff --git a/src/library/tags/taglabel.h b/src/library/tags/taglabel.h index 1e7fb22fd19..52aabb0d285 100644 --- a/src/library/tags/taglabel.h +++ b/src/library/tags/taglabel.h @@ -17,6 +17,7 @@ namespace tags { /// be displayed to the user. /// /// Value constraints: +/// - charset: Unicode /// - no leading/trailing whitespace class TagLabel final { public: diff --git a/src/test/customtags_test.cpp b/src/test/customtags_test.cpp index 65a0d24c9c9..39647aab4c8 100644 --- a/src/test/customtags_test.cpp +++ b/src/test/customtags_test.cpp @@ -15,38 +15,6 @@ namespace tags { class CustomTagsTest : public testing::Test { }; -TEST_F(CustomTagsTest, isValidFacetValue) { - EXPECT_TRUE(TagFacet::isValidValue(QString())); - EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1"))); - EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("with\\backslash"))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("Uppercase"))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1_and_whitespace "))); - EXPECT_FALSE(TagFacet::isValidValue(QLatin1String(""))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral(" "))); -} - -TEST_F(CustomTagsTest, clampFacetValue) { - EXPECT_EQ(QString(), TagFacet::convertIntoValidValue(QString())); - EXPECT_EQ(QString("lower-case_with_digit_1_and_whitespaces"), - TagFacet::convertIntoValidValue(QStringLiteral( - " lower -\tcase_with _digit_1_and_whitespaces "))); - EXPECT_EQ(QString(), TagFacet::convertIntoValidValue(QStringLiteral(" \n \t \r"))); - EXPECT_EQ(QString("uppercase"), TagFacet::convertIntoValidValue(QStringLiteral("Uppercase"))); - EXPECT_EQ(QString("with\\back/slash"), - TagFacet::convertIntoValidValue(QStringLiteral("with \\ back/\tslash"))); -} - -TEST_F(CustomTagsTest, isValidLabelValue) { - EXPECT_TRUE(TagLabel::isValidValue(QString())); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("lower-case_with_digit_1"))); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("With\\backslash"))); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("Uppercase with\twhitespace"))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" With leading space"))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral("With trailing space\n"))); - EXPECT_FALSE(TagLabel::isValidValue(QLatin1String(""))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" "))); -} - TEST_F(CustomTagsTest, isEmptyAndCount) { const auto facet = TagFacet(QStringLiteral("facet")); const auto label = TagLabel(QStringLiteral("Label")); diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp index 1568c505b52..27c59a3a2b2 100644 --- a/src/test/librarytags_test.cpp +++ b/src/test/librarytags_test.cpp @@ -44,29 +44,53 @@ TEST_F(TagFacetTest, filterEmptyValue) { TEST_F(TagFacetTest, convertIntoValidValue) { // Clamp empty string EXPECT_EQ( - TagFacet(), - TagFacet(TagFacet::convertIntoValidValue(""))); + TagFacet::value_t{}, + TagFacet::convertIntoValidValue("")); // Clamped whitespace string EXPECT_EQ( - TagFacet(), - TagFacet(TagFacet::convertIntoValidValue(" \t\n "))); + TagFacet::value_t{}, + TagFacet::convertIntoValidValue(" \t\n ")); // Lowercase EXPECT_EQ( - TagFacet("x"), - TagFacet(TagFacet::convertIntoValidValue(" \tX\n "))); + TagFacet::value_t{"x"}, + TagFacet::convertIntoValidValue(" \tX\n ")); // Whitespace replacement EXPECT_EQ( - TagFacet("xy_{+}"), - TagFacet(TagFacet::convertIntoValidValue("X y _\n{+} "))); - // Lowercase ASCII alphabet without whitespace - EXPECT_EQ( - TagFacet("!\"#$%&'()*+,-./0123456789:;<=>?" - "@abcdefghijklmnopqrstuvwxyz[\\]^_" - "`abcdefghijklmnopqrstuvwxyz{|}~"), - TagFacet(TagFacet::convertIntoValidValue( - " !\"#$%&'()*+,-./0123456789:;<=>?" - "@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_" - "`abcdefghijklmnopqrstuvwxyz{|}~"))); + TagFacet::value_t{"xy_[+]"}, + TagFacet::convertIntoValidValue("X y _\n[+] ")); + // Valid characters without whitespace + EXPECT_EQ( + TagFacet::value_t{TagFacet::kAlphabet}, + TagFacet::convertIntoValidValue( + TagFacet::value_t{TagFacet::kAlphabet})); + EXPECT_EQ( + TagFacet::value_t{"+-./0123456789" + "@abcdefghijklmnopqrstuvwxyz[]_" + "abcdefghijklmnopqrstuvwxyz"}, + TagFacet::convertIntoValidValue( + "\t!\"#$%&'()*+,-./0123456789:;<=>?" + " @ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_" + " `abcdefghijklmnopqrstuvwxyz{|}~\n")); +} + +TEST_F(TagFacetTest, isValidValue) { + EXPECT_TRUE(TagFacet::isValidValue(QString())); + EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("facet@mixxx.org/simple-test[1]"))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("Uppercase"))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1_and_whitespace "))); + EXPECT_FALSE(TagFacet::isValidValue(QLatin1String(""))); + EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral(" "))); +} + +TEST_F(TagFacetTest, convertedEmptyValueIsValid) { + EXPECT_TRUE( + TagFacet::isValidValue(TagFacet::convertIntoValidValue(TagFacet::value_t{}))); + EXPECT_TRUE( + TagFacet::isValidValue(TagFacet::convertIntoValidValue(""))); + EXPECT_TRUE( + TagFacet::isValidValue(TagFacet::convertIntoValidValue(" "))); + EXPECT_TRUE( + TagFacet::isValidValue(TagFacet::convertIntoValidValue(" \t \n \r "))); } class TagLabelTest : public testing::Test { @@ -86,15 +110,6 @@ TEST_F(TagLabelTest, isEmpty) { EXPECT_FALSE(TagLabel("X").isEmpty()); } -TEST_F(TagLabelTest, clampEmptyValueIsValid) { - EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(TagLabel::value_t()))); - EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); - EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(" "))); -} - TEST_F(TagLabelTest, filterEmptyValue) { EXPECT_EQ( TagLabel::value_t(), @@ -113,12 +128,34 @@ TEST_F(TagLabelTest, convertIntoValidValue) { TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); // Clamped whitespace string EXPECT_EQ( - TagLabel(), - TagLabel(TagLabel::convertIntoValidValue(" \t\n "))); + TagLabel::value_t{}, + TagLabel::convertIntoValidValue(" \t\n ")); // Trimmed EXPECT_EQ( - TagLabel("X y"), - TagLabel(TagLabel::convertIntoValidValue(" \tX y\n "))); + TagLabel::value_t{"X y"}, + TagLabel::convertIntoValidValue(" \tX y\n ")); +} + +TEST_F(TagLabelTest, isValidValue) { + EXPECT_TRUE(TagLabel::isValidValue(QString())); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("lower-case_with_digit_1"))); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("With\\backslash"))); + EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("Uppercase with\twhitespace"))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" With leading space"))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral("With trailing space\n"))); + EXPECT_FALSE(TagLabel::isValidValue(QLatin1String(""))); + EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" "))); +} + +TEST_F(TagLabelTest, convertedEmptyValueIsValid) { + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::convertIntoValidValue(TagLabel::value_t{}))); + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); + EXPECT_TRUE( + TagLabel::isValidValue(TagLabel::convertIntoValidValue(" "))); + EXPECT_TRUE( + TagLabel::isValidValue(TagFacet::convertIntoValidValue(" \t \n \r "))); } class TagTest : public testing::Test { From 3e981158547b08783c7329a48d238c5661e53697 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 31 Aug 2021 12:00:06 +0200 Subject: [PATCH 13/39] Update tag facet doc comment --- src/library/tags/tagfacet.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacet.h index 996001249db..764fb111f4d 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacet.h @@ -23,9 +23,14 @@ namespace tags { /// could be mapped to "Veranstaltungsort" in German. /// /// Value constraints: -/// - charset: +-._/0123456789abcdefghijklmnopqrstuvwxyz +/// - charset/alphabet: +-./0123456789@[]_abcdefghijklmnopqrstuvwxyz /// - no leading/trailing/inner whitespace /// +/// Rationale for the value constraints: +/// - Facet identifiers are intended to be created, shared, and parsed worldwide +/// - The Lingua franca of IT is English +/// - ASCII characters can be encoded by a single byte in UTF-8 +/// /// References: /// - https://en.wikipedia.org/wiki/Faceted_classification class TagFacet final { From b2bc645087aabd1e73bdc1cae6e48929ff96f4a7 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 31 Aug 2021 13:57:24 +0200 Subject: [PATCH 14/39] Renaming: TagFacet -> TagFacetId --- CMakeLists.txt | 2 +- src/library/tags/customtags.cpp | 108 +++++++++--------- src/library/tags/customtags.h | 62 +++++----- .../tags/{tagfacet.cpp => tagfacetid.cpp} | 6 +- src/library/tags/{tagfacet.h => tagfacetid.h} | 63 +++++----- src/mixxxapplication.cpp | 4 +- src/test/customtags_test.cpp | 76 ++++++------ src/test/librarytags_test.cpp | 91 +++++++-------- 8 files changed, 207 insertions(+), 205 deletions(-) rename src/library/tags/{tagfacet.cpp => tagfacetid.cpp} (89%) rename src/library/tags/{tagfacet.h => tagfacetid.h} (68%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 53062983382..342e4d8b8ff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -854,7 +854,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyutils.cpp src/library/tags/customtags.cpp src/library/tags/tag.cpp - src/library/tags/tagfacet.cpp + src/library/tags/tagfacetid.cpp src/library/tags/taglabel.cpp src/track/playcounter.cpp src/track/replaygain.cpp diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index f9b30a281ca..0f9992d2332 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -57,7 +57,7 @@ bool CustomTags::validate() const { } else { // faceted tags if (!i.key().isValid()) { - // invalid facet + // invalid facetId return false; } for (auto j = i.value().begin(); @@ -76,8 +76,8 @@ bool CustomTags::validate() const { bool CustomTags::containsTag( const TagLabel& label, - const TagFacet& facet) const { - const auto i = getFacetedTags().find(facet); + const TagFacetId& facetId) const { + const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return false; } @@ -86,8 +86,8 @@ bool CustomTags::containsTag( int CustomTags::countTags( const TagLabel& label, - const TagFacet& facet) const { - const auto i = getFacetedTags().find(facet); + const TagFacetId& facetId) const { + const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return 0; } @@ -95,8 +95,8 @@ int CustomTags::countTags( } int CustomTags::countFacetedTags( - const TagFacet& facet) const { - const auto i = getFacetedTags().find(facet); + const TagFacetId& facetId) const { + const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return 0; } @@ -105,11 +105,11 @@ int CustomTags::countFacetedTags( bool CustomTags::addOrReplaceTag( const Tag& tag, - const TagFacet& facet) { - DEBUG_ASSERT(countTags(tag.getLabel(), facet) <= 1); - auto i = refFacetedTags().find(facet); + const TagFacetId& facetId) { + DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); + auto i = refFacetedTags().find(facetId); if (i == refFacetedTags().end()) { - i = refFacetedTags().insert(facet, TagMap{}); + i = refFacetedTags().insert(facetId, TagMap{}); } DEBUG_ASSERT(!tag.getLabel().isEmpty() || i.value().size() <= 1); DEBUG_ASSERT(i.value().count(tag.getLabel()) <= 1); @@ -127,9 +127,9 @@ bool CustomTags::addOrReplaceTag( bool CustomTags::removeTag( const TagLabel& label, - const TagFacet& facet) { - DEBUG_ASSERT(countTags(label, facet) <= 1); - const auto i = refFacetedTags().find(facet); + const TagFacetId& facetId) { + DEBUG_ASSERT(countTags(label, facetId) <= 1); + const auto i = refFacetedTags().find(facetId); if (i == refFacetedTags().end()) { return false; } @@ -137,7 +137,7 @@ bool CustomTags::removeTag( DEBUG_ASSERT(i.value().count(label) <= 1); if (i.value().remove(label) > 0) { if (i.value().isEmpty()) { - // Compact entry for this facet, i.e. remove it entirely + // Compact entry for this facetId, i.e. remove it entirely refFacetedTags().erase(i); } return true; @@ -148,7 +148,7 @@ bool CustomTags::removeTag( TagVector CustomTags::getTags() const { TagVector tags; - const auto i = getFacetedTags().find(TagFacet()); + const auto i = getFacetedTags().find(TagFacetId()); if (i == getFacetedTags().end()) { return tags; } @@ -162,32 +162,32 @@ TagVector CustomTags::getTags() const { } FacetTagMap::iterator CustomTags::replaceAllFacetedTags( - const TagFacet& facet, + const TagFacetId& facetId, const Tag& tag) { - DEBUG_ASSERT(!facet.isEmpty()); - DEBUG_ASSERT(countTags(tag.getLabel(), facet) <= 1); - auto i = refFacetedTags().find(facet); + DEBUG_ASSERT(!facetId.isEmpty()); + DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); + auto i = refFacetedTags().find(facetId); if (i == refFacetedTags().end()) { - i = refFacetedTags().insert(facet, TagMap{}); + i = refFacetedTags().insert(facetId, TagMap{}); } else { i.value().clear(); } i.value().insert(tag.getLabel(), tag.getScore()); - DEBUG_ASSERT(countFacetedTags(facet) == 1); + DEBUG_ASSERT(countFacetedTags(facetId) == 1); return i; } int CustomTags::removeAllFacetedTags( - const TagFacet& facet) { - DEBUG_ASSERT(!facet.isEmpty()); - return refFacetedTags().remove(facet); + const TagFacetId& facetId) { + DEBUG_ASSERT(!facetId.isEmpty()); + return refFacetedTags().remove(facetId); } TagVector CustomTags::getFacetedTagsOrdered( - const TagFacet& facet) const { - DEBUG_ASSERT(!facet.isEmpty()); + const TagFacetId& facetId) const { + DEBUG_ASSERT(!facetId.isEmpty()); TagVector tags; - auto i = getFacetedTags().find(facet); + auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return tags; } @@ -195,7 +195,7 @@ TagVector CustomTags::getFacetedTagsOrdered( for (auto j = i.value().begin(); j != i.value().end(); ++j) { tags += Tag(j.key(), j.value()); } - DEBUG_ASSERT(tags.size() == countFacetedTags(facet)); + DEBUG_ASSERT(tags.size() == countFacetedTags(facetId)); std::sort( tags.begin(), tags.end(), @@ -206,9 +206,9 @@ TagVector CustomTags::getFacetedTagsOrdered( } TagLabel CustomTags::getFacetedTagLabel( - const TagFacet& facet) const { - DEBUG_ASSERT(countFacetedTags(facet) <= 1); - const auto i = getFacetedTags().find(facet); + const TagFacetId& facetId) const { + DEBUG_ASSERT(countFacetedTags(facetId) <= 1); + const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end() || i.value().isEmpty()) { return TagLabel(); @@ -219,10 +219,10 @@ TagLabel CustomTags::getFacetedTagLabel( } std::optional CustomTags::getTagScore( - const TagFacet& facet, + const TagFacetId& facetId, const TagLabel& label) const { - DEBUG_ASSERT(countFacetedTags(facet) <= 1); - const auto i = getFacetedTags().find(facet); + DEBUG_ASSERT(countFacetedTags(facetId) <= 1); + const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end() || i.value().isEmpty()) { return std::nullopt; @@ -237,11 +237,11 @@ std::optional CustomTags::getTagScore( } Tag CustomTags::mergeFacetedTags( - const TagFacet& facet, + const TagFacetId& facetId, AggregateScoring aggregateScoring, const QString& joinLabelSeparator) const { - DEBUG_ASSERT(!facet.isEmpty()); - const auto tags = getFacetedTagsOrdered(facet); + DEBUG_ASSERT(!facetId.isEmpty()); + const auto tags = getFacetedTagsOrdered(facetId); if (tags.isEmpty()) { return Tag(); } @@ -302,33 +302,33 @@ std::optional CustomTags::fromJsonObject( for (auto i = jsonObject.begin(); i != jsonObject.end(); ++i) { - auto facetValue = TagFacet::filterEmptyValue(i.key()); - if (!TagFacet::isValidValue(facetValue)) { + auto facetIdValue = TagFacetId::filterEmptyValue(i.key()); + if (!TagFacetId::isValidValue(facetIdValue)) { VERIFY_OR_DEBUG_ASSERT(mode == FromJsonMode::Lenient) { qCWarning(kLogger) - << "Invalid facet" - << facetValue + << "Invalid facetId" + << facetIdValue << "from JSON"; return std::nullopt; } qCWarning(kLogger) - << "Skipping invalid facet" - << facetValue + << "Skipping invalid facetId" + << facetIdValue << "from JSON"; continue; } - const auto facet = TagFacet(std::move(facetValue)); + const auto facetId = TagFacetId(std::move(facetIdValue)); DEBUG_ASSERT(i.value().isArray()); const auto jsonArray = i.value().toArray(); if (jsonArray.isEmpty()) { - // Add an empty placeholder slot just for the facet. + // Add an empty placeholder slot just for the facetId. // This is behavior intended and needed, i.e. when // loading a set of predefined tags from a JSON file. - // Some entries may only contains the name of the facet, - // but no predefined tags/labels. The facet will be + // Some entries may only contains the name of the facetId, + // but no predefined tags/labels. The facetId will be // displayed in the UI with the option to add custom // labels. - customTags.addOrIgnoreFacet(facet); + customTags.addOrIgnoreFacet(facetId); continue; } for (const auto& jsonValue : jsonArray) { @@ -339,20 +339,20 @@ std::optional CustomTags::fromJsonObject( qCWarning(kLogger) << "Invalid tag" << jsonValue - << "from JSON for facet" - << facet.value(); + << "from JSON for facetId" + << facetId.value(); return std::nullopt; } qCWarning(kLogger) << "Skipping invalid tag" << jsonValue - << "from JSON for facet" - << facet.value(); + << "from JSON for facetId" + << facetId.value(); continue; } customTags.addOrReplaceTag( std::move(*tag), - facet); + facetId); } } return customTags; diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index 4d6f0bfe02e..494646db720 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -5,7 +5,7 @@ #include #include "library/tags/tag.h" -#include "library/tags/tagfacet.h" +#include "library/tags/tagfacetid.h" namespace mixxx { @@ -14,19 +14,21 @@ namespace library { namespace tags { typedef QMap TagMap; -typedef QMap FacetTagMap; +typedef QMap FacetTagMap; /// Custom tags /// -/// Each custom tag is represented by a 3-tuple (triple): a facet, a label, and a score: +/// Each custom tag is represented by a 3-tuple (triple) of a *facet (`TagFacetId`), +/// a *label* (`TagLabel`), and a *score* (`TagScore`): /// -/// - The *facet* is a lowercase, non-empty ASCII string without whitespace -/// - The *label* is non-empty free text +/// - The *facet* is represented by a *facet identifier* string +/// - The *label* is non-empty, free text without leading/trailing whitespace /// - The *score* is a floating-point value in the interval [0.0, 1.0] /// -/// Both *facet* and *label* are optional (= nullable), but at least one of them must be present. -/// The *score* is mandatory and is supposed to be interpreted as a *weight*, *degree of cohesion*, -/// or *normalized level* with a default value of 1.0. +/// Both *facet* and *label* are optional (= nullable), but at least one of them +/// must be present. The *score* is mandatory and is supposed to be interpreted +/// as a *weight*, *degree of cohesion*, or *normalized level* with a default value +/// of 1.0. /// /// The following combinations are allowed, missing values are indicated by `-`: /// @@ -63,46 +65,46 @@ class CustomTags final { bool isEmpty() const; bool containsFacet( - const TagFacet& facet) const { - return getFacetedTags().contains(facet); + const TagFacetId& facetId) const { + return getFacetedTags().contains(facetId); } /// Add an (empty) entry for the given facet if it does not exist yet. /// /// Existing entries are not affected. void addOrIgnoreFacet( - const TagFacet& facet) { - refFacetedTags()[facet]; - DEBUG_ASSERT(containsFacet(facet)); + const TagFacetId& facetId) { + refFacetedTags()[facetId]; + DEBUG_ASSERT(containsFacet(facetId)); } bool containsTag( const TagLabel& label, - const TagFacet& facet = TagFacet()) const; + const TagFacetId& facetId = TagFacetId()) const; int countTags( const TagLabel& label, - const TagFacet& facet = TagFacet()) const; + const TagFacetId& facetId = TagFacetId()) const; int countTags() const { - return countFacetedTags(TagFacet()); + return countFacetedTags(TagFacetId()); } TagMap& refTags() { - return refFacetedTags()[TagFacet()]; + return refFacetedTags()[TagFacetId()]; } int countFacetedTags( - const TagFacet& facet) const; + const TagFacetId& facetId) const; TagMap& refFacetedTags( - const TagFacet& facet) { - return refFacetedTags()[facet]; + const TagFacetId& facetId) { + return refFacetedTags()[facetId]; } bool addOrReplaceTag( const Tag& tag, - const TagFacet& facet = TagFacet()); + const TagFacetId& facetId = TagFacetId()); bool removeTag( const TagLabel& label, - const TagFacet& facet = TagFacet()); + const TagFacetId& facetId = TagFacetId()); bool addOrReplaceAllTags( const CustomTags& tags); @@ -113,28 +115,28 @@ class CustomTags final { /// Replace all existing tags of this facet with a single /// faceted tag or insert a new faceted tag. FacetTagMap::iterator replaceAllFacetedTags( - const TagFacet& facet, + const TagFacetId& facetId, const Tag& tag); int removeAllFacetedTags( - const TagFacet& facet); + const TagFacetId& facetId); /// Get all tags of a given facet sorted by score in descending order TagVector getFacetedTagsOrdered( - const TagFacet& facet) const; + const TagFacetId& facetId) const; /// Get the label of a single faceted tag, i.e. that /// occurs at most once and has no custom score. If the /// facet is unknown/absent an empty label is returned. TagLabel getFacetedTagLabel( - const TagFacet& facet) const; + const TagFacetId& facetId) const; /// Get the score of a tag if present. /// /// Returns `std::nullopt` if the corresponding (facet, label) /// combination that serves as the key does not exist. std::optional getTagScore( - const TagFacet& facet, + const TagFacetId& facetId, const TagLabel& label = TagLabel()) const; enum class AggregateScoring { @@ -146,14 +148,14 @@ class CustomTags final { // a single plain tag. The strings of the labels are joined // with a separator in between and the scores are aggregated. Tag mergeFacetedTags( - const TagFacet& facet, + const TagFacetId& facetId, AggregateScoring aggregateScoring, const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const; TagLabel joinFacetedTagsLabel( - const TagFacet& facet, + const TagFacetId& facetId, const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const { return mergeFacetedTags( - facet, + facetId, AggregateScoring::Maximum, joinLabelSeparator) .getLabel(); diff --git a/src/library/tags/tagfacet.cpp b/src/library/tags/tagfacetid.cpp similarity index 89% rename from src/library/tags/tagfacet.cpp rename to src/library/tags/tagfacetid.cpp index 63020e14cfd..39a3cfa988e 100644 --- a/src/library/tags/tagfacet.cpp +++ b/src/library/tags/tagfacetid.cpp @@ -1,4 +1,4 @@ -#include "library/tags/tagfacet.h" +#include "library/tags/tagfacetid.h" #include @@ -18,7 +18,7 @@ namespace library { namespace tags { //static -bool TagFacet::isValidValue( +bool TagFacetId::isValidValue( const value_t& value) { if (value.isNull()) { return true; @@ -35,7 +35,7 @@ bool TagFacet::isValidValue( } //static -TagFacet::value_t TagFacet::convertIntoValidValue( +TagFacetId::value_t TagFacetId::convertIntoValidValue( const value_t& value) { auto validValue = filterEmptyValue(value.toLower().remove(kInversekValidFacetStringNotEmpty)); DEBUG_ASSERT(isValidValue(validValue)); diff --git a/src/library/tags/tagfacet.h b/src/library/tags/tagfacetid.h similarity index 68% rename from src/library/tags/tagfacet.h rename to src/library/tags/tagfacetid.h index 764fb111f4d..f998ceeda13 100644 --- a/src/library/tags/tagfacet.h +++ b/src/library/tags/tagfacetid.h @@ -11,16 +11,15 @@ namespace library { namespace tags { -/// A category for tags. +/// An identifier for referencing tag categories. /// /// Facets are used for grouping/categorizing and providing context or meaning. /// -/// Their value serves as a symbolic, internal identifier that is not intended -/// to be displayed literally in the UI. The restrictive nameing constraints -/// ensure that they are not used for storing arbitrary text. Instead facet -/// identifiers should be mapped to translated display strings, e.g. the -/// facet "genre" could be mapped to "Genre" in English and the facet "venue" -/// could be mapped to "Veranstaltungsort" in German. +/// Serves as a symbolic, internal identifier that is not intended to be displayed +/// literally in the UI. The restrictive nameing constraints ensure that they are +/// not used for storing arbitrary text. Instead facet identifiers should be mapped +/// to translated display strings, e.g. the facet "genre" could be mapped to "Genre" +/// in English and the facet "venue" could be mapped to "Veranstaltungsort" in German. /// /// Value constraints: /// - charset/alphabet: +-./0123456789@[]_abcdefghijklmnopqrstuvwxyz @@ -33,7 +32,7 @@ namespace tags { /// /// References: /// - https://en.wikipedia.org/wiki/Faceted_classification -class TagFacet final { +class TagFacetId final { public: typedef QString value_t; @@ -59,12 +58,12 @@ class TagFacet final { } /// Default constructor. - TagFacet() = default; + TagFacetId() = default; /// Create a new instance. /// /// This constructor must not be used for static constants! - explicit TagFacet( + explicit TagFacetId( value_t value) : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); @@ -78,21 +77,21 @@ class TagFacet final { enum struct StaticCtor {}; /// Constructor for creating non-validated, static constants. - TagFacet( + TagFacetId( StaticCtor, value_t value) : m_value(std::move(value)) { } - static TagFacet staticConst(value_t value) { - return TagFacet(StaticCtor{}, std::move(value)); + static TagFacetId staticConst(value_t value) { + return TagFacetId(StaticCtor{}, std::move(value)); } - TagFacet(const TagFacet&) = default; - TagFacet(TagFacet&&) = default; + TagFacetId(const TagFacetId&) = default; + TagFacetId(TagFacetId&&) = default; - TagFacet& operator=(const TagFacet&) = default; - TagFacet& operator=(TagFacet&&) = default; + TagFacetId& operator=(const TagFacetId&) = default; + TagFacetId& operator=(TagFacetId&&) = default; bool isValid() const { return isValidValue(m_value); @@ -116,45 +115,45 @@ class TagFacet final { }; inline bool operator==( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return lhs.value() == rhs.value(); } inline bool operator!=( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return !(lhs == rhs); } inline bool operator<( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return lhs.value() < rhs.value(); } inline bool operator>( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return !(lhs < rhs); } inline bool operator<=( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return !(lhs > rhs); } inline bool operator>=( - const TagFacet& lhs, - const TagFacet& rhs) { + const TagFacetId& lhs, + const TagFacetId& rhs) { return !(lhs < rhs); } inline uint qHash( - const TagFacet& facet, + const TagFacetId& facetId, uint seed = 0) { - return qHash(facet.value(), seed); + return qHash(facetId.value(), seed); } } // namespace tags @@ -163,4 +162,4 @@ inline uint qHash( } // namespace mixxx -Q_DECLARE_METATYPE(mixxx::library::tags::TagFacet) +Q_DECLARE_METATYPE(mixxx::library::tags::TagFacetId) diff --git a/src/mixxxapplication.cpp b/src/mixxxapplication.cpp index 6be256ad211..16f0ace8f4f 100644 --- a/src/mixxxapplication.cpp +++ b/src/mixxxapplication.cpp @@ -8,7 +8,7 @@ #include "audio/types.h" #include "control/controlproxy.h" #include "library/tags/tag.h" -#include "library/tags/tagfacet.h" +#include "library/tags/tagfacetid.h" #include "library/trackset/crate/crateid.h" #include "moc_mixxxapplication.cpp" #include "soundio/soundmanagerutil.h" @@ -97,7 +97,7 @@ void MixxxApplication::registerMetaTypes() { QMetaType::registerComparators(); // Library: Tags - qRegisterMetaType("mixxx::library::tags::TagFacet"); + qRegisterMetaType("mixxx::library::tags::TagFacetId"); qRegisterMetaType("mixxx::library::tags::TagLabel"); qRegisterMetaType("mixxx::library::tags::TagScore"); qRegisterMetaType("mixxx::library::tags::Tag"); diff --git a/src/test/customtags_test.cpp b/src/test/customtags_test.cpp index 39647aab4c8..ad32620ff33 100644 --- a/src/test/customtags_test.cpp +++ b/src/test/customtags_test.cpp @@ -16,64 +16,64 @@ class CustomTagsTest : public testing::Test { }; TEST_F(CustomTagsTest, isEmptyAndCount) { - const auto facet = TagFacet(QStringLiteral("facet")); + const auto facetId = TagFacetId(QStringLiteral("facet-id")); const auto label = TagLabel(QStringLiteral("Label")); CustomTags customTags; EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacet())); - EXPECT_EQ(0, customTags.countTags(label, facet)); - EXPECT_EQ(0, customTags.countFacetedTags(facet)); + EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, facetId)); + EXPECT_EQ(0, customTags.countFacetedTags(facetId)); customTags.addOrReplaceTag(Tag(label)); EXPECT_FALSE(customTags.isEmpty()); EXPECT_EQ(1, customTags.countTags(label)); - EXPECT_EQ(1, customTags.countTags(label, TagFacet())); - EXPECT_EQ(0, customTags.countTags(label, facet)); - EXPECT_EQ(0, customTags.countFacetedTags(facet)); + EXPECT_EQ(1, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, facetId)); + EXPECT_EQ(0, customTags.countFacetedTags(facetId)); customTags.removeTag(label); EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacet())); - EXPECT_EQ(0, customTags.countTags(label, facet)); - EXPECT_EQ(0, customTags.countFacetedTags(facet)); + EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, facetId)); + EXPECT_EQ(0, customTags.countFacetedTags(facetId)); - customTags.addOrReplaceTag(Tag(label), facet); + customTags.addOrReplaceTag(Tag(label), facetId); EXPECT_FALSE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacet())); - EXPECT_EQ(1, customTags.countTags(label, facet)); - EXPECT_EQ(1, customTags.countFacetedTags(facet)); + EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(1, customTags.countTags(label, facetId)); + EXPECT_EQ(1, customTags.countFacetedTags(facetId)); - customTags.removeTag(label, facet); + customTags.removeTag(label, facetId); EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacet())); - EXPECT_EQ(0, customTags.countTags(label, facet)); - EXPECT_EQ(0, customTags.countFacetedTags(facet)); + EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, facetId)); + EXPECT_EQ(0, customTags.countFacetedTags(facetId)); } TEST_F(CustomTagsTest, isEmptyWithEmptyFacetedSlot) { - const auto facet = TagFacet(QStringLiteral("facet")); + const auto facetId = TagFacetId(QStringLiteral("facet")); CustomTags customTags; ASSERT_EQ(0, customTags.getFacetedTags().size()); ASSERT_TRUE(customTags.isEmpty()); - // Add an empty placeholder slot just for the facet - customTags.refFacetedTags(facet); + // Add an empty placeholder slot just for the facetId + customTags.refFacetedTags(facetId); // Verify that an empty slot for faceted tags does // not affect the emptiness check. EXPECT_EQ(1, customTags.getFacetedTags().size()); EXPECT_TRUE(customTags.isEmpty()); - // Add an empty placeholder slot with an empty facet - customTags.refFacetedTags(TagFacet()); + // Add an empty placeholder slot with an empty facetId + customTags.refFacetedTags(TagFacetId()); - // Verify that an empty slot for an empty facet does + // Verify that an empty slot for an empty facetId does // not affect the emptiness check. EXPECT_EQ(2, customTags.getFacetedTags().size()); EXPECT_TRUE(customTags.isEmpty()); @@ -155,9 +155,9 @@ TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { }, }; - const auto facet1 = TagFacet(QStringLiteral("facet1")); - const auto facet2 = TagFacet(QStringLiteral("facet2")); - const auto facet3 = TagFacet(QStringLiteral("facet3")); + const auto facetId1 = TagFacetId(QStringLiteral("facet1")); + const auto facetId2 = TagFacetId(QStringLiteral("facet2")); + const auto facetId3 = TagFacetId(QStringLiteral("facet3")); const auto label1 = TagLabel(QStringLiteral("Label 1")); const auto label2 = TagLabel(QStringLiteral("Label 2")); @@ -181,27 +181,27 @@ TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { EXPECT_EQ(1, decoded->countTags(label2)); EXPECT_EQ( TagScore(), - decoded->getFacetedTags().value(TagFacet()).value(label1)); + decoded->getFacetedTags().value(TagFacetId()).value(label1)); EXPECT_EQ( TagScore(0.5), - decoded->getFacetedTags().value(TagFacet()).value(label2)); + decoded->getFacetedTags().value(TagFacetId()).value(label2)); // Faceted tags - EXPECT_EQ(2, decoded->countFacetedTags(facet1)); - EXPECT_EQ(2, decoded->countFacetedTags(facet2)); - EXPECT_EQ(0, decoded->countFacetedTags(facet3)); - const auto facet1TagsOrdered = TagVector{ + EXPECT_EQ(2, decoded->countFacetedTags(facetId1)); + EXPECT_EQ(2, decoded->countFacetedTags(facetId2)); + EXPECT_EQ(0, decoded->countFacetedTags(facetId3)); + const auto facetId1TagsOrdered = TagVector{ Tag(label1), Tag(TagScore(0.2))}; EXPECT_EQ( - facet1TagsOrdered, - decoded->getFacetedTagsOrdered(facet1)); - const auto facet2TagsOrdered = TagVector{ + facetId1TagsOrdered, + decoded->getFacetedTagsOrdered(facetId1)); + const auto facetId2TagsOrdered = TagVector{ Tag(), Tag(label2, TagScore(0.4))}; EXPECT_EQ( - facet2TagsOrdered, - decoded->getFacetedTagsOrdered(facet2)); + facetId2TagsOrdered, + decoded->getFacetedTagsOrdered(facetId2)); // Compaction decoded->compact(); diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp index 27c59a3a2b2..679651e654b 100644 --- a/src/test/librarytags_test.cpp +++ b/src/test/librarytags_test.cpp @@ -4,7 +4,7 @@ #include #include "library/tags/tag.h" -#include "library/tags/tagfacet.h" +#include "library/tags/tagfacetid.h" namespace mixxx { @@ -12,85 +12,86 @@ namespace library { namespace tags { -class TagFacetTest : public testing::Test { +class TagFacetIdTest : public testing::Test { }; -TEST_F(TagFacetTest, isEmpty) { - ASSERT_TRUE(TagFacet().isEmpty()); +TEST_F(TagFacetIdTest, isEmpty) { + ASSERT_TRUE(TagFacetId().isEmpty()); // Null string EXPECT_EQ( - TagFacet(), - TagFacet(TagFacet::value_t())); + TagFacetId(), + TagFacetId(TagFacetId::value_t())); // Empty string EXPECT_EQ( - TagFacet(), - TagFacet(TagFacet::filterEmptyValue(""))); + TagFacetId(), + TagFacetId(TagFacetId::filterEmptyValue(""))); // Non-empty string - EXPECT_FALSE(TagFacet("x").isEmpty()); + EXPECT_FALSE(TagFacetId("x").isEmpty()); } -TEST_F(TagFacetTest, filterEmptyValue) { +TEST_F(TagFacetIdTest, filterEmptyValue) { EXPECT_EQ( - TagFacet::value_t(), - TagFacet::filterEmptyValue(TagFacet::value_t())); + TagFacetId::value_t(), + TagFacetId::filterEmptyValue(TagFacetId::value_t())); EXPECT_EQ( - TagFacet::value_t(), - TagFacet::filterEmptyValue("")); + TagFacetId::value_t(), + TagFacetId::filterEmptyValue("")); EXPECT_EQ( - TagFacet::value_t("x"), - TagFacet::filterEmptyValue("x")); + TagFacetId::value_t("x"), + TagFacetId::filterEmptyValue("x")); } -TEST_F(TagFacetTest, convertIntoValidValue) { +TEST_F(TagFacetIdTest, convertIntoValidValue) { // Clamp empty string EXPECT_EQ( - TagFacet::value_t{}, - TagFacet::convertIntoValidValue("")); + TagFacetId::value_t{}, + TagFacetId::convertIntoValidValue("")); // Clamped whitespace string EXPECT_EQ( - TagFacet::value_t{}, - TagFacet::convertIntoValidValue(" \t\n ")); + TagFacetId::value_t{}, + TagFacetId::convertIntoValidValue(" \t\n ")); // Lowercase EXPECT_EQ( - TagFacet::value_t{"x"}, - TagFacet::convertIntoValidValue(" \tX\n ")); + TagFacetId::value_t{"x"}, + TagFacetId::convertIntoValidValue(" \tX\n ")); // Whitespace replacement EXPECT_EQ( - TagFacet::value_t{"xy_[+]"}, - TagFacet::convertIntoValidValue("X y _\n[+] ")); + TagFacetId::value_t{"xy_[+]"}, + TagFacetId::convertIntoValidValue("X y _\n[+] ")); // Valid characters without whitespace EXPECT_EQ( - TagFacet::value_t{TagFacet::kAlphabet}, - TagFacet::convertIntoValidValue( - TagFacet::value_t{TagFacet::kAlphabet})); + TagFacetId::value_t{TagFacetId::kAlphabet}, + TagFacetId::convertIntoValidValue( + TagFacetId::value_t{TagFacetId::kAlphabet})); EXPECT_EQ( - TagFacet::value_t{"+-./0123456789" - "@abcdefghijklmnopqrstuvwxyz[]_" - "abcdefghijklmnopqrstuvwxyz"}, - TagFacet::convertIntoValidValue( + TagFacetId::value_t{"+-./0123456789" + "@abcdefghijklmnopqrstuvwxyz[]_" + "abcdefghijklmnopqrstuvwxyz"}, + TagFacetId::convertIntoValidValue( "\t!\"#$%&'()*+,-./0123456789:;<=>?" " @ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_" " `abcdefghijklmnopqrstuvwxyz{|}~\n")); } -TEST_F(TagFacetTest, isValidValue) { - EXPECT_TRUE(TagFacet::isValidValue(QString())); - EXPECT_TRUE(TagFacet::isValidValue(QStringLiteral("facet@mixxx.org/simple-test[1]"))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("Uppercase"))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral("lower-case_with_digit_1_and_whitespace "))); - EXPECT_FALSE(TagFacet::isValidValue(QLatin1String(""))); - EXPECT_FALSE(TagFacet::isValidValue(QStringLiteral(" "))); +TEST_F(TagFacetIdTest, isValidValue) { + EXPECT_TRUE(TagFacetId::isValidValue(QString())); + EXPECT_TRUE(TagFacetId::isValidValue(QStringLiteral("facet@mixxx.org/simple-test[1]"))); + EXPECT_FALSE(TagFacetId::isValidValue(QStringLiteral("Uppercase"))); + EXPECT_FALSE(TagFacetId::isValidValue( + QStringLiteral("lower-case_with_digit_1_and_whitespace "))); + EXPECT_FALSE(TagFacetId::isValidValue(QLatin1String(""))); + EXPECT_FALSE(TagFacetId::isValidValue(QStringLiteral(" "))); } -TEST_F(TagFacetTest, convertedEmptyValueIsValid) { +TEST_F(TagFacetIdTest, convertedEmptyValueIsValid) { EXPECT_TRUE( - TagFacet::isValidValue(TagFacet::convertIntoValidValue(TagFacet::value_t{}))); + TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(TagFacetId::value_t{}))); EXPECT_TRUE( - TagFacet::isValidValue(TagFacet::convertIntoValidValue(""))); + TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(""))); EXPECT_TRUE( - TagFacet::isValidValue(TagFacet::convertIntoValidValue(" "))); + TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(" "))); EXPECT_TRUE( - TagFacet::isValidValue(TagFacet::convertIntoValidValue(" \t \n \r "))); + TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(" \t \n \r "))); } class TagLabelTest : public testing::Test { @@ -155,7 +156,7 @@ TEST_F(TagLabelTest, convertedEmptyValueIsValid) { EXPECT_TRUE( TagLabel::isValidValue(TagLabel::convertIntoValidValue(" "))); EXPECT_TRUE( - TagLabel::isValidValue(TagFacet::convertIntoValidValue(" \t \n \r "))); + TagLabel::isValidValue(TagFacetId::convertIntoValidValue(" \t \n \r "))); } class TagTest : public testing::Test { From 56fd0cc92a06846f126644a188419b74a8bf46e6 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 31 Aug 2021 21:34:31 +0200 Subject: [PATCH 15/39] Tags: Add constants for predefined facet identifiers --- src/library/tags/tagfacetid.cpp | 83 +++++++++++++++++++++++++++++++++ src/library/tags/tagfacetid.h | 53 +++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/src/library/tags/tagfacetid.cpp b/src/library/tags/tagfacetid.cpp index 39a3cfa988e..a15b1c1cd86 100644 --- a/src/library/tags/tagfacetid.cpp +++ b/src/library/tags/tagfacetid.cpp @@ -47,3 +47,86 @@ TagFacetId::value_t TagFacetId::convertIntoValidValue( } // namespace library } // namespace mixxx + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcoustidFingerprint = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acoustid_fingerprint")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcoustidId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acoustid_id")); + +// MusicBrainz: "comment:description" +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetComment = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("comment")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetDecade = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("decade")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetGenre = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("genre")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetGrouping = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("grouping")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetIsrc = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("isrc")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetLanguage = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("language")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMood = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("mood")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzAlbumArtistId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_albumartistid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzAlbumId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_albumid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzArtistId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_artistid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzRecordingId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_recordingid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzReleaseGroupId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_releasegroupid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzReleaseTrackId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_releasetrackid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzTrackId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_trackid")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzWorkId = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_workid")); + +// MusicBrainz: "_rating" +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetRating = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("rating")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetArousal = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("arousal")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetValence = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("valence")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcousticness = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acousticness")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetDanceability = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("danceability")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetEnergy = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("energy")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetInstrumentalness = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("instrumentalness")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetLiveness = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("liveness")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetPopularity = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("popularity")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetSpeechiness = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("speechiness")); diff --git a/src/library/tags/tagfacetid.h b/src/library/tags/tagfacetid.h index f998ceeda13..ea6bcad262b 100644 --- a/src/library/tags/tagfacetid.h +++ b/src/library/tags/tagfacetid.h @@ -156,6 +156,59 @@ inline uint qHash( return qHash(facetId.value(), seed); } +/// Some predefined facets, mostly adopting the corresponding MusicBrainz +/// Picard naming conventions ("Internal Name") if available and suitable. +/// +/// https://picard-docs.musicbrainz.org/en/variables/variables.html +/// https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html +extern const TagFacetId kTagFacetAcoustidFingerprint; +extern const TagFacetId kTagFacetAcoustidId; +extern const TagFacetId kTagFacetComment; // multi-valued comment(s) +extern const TagFacetId kTagFacetDecade; // e.g. "1980s" +extern const TagFacetId kTagFacetGenre; // multi-valued genre(s) +extern const TagFacetId kTagFacetGrouping; // aka content group +extern const TagFacetId kTagFacetIsrc; // ISRC +extern const TagFacetId kTagFacetLanguage; // ISO 639-3 +extern const TagFacetId kTagFacetMood; // multi-valued mood(s) +extern const TagFacetId kTagFacetMusicBrainzAlbumArtistId; +extern const TagFacetId kTagFacetMusicBrainzAlbumId; +extern const TagFacetId kTagFacetMusicBrainzArtistId; +extern const TagFacetId kTagFacetMusicBrainzRecordingId; +extern const TagFacetId kTagFacetMusicBrainzReleaseGroupId; +extern const TagFacetId kTagFacetMusicBrainzReleaseTrackId; +extern const TagFacetId kTagFacetMusicBrainzTrackId; +extern const TagFacetId kTagFacetMusicBrainzWorkId; + +/// The score of this facet captures a user's subjective like (or dislike) +/// level. +/// +/// The label is optional and used for discrimination. It may either refer +/// to an organization or contains the personal e-mail address of the +/// owner if known. +/// +/// The normalized score is usually mapped to a star rating, typically +/// ranging from 0 to 5 stars. +extern const TagFacetId kTagFacetRating; + +/// Predefined musical or audio feature scores as of Spotify/EchoNest. +/// +/// A label is optional and could be used for identifying the source of +/// the assigned score. If the source it unspecified or unknown it should +/// be empty (= absent). +/// +/// The combination of kTagFacetArousal and kTagFacetValence could +/// be used for classifying emotion (= mood) according to Thayer's +/// arousel-valence emotion plane. +extern const TagFacetId kTagFacetArousal; // for emotion classification +extern const TagFacetId kTagFacetValence; // for emotion classification +extern const TagFacetId kTagFacetAcousticness; +extern const TagFacetId kTagFacetDanceability; +extern const TagFacetId kTagFacetEnergy; +extern const TagFacetId kTagFacetInstrumentalness; +extern const TagFacetId kTagFacetLiveness; +extern const TagFacetId kTagFacetPopularity; +extern const TagFacetId kTagFacetSpeechiness; + } // namespace tags } // namespace library From c2fb4e2d4724f8c7f768ea168b29a553112e3f22 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 1 Sep 2021 10:33:38 +0200 Subject: [PATCH 16/39] Fix naming of constant --- src/library/tags/tagfacetid.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/tags/tagfacetid.cpp b/src/library/tags/tagfacetid.cpp index a15b1c1cd86..58959bc8c0a 100644 --- a/src/library/tags/tagfacetid.cpp +++ b/src/library/tags/tagfacetid.cpp @@ -6,7 +6,7 @@ namespace { const QRegularExpression kValidFacetStringNotEmpty( QStringLiteral("^[\\+\\-\\./0-9@a-z\\[\\]_]+")); -const QRegularExpression kInversekValidFacetStringNotEmpty( +const QRegularExpression kInverseValidFacetStringNotEmpty( QStringLiteral("[^\\+\\-\\./0-9@a-z\\[\\]_]+")); } // anonymous namespace @@ -37,7 +37,7 @@ bool TagFacetId::isValidValue( //static TagFacetId::value_t TagFacetId::convertIntoValidValue( const value_t& value) { - auto validValue = filterEmptyValue(value.toLower().remove(kInversekValidFacetStringNotEmpty)); + auto validValue = filterEmptyValue(value.toLower().remove(kInverseValidFacetStringNotEmpty)); DEBUG_ASSERT(isValidValue(validValue)); return validValue; } From 35c387c7b169736b4c51c11f6d854149134e11aa Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 1 Sep 2021 10:58:45 +0200 Subject: [PATCH 17/39] Add more predefined tag facets and labels --- src/library/tags/tagfacetid.cpp | 44 +++++++++++++++++++++++++++++++++ src/library/tags/tagfacetid.h | 18 ++++++++++++-- src/library/tags/taglabel.cpp | 3 +++ src/library/tags/taglabel.h | 6 +++++ 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/library/tags/tagfacetid.cpp b/src/library/tags/tagfacetid.cpp index 58959bc8c0a..bdee5ff5497 100644 --- a/src/library/tags/tagfacetid.cpp +++ b/src/library/tags/tagfacetid.cpp @@ -61,6 +61,9 @@ const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetComment = const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetDecade = mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("decade")); +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetEthno = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("ethno")); + const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetGenre = mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("genre")); @@ -76,6 +79,12 @@ const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetLanguage = const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMood = mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("mood")); +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetVenue = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("venue")); + +const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetVibe = + mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("vibe")); + const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzAlbumArtistId = mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_albumartistid")); @@ -130,3 +139,38 @@ const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetPopularity const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetSpeechiness = mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("speechiness")); + +const QVector + mixxx::library::tags::kReservedTagFacetIds = + QVector{ + mixxx::library::tags::kTagFacetAcoustidFingerprint, + mixxx::library::tags::kTagFacetAcoustidId, + mixxx::library::tags::kTagFacetComment, + mixxx::library::tags::kTagFacetDecade, + mixxx::library::tags::kTagFacetEthno, + mixxx::library::tags::kTagFacetGenre, + mixxx::library::tags::kTagFacetGrouping, + mixxx::library::tags::kTagFacetIsrc, + mixxx::library::tags::kTagFacetLanguage, + mixxx::library::tags::kTagFacetMood, + mixxx::library::tags::kTagFacetVenue, + mixxx::library::tags::kTagFacetVibe, + mixxx::library::tags::kTagFacetMusicBrainzAlbumArtistId, + mixxx::library::tags::kTagFacetMusicBrainzAlbumId, + mixxx::library::tags::kTagFacetMusicBrainzArtistId, + mixxx::library::tags::kTagFacetMusicBrainzRecordingId, + mixxx::library::tags::kTagFacetMusicBrainzReleaseGroupId, + mixxx::library::tags::kTagFacetMusicBrainzReleaseTrackId, + mixxx::library::tags::kTagFacetMusicBrainzTrackId, + mixxx::library::tags::kTagFacetMusicBrainzWorkId, + mixxx::library::tags::kTagFacetRating, + mixxx::library::tags::kTagFacetArousal, + mixxx::library::tags::kTagFacetValence, + mixxx::library::tags::kTagFacetAcousticness, + mixxx::library::tags::kTagFacetDanceability, + mixxx::library::tags::kTagFacetEnergy, + mixxx::library::tags::kTagFacetInstrumentalness, + mixxx::library::tags::kTagFacetLiveness, + mixxx::library::tags::kTagFacetPopularity, + mixxx::library::tags::kTagFacetSpeechiness, + }; diff --git a/src/library/tags/tagfacetid.h b/src/library/tags/tagfacetid.h index ea6bcad262b..028c91103e2 100644 --- a/src/library/tags/tagfacetid.h +++ b/src/library/tags/tagfacetid.h @@ -2,6 +2,7 @@ #include #include +#include #include "util/assert.h" @@ -161,15 +162,21 @@ inline uint qHash( /// /// https://picard-docs.musicbrainz.org/en/variables/variables.html /// https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html +/// +/// TODO: These predefined facets are only intended as a starting point +/// and for inspiration. They could be modified as needed. extern const TagFacetId kTagFacetAcoustidFingerprint; extern const TagFacetId kTagFacetAcoustidId; extern const TagFacetId kTagFacetComment; // multi-valued comment(s) -extern const TagFacetId kTagFacetDecade; // e.g. "1980s" -extern const TagFacetId kTagFacetGenre; // multi-valued genre(s) +extern const TagFacetId kTagFacetDecade; // e.g. "1980s" +extern const TagFacetId kTagFacetEthno; +extern const TagFacetId kTagFacetGenre; // multi-valued genre(s) extern const TagFacetId kTagFacetGrouping; // aka content group extern const TagFacetId kTagFacetIsrc; // ISRC extern const TagFacetId kTagFacetLanguage; // ISO 639-3 extern const TagFacetId kTagFacetMood; // multi-valued mood(s) +extern const TagFacetId kTagFacetVenue; +extern const TagFacetId kTagFacetVibe; extern const TagFacetId kTagFacetMusicBrainzAlbumArtistId; extern const TagFacetId kTagFacetMusicBrainzAlbumId; extern const TagFacetId kTagFacetMusicBrainzArtistId; @@ -209,6 +216,13 @@ extern const TagFacetId kTagFacetLiveness; extern const TagFacetId kTagFacetPopularity; extern const TagFacetId kTagFacetSpeechiness; +extern const QVector kReservedTagFacetIds; + +inline bool isReservedTagFacetId( + const TagFacetId& facetId) { + return kReservedTagFacetIds.contains(facetId); +} + } // namespace tags } // namespace library diff --git a/src/library/tags/taglabel.cpp b/src/library/tags/taglabel.cpp index 8a785167a95..ea3ba102466 100644 --- a/src/library/tags/taglabel.cpp +++ b/src/library/tags/taglabel.cpp @@ -32,3 +32,6 @@ TagLabel::value_t TagLabel::convertIntoValidValue( } // namespace library } // namespace mixxx + +const mixxx::library::tags::TagLabel mixxx::library::tags::kTagLabelOrgMixxx = + mixxx::library::tags::TagLabel{QStringLiteral("org.mixxx")}; diff --git a/src/library/tags/taglabel.h b/src/library/tags/taglabel.h index 52aabb0d285..5275814d95b 100644 --- a/src/library/tags/taglabel.h +++ b/src/library/tags/taglabel.h @@ -112,6 +112,12 @@ inline uint qHash( return qHash(label.value(), seed); } +/// Predefined labels for identifying the source or owner of a score, +/// e.g. for custom ratings. +/// +/// Naming convention: Reverse domain name notation +extern const TagLabel kTagLabelOrgMixxx; + } // namespace tags } // namespace library From ca2fb2db0ebbf9fe7fc6858a4269c10852fc6309 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 11 Sep 2021 12:04:54 +0200 Subject: [PATCH 18/39] Remove redundant "Tag" prefix from type names --- CMakeLists.txt | 4 +- src/library/tags/customtags.cpp | 62 +++---- src/library/tags/customtags.h | 62 +++---- src/library/tags/facetid.cpp | 176 +++++++++++++++++++ src/library/tags/{tagfacetid.h => facetid.h} | 122 ++++++------- src/library/tags/{taglabel.cpp => label.cpp} | 10 +- src/library/tags/{taglabel.h => label.h} | 42 ++--- src/library/tags/{tagscore.h => score.h} | 6 +- src/library/tags/tag.cpp | 22 +-- src/library/tags/tag.h | 14 +- src/library/tags/tagfacetid.cpp | 176 ------------------- src/mixxxapplication.cpp | 8 +- src/test/customtags_test.cpp | 40 ++--- src/test/librarytags_test.cpp | 174 +++++++++--------- 14 files changed, 459 insertions(+), 459 deletions(-) create mode 100644 src/library/tags/facetid.cpp rename src/library/tags/{tagfacetid.h => facetid.h} (63%) rename src/library/tags/{taglabel.cpp => label.cpp} (66%) rename src/library/tags/{taglabel.h => label.h} (74%) rename src/library/tags/{tagscore.h => score.h} (92%) delete mode 100644 src/library/tags/tagfacetid.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 342e4d8b8ff..f2c6f907cd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -853,9 +853,9 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keys.cpp src/track/keyutils.cpp src/library/tags/customtags.cpp + src/library/tags/facetid.cpp + src/library/tags/label.cpp src/library/tags/tag.cpp - src/library/tags/tagfacetid.cpp - src/library/tags/taglabel.cpp src/track/playcounter.cpp src/track/replaygain.cpp src/track/serato/beatgrid.cpp diff --git a/src/library/tags/customtags.cpp b/src/library/tags/customtags.cpp index 0f9992d2332..8e125350220 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/customtags.cpp @@ -75,8 +75,8 @@ bool CustomTags::validate() const { } bool CustomTags::containsTag( - const TagLabel& label, - const TagFacetId& facetId) const { + const Label& label, + const FacetId& facetId) const { const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return false; @@ -85,8 +85,8 @@ bool CustomTags::containsTag( } int CustomTags::countTags( - const TagLabel& label, - const TagFacetId& facetId) const { + const Label& label, + const FacetId& facetId) const { const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return 0; @@ -95,7 +95,7 @@ int CustomTags::countTags( } int CustomTags::countFacetedTags( - const TagFacetId& facetId) const { + const FacetId& facetId) const { const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end()) { return 0; @@ -105,7 +105,7 @@ int CustomTags::countFacetedTags( bool CustomTags::addOrReplaceTag( const Tag& tag, - const TagFacetId& facetId) { + const FacetId& facetId) { DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); auto i = refFacetedTags().find(facetId); if (i == refFacetedTags().end()) { @@ -126,8 +126,8 @@ bool CustomTags::addOrReplaceTag( } bool CustomTags::removeTag( - const TagLabel& label, - const TagFacetId& facetId) { + const Label& label, + const FacetId& facetId) { DEBUG_ASSERT(countTags(label, facetId) <= 1); const auto i = refFacetedTags().find(facetId); if (i == refFacetedTags().end()) { @@ -148,7 +148,7 @@ bool CustomTags::removeTag( TagVector CustomTags::getTags() const { TagVector tags; - const auto i = getFacetedTags().find(TagFacetId()); + const auto i = getFacetedTags().find(FacetId()); if (i == getFacetedTags().end()) { return tags; } @@ -162,7 +162,7 @@ TagVector CustomTags::getTags() const { } FacetTagMap::iterator CustomTags::replaceAllFacetedTags( - const TagFacetId& facetId, + const FacetId& facetId, const Tag& tag) { DEBUG_ASSERT(!facetId.isEmpty()); DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); @@ -178,13 +178,13 @@ FacetTagMap::iterator CustomTags::replaceAllFacetedTags( } int CustomTags::removeAllFacetedTags( - const TagFacetId& facetId) { + const FacetId& facetId) { DEBUG_ASSERT(!facetId.isEmpty()); return refFacetedTags().remove(facetId); } TagVector CustomTags::getFacetedTagsOrdered( - const TagFacetId& facetId) const { + const FacetId& facetId) const { DEBUG_ASSERT(!facetId.isEmpty()); TagVector tags; auto i = getFacetedTags().find(facetId); @@ -205,22 +205,22 @@ TagVector CustomTags::getFacetedTagsOrdered( return tags; } -TagLabel CustomTags::getFacetedTagLabel( - const TagFacetId& facetId) const { +Label CustomTags::getFacetedLabel( + const FacetId& facetId) const { DEBUG_ASSERT(countFacetedTags(facetId) <= 1); const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end() || i.value().isEmpty()) { - return TagLabel(); + return Label(); } DEBUG_ASSERT(i.value().size() == 1); - DEBUG_ASSERT(i.value().first() == TagScore()); + DEBUG_ASSERT(i.value().first() == Score()); return i.value().firstKey(); } -std::optional CustomTags::getTagScore( - const TagFacetId& facetId, - const TagLabel& label) const { +std::optional CustomTags::getScore( + const FacetId& facetId, + const Label& label) const { DEBUG_ASSERT(countFacetedTags(facetId) <= 1); const auto i = getFacetedTags().find(facetId); if (i == getFacetedTags().end() || @@ -237,7 +237,7 @@ std::optional CustomTags::getTagScore( } Tag CustomTags::mergeFacetedTags( - const TagFacetId& facetId, + const FacetId& facetId, AggregateScoring aggregateScoring, const QString& joinLabelSeparator) const { DEBUG_ASSERT(!facetId.isEmpty()); @@ -245,7 +245,7 @@ Tag CustomTags::mergeFacetedTags( if (tags.isEmpty()) { return Tag(); } - TagScore::value_t scoreValue; + Score::value_t scoreValue; switch (aggregateScoring) { case AggregateScoring::Maximum: scoreValue = tags.first().getScore(); @@ -253,14 +253,14 @@ Tag CustomTags::mergeFacetedTags( case AggregateScoring::Average: scoreValue = 0.0; for (const auto& tag : tags) { - scoreValue += tag.getScore() - TagScore::kMinValue; + scoreValue += tag.getScore() - Score::kMinValue; } - scoreValue = TagScore::kMinValue + scoreValue / tags.size(); - DEBUG_ASSERT(TagScore::isValidValue(scoreValue)); + scoreValue = Score::kMinValue + scoreValue / tags.size(); + DEBUG_ASSERT(Score::isValidValue(scoreValue)); break; default: DEBUG_ASSERT(!"unreachable"); - scoreValue = TagScore(); + scoreValue = Score(); } QStringList labels; labels.reserve(tags.size()); @@ -268,10 +268,10 @@ Tag CustomTags::mergeFacetedTags( labels += tag.getLabel(); } auto labelValue = - TagLabel::convertIntoValidValue(labels.join(joinLabelSeparator)); + Label::convertIntoValidValue(labels.join(joinLabelSeparator)); return Tag( - TagLabel(std::move(labelValue)), - TagScore(scoreValue)); + Label(std::move(labelValue)), + Score(scoreValue)); } bool CustomTags::addOrReplaceAllTags( @@ -302,8 +302,8 @@ std::optional CustomTags::fromJsonObject( for (auto i = jsonObject.begin(); i != jsonObject.end(); ++i) { - auto facetIdValue = TagFacetId::filterEmptyValue(i.key()); - if (!TagFacetId::isValidValue(facetIdValue)) { + auto facetIdValue = FacetId::filterEmptyValue(i.key()); + if (!FacetId::isValidValue(facetIdValue)) { VERIFY_OR_DEBUG_ASSERT(mode == FromJsonMode::Lenient) { qCWarning(kLogger) << "Invalid facetId" @@ -317,7 +317,7 @@ std::optional CustomTags::fromJsonObject( << "from JSON"; continue; } - const auto facetId = TagFacetId(std::move(facetIdValue)); + const auto facetId = FacetId(std::move(facetIdValue)); DEBUG_ASSERT(i.value().isArray()); const auto jsonArray = i.value().toArray(); if (jsonArray.isEmpty()) { diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h index 494646db720..a26490ca55d 100644 --- a/src/library/tags/customtags.h +++ b/src/library/tags/customtags.h @@ -4,8 +4,8 @@ #include #include +#include "library/tags/facetid.h" #include "library/tags/tag.h" -#include "library/tags/tagfacetid.h" namespace mixxx { @@ -13,13 +13,13 @@ namespace library { namespace tags { -typedef QMap TagMap; -typedef QMap FacetTagMap; +typedef QMap TagMap; +typedef QMap FacetTagMap; /// Custom tags /// -/// Each custom tag is represented by a 3-tuple (triple) of a *facet (`TagFacetId`), -/// a *label* (`TagLabel`), and a *score* (`TagScore`): +/// Each custom tag is represented by a 3-tuple (triple) of a *facet (`FacetId`), +/// a *label* (`Label`), and a *score* (`Score`): /// /// - The *facet* is represented by a *facet identifier* string /// - The *label* is non-empty, free text without leading/trailing whitespace @@ -65,7 +65,7 @@ class CustomTags final { bool isEmpty() const; bool containsFacet( - const TagFacetId& facetId) const { + const FacetId& facetId) const { return getFacetedTags().contains(facetId); } @@ -73,38 +73,38 @@ class CustomTags final { /// /// Existing entries are not affected. void addOrIgnoreFacet( - const TagFacetId& facetId) { + const FacetId& facetId) { refFacetedTags()[facetId]; DEBUG_ASSERT(containsFacet(facetId)); } bool containsTag( - const TagLabel& label, - const TagFacetId& facetId = TagFacetId()) const; + const Label& label, + const FacetId& facetId = FacetId()) const; int countTags( - const TagLabel& label, - const TagFacetId& facetId = TagFacetId()) const; + const Label& label, + const FacetId& facetId = FacetId()) const; int countTags() const { - return countFacetedTags(TagFacetId()); + return countFacetedTags(FacetId()); } TagMap& refTags() { - return refFacetedTags()[TagFacetId()]; + return refFacetedTags()[FacetId()]; } int countFacetedTags( - const TagFacetId& facetId) const; + const FacetId& facetId) const; TagMap& refFacetedTags( - const TagFacetId& facetId) { + const FacetId& facetId) { return refFacetedTags()[facetId]; } bool addOrReplaceTag( const Tag& tag, - const TagFacetId& facetId = TagFacetId()); + const FacetId& facetId = FacetId()); bool removeTag( - const TagLabel& label, - const TagFacetId& facetId = TagFacetId()); + const Label& label, + const FacetId& facetId = FacetId()); bool addOrReplaceAllTags( const CustomTags& tags); @@ -115,29 +115,29 @@ class CustomTags final { /// Replace all existing tags of this facet with a single /// faceted tag or insert a new faceted tag. FacetTagMap::iterator replaceAllFacetedTags( - const TagFacetId& facetId, + const FacetId& facetId, const Tag& tag); int removeAllFacetedTags( - const TagFacetId& facetId); + const FacetId& facetId); /// Get all tags of a given facet sorted by score in descending order TagVector getFacetedTagsOrdered( - const TagFacetId& facetId) const; + const FacetId& facetId) const; /// Get the label of a single faceted tag, i.e. that /// occurs at most once and has no custom score. If the /// facet is unknown/absent an empty label is returned. - TagLabel getFacetedTagLabel( - const TagFacetId& facetId) const; + Label getFacetedLabel( + const FacetId& facetId) const; /// Get the score of a tag if present. /// /// Returns `std::nullopt` if the corresponding (facet, label) /// combination that serves as the key does not exist. - std::optional getTagScore( - const TagFacetId& facetId, - const TagLabel& label = TagLabel()) const; + std::optional getScore( + const FacetId& facetId, + const Label& label = Label()) const; enum class AggregateScoring { Maximum, @@ -148,12 +148,12 @@ class CustomTags final { // a single plain tag. The strings of the labels are joined // with a separator in between and the scores are aggregated. Tag mergeFacetedTags( - const TagFacetId& facetId, + const FacetId& facetId, AggregateScoring aggregateScoring, - const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const; - TagLabel joinFacetedTagsLabel( - const TagFacetId& facetId, - const TagLabel::value_t& joinLabelSeparator = TagLabel::value_t()) const { + const Label::value_t& joinLabelSeparator = Label::value_t()) const; + Label joinFacetedTagsLabel( + const FacetId& facetId, + const Label::value_t& joinLabelSeparator = Label::value_t()) const { return mergeFacetedTags( facetId, AggregateScoring::Maximum, diff --git a/src/library/tags/facetid.cpp b/src/library/tags/facetid.cpp new file mode 100644 index 00000000000..c4bf49144c9 --- /dev/null +++ b/src/library/tags/facetid.cpp @@ -0,0 +1,176 @@ +#include "library/tags/facetid.h" + +#include + +namespace { + +const QRegularExpression kValidFacetStringNotEmpty( + QStringLiteral("^[\\+\\-\\./0-9@a-z\\[\\]_]+")); +const QRegularExpression kInverseValidFacetStringNotEmpty( + QStringLiteral("[^\\+\\-\\./0-9@a-z\\[\\]_]+")); + +} // anonymous namespace + +namespace mixxx { + +namespace library { + +namespace tags { + +//static +bool FacetId::isValidValue( + const value_t& value) { + if (value.isNull()) { + return true; + } + if (value.isEmpty()) { + // for disambiguation with null + return false; + } + const auto match = kValidFacetStringNotEmpty.match(value); + DEBUG_ASSERT(match.isValid()); + DEBUG_ASSERT(value.length() > 0); + // match = exact match + return match.capturedLength() == value.length(); +} + +//static +FacetId::value_t FacetId::convertIntoValidValue( + const value_t& value) { + auto validValue = filterEmptyValue(value.toLower().remove(kInverseValidFacetStringNotEmpty)); + DEBUG_ASSERT(isValidValue(validValue)); + return validValue; +} + +} // namespace tags + +} // namespace library + +} // namespace mixxx + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidFingerprint = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_fingerprint")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_id")); + +// MusicBrainz: "comment:description" +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetComment = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("comment")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetDecade = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("decade")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetEthno = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("ethno")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetGenre = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("genre")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetGrouping = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("grouping")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetIsrc = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("isrc")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetLanguage = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("language")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMood = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("mood")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetVenue = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("venue")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetVibe = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("vibe")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzAlbumArtistId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_albumartistid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzAlbumId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_albumid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzArtistId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_artistid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzRecordingId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_recordingid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzReleaseGroupId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_releasegroupid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzReleaseTrackId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_releasetrackid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzTrackId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_trackid")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzWorkId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_workid")); + +// MusicBrainz: "_rating" +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetRating = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("rating")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetArousal = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("arousal")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetValence = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("valence")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcousticness = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("acousticness")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetDanceability = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("danceability")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetEnergy = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("energy")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetInstrumentalness = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("instrumentalness")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetLiveness = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("liveness")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetPopularity = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("popularity")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetSpeechiness = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("speechiness")); + +const QVector + mixxx::library::tags::kReservedFacetIds = + QVector{ + mixxx::library::tags::kFacetAcoustidFingerprint, + mixxx::library::tags::kFacetAcoustidId, + mixxx::library::tags::kFacetComment, + mixxx::library::tags::kFacetDecade, + mixxx::library::tags::kFacetEthno, + mixxx::library::tags::kFacetGenre, + mixxx::library::tags::kFacetGrouping, + mixxx::library::tags::kFacetIsrc, + mixxx::library::tags::kFacetLanguage, + mixxx::library::tags::kFacetMood, + mixxx::library::tags::kFacetVenue, + mixxx::library::tags::kFacetVibe, + mixxx::library::tags::kFacetMusicBrainzAlbumArtistId, + mixxx::library::tags::kFacetMusicBrainzAlbumId, + mixxx::library::tags::kFacetMusicBrainzArtistId, + mixxx::library::tags::kFacetMusicBrainzRecordingId, + mixxx::library::tags::kFacetMusicBrainzReleaseGroupId, + mixxx::library::tags::kFacetMusicBrainzReleaseTrackId, + mixxx::library::tags::kFacetMusicBrainzTrackId, + mixxx::library::tags::kFacetMusicBrainzWorkId, + mixxx::library::tags::kFacetRating, + mixxx::library::tags::kFacetArousal, + mixxx::library::tags::kFacetValence, + mixxx::library::tags::kFacetAcousticness, + mixxx::library::tags::kFacetDanceability, + mixxx::library::tags::kFacetEnergy, + mixxx::library::tags::kFacetInstrumentalness, + mixxx::library::tags::kFacetLiveness, + mixxx::library::tags::kFacetPopularity, + mixxx::library::tags::kFacetSpeechiness, + }; diff --git a/src/library/tags/tagfacetid.h b/src/library/tags/facetid.h similarity index 63% rename from src/library/tags/tagfacetid.h rename to src/library/tags/facetid.h index 028c91103e2..b514114f54a 100644 --- a/src/library/tags/tagfacetid.h +++ b/src/library/tags/facetid.h @@ -33,7 +33,7 @@ namespace tags { /// /// References: /// - https://en.wikipedia.org/wiki/Faceted_classification -class TagFacetId final { +class FacetId final { public: typedef QString value_t; @@ -59,12 +59,12 @@ class TagFacetId final { } /// Default constructor. - TagFacetId() = default; + FacetId() = default; /// Create a new instance. /// /// This constructor must not be used for static constants! - explicit TagFacetId( + explicit FacetId( value_t value) : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); @@ -78,21 +78,21 @@ class TagFacetId final { enum struct StaticCtor {}; /// Constructor for creating non-validated, static constants. - TagFacetId( + FacetId( StaticCtor, value_t value) : m_value(std::move(value)) { } - static TagFacetId staticConst(value_t value) { - return TagFacetId(StaticCtor{}, std::move(value)); + static FacetId staticConst(value_t value) { + return FacetId(StaticCtor{}, std::move(value)); } - TagFacetId(const TagFacetId&) = default; - TagFacetId(TagFacetId&&) = default; + FacetId(const FacetId&) = default; + FacetId(FacetId&&) = default; - TagFacetId& operator=(const TagFacetId&) = default; - TagFacetId& operator=(TagFacetId&&) = default; + FacetId& operator=(const FacetId&) = default; + FacetId& operator=(FacetId&&) = default; bool isValid() const { return isValidValue(m_value); @@ -116,43 +116,43 @@ class TagFacetId final { }; inline bool operator==( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return lhs.value() == rhs.value(); } inline bool operator!=( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return !(lhs == rhs); } inline bool operator<( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return lhs.value() < rhs.value(); } inline bool operator>( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return !(lhs < rhs); } inline bool operator<=( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return !(lhs > rhs); } inline bool operator>=( - const TagFacetId& lhs, - const TagFacetId& rhs) { + const FacetId& lhs, + const FacetId& rhs) { return !(lhs < rhs); } inline uint qHash( - const TagFacetId& facetId, + const FacetId& facetId, uint seed = 0) { return qHash(facetId.value(), seed); } @@ -165,26 +165,26 @@ inline uint qHash( /// /// TODO: These predefined facets are only intended as a starting point /// and for inspiration. They could be modified as needed. -extern const TagFacetId kTagFacetAcoustidFingerprint; -extern const TagFacetId kTagFacetAcoustidId; -extern const TagFacetId kTagFacetComment; // multi-valued comment(s) -extern const TagFacetId kTagFacetDecade; // e.g. "1980s" -extern const TagFacetId kTagFacetEthno; -extern const TagFacetId kTagFacetGenre; // multi-valued genre(s) -extern const TagFacetId kTagFacetGrouping; // aka content group -extern const TagFacetId kTagFacetIsrc; // ISRC -extern const TagFacetId kTagFacetLanguage; // ISO 639-3 -extern const TagFacetId kTagFacetMood; // multi-valued mood(s) -extern const TagFacetId kTagFacetVenue; -extern const TagFacetId kTagFacetVibe; -extern const TagFacetId kTagFacetMusicBrainzAlbumArtistId; -extern const TagFacetId kTagFacetMusicBrainzAlbumId; -extern const TagFacetId kTagFacetMusicBrainzArtistId; -extern const TagFacetId kTagFacetMusicBrainzRecordingId; -extern const TagFacetId kTagFacetMusicBrainzReleaseGroupId; -extern const TagFacetId kTagFacetMusicBrainzReleaseTrackId; -extern const TagFacetId kTagFacetMusicBrainzTrackId; -extern const TagFacetId kTagFacetMusicBrainzWorkId; +extern const FacetId kFacetAcoustidFingerprint; +extern const FacetId kFacetAcoustidId; +extern const FacetId kFacetComment; // multi-valued comment(s) +extern const FacetId kFacetDecade; // e.g. "1980s" +extern const FacetId kFacetEthno; +extern const FacetId kFacetGenre; // multi-valued genre(s) +extern const FacetId kFacetGrouping; // aka content group +extern const FacetId kFacetIsrc; // ISRC +extern const FacetId kFacetLanguage; // ISO 639-3 +extern const FacetId kFacetMood; // multi-valued mood(s) +extern const FacetId kFacetVenue; +extern const FacetId kFacetVibe; +extern const FacetId kFacetMusicBrainzAlbumArtistId; +extern const FacetId kFacetMusicBrainzAlbumId; +extern const FacetId kFacetMusicBrainzArtistId; +extern const FacetId kFacetMusicBrainzRecordingId; +extern const FacetId kFacetMusicBrainzReleaseGroupId; +extern const FacetId kFacetMusicBrainzReleaseTrackId; +extern const FacetId kFacetMusicBrainzTrackId; +extern const FacetId kFacetMusicBrainzWorkId; /// The score of this facet captures a user's subjective like (or dislike) /// level. @@ -195,7 +195,7 @@ extern const TagFacetId kTagFacetMusicBrainzWorkId; /// /// The normalized score is usually mapped to a star rating, typically /// ranging from 0 to 5 stars. -extern const TagFacetId kTagFacetRating; +extern const FacetId kFacetRating; /// Predefined musical or audio feature scores as of Spotify/EchoNest. /// @@ -203,24 +203,24 @@ extern const TagFacetId kTagFacetRating; /// the assigned score. If the source it unspecified or unknown it should /// be empty (= absent). /// -/// The combination of kTagFacetArousal and kTagFacetValence could +/// The combination of kFacetArousal and kFacetValence could /// be used for classifying emotion (= mood) according to Thayer's /// arousel-valence emotion plane. -extern const TagFacetId kTagFacetArousal; // for emotion classification -extern const TagFacetId kTagFacetValence; // for emotion classification -extern const TagFacetId kTagFacetAcousticness; -extern const TagFacetId kTagFacetDanceability; -extern const TagFacetId kTagFacetEnergy; -extern const TagFacetId kTagFacetInstrumentalness; -extern const TagFacetId kTagFacetLiveness; -extern const TagFacetId kTagFacetPopularity; -extern const TagFacetId kTagFacetSpeechiness; - -extern const QVector kReservedTagFacetIds; - -inline bool isReservedTagFacetId( - const TagFacetId& facetId) { - return kReservedTagFacetIds.contains(facetId); +extern const FacetId kFacetArousal; // for emotion classification +extern const FacetId kFacetValence; // for emotion classification +extern const FacetId kFacetAcousticness; +extern const FacetId kFacetDanceability; +extern const FacetId kFacetEnergy; +extern const FacetId kFacetInstrumentalness; +extern const FacetId kFacetLiveness; +extern const FacetId kFacetPopularity; +extern const FacetId kFacetSpeechiness; + +extern const QVector kReservedFacetIds; + +inline bool isReservedFacetId( + const FacetId& facetId) { + return kReservedFacetIds.contains(facetId); } } // namespace tags @@ -229,4 +229,4 @@ inline bool isReservedTagFacetId( } // namespace mixxx -Q_DECLARE_METATYPE(mixxx::library::tags::TagFacetId) +Q_DECLARE_METATYPE(mixxx::library::tags::FacetId) diff --git a/src/library/tags/taglabel.cpp b/src/library/tags/label.cpp similarity index 66% rename from src/library/tags/taglabel.cpp rename to src/library/tags/label.cpp index ea3ba102466..7532da015f8 100644 --- a/src/library/tags/taglabel.cpp +++ b/src/library/tags/label.cpp @@ -1,4 +1,4 @@ -#include "library/tags/taglabel.h" +#include "library/tags/label.h" namespace mixxx { @@ -7,7 +7,7 @@ namespace library { namespace tags { //static -bool TagLabel::isValidValue( +bool Label::isValidValue( const value_t& value) { if (value.isNull()) { return true; @@ -20,7 +20,7 @@ bool TagLabel::isValidValue( } //static -TagLabel::value_t TagLabel::convertIntoValidValue( +Label::value_t Label::convertIntoValidValue( const value_t& value) { auto validValue = filterEmptyValue(value.trimmed()); DEBUG_ASSERT(isValidValue(validValue)); @@ -33,5 +33,5 @@ TagLabel::value_t TagLabel::convertIntoValidValue( } // namespace mixxx -const mixxx::library::tags::TagLabel mixxx::library::tags::kTagLabelOrgMixxx = - mixxx::library::tags::TagLabel{QStringLiteral("org.mixxx")}; +const mixxx::library::tags::Label mixxx::library::tags::kLabelOrgMixxx = + mixxx::library::tags::Label{QStringLiteral("org.mixxx")}; diff --git a/src/library/tags/taglabel.h b/src/library/tags/label.h similarity index 74% rename from src/library/tags/taglabel.h rename to src/library/tags/label.h index 5275814d95b..cce183b5ffd 100644 --- a/src/library/tags/taglabel.h +++ b/src/library/tags/label.h @@ -19,7 +19,7 @@ namespace tags { /// Value constraints: /// - charset: Unicode /// - no leading/trailing whitespace -class TagLabel final { +class Label final { public: typedef QString value_t; @@ -38,16 +38,16 @@ class TagLabel final { return value.isEmpty() ? value_t{} : std::move(value); } - explicit TagLabel( + explicit Label( value_t value = value_t{}) : m_value(std::move(value)) { DEBUG_ASSERT(isValid()); } - TagLabel(const TagLabel&) = default; - TagLabel(TagLabel&&) = default; + Label(const Label&) = default; + Label(Label&&) = default; - TagLabel& operator=(const TagLabel&) = default; - TagLabel& operator=(TagLabel&&) = default; + Label& operator=(const Label&) = default; + Label& operator=(Label&&) = default; bool isValid() const { return isValidValue(m_value); @@ -71,43 +71,43 @@ class TagLabel final { }; inline bool operator==( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return lhs.value() == rhs.value(); } inline bool operator!=( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return !(lhs == rhs); } inline bool operator<( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return lhs.value() < rhs.value(); } inline bool operator>( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return !(lhs < rhs); } inline bool operator<=( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return !(lhs > rhs); } inline bool operator>=( - const TagLabel& lhs, - const TagLabel& rhs) { + const Label& lhs, + const Label& rhs) { return !(lhs < rhs); } inline uint qHash( - const TagLabel& label, + const Label& label, uint seed = 0) { return qHash(label.value(), seed); } @@ -116,7 +116,7 @@ inline uint qHash( /// e.g. for custom ratings. /// /// Naming convention: Reverse domain name notation -extern const TagLabel kTagLabelOrgMixxx; +extern const Label kLabelOrgMixxx; } // namespace tags @@ -124,4 +124,4 @@ extern const TagLabel kTagLabelOrgMixxx; } // namespace mixxx -Q_DECLARE_METATYPE(mixxx::library::tags::TagLabel) +Q_DECLARE_METATYPE(mixxx::library::tags::Label) diff --git a/src/library/tags/tagscore.h b/src/library/tags/score.h similarity index 92% rename from src/library/tags/tagscore.h rename to src/library/tags/score.h index b3c5788305d..f3ff530abde 100644 --- a/src/library/tags/tagscore.h +++ b/src/library/tags/score.h @@ -14,7 +14,7 @@ namespace tags { /// The score or weight of a tag relationship, defaulting to 1.0. /// /// Constraints: Normalized to the unit interval [0.0, 1.0] -class TagScore final { +class Score final { public: typedef double value_t; @@ -33,7 +33,7 @@ class TagScore final { return math_min(kMaxValue, math_max(kMinValue, value)); } - explicit TagScore( + explicit Score( value_t value = kDefaultValue) : m_value(value) { DEBUG_ASSERT(isValid()); @@ -60,4 +60,4 @@ class TagScore final { } // namespace mixxx -Q_DECLARE_METATYPE(mixxx::library::tags::TagScore) +Q_DECLARE_METATYPE(mixxx::library::tags::Score) diff --git a/src/library/tags/tag.cpp b/src/library/tags/tag.cpp index cb6c4d1c7d7..5828c0e26b0 100644 --- a/src/library/tags/tag.cpp +++ b/src/library/tags/tag.cpp @@ -22,15 +22,15 @@ std::optional Tag::fromJsonValue( const QJsonValue& jsonValue) { if (jsonValue.isString()) { // label: string - auto labelValue = TagLabel::filterEmptyValue(jsonValue.toString()); - if (TagLabel::isValidValue(labelValue)) { - return Tag(TagLabel(std::move(labelValue))); + auto labelValue = Label::filterEmptyValue(jsonValue.toString()); + if (Label::isValidValue(labelValue)) { + return Tag(Label(std::move(labelValue))); } } else if (jsonValue.isDouble()) { // score: number auto scoreValue = jsonValue.toDouble(); - if (TagScore::isValidValue(scoreValue)) { - return Tag(TagScore(scoreValue)); + if (Score::isValidValue(scoreValue)) { + return Tag(Score(scoreValue)); } } else if (jsonValue.isArray()) { // [label: string, score: number] @@ -38,13 +38,13 @@ std::optional Tag::fromJsonValue( if (jsonArray.size() == 2 && jsonArray.at(0).isString() && jsonArray.at(1).isDouble()) { - auto labelValue = TagLabel::filterEmptyValue(jsonArray.at(0).toString()); + auto labelValue = Label::filterEmptyValue(jsonArray.at(0).toString()); auto scoreValue = jsonArray.at(1).toDouble(); - if (TagLabel::isValidValue(labelValue) && - TagScore::isValidValue(scoreValue)) { + if (Label::isValidValue(labelValue) && + Score::isValidValue(scoreValue)) { return Tag( - TagLabel(std::move(labelValue)), - TagScore(scoreValue)); + Label(std::move(labelValue)), + Score(scoreValue)); } } } @@ -55,7 +55,7 @@ QJsonValue Tag::toJsonValue() const { if (hasLabel()) { // Regular plain tag DEBUG_ASSERT(isValid()); - if (getScore() != TagScore()) { + if (getScore() != Score()) { return QJsonArray{ QJsonValue{getLabel()}, QJsonValue{getScore()}}; diff --git a/src/library/tags/tag.h b/src/library/tags/tag.h index ba6da5d693c..f81e5636510 100644 --- a/src/library/tags/tag.h +++ b/src/library/tags/tag.h @@ -3,8 +3,8 @@ #include #include -#include "library/tags/taglabel.h" -#include "library/tags/tagscore.h" +#include "library/tags/label.h" +#include "library/tags/score.h" #include "util/assert.h" #include "util/macros.h" #include "util/optional.h" @@ -25,18 +25,18 @@ namespace tags { /// VorbisComment) for storing metadata in media files. class Tag final { // Properties - MIXXX_DECL_PROPERTY(TagLabel, label, Label) - MIXXX_DECL_PROPERTY(TagScore, score, Score) + MIXXX_DECL_PROPERTY(Label, label, Label) + MIXXX_DECL_PROPERTY(Score, score, Score) public: explicit Tag( - const TagLabel& label, - TagScore score = TagScore{}) + const Label& label, + Score score = Score{}) : m_label(label), m_score(score) { } explicit Tag( - TagScore score = TagScore{}) + Score score = Score{}) : m_score(score) { } Tag(const Tag&) = default; diff --git a/src/library/tags/tagfacetid.cpp b/src/library/tags/tagfacetid.cpp deleted file mode 100644 index bdee5ff5497..00000000000 --- a/src/library/tags/tagfacetid.cpp +++ /dev/null @@ -1,176 +0,0 @@ -#include "library/tags/tagfacetid.h" - -#include - -namespace { - -const QRegularExpression kValidFacetStringNotEmpty( - QStringLiteral("^[\\+\\-\\./0-9@a-z\\[\\]_]+")); -const QRegularExpression kInverseValidFacetStringNotEmpty( - QStringLiteral("[^\\+\\-\\./0-9@a-z\\[\\]_]+")); - -} // anonymous namespace - -namespace mixxx { - -namespace library { - -namespace tags { - -//static -bool TagFacetId::isValidValue( - const value_t& value) { - if (value.isNull()) { - return true; - } - if (value.isEmpty()) { - // for disambiguation with null - return false; - } - const auto match = kValidFacetStringNotEmpty.match(value); - DEBUG_ASSERT(match.isValid()); - DEBUG_ASSERT(value.length() > 0); - // match = exact match - return match.capturedLength() == value.length(); -} - -//static -TagFacetId::value_t TagFacetId::convertIntoValidValue( - const value_t& value) { - auto validValue = filterEmptyValue(value.toLower().remove(kInverseValidFacetStringNotEmpty)); - DEBUG_ASSERT(isValidValue(validValue)); - return validValue; -} - -} // namespace tags - -} // namespace library - -} // namespace mixxx - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcoustidFingerprint = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acoustid_fingerprint")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcoustidId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acoustid_id")); - -// MusicBrainz: "comment:description" -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetComment = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("comment")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetDecade = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("decade")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetEthno = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("ethno")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetGenre = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("genre")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetGrouping = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("grouping")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetIsrc = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("isrc")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetLanguage = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("language")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMood = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("mood")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetVenue = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("venue")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetVibe = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("vibe")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzAlbumArtistId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_albumartistid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzAlbumId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_albumid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzArtistId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_artistid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzRecordingId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_recordingid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzReleaseGroupId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_releasegroupid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzReleaseTrackId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_releasetrackid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzTrackId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_trackid")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetMusicBrainzWorkId = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("musicbrainz_workid")); - -// MusicBrainz: "_rating" -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetRating = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("rating")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetArousal = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("arousal")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetValence = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("valence")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetAcousticness = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("acousticness")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetDanceability = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("danceability")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetEnergy = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("energy")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetInstrumentalness = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("instrumentalness")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetLiveness = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("liveness")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetPopularity = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("popularity")); - -const mixxx::library::tags::TagFacetId mixxx::library::tags::kTagFacetSpeechiness = - mixxx::library::tags::TagFacetId::staticConst(QStringLiteral("speechiness")); - -const QVector - mixxx::library::tags::kReservedTagFacetIds = - QVector{ - mixxx::library::tags::kTagFacetAcoustidFingerprint, - mixxx::library::tags::kTagFacetAcoustidId, - mixxx::library::tags::kTagFacetComment, - mixxx::library::tags::kTagFacetDecade, - mixxx::library::tags::kTagFacetEthno, - mixxx::library::tags::kTagFacetGenre, - mixxx::library::tags::kTagFacetGrouping, - mixxx::library::tags::kTagFacetIsrc, - mixxx::library::tags::kTagFacetLanguage, - mixxx::library::tags::kTagFacetMood, - mixxx::library::tags::kTagFacetVenue, - mixxx::library::tags::kTagFacetVibe, - mixxx::library::tags::kTagFacetMusicBrainzAlbumArtistId, - mixxx::library::tags::kTagFacetMusicBrainzAlbumId, - mixxx::library::tags::kTagFacetMusicBrainzArtistId, - mixxx::library::tags::kTagFacetMusicBrainzRecordingId, - mixxx::library::tags::kTagFacetMusicBrainzReleaseGroupId, - mixxx::library::tags::kTagFacetMusicBrainzReleaseTrackId, - mixxx::library::tags::kTagFacetMusicBrainzTrackId, - mixxx::library::tags::kTagFacetMusicBrainzWorkId, - mixxx::library::tags::kTagFacetRating, - mixxx::library::tags::kTagFacetArousal, - mixxx::library::tags::kTagFacetValence, - mixxx::library::tags::kTagFacetAcousticness, - mixxx::library::tags::kTagFacetDanceability, - mixxx::library::tags::kTagFacetEnergy, - mixxx::library::tags::kTagFacetInstrumentalness, - mixxx::library::tags::kTagFacetLiveness, - mixxx::library::tags::kTagFacetPopularity, - mixxx::library::tags::kTagFacetSpeechiness, - }; diff --git a/src/mixxxapplication.cpp b/src/mixxxapplication.cpp index 16f0ace8f4f..4d1a2a30c3f 100644 --- a/src/mixxxapplication.cpp +++ b/src/mixxxapplication.cpp @@ -7,8 +7,8 @@ #include "audio/frame.h" #include "audio/types.h" #include "control/controlproxy.h" +#include "library/tags/facetid.h" #include "library/tags/tag.h" -#include "library/tags/tagfacetid.h" #include "library/trackset/crate/crateid.h" #include "moc_mixxxapplication.cpp" #include "soundio/soundmanagerutil.h" @@ -97,9 +97,9 @@ void MixxxApplication::registerMetaTypes() { QMetaType::registerComparators(); // Library: Tags - qRegisterMetaType("mixxx::library::tags::TagFacetId"); - qRegisterMetaType("mixxx::library::tags::TagLabel"); - qRegisterMetaType("mixxx::library::tags::TagScore"); + qRegisterMetaType("mixxx::library::tags::FacetId"); + qRegisterMetaType("mixxx::library::tags::Label"); + qRegisterMetaType("mixxx::library::tags::Score"); qRegisterMetaType("mixxx::library::tags::Tag"); qRegisterMetaType("mixxx::library::tags::TagVector"); diff --git a/src/test/customtags_test.cpp b/src/test/customtags_test.cpp index ad32620ff33..9bc1bb0a237 100644 --- a/src/test/customtags_test.cpp +++ b/src/test/customtags_test.cpp @@ -16,47 +16,47 @@ class CustomTagsTest : public testing::Test { }; TEST_F(CustomTagsTest, isEmptyAndCount) { - const auto facetId = TagFacetId(QStringLiteral("facet-id")); - const auto label = TagLabel(QStringLiteral("Label")); + const auto facetId = FacetId(QStringLiteral("facet-id")); + const auto label = Label(QStringLiteral("Label")); CustomTags customTags; EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, FacetId())); EXPECT_EQ(0, customTags.countTags(label, facetId)); EXPECT_EQ(0, customTags.countFacetedTags(facetId)); customTags.addOrReplaceTag(Tag(label)); EXPECT_FALSE(customTags.isEmpty()); EXPECT_EQ(1, customTags.countTags(label)); - EXPECT_EQ(1, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(1, customTags.countTags(label, FacetId())); EXPECT_EQ(0, customTags.countTags(label, facetId)); EXPECT_EQ(0, customTags.countFacetedTags(facetId)); customTags.removeTag(label); EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, FacetId())); EXPECT_EQ(0, customTags.countTags(label, facetId)); EXPECT_EQ(0, customTags.countFacetedTags(facetId)); customTags.addOrReplaceTag(Tag(label), facetId); EXPECT_FALSE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, FacetId())); EXPECT_EQ(1, customTags.countTags(label, facetId)); EXPECT_EQ(1, customTags.countFacetedTags(facetId)); customTags.removeTag(label, facetId); EXPECT_TRUE(customTags.isEmpty()); EXPECT_EQ(0, customTags.countTags(label)); - EXPECT_EQ(0, customTags.countTags(label, TagFacetId())); + EXPECT_EQ(0, customTags.countTags(label, FacetId())); EXPECT_EQ(0, customTags.countTags(label, facetId)); EXPECT_EQ(0, customTags.countFacetedTags(facetId)); } TEST_F(CustomTagsTest, isEmptyWithEmptyFacetedSlot) { - const auto facetId = TagFacetId(QStringLiteral("facet")); + const auto facetId = FacetId(QStringLiteral("facet")); CustomTags customTags; ASSERT_EQ(0, customTags.getFacetedTags().size()); @@ -71,7 +71,7 @@ TEST_F(CustomTagsTest, isEmptyWithEmptyFacetedSlot) { EXPECT_TRUE(customTags.isEmpty()); // Add an empty placeholder slot with an empty facetId - customTags.refFacetedTags(TagFacetId()); + customTags.refFacetedTags(FacetId()); // Verify that an empty slot for an empty facetId does // not affect the emptiness check. @@ -155,12 +155,12 @@ TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { }, }; - const auto facetId1 = TagFacetId(QStringLiteral("facet1")); - const auto facetId2 = TagFacetId(QStringLiteral("facet2")); - const auto facetId3 = TagFacetId(QStringLiteral("facet3")); + const auto facetId1 = FacetId(QStringLiteral("facet1")); + const auto facetId2 = FacetId(QStringLiteral("facet2")); + const auto facetId3 = FacetId(QStringLiteral("facet3")); - const auto label1 = TagLabel(QStringLiteral("Label 1")); - const auto label2 = TagLabel(QStringLiteral("Label 2")); + const auto label1 = Label(QStringLiteral("Label 1")); + const auto label2 = Label(QStringLiteral("Label 2")); // Decode JSON const QByteArray jsonData = @@ -180,11 +180,11 @@ TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { EXPECT_EQ(1, decoded->countTags(label1)); EXPECT_EQ(1, decoded->countTags(label2)); EXPECT_EQ( - TagScore(), - decoded->getFacetedTags().value(TagFacetId()).value(label1)); + Score(), + decoded->getFacetedTags().value(FacetId()).value(label1)); EXPECT_EQ( - TagScore(0.5), - decoded->getFacetedTags().value(TagFacetId()).value(label2)); + Score(0.5), + decoded->getFacetedTags().value(FacetId()).value(label2)); // Faceted tags EXPECT_EQ(2, decoded->countFacetedTags(facetId1)); @@ -192,13 +192,13 @@ TEST_F(CustomTagsTest, encodeDecodeJsonRoundtrip) { EXPECT_EQ(0, decoded->countFacetedTags(facetId3)); const auto facetId1TagsOrdered = TagVector{ Tag(label1), - Tag(TagScore(0.2))}; + Tag(Score(0.2))}; EXPECT_EQ( facetId1TagsOrdered, decoded->getFacetedTagsOrdered(facetId1)); const auto facetId2TagsOrdered = TagVector{ Tag(), - Tag(label2, TagScore(0.4))}; + Tag(label2, Score(0.4))}; EXPECT_EQ( facetId2TagsOrdered, decoded->getFacetedTagsOrdered(facetId2)); diff --git a/src/test/librarytags_test.cpp b/src/test/librarytags_test.cpp index 679651e654b..42ba196457f 100644 --- a/src/test/librarytags_test.cpp +++ b/src/test/librarytags_test.cpp @@ -3,8 +3,8 @@ #include #include +#include "library/tags/facetid.h" #include "library/tags/tag.h" -#include "library/tags/tagfacetid.h" namespace mixxx { @@ -12,151 +12,151 @@ namespace library { namespace tags { -class TagFacetIdTest : public testing::Test { +class FacetIdTest : public testing::Test { }; -TEST_F(TagFacetIdTest, isEmpty) { - ASSERT_TRUE(TagFacetId().isEmpty()); +TEST_F(FacetIdTest, isEmpty) { + ASSERT_TRUE(FacetId().isEmpty()); // Null string EXPECT_EQ( - TagFacetId(), - TagFacetId(TagFacetId::value_t())); + FacetId(), + FacetId(FacetId::value_t())); // Empty string EXPECT_EQ( - TagFacetId(), - TagFacetId(TagFacetId::filterEmptyValue(""))); + FacetId(), + FacetId(FacetId::filterEmptyValue(""))); // Non-empty string - EXPECT_FALSE(TagFacetId("x").isEmpty()); + EXPECT_FALSE(FacetId("x").isEmpty()); } -TEST_F(TagFacetIdTest, filterEmptyValue) { +TEST_F(FacetIdTest, filterEmptyValue) { EXPECT_EQ( - TagFacetId::value_t(), - TagFacetId::filterEmptyValue(TagFacetId::value_t())); + FacetId::value_t(), + FacetId::filterEmptyValue(FacetId::value_t())); EXPECT_EQ( - TagFacetId::value_t(), - TagFacetId::filterEmptyValue("")); + FacetId::value_t(), + FacetId::filterEmptyValue("")); EXPECT_EQ( - TagFacetId::value_t("x"), - TagFacetId::filterEmptyValue("x")); + FacetId::value_t("x"), + FacetId::filterEmptyValue("x")); } -TEST_F(TagFacetIdTest, convertIntoValidValue) { +TEST_F(FacetIdTest, convertIntoValidValue) { // Clamp empty string EXPECT_EQ( - TagFacetId::value_t{}, - TagFacetId::convertIntoValidValue("")); + FacetId::value_t{}, + FacetId::convertIntoValidValue("")); // Clamped whitespace string EXPECT_EQ( - TagFacetId::value_t{}, - TagFacetId::convertIntoValidValue(" \t\n ")); + FacetId::value_t{}, + FacetId::convertIntoValidValue(" \t\n ")); // Lowercase EXPECT_EQ( - TagFacetId::value_t{"x"}, - TagFacetId::convertIntoValidValue(" \tX\n ")); + FacetId::value_t{"x"}, + FacetId::convertIntoValidValue(" \tX\n ")); // Whitespace replacement EXPECT_EQ( - TagFacetId::value_t{"xy_[+]"}, - TagFacetId::convertIntoValidValue("X y _\n[+] ")); + FacetId::value_t{"xy_[+]"}, + FacetId::convertIntoValidValue("X y _\n[+] ")); // Valid characters without whitespace EXPECT_EQ( - TagFacetId::value_t{TagFacetId::kAlphabet}, - TagFacetId::convertIntoValidValue( - TagFacetId::value_t{TagFacetId::kAlphabet})); + FacetId::value_t{FacetId::kAlphabet}, + FacetId::convertIntoValidValue( + FacetId::value_t{FacetId::kAlphabet})); EXPECT_EQ( - TagFacetId::value_t{"+-./0123456789" - "@abcdefghijklmnopqrstuvwxyz[]_" - "abcdefghijklmnopqrstuvwxyz"}, - TagFacetId::convertIntoValidValue( + FacetId::value_t{"+-./0123456789" + "@abcdefghijklmnopqrstuvwxyz[]_" + "abcdefghijklmnopqrstuvwxyz"}, + FacetId::convertIntoValidValue( "\t!\"#$%&'()*+,-./0123456789:;<=>?" " @ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_" " `abcdefghijklmnopqrstuvwxyz{|}~\n")); } -TEST_F(TagFacetIdTest, isValidValue) { - EXPECT_TRUE(TagFacetId::isValidValue(QString())); - EXPECT_TRUE(TagFacetId::isValidValue(QStringLiteral("facet@mixxx.org/simple-test[1]"))); - EXPECT_FALSE(TagFacetId::isValidValue(QStringLiteral("Uppercase"))); - EXPECT_FALSE(TagFacetId::isValidValue( +TEST_F(FacetIdTest, isValidValue) { + EXPECT_TRUE(FacetId::isValidValue(QString())); + EXPECT_TRUE(FacetId::isValidValue(QStringLiteral("facet@mixxx.org/simple-test[1]"))); + EXPECT_FALSE(FacetId::isValidValue(QStringLiteral("Uppercase"))); + EXPECT_FALSE(FacetId::isValidValue( QStringLiteral("lower-case_with_digit_1_and_whitespace "))); - EXPECT_FALSE(TagFacetId::isValidValue(QLatin1String(""))); - EXPECT_FALSE(TagFacetId::isValidValue(QStringLiteral(" "))); + EXPECT_FALSE(FacetId::isValidValue(QLatin1String(""))); + EXPECT_FALSE(FacetId::isValidValue(QStringLiteral(" "))); } -TEST_F(TagFacetIdTest, convertedEmptyValueIsValid) { +TEST_F(FacetIdTest, convertedEmptyValueIsValid) { EXPECT_TRUE( - TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(TagFacetId::value_t{}))); + FacetId::isValidValue(FacetId::convertIntoValidValue(FacetId::value_t{}))); EXPECT_TRUE( - TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(""))); + FacetId::isValidValue(FacetId::convertIntoValidValue(""))); EXPECT_TRUE( - TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(" "))); + FacetId::isValidValue(FacetId::convertIntoValidValue(" "))); EXPECT_TRUE( - TagFacetId::isValidValue(TagFacetId::convertIntoValidValue(" \t \n \r "))); + FacetId::isValidValue(FacetId::convertIntoValidValue(" \t \n \r "))); } -class TagLabelTest : public testing::Test { +class LabelTest : public testing::Test { }; -TEST_F(TagLabelTest, isEmpty) { - ASSERT_TRUE(TagLabel().isEmpty()); +TEST_F(LabelTest, isEmpty) { + ASSERT_TRUE(Label().isEmpty()); // Null string EXPECT_EQ( - TagLabel(), - TagLabel(TagLabel::value_t())); + Label(), + Label(Label::value_t())); // Empty string EXPECT_EQ( - TagLabel(), - TagLabel(TagLabel::filterEmptyValue(""))); + Label(), + Label(Label::filterEmptyValue(""))); // Non-empty string - EXPECT_FALSE(TagLabel("X").isEmpty()); + EXPECT_FALSE(Label("X").isEmpty()); } -TEST_F(TagLabelTest, filterEmptyValue) { +TEST_F(LabelTest, filterEmptyValue) { EXPECT_EQ( - TagLabel::value_t(), - TagLabel::filterEmptyValue(TagLabel::value_t())); + Label::value_t(), + Label::filterEmptyValue(Label::value_t())); EXPECT_EQ( - TagLabel::value_t(), - TagLabel::filterEmptyValue("")); + Label::value_t(), + Label::filterEmptyValue("")); EXPECT_EQ( - TagLabel::value_t("X"), - TagLabel::filterEmptyValue("X")); + Label::value_t("X"), + Label::filterEmptyValue("X")); } -TEST_F(TagLabelTest, convertIntoValidValue) { +TEST_F(LabelTest, convertIntoValidValue) { // Clamp empty string EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); + Label::isValidValue(Label::convertIntoValidValue(""))); // Clamped whitespace string EXPECT_EQ( - TagLabel::value_t{}, - TagLabel::convertIntoValidValue(" \t\n ")); + Label::value_t{}, + Label::convertIntoValidValue(" \t\n ")); // Trimmed EXPECT_EQ( - TagLabel::value_t{"X y"}, - TagLabel::convertIntoValidValue(" \tX y\n ")); + Label::value_t{"X y"}, + Label::convertIntoValidValue(" \tX y\n ")); } -TEST_F(TagLabelTest, isValidValue) { - EXPECT_TRUE(TagLabel::isValidValue(QString())); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("lower-case_with_digit_1"))); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("With\\backslash"))); - EXPECT_TRUE(TagLabel::isValidValue(QStringLiteral("Uppercase with\twhitespace"))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" With leading space"))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral("With trailing space\n"))); - EXPECT_FALSE(TagLabel::isValidValue(QLatin1String(""))); - EXPECT_FALSE(TagLabel::isValidValue(QStringLiteral(" "))); +TEST_F(LabelTest, isValidValue) { + EXPECT_TRUE(Label::isValidValue(QString())); + EXPECT_TRUE(Label::isValidValue(QStringLiteral("lower-case_with_digit_1"))); + EXPECT_TRUE(Label::isValidValue(QStringLiteral("With\\backslash"))); + EXPECT_TRUE(Label::isValidValue(QStringLiteral("Uppercase with\twhitespace"))); + EXPECT_FALSE(Label::isValidValue(QStringLiteral(" With leading space"))); + EXPECT_FALSE(Label::isValidValue(QStringLiteral("With trailing space\n"))); + EXPECT_FALSE(Label::isValidValue(QLatin1String(""))); + EXPECT_FALSE(Label::isValidValue(QStringLiteral(" "))); } -TEST_F(TagLabelTest, convertedEmptyValueIsValid) { +TEST_F(LabelTest, convertedEmptyValueIsValid) { EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(TagLabel::value_t{}))); + Label::isValidValue(Label::convertIntoValidValue(Label::value_t{}))); EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(""))); + Label::isValidValue(Label::convertIntoValidValue(""))); EXPECT_TRUE( - TagLabel::isValidValue(TagLabel::convertIntoValidValue(" "))); + Label::isValidValue(Label::convertIntoValidValue(" "))); EXPECT_TRUE( - TagLabel::isValidValue(TagFacetId::convertIntoValidValue(" \t \n \r "))); + Label::isValidValue(FacetId::convertIntoValidValue(" \t \n \r "))); } class TagTest : public testing::Test { @@ -164,9 +164,9 @@ class TagTest : public testing::Test { TEST_F(TagTest, fromJsonValue) { const char* labelText = "Label"; - const auto label = TagLabel(labelText); - const TagScore::value_t scoreValue = 0.1357; - const auto score = TagScore(scoreValue); + const auto label = Label(labelText); + const Score::value_t scoreValue = 0.1357; + const auto score = Score(scoreValue); // Empty array EXPECT_EQ( std::nullopt, @@ -181,7 +181,7 @@ TEST_F(TagTest, fromJsonValue) { EXPECT_EQ( std::make_optional(Tag()), Tag::fromJsonValue( - TagScore::kDefaultValue)); + Score::kDefaultValue)); // Label EXPECT_EQ( std::make_optional(Tag(label)), @@ -216,16 +216,16 @@ TEST_F(TagTest, fromJsonValue) { TEST_F(TagTest, toJsonValue) { const char* labelText = "Label"; - const auto label = TagLabel(labelText); - const TagScore::value_t scoreValue = 0.1357; - const auto score = TagScore(scoreValue); + const auto label = Label(labelText); + const Score::value_t scoreValue = 0.1357; + const auto score = Score(scoreValue); // Only label, no score EXPECT_EQ( QJsonValue{labelText}, Tag(label).toJsonValue()); // Only score, no label EXPECT_EQ( - QJsonValue{TagScore::kDefaultValue}, + QJsonValue{Score::kDefaultValue}, Tag().toJsonValue()); EXPECT_EQ( QJsonValue{scoreValue}, From 25c732840aceb6904eeb4192ec80834cd6c4e56d Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 1 Sep 2021 15:25:32 +0200 Subject: [PATCH 19/39] Rename CustomTags as Facets --- CMakeLists.txt | 6 +- src/library/tags/customtags.h | 205 ------------ src/library/tags/facetid.cpp | 16 +- src/library/tags/facetid.h | 4 +- .../tags/{customtags.cpp => facets.cpp} | 271 +++++++++------- src/library/tags/facets.h | 293 ++++++++++++++++++ src/library/tags/tag.h | 2 +- src/test/customtags_test.cpp | 216 ------------- src/test/facets_test.cpp | 220 +++++++++++++ src/track/track.h | 8 +- src/widget/wtrackmenu.cpp | 2 +- 11 files changed, 685 insertions(+), 558 deletions(-) delete mode 100644 src/library/tags/customtags.h rename src/library/tags/{customtags.cpp => facets.cpp} (62%) create mode 100644 src/library/tags/facets.h delete mode 100644 src/test/customtags_test.cpp create mode 100644 src/test/facets_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index f2c6f907cd3..02ba6732b81 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -852,7 +852,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyfactory.cpp src/track/keys.cpp src/track/keyutils.cpp - src/library/tags/customtags.cpp + src/library/tags/facets.cpp src/library/tags/facetid.cpp src/library/tags/label.cpp src/library/tags/tag.cpp @@ -1575,7 +1575,6 @@ add_executable(mixxx-test src/test/cratestorage_test.cpp src/test/cue_test.cpp src/test/cuecontrol_test.cpp - src/test/customtags_test.cpp src/test/dbconnectionpool_test.cpp src/test/dbidtest.cpp src/test/directorydaotest.cpp @@ -1590,7 +1589,7 @@ add_executable(mixxx-test src/test/enginemastertest.cpp src/test/enginemicrophonetest.cpp src/test/enginesynctest.cpp - src/test/librarytags_test.cpp + src/test/facets_test.cpp src/test/fileinfo_test.cpp src/test/frametest.cpp src/test/globaltrackcache_test.cpp @@ -1601,6 +1600,7 @@ add_executable(mixxx-test src/test/lcstest.cpp src/test/learningutilstest.cpp src/test/libraryscannertest.cpp + src/test/librarytags_test.cpp src/test/librarytest.cpp src/test/looping_control_test.cpp src/test/main.cpp diff --git a/src/library/tags/customtags.h b/src/library/tags/customtags.h deleted file mode 100644 index a26490ca55d..00000000000 --- a/src/library/tags/customtags.h +++ /dev/null @@ -1,205 +0,0 @@ -#pragma once - -#include -#include -#include - -#include "library/tags/facetid.h" -#include "library/tags/tag.h" - -namespace mixxx { - -namespace library { - -namespace tags { - -typedef QMap TagMap; -typedef QMap FacetTagMap; - -/// Custom tags -/// -/// Each custom tag is represented by a 3-tuple (triple) of a *facet (`FacetId`), -/// a *label* (`Label`), and a *score* (`Score`): -/// -/// - The *facet* is represented by a *facet identifier* string -/// - The *label* is non-empty, free text without leading/trailing whitespace -/// - The *score* is a floating-point value in the interval [0.0, 1.0] -/// -/// Both *facet* and *label* are optional (= nullable), but at least one of them -/// must be present. The *score* is mandatory and is supposed to be interpreted -/// as a *weight*, *degree of cohesion*, or *normalized level* with a default value -/// of 1.0. -/// -/// The following combinations are allowed, missing values are indicated by `-`: -/// -/// - `(-, label, score)`, e.g. `label` = "Top 40", `score` = 1.0 for tagging a track with -/// the label "Top 40" and full (= no particular, i.e. binary) score. Instead of using -/// a score of 0.0 you would simply remove the entire tag then. -/// - `(facet, -, score)`, e.g. `facet` = "energy", `score` = 0.8 for tagging a track with -/// a fairly high energy level. -/// - `(facet, label, score)`, e.g. `facet` = "genre", `label` = "Hip Hop/Rap", `score` = 0.3 -/// for tagging a track that resembles the genre "Hip Hop/Rap" to a fairly low amount and -/// might be a candidate during a genre change. The "genre" tag with the highest score will -/// represent the main genre and the score defines an ordering between multiple tags of the -/// same facet. -/// -/// The tuple (`facet`, `label`) defines a uniqueness constraint among all custom tags, i.e. within -/// an instance of `CustomTags` which is basically two nested maps. The 1st key is `facet` (nullable) -/// and the 2nd key is `label` (nullable). -class CustomTags final { - MIXXX_DECL_PROPERTY(FacetTagMap, facetedTags, FacetedTags) - - public: - CustomTags() = default; - CustomTags(CustomTags&&) = default; - CustomTags(const CustomTags&) = default; - CustomTags& operator=(CustomTags&&) = default; - CustomTags& operator=(const CustomTags&) = default; - - /// Purge all empty and unused map entries - void compact(); - - /// Check for consistency - bool validate() const; - - bool isEmpty() const; - - bool containsFacet( - const FacetId& facetId) const { - return getFacetedTags().contains(facetId); - } - - /// Add an (empty) entry for the given facet if it does not exist yet. - /// - /// Existing entries are not affected. - void addOrIgnoreFacet( - const FacetId& facetId) { - refFacetedTags()[facetId]; - DEBUG_ASSERT(containsFacet(facetId)); - } - - bool containsTag( - const Label& label, - const FacetId& facetId = FacetId()) const; - int countTags( - const Label& label, - const FacetId& facetId = FacetId()) const; - - int countTags() const { - return countFacetedTags(FacetId()); - } - TagMap& refTags() { - return refFacetedTags()[FacetId()]; - } - - int countFacetedTags( - const FacetId& facetId) const; - TagMap& refFacetedTags( - const FacetId& facetId) { - return refFacetedTags()[facetId]; - } - - bool addOrReplaceTag( - const Tag& tag, - const FacetId& facetId = FacetId()); - bool removeTag( - const Label& label, - const FacetId& facetId = FacetId()); - - bool addOrReplaceAllTags( - const CustomTags& tags); - - /// Get all plain tags as a list (in no particular order) - TagVector getTags() const; - - /// Replace all existing tags of this facet with a single - /// faceted tag or insert a new faceted tag. - FacetTagMap::iterator replaceAllFacetedTags( - const FacetId& facetId, - const Tag& tag); - - int removeAllFacetedTags( - const FacetId& facetId); - - /// Get all tags of a given facet sorted by score in descending order - TagVector getFacetedTagsOrdered( - const FacetId& facetId) const; - - /// Get the label of a single faceted tag, i.e. that - /// occurs at most once and has no custom score. If the - /// facet is unknown/absent an empty label is returned. - Label getFacetedLabel( - const FacetId& facetId) const; - - /// Get the score of a tag if present. - /// - /// Returns `std::nullopt` if the corresponding (facet, label) - /// combination that serves as the key does not exist. - std::optional getScore( - const FacetId& facetId, - const Label& label = Label()) const; - - enum class AggregateScoring { - Maximum, - Average, - }; - - // Merge the score and label of all faceted tags into - // a single plain tag. The strings of the labels are joined - // with a separator in between and the scores are aggregated. - Tag mergeFacetedTags( - const FacetId& facetId, - AggregateScoring aggregateScoring, - const Label::value_t& joinLabelSeparator = Label::value_t()) const; - Label joinFacetedTagsLabel( - const FacetId& facetId, - const Label::value_t& joinLabelSeparator = Label::value_t()) const { - return mergeFacetedTags( - facetId, - AggregateScoring::Maximum, - joinLabelSeparator) - .getLabel(); - } - - enum class FromJsonMode { - Lenient, - Strict, - }; - static std::optional fromJsonObject( - const QJsonObject& jsonObject, - FromJsonMode mode = FromJsonMode::Lenient); - enum class ToJsonMode { - Plain, - Compact, - }; - QJsonObject toJsonObject( - ToJsonMode mode = ToJsonMode::Compact) const; - - enum class ParseJsonDataResult { - Ok, - DeserializationError, - TransformationError, - }; - /// Create a new instance from JSON data - /// - /// Returns a new instance if no error occurred. The resulting - /// instance should be validated for consistency using `validate()`. - static std::pair, ParseJsonDataResult> parseJsonData( - const QByteArray& jsonData, - FromJsonMode mode = FromJsonMode::Lenient); - QByteArray dumpJsonData() const; -}; - -bool operator==(const CustomTags& lhs, const CustomTags& rhs); - -inline bool operator!=(const CustomTags& lhs, const CustomTags& rhs) { - return !(lhs == rhs); -} - -QDebug operator<<(QDebug dbg, const CustomTags& arg); - -} // namespace tags - -} // namespace library - -} // namespace mixxx diff --git a/src/library/tags/facetid.cpp b/src/library/tags/facetid.cpp index c4bf49144c9..780f2d692dd 100644 --- a/src/library/tags/facetid.cpp +++ b/src/library/tags/facetid.cpp @@ -48,12 +48,6 @@ FacetId::value_t FacetId::convertIntoValidValue( } // namespace mixxx -const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidFingerprint = - mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_fingerprint")); - -const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidId = - mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_id")); - // MusicBrainz: "comment:description" const mixxx::library::tags::FacetId mixxx::library::tags::kFacetComment = mixxx::library::tags::FacetId::staticConst(QStringLiteral("comment")); @@ -85,6 +79,12 @@ const mixxx::library::tags::FacetId mixxx::library::tags::kFacetVenue = const mixxx::library::tags::FacetId mixxx::library::tags::kFacetVibe = mixxx::library::tags::FacetId::staticConst(QStringLiteral("vibe")); +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidFingerprint = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_fingerprint")); + +const mixxx::library::tags::FacetId mixxx::library::tags::kFacetAcoustidId = + mixxx::library::tags::FacetId::staticConst(QStringLiteral("acoustid_id")); + const mixxx::library::tags::FacetId mixxx::library::tags::kFacetMusicBrainzAlbumArtistId = mixxx::library::tags::FacetId::staticConst(QStringLiteral("musicbrainz_albumartistid")); @@ -143,8 +143,6 @@ const mixxx::library::tags::FacetId mixxx::library::tags::kFacetSpeechiness = const QVector mixxx::library::tags::kReservedFacetIds = QVector{ - mixxx::library::tags::kFacetAcoustidFingerprint, - mixxx::library::tags::kFacetAcoustidId, mixxx::library::tags::kFacetComment, mixxx::library::tags::kFacetDecade, mixxx::library::tags::kFacetEthno, @@ -155,6 +153,8 @@ const QVector mixxx::library::tags::kFacetMood, mixxx::library::tags::kFacetVenue, mixxx::library::tags::kFacetVibe, + mixxx::library::tags::kFacetAcoustidFingerprint, + mixxx::library::tags::kFacetAcoustidId, mixxx::library::tags::kFacetMusicBrainzAlbumArtistId, mixxx::library::tags::kFacetMusicBrainzAlbumId, mixxx::library::tags::kFacetMusicBrainzArtistId, diff --git a/src/library/tags/facetid.h b/src/library/tags/facetid.h index b514114f54a..dbfdd973381 100644 --- a/src/library/tags/facetid.h +++ b/src/library/tags/facetid.h @@ -165,8 +165,6 @@ inline uint qHash( /// /// TODO: These predefined facets are only intended as a starting point /// and for inspiration. They could be modified as needed. -extern const FacetId kFacetAcoustidFingerprint; -extern const FacetId kFacetAcoustidId; extern const FacetId kFacetComment; // multi-valued comment(s) extern const FacetId kFacetDecade; // e.g. "1980s" extern const FacetId kFacetEthno; @@ -177,6 +175,8 @@ extern const FacetId kFacetLanguage; // ISO 639-3 extern const FacetId kFacetMood; // multi-valued mood(s) extern const FacetId kFacetVenue; extern const FacetId kFacetVibe; +extern const FacetId kFacetAcoustidFingerprint; +extern const FacetId kFacetAcoustidId; extern const FacetId kFacetMusicBrainzAlbumArtistId; extern const FacetId kFacetMusicBrainzAlbumId; extern const FacetId kFacetMusicBrainzArtistId; diff --git a/src/library/tags/customtags.cpp b/src/library/tags/facets.cpp similarity index 62% rename from src/library/tags/customtags.cpp rename to src/library/tags/facets.cpp index 8e125350220..212f4268cce 100644 --- a/src/library/tags/customtags.cpp +++ b/src/library/tags/facets.cpp @@ -1,4 +1,4 @@ -#include "library/tags/customtags.h" +#include "library/tags/facets.h" #include #include @@ -7,8 +7,7 @@ namespace { -Q_LOGGING_CATEGORY(kLogger, "mixxx.library.tags.CustomTags") - +Q_LOGGING_CATEGORY(kLogger, "mixxx.library.tags.Facets") } namespace mixxx { @@ -17,9 +16,9 @@ namespace library { namespace tags { -bool CustomTags::isEmpty() const { - for (auto i = getFacetedTags().begin(); - i != getFacetedTags().end(); +bool Facets::isEmpty() const { + for (auto i = m_impl.begin(); + i != m_impl.end(); ++i) { if (!i.value().isEmpty()) { return false; @@ -28,20 +27,30 @@ bool CustomTags::isEmpty() const { return true; } -void CustomTags::compact() { - auto i = refFacetedTags().begin(); - while (i != refFacetedTags().end()) { +int Facets::countAllTags() const { + int count = 0; + for (auto i = m_impl.begin(); + i != m_impl.end(); + ++i) { + count += i.value().size(); + } + return count; +} + +void Facets::compact() { + auto i = m_impl.begin(); + while (i != m_impl.end()) { if (i.value().isEmpty()) { - i = refFacetedTags().erase(i); + i = m_impl.erase(i); } else { ++i; } } } -bool CustomTags::validate() const { - for (auto i = getFacetedTags().begin(); - i != getFacetedTags().end(); +bool Facets::validate() const { + for (auto i = m_impl.begin(); + i != m_impl.end(); ++i) { if (i.key().isEmpty()) { // plain tags @@ -74,42 +83,42 @@ bool CustomTags::validate() const { return true; } -bool CustomTags::containsTag( +bool Facets::containsTagLabeled( const Label& label, const FacetId& facetId) const { - const auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end()) { + const auto i = m_impl.find(facetId); + if (i == m_impl.end()) { return false; } return i.value().contains(label); } -int CustomTags::countTags( +int Facets::countTagsLabeled( const Label& label, const FacetId& facetId) const { - const auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end()) { + const auto i = m_impl.find(facetId); + if (i == m_impl.end()) { return 0; } return i.value().count(label); } -int CustomTags::countFacetedTags( +int Facets::countTags( const FacetId& facetId) const { - const auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end()) { + const auto i = m_impl.find(facetId); + if (i == m_impl.end()) { return 0; } return i.value().size(); } -bool CustomTags::addOrReplaceTag( +bool Facets::addOrUpdateTag( const Tag& tag, const FacetId& facetId) { - DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); - auto i = refFacetedTags().find(facetId); - if (i == refFacetedTags().end()) { - i = refFacetedTags().insert(facetId, TagMap{}); + DEBUG_ASSERT(countTagsLabeled(tag.getLabel(), facetId) <= 1); + auto i = m_impl.find(facetId); + if (i == m_impl.end()) { + i = m_impl.insert(facetId, TagMap{}); } DEBUG_ASSERT(!tag.getLabel().isEmpty() || i.value().size() <= 1); DEBUG_ASSERT(i.value().count(tag.getLabel()) <= 1); @@ -125,12 +134,12 @@ bool CustomTags::addOrReplaceTag( return true; } -bool CustomTags::removeTag( +bool Facets::removeTagLabeled( const Label& label, const FacetId& facetId) { - DEBUG_ASSERT(countTags(label, facetId) <= 1); - const auto i = refFacetedTags().find(facetId); - if (i == refFacetedTags().end()) { + DEBUG_ASSERT(countTagsLabeled(label, facetId) <= 1); + const auto i = m_impl.find(facetId); + if (i == m_impl.end()) { return false; } DEBUG_ASSERT(!label.isEmpty() || i.value().size() <= 1); @@ -138,7 +147,7 @@ bool CustomTags::removeTag( if (i.value().remove(label) > 0) { if (i.value().isEmpty()) { // Compact entry for this facetId, i.e. remove it entirely - refFacetedTags().erase(i); + m_impl.erase(i); } return true; } else { @@ -146,10 +155,11 @@ bool CustomTags::removeTag( } } -TagVector CustomTags::getTags() const { +TagVector Facets::collectTags( + const FacetId& facetId) const { TagVector tags; - const auto i = getFacetedTags().find(FacetId()); - if (i == getFacetedTags().end()) { + const auto i = m_impl.find(facetId); + if (i == m_impl.end()) { return tags; } tags.reserve(i.value().size()); @@ -161,69 +171,74 @@ TagVector CustomTags::getTags() const { return tags; } -FacetTagMap::iterator CustomTags::replaceAllFacetedTags( - const FacetId& facetId, - const Tag& tag) { - DEBUG_ASSERT(!facetId.isEmpty()); - DEBUG_ASSERT(countTags(tag.getLabel(), facetId) <= 1); - auto i = refFacetedTags().find(facetId); - if (i == refFacetedTags().end()) { - i = refFacetedTags().insert(facetId, TagMap{}); +TagVector Facets::collectTagsOrdered( + ScoreOrdering scoreOrdering, + const FacetId& facetId) const { + TagVector tags = collectTags(facetId); + switch (scoreOrdering) { + case ScoreOrdering::Ascending: + std::sort( + tags.begin(), + tags.end(), + [](const Tag& lhs, const Tag& rhs) { + return lhs.getScore() < rhs.getScore(); + }); + break; + case ScoreOrdering::Descending: + std::sort( + tags.begin(), + tags.end(), + [](const Tag& lhs, const Tag& rhs) { + return lhs.getScore() > rhs.getScore(); + }); + break; + } + return tags; +} + +FacetTagMap::iterator Facets::addOrReplaceTagsWithSingleTag( + const Tag& tag, + const FacetId& facetId) { + DEBUG_ASSERT(countTagsLabeled(tag.getLabel(), facetId) <= 1); + auto i = m_impl.find(facetId); + if (i == m_impl.end()) { + i = m_impl.insert(facetId, TagMap{}); } else { i.value().clear(); } i.value().insert(tag.getLabel(), tag.getScore()); - DEBUG_ASSERT(countFacetedTags(facetId) == 1); + DEBUG_ASSERT(countTags(facetId) == 1); return i; } -int CustomTags::removeAllFacetedTags( +int Facets::removeTags( const FacetId& facetId) { DEBUG_ASSERT(!facetId.isEmpty()); - return refFacetedTags().remove(facetId); + return m_impl.remove(facetId); } -TagVector CustomTags::getFacetedTagsOrdered( +std::optional Facets::getSingleTag( const FacetId& facetId) const { - DEBUG_ASSERT(!facetId.isEmpty()); - TagVector tags; - auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end()) { - return tags; + DEBUG_ASSERT(countTags(facetId) <= 1); + const auto i = m_impl.find(facetId); + if (i == m_impl.end() || + i.value().isEmpty()) { + return std::nullopt; } - tags.reserve(i.value().size()); - for (auto j = i.value().begin(); j != i.value().end(); ++j) { - tags += Tag(j.key(), j.value()); + DEBUG_ASSERT(i.value().size() <= 1); + const auto j = i.value().begin(); + if (j == i.value().end()) { + return std::nullopt; } - DEBUG_ASSERT(tags.size() == countFacetedTags(facetId)); - std::sort( - tags.begin(), - tags.end(), - [](const Tag& lhs, const Tag& rhs) { - return lhs.getScore() > rhs.getScore(); - }); - return tags; + return Tag{j.key(), j.value()}; } -Label CustomTags::getFacetedLabel( +std::optional Facets::getTagScore( + const Label& label, const FacetId& facetId) const { - DEBUG_ASSERT(countFacetedTags(facetId) <= 1); - const auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end() || - i.value().isEmpty()) { - return Label(); - } - DEBUG_ASSERT(i.value().size() == 1); - DEBUG_ASSERT(i.value().first() == Score()); - return i.value().firstKey(); -} - -std::optional CustomTags::getScore( - const FacetId& facetId, - const Label& label) const { - DEBUG_ASSERT(countFacetedTags(facetId) <= 1); - const auto i = getFacetedTags().find(facetId); - if (i == getFacetedTags().end() || + DEBUG_ASSERT(countTagsLabeled(label, facetId) <= 1); + const auto i = m_impl.find(facetId); + if (i == m_impl.end() || i.value().isEmpty()) { return std::nullopt; } @@ -236,14 +251,14 @@ std::optional CustomTags::getScore( return j.value(); } -Tag CustomTags::mergeFacetedTags( - const FacetId& facetId, +Tag Facets::mergeTags( + const QString& joinLabelSeparator, AggregateScoring aggregateScoring, - const QString& joinLabelSeparator) const { + const FacetId& facetId) const { DEBUG_ASSERT(!facetId.isEmpty()); - const auto tags = getFacetedTagsOrdered(facetId); + const auto tags = collectTagsOrdered(ScoreOrdering::Descending, facetId); if (tags.isEmpty()) { - return Tag(); + return Tag{}; } Score::value_t scoreValue; switch (aggregateScoring) { @@ -274,31 +289,51 @@ Tag CustomTags::mergeFacetedTags( Score(scoreValue)); } -bool CustomTags::addOrReplaceAllTags( - const CustomTags& tags) { - bool modified = false; - VERIFY_OR_DEBUG_ASSERT(this != &tags) { - return false; +int Facets::addOrUpdateAllTags( + const Facets& facets) { + VERIFY_OR_DEBUG_ASSERT(this != &facets) { + return 0; } - for (auto i = tags.getFacetedTags().begin(); - i != tags.getFacetedTags().end(); + int count = 0; + for (auto i = facets.m_impl.begin(); + i != facets.m_impl.end(); ++i) { for (auto j = i.value().begin(); j != i.value().end(); ++j) { - modified |= addOrReplaceTag( - Tag(j.key(), j.value()), - i.key()); + if (addOrUpdateTag( + Tag{j.key(), j.value()}, + i.key())) { + ++count; + } } } - return modified; + return count; +} + +void Facets::addOrReplaceFacet( + const TagMap& tags, + const FacetId& facetId) { + m_impl.insert(facetId, tags); +} + +void Facets::addOrReplaceAllFacets( + const Facets& facets) { + VERIFY_OR_DEBUG_ASSERT(this != &facets) { + return; + } + for (auto i = facets.m_impl.begin(); + i != facets.m_impl.end(); + ++i) { + addOrReplaceFacet(i.value(), i.key()); + } } //static -std::optional CustomTags::fromJsonObject( +std::optional Facets::fromJsonObject( const QJsonObject& jsonObject, FromJsonMode mode) { - CustomTags customTags; + Facets facets; for (auto i = jsonObject.begin(); i != jsonObject.end(); ++i) { @@ -328,7 +363,7 @@ std::optional CustomTags::fromJsonObject( // but no predefined tags/labels. The facetId will be // displayed in the UI with the option to add custom // labels. - customTags.addOrIgnoreFacet(facetId); + facets.addOrIgnoreFacet(facetId); continue; } for (const auto& jsonValue : jsonArray) { @@ -350,19 +385,19 @@ std::optional CustomTags::fromJsonObject( << facetId.value(); continue; } - customTags.addOrReplaceTag( - std::move(*tag), + facets.addOrUpdateTag( + *tag, facetId); } } - return customTags; + return facets; } -QJsonObject CustomTags::toJsonObject( +QJsonObject Facets::toJsonObject( ToJsonMode mode) const { QJsonObject jsonObject; - for (auto i = getFacetedTags().begin(); - i != getFacetedTags().end(); + for (auto i = m_impl.begin(); + i != m_impl.end(); ++i) { QJsonArray jsonArray; for (auto j = i.value().begin(); @@ -381,7 +416,7 @@ QJsonObject CustomTags::toJsonObject( } //static -std::pair, CustomTags::ParseJsonDataResult> CustomTags::parseJsonData( +std::pair, Facets::ParseJsonDataResult> Facets::parseJsonData( const QByteArray& jsonData, FromJsonMode mode) { const auto [jsonObject, parseError] = json::parseObject(jsonData); @@ -391,32 +426,32 @@ std::pair, CustomTags::ParseJsonDataResult> CustomTags << parseError; return std::make_pair(std::nullopt, ParseJsonDataResult::DeserializationError); } - auto customTags = CustomTags::fromJsonObject(jsonObject, mode); - if (!customTags) { + auto facets = Facets::fromJsonObject(jsonObject, mode); + if (!facets) { qCWarning(kLogger) << "Failed to transform JSON object" << jsonObject; return std::make_pair(std::nullopt, ParseJsonDataResult::TransformationError); } - return std::make_pair(customTags, ParseJsonDataResult::Ok); + return std::make_pair(facets, ParseJsonDataResult::Ok); } -QByteArray CustomTags::dumpJsonData() const { +QByteArray Facets::dumpJsonData() const { return QJsonDocument(toJsonObject()).toJson(QJsonDocument::Compact); } bool operator==( - const CustomTags& lhs, - const CustomTags& rhs) { - return lhs.getFacetedTags() == rhs.getFacetedTags(); + const Facets& lhs, + const Facets& rhs) { + return lhs.m_impl == rhs.m_impl; } QDebug operator<<( QDebug dbg, - const CustomTags& arg) { - dbg << "CustomTags{"; - arg.dbgFacetedTags(dbg); - dbg << '}'; + const Facets& arg) { + dbg << "Facets{" + << arg.m_impl + << '}'; return dbg; } diff --git a/src/library/tags/facets.h b/src/library/tags/facets.h new file mode 100644 index 00000000000..2b339fc9308 --- /dev/null +++ b/src/library/tags/facets.h @@ -0,0 +1,293 @@ +#pragma once + +#include +#include +#include + +#include "library/tags/facetid.h" +#include "library/tags/tag.h" + +namespace mixxx { + +namespace library { + +namespace tags { + +typedef QMap TagMap; +typedef QMap FacetTagMap; + +/// Set of both faceted and non-faceted tags. +/// +/// Each facet is represented by a 3-tuple (triple) of a *facet (`FacetId`), +/// a *label* (`Label`), and a *score* (`Score`): +/// +/// - The *facet* is represented by a *facet identifier* string +/// - The *label* is non-empty, free text without leading/trailing whitespace +/// - The *score* is a floating-point value in the interval [0.0, 1.0] +/// +/// Both *facet* and *label* are optional (= nullable), but at least one of them +/// must be present. The *score* is mandatory and is supposed to be interpreted +/// as a *weight*, *degree of cohesion*, or *normalized level* with a default value +/// of 1.0. +/// +/// The following combinations are allowed, missing values are indicated by `-`: +/// +/// - `(-, label, score)`, e.g. `label` = "Top 40", `score` = 1.0 for tagging a track with +/// the label "Top 40" and full (= no particular, i.e. binary) score. Instead of using +/// a score of 0.0 you would simply remove the entire tag then. +/// - `(facet, -, score)`, e.g. `facet` = "energy", `score` = 0.8 for tagging a track with +/// a fairly high energy level. +/// - `(facet, label, score)`, e.g. `facet` = "genre", `label` = "Hip Hop/Rap", `score` = 0.3 +/// for tagging a track that resembles the genre "Hip Hop/Rap" to a fairly low amount and +/// might be a candidate during a genre change. The "genre" tag with the highest score will +/// represent the main genre and the score defines an ordering between multiple tags of the +/// same facet. +/// +/// The tuple (`facet`, `label`) defines a uniqueness constraint among all facets, +/// i.e. within an instance of `Facets` which is basically two nested maps. The +/// 1st key is `facet` (nullable) and the 2nd key is `label` (nullable). +/// +/// Plain, non-faceted tags are accessed by an empty facet. For this purpose +/// most operations have an optional `facetId` parameter that could be omitted +/// for accessing the plain, non-faceted tags. +class Facets final { + public: + Facets() = default; + Facets(Facets&&) = default; + Facets(const Facets&) = default; + Facets& operator=(Facets&&) = default; + Facets& operator=(const Facets&) = default; + + /// Check for consistency + bool validate() const; + + /// Purge all empty and unused map entries, i.e. facets + /// without any tags. + void compact(); + + FacetTagMap::const_iterator begin() const { + return m_impl.begin(); + } + FacetTagMap::const_iterator end() const { + return m_impl.end(); + } + + bool isEmpty() const; + + int countAllTags() const; + + int countFacets() const { + return m_impl.size(); + } + + bool containsFacet( + const FacetId& facetId) const { + return m_impl.contains(facetId); + } + + /// Count all tags + int countTags( + const FacetId& facetId = FacetId{}) const; + + /// Access all tags (mutable) + /// + /// This operation will create a new, empty entry in the internal + /// map if the given facet does not exist. + TagMap& refTags( + const FacetId& facetId = FacetId{}) { + return m_impl[facetId]; + } + + int removeTags( + const FacetId& facetId = FacetId{}); + + /// Checks if a tag with the given label exists + bool containsTagLabeled( + const Label& label, + const FacetId& facetId = FacetId{}) const; + + /// Count all tags by the given label + /// + /// Should return either 0 or 1. + int countTagsLabeled( + const Label& label, + const FacetId& facetId = FacetId{}) const; + + /// Remove a tag by label + /// + /// Returns true if a tag has been found and removed. + bool removeTagLabeled( + const Label& label, + const FacetId& facetId = FacetId{}); + + /// Get the singular tag of a facet + /// + /// Triggers a debug assertion if the facet has more than 1 tag. + /// + /// Returns `std::nullopt` if the facet has no tags. + std::optional getSingleTag( + const FacetId& facetId = FacetId{}) const; + + /// Get the singular tag label of a facet + /// + /// Triggers a debug assertion if the facet has more than 1 tag. + /// + /// Returns `std::nullopt` if the facet has no tags. + std::optional