diff --git a/README.md b/README.md index c8382f6..3ae5d07 100644 --- a/README.md +++ b/README.md @@ -174,17 +174,62 @@ An artist neither key resolves is recorded as fetched with no tags, so the next pass does not spend a request on it again. A genuine failure — a rate limit, a bad key — is *not* recorded, so that one is retried. -The built-in moods are chosen for this library rather than as a general -taxonomy: +The built-in moods are built from the **tag distribution of this library**, +measured, rather than from a general taxonomy: | Mood | Selected on | -| ------------------ | ------------------------------------------------- | -| `80s-synths` | synthpop, new wave, synthwave — released 1975-1992 | -| `high-energy-rock` | hard rock, punk, pop punk, alternative | -| `screamo` | screamo, post-hardcore, metalcore, emo | -| `drum-and-bass` | drum and bass, liquid funk, neurofunk, jungle | -| `dance` | house, big room, hardstyle, trance, dubstep | -| `classic-rock` | classic rock, prog, psychedelic, blues rock | +| --------------- | -------------------------------------------------------- | +| `drum-and-bass` | drum and bass and its six spellings, liquid funk, neurofunk, jungle, techstep, hospital records | +| `bass` | dubstep, brostep, grime, trip-hop, big beat | +| `dance` | house and its variants, trance, techno, electro, rave | +| `pop-punk` | pop punk, punk, emo, emocore, easycore, power pop | +| `screamo` | screamo, post-hardcore, metalcore, melodic hardcore, trancecore | +| `heavy-metal` | heavy metal, thrash, speed, power, death, prog, NWOBHM | +| `hair-metal` | hair metal, glam metal, glam rock, arena rock, AOR — 1975-1994 | +| `nu-metal` | nu metal, alternative metal, rapcore, industrial | +| `classic-rock` | classic rock, prog, psychedelic, blues rock, 70s, 60s | +| `80s-synths` | 80s, new wave, synth pop, electropop, post-punk — 1975-1992, rock excluded | +| `indie` | indie, indie rock, indie pop, britpop, singer-songwriter | + +Measuring first mattered. `synthwave`, `edm`, `big room` and `hardstyle` are +plausible tags that carry **nothing at all** here, while `techstep`, `easycore` +and `hospital records` carry real weight. Guessing produces the first list. + +Three kinds of tag are never used, and there is a test enforcing it: + +- **Nationality** — `american` alone spans 241 artists. A passport is not a + sound. +- **`rock` and `electronic`** — 340 and 275 artists, most of the library. A + mood that matches everything is not a mood. +- **Artist names** — Last.fm's most popular tag for an artist is frequently + their own name. `green day`, `paramore` and `queen` are single-artist + playlists waiting to happen. + +### Exclusions + +A mood may also list `exclude`. An excluded tag drops the artist outright rather +than docking their score, and it exists because `80s-synths` cannot be written +any other way. + +`80s` is the eleventh most-played tag here, and it sits on Def Leppard and Bon +Jovi exactly as heavily as on Eurythmics. Weighting cannot separate them, +because the tag it would weight is the one they share. What does separate them +is that the stadium rock also carries `hard rock` and `hair metal`, and the +synth acts do not. + +Checked against live Last.fm pages, since the tag census only sees artists +already in the library: + +| Artist | Tags | +| --- | --- | +| Eurythmics | `80s`, `new wave`, `pop`, `female vocalists`, `synth pop` | +| Frankie Goes to Hollywood | `80s`, `new wave`, `pop`, `british`, `dance` | +| Depeche Mode | `electronic`, `synthpop`, `new wave`, `80s`, `synth pop` | +| Duran Duran | `new wave`, `80s`, `pop`, `synth pop`, `rock` | + +Four of Eurythmics' five tags are ones no mood may use. Three of the four +artists spell it **`synth pop`** with a space; only one spells it `synthpop`. +Guessing one spelling would have missed most of the canon. An artist qualifies when their tag weights inside a mood sum to at least 30 out of Last.fm's 0-100 scale. One low-weight tag is not a genre, it is somebody's @@ -216,6 +261,13 @@ in step by hand. Override it if that guess is wrong. Entries are written **relative to the playlist file**, so one playlist works from the NAS, from a Mac over SMB, and from Linux, without rewriting. +Each playlist is given the **owner and group of the mirror** it is written +into. The image runs as root by default so that a bind mount of any ownership +stays writable, and the cost of that is output owned by root — which the account +serving the share cannot read, group bit or no group bit, because the group is +also root. Copying the mirror's own ownership avoids having to be told what it +should be, and does nothing when the two already agree. + A track is only listed once its mirror file has been confirmed to exist. Lidarr holding the FLAC says nothing about whether the MP3 has been encoded yet. If a large number are missing, the run says so — that is what a wrong `--library-root` diff --git a/music_curator.py b/music_curator.py index 03687f0..31e38c4 100644 --- a/music_curator.py +++ b/music_curator.py @@ -1242,49 +1242,108 @@ PLAYLISTS = ( # `years` filters on the album's release date, which is what separates eighties # synth records from everything a synthpop tag would otherwise drag in. DEFAULT_VIBES = ( + # Built from the tag distribution of this library rather than from a general + # taxonomy, which is why some obvious-looking tags are absent and some + # unobvious ones are here. `synthwave`, `edm`, `big room` and `hardstyle` + # carry nothing at all; `techstep`, `easycore` and `hospital records` carry + # real weight. + # + # Three kinds of tag are deliberately never used. Nationality -- american, + # british, swedish -- describes a passport, not a sound, and `american` + # alone spans 241 artists. `rock` and `electronic` span 340 and 275, which + # is most of the library and therefore no mood at all. And Last.fm's most + # popular tag for an artist is frequently their own name, so `green day`, + # `paramore` and `queen` are single-artist playlists waiting to happen. { - "name": "80s-synths", + "name": "drum-and-bass", "tags": [ - "synthpop", "synth pop", "synth-pop", "new wave", "synthwave", - "new romantic", "electropop", "80s", "1980s", + "drum and bass", "dnb", "drum n bass", "drum'n'bass", "drum & bass", + "drum 'n' bass", "liquid funk", "neurofunk", "jungle", "techstep", + "darkstep", "drumstep", "breakbeat", "hospital records", ], - "years": [1975, 1992], }, { - "name": "high-energy-rock", + "name": "bass", + "tags": ["dubstep", "brostep", "grime", "trip-hop", "big beat"], + }, + { + "name": "dance", "tags": [ - "hard rock", "punk rock", "pop punk", "punk", "alternative rock", - "rock", "garage rock", "skate punk", + "house", "electro house", "progressive house", "tech house", "trance", + "techno", "electro", "rave", "dance", "minimal", + ], + }, + { + "name": "pop-punk", + "tags": [ + "pop punk", "pop-punk", "punk rock", "punk", "skate punk", "emo", + "emocore", "easycore", "powerpop", "power pop", "post-grunge", ], }, { "name": "screamo", "tags": [ - "screamo", "post-hardcore", "metalcore", "emo", "hardcore", - "melodic hardcore", "emocore", + "screamo", "post-hardcore", "metalcore", "melodic metalcore", + "melodic hardcore", "hardcore", "trancecore", "deathcore", ], }, { - "name": "drum-and-bass", + "name": "heavy-metal", "tags": [ - "drum and bass", "drum n bass", "dnb", "liquid funk", "neurofunk", - "jungle", "breakbeat", + "heavy metal", "metal", "thrash metal", "thrash", "speed metal", + "power metal", "death metal", "progressive metal", "nwobhm", + "classic metal", ], }, { - "name": "dance", + "name": "hair-metal", "tags": [ - "electro house", "house", "big room", "electronic dance music", - "edm", "hardstyle", "trance", "dubstep", "electro", + "hair metal", "glam metal", "glam rock", "arena rock", "aor", + "rock and roll", "rock n roll", + ], + "years": [1975, 1994], + }, + { + "name": "nu-metal", + "tags": [ + "nu metal", "nu-metal", "alternative metal", "rapcore", + "industrial metal", "industrial rock", "industrial", ], }, { "name": "classic-rock", "tags": [ - "classic rock", "progressive rock", "psychedelic rock", - "blues rock", "70s", "60s", + "classic rock", "progressive rock", "psychedelic rock", "psychedelic", + "blues rock", "blues", "southern rock", "art rock", "space rock", + "british invasion", "folk rock", "70s", "60s", ], }, + { + "name": "80s-synths", + # `80s` is included, which on its own would drag in Def Leppard and Bon + # Jovi -- they carry it as heavily as Eurythmics does. The exclusion is + # what separates them: the stadium rock is also tagged hard rock and + # hair metal, and the synth acts are not. Weighting cannot do this, + # because the tag it would weight is the one they share. + # + # Both spellings of synth pop are listed. Of Depeche Mode, Duran Duran, + # Eurythmics and Frankie Goes to Hollywood, three carry "synth pop" with + # a space and only one carries "synthpop" without. + "tags": [ + "80s", "new wave", "synth pop", "synthpop", "synth-pop", "synthwave", + "electropop", "new romantic", "post-punk", "post-punk revival", + ], + "exclude": [ + "hard rock", "hair metal", "glam metal", "glam rock", "heavy metal", + "metal", "arena rock", "aor", "nwobhm", "thrash metal", + "classic rock", "southern rock", "blues rock", + ], + "years": [1975, 1992], + }, + { + "name": "indie", + "tags": ["indie", "indie rock", "indie pop", "britpop", "singer-songwriter"], + }, ) @@ -1310,6 +1369,13 @@ def load_vibes(path): raise ValueError(f"{path}: {name!r} is not a usable vibe name (a-z, 0-9, -)") if not vibe.get("tags"): raise ValueError(f"{path}: vibe {name!r} lists no tags") + overlap = {str(tag).casefold() for tag in vibe["tags"]} & { + str(tag).casefold() for tag in vibe.get("exclude", []) + } + if overlap: + raise ValueError( + f"{path}: vibe {name!r} both selects on and excludes {sorted(overlap)}" + ) return tuple(loaded) @@ -1416,8 +1482,24 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): for vibe in vibes: tags = [str(tag).strip().casefold() for tag in vibe["tags"]] placeholders = ",".join("?" * len(tags)) - years = vibe.get("years") parameters = [*tags, vibe.get("min_score", VIBE_MIN_SCORE)] + + # An exclusion drops the artist outright rather than docking their + # score. It is the only way to write "the eighties, but not the stadium + # rock": those artists carry `80s` as heavily as the synth acts do, so + # no amount of weighting separates them -- but they also carry `hard + # rock`, and the synth acts do not. + excluded = [str(tag).strip().casefold() for tag in vibe.get("exclude", [])] + exclude_clause = "" + if excluded: + exclude_clause = ( + " AND NOT EXISTS (SELECT 1 FROM artist_tag x" + " WHERE x.norm_artist = a.norm_name" + f" AND x.tag IN ({','.join('?' * len(excluded))}))" + ) + parameters += excluded + + years = vibe.get("years") year_clause = "" if years: year_clause = ( @@ -1440,7 +1522,7 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): JOIN lidarr_artist a ON a.id = t.artist_id JOIN vibe v ON v.norm_artist = a.norm_name LEFT JOIN lidarr_album al ON al.id = t.album_id - WHERE {PLAYABLE}{year_clause} + WHERE {PLAYABLE}{exclude_clause}{year_clause} ORDER BY ((t.id * {SHUFFLE_MULTIPLIER}) + ?) % {SHUFFLE_MODULUS} LIMIT ? """ @@ -1452,7 +1534,7 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): continue entries.append({**dict(row), "mirror": mirror}) - write_playlist(directory / f"{vibe['name']}.m3u", entries) + write_playlist(directory / f"{vibe['name']}.m3u", entries, Path(mirror_root)) total += len(entries) logger.info("playlist %-20s %4d tracks -- by tag", vibe["name"], len(entries)) @@ -1490,7 +1572,40 @@ def mirror_path_for(source, library_root, mirror_root): return (Path(mirror_root) / relative).with_suffix(MIRROR_SUFFIX) -def write_playlist(path, entries): +def set_ownership(path, uid, gid): + """Give a path an owner and group. Returns whether anything changed.""" + try: + current = path.stat() + if (current.st_uid, current.st_gid) == (uid, gid): + return False + os.chown(path, uid, gid) + except OSError: + # Not permitted unless running as root, which is the case where the + # ownership is already whatever the caller runs as. + return False + return True + + +def match_ownership(path, reference): + """Give a path the owner and group of the tree it is joining. + + The image runs as root by default, so that a bind mount of any ownership + stays writable. The cost is that everything it writes comes out root-owned, + and a root-owned playlist inside a mirror owned by the apps account is + unreadable to the thing that serves it -- the group bit does not help when + the group is root. + + Copying the mirror's own ownership avoids having to be told what it should + be, and is a no-op when the two already agree. + """ + try: + wanted = reference.stat() + except OSError: + return False + return set_ownership(path, wanted.st_uid, wanted.st_gid) + + +def write_playlist(path, entries, reference=None): """Write one extended M3U, atomically. Paths are relative to the playlist file, so the same playlist works from the @@ -1502,7 +1617,11 @@ def write_playlist(path, entries): lines.append(f"#EXTINF:{seconds},{entry['artist']} - {entry['title']}") lines.append(os.path.relpath(entry["mirror"], path.parent)) + fresh = not path.parent.exists() path.parent.mkdir(parents=True, exist_ok=True) + if fresh and reference is not None: + match_ownership(path.parent, reference) + handle, temporary = tempfile.mkstemp(dir=path.parent, suffix=".m3u.part") os.close(handle) temporary = Path(temporary) @@ -1513,6 +1632,10 @@ def write_playlist(path, entries): mode = temporary.stat().st_mode if not mode & GROUP_READ: temporary.chmod(mode | GROUP_READ) + # Before the rename, so the playlist is never briefly visible owned by + # the wrong account. + if reference is not None: + match_ownership(temporary, reference) os.replace(temporary, path) finally: temporary.unlink(missing_ok=True) @@ -1543,7 +1666,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): missing += 1 continue entries.append({**dict(row), "mirror": mirror}) - write_playlist(directory / f"{name}.m3u", entries) + write_playlist(directory / f"{name}.m3u", entries, Path(mirror_root)) total += len(entries) logger.info("playlist %-20s %4d tracks -- %s", name, len(entries), description) diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index d47d353..df74267 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -1,4 +1,5 @@ import json +import os import stat import urllib.error from pathlib import Path @@ -1198,3 +1199,146 @@ def test_a_real_failure_is_not_recorded_so_the_next_pass_retries(tmp_path): # One artist failed hard and must still be pending. assert store.scalar("SELECT COUNT(*) FROM artist_tag_fetched") == 1 + + +# Tags that carry real weight in a real library, against tags that describe a +# passport, span most of the collection, or are somebody's artist name. +USELESS_TAGS = { + "american", "british", "australian", "canadian", "swedish", "dutch", "german", + "scottish", "english", "uk", "usa", "canada", + "rock", "electronic", "pop", "alternative", "metal ", "all", "heavy", + "female vocalists", "male vocalists", "female vocalist", + "my top songs", "cover", "covers", "not emo", + "green day", "paramore", "queen", "bon jovi", "shinedown", "aerosmith", + "journey", "fleetwood mac", +} + + +def test_no_mood_selects_on_a_useless_tag(): + """Nationality is not a sound; `rock` and `electronic` span most of the + library; and Last.fm's top tag for an artist is often their own name.""" + for vibe in music_curator.DEFAULT_VIBES: + overlap = {tag.casefold() for tag in vibe["tags"]} & USELESS_TAGS + assert not overlap, f"{vibe['name']} selects on {overlap}" + + +def test_mood_names_are_unique(): + names = [vibe["name"] for vibe in music_curator.DEFAULT_VIBES] + assert len(names) == len(set(names)) + + +ROCK_TAGS = { + "classic rock", "hard rock", "blues rock", "southern rock", "arena rock", + "glam rock", "hair metal", "glam metal", "heavy metal", "metal", "art rock", + "psychedelic rock", "progressive rock", "rock and roll", "rock n roll", +} + + +def test_a_decade_tag_in_a_non_rock_mood_must_exclude_the_rock(): + """`80s` sits on Def Leppard and Bon Jovi as heavily as on Eurythmics, so a + mood that reaches for a decade without wanting rock has to say so. A mood + that does want it -- classic-rock reaching for 70s -- is exempt.""" + for vibe in music_curator.DEFAULT_VIBES: + tags = {tag.casefold() for tag in vibe["tags"]} + decades = {"60s", "70s", "80s", "90s"} & tags + if not decades or tags & ROCK_TAGS: + continue + excluded = {tag.casefold() for tag in vibe.get("exclude", [])} + missing = {"hard rock", "hair metal"} - excluded + assert not missing, f"{vibe['name']} selects on {decades} without excluding {missing}" + + +def test_the_eighties_mood_covers_the_canon(): + """Depeche Mode, Duran Duran, Eurythmics and Frankie Goes to Hollywood -- + checked against their live Last.fm tags. Three of the four carry "synth pop" + with a space; only one carries it without.""" + synths = next(v for v in music_curator.DEFAULT_VIBES if v["name"] == "80s-synths") + tags = {t.casefold() for t in synths["tags"]} + for artist_tags in ( + {"80s", "new wave", "pop", "female vocalists", "synth pop"}, # Eurythmics + {"80s", "new wave", "pop", "british", "dance"}, # Frankie + {"electronic", "synthpop", "new wave", "80s", "synth pop"}, # Depeche Mode + {"new wave", "80s", "pop", "synth pop", "rock"}, # Duran Duran + ): + assert tags & artist_tags, artist_tags + assert synths["years"] == [1975, 1992] + + +def test_an_excluded_tag_drops_the_artist(tmp_path): + """Weighting cannot separate the eighties synth acts from the eighties + stadium rock, because the tag they would be weighted on is the one they + share.""" + store, source, mirror = tagged_store( + tmp_path, + { + "Played Band": [{"name": "80s", "count": 100}, {"name": "hard rock", "count": 90}], + "Silent Band": [{"name": "80s", "count": 100}, {"name": "synth pop", "count": 90}], + }, + ) + vibes = [{"name": "eighties", "tags": ["80s", "synth pop"], "exclude": ["hard rock"]}] + + music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW) + + written = (mirror / "_playlists" / "eighties.m3u").read_text() + assert "Silent Band" in written + assert "Played Band" not in written + + +def test_a_vibe_cannot_both_select_and_exclude_a_tag(tmp_path): + path = tmp_path / "vibes.json" + path.write_text(json.dumps([{"name": "x", "tags": ["80s"], "exclude": ["80s"]}])) + + with pytest.raises(ValueError, match="selects on and excludes"): + music_curator.load_vibes(str(path)) + + +def test_matching_ownership_is_a_no_op_when_it_already_agrees(tmp_path): + target = tmp_path / "file" + target.write_text("x") + + assert music_curator.match_ownership(target, tmp_path) is False + + +def test_ownership_failure_is_tolerated(tmp_path): + """Not permitted unless running as root -- which is exactly the case where + the ownership is already whatever the caller runs as.""" + target = tmp_path / "file" + target.write_text("x") + + # uid 0 from a non-root test process: refused, and must not raise. + assert music_curator.set_ownership(target, 0, 0) is False + + +def test_a_playlist_is_chowned_to_match_the_mirror(tmp_path, monkeypatch): + """The image runs as root, so its output is root-owned, and a root-owned + playlist in an apps-owned mirror is unreadable to whatever serves it.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + music_curator.index_library( + music_curator.Lidarr("http://lidarr", "key", transport=api), store + ) + music_curator.match_library(store) + + attempted = [] + real_stat = music_curator.Path.stat + + def pretend_mirror_is_owned_by_568(self, *args, **kwargs): + info = real_stat(self, *args, **kwargs) + if self == mirror: + return os.stat_result( + (info.st_mode, info.st_ino, info.st_dev, info.st_nlink, 568, 568, + info.st_size, int(info.st_atime), int(info.st_mtime), int(info.st_ctime)) + ) + return info + + monkeypatch.setattr(music_curator.Path, "stat", pretend_mirror_is_owned_by_568) + monkeypatch.setattr( + music_curator.os, "chown", lambda p, u, g: attempted.append((str(p), u, g)) + ) + + music_curator.build_playlists(store, mirror, str(source), 100, NOW) + + assert attempted, "no ownership was applied" + assert all(tuple(owner) == (568, 568) for _, *owner in attempted) + # The temporary file, before the rename, never the finished playlist. + assert all(path.endswith(".part") or path.endswith("_playlists") for path, *_ in attempted)