From 4fdf3a204f7795f4751493cb63d1cd892dbe6424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 12 Sep 2021 13:53:02 +0100 Subject: [PATCH 1/7] 18-19 handle absent track durations --- CHANGELOG.md | 7 + beetsplug/bandcamp/_metaguru.py | 11 +- pyproject.toml | 2 +- setup.cfg | 1 + tests/conftest.py | 62 +++ tests/json/issues/18.json | 697 ++++++++++++++++++++++++++++++++ tests/test_jsons.py | 83 ++++ tests/test_parsing.py | 74 +--- url2json | 11 +- 9 files changed, 866 insertions(+), 82 deletions(-) create mode 100644 tests/json/issues/18.json create mode 100644 tests/test_jsons.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8159a91..03cf94b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## [0.10.1] 2021-09-12 + +### Fixed + +- Fixed #18 by handling cases when a track duration is not given. + + ## [0.10.0] 2021-09-10 ### Fixed diff --git a/beetsplug/bandcamp/_metaguru.py b/beetsplug/bandcamp/_metaguru.py index d6d5aec..bcf52e4 100644 --- a/beetsplug/bandcamp/_metaguru.py +++ b/beetsplug/bandcamp/_metaguru.py @@ -151,11 +151,14 @@ def parse_catalognum(album: str, disctitle: str, description: str, label: str) - @staticmethod def get_duration(source: JSONDict) -> int: - return [ - floor(x.get("value", 0)) + prop = [ + x.get("value") or 0 for x in source.get("additionalProperty", []) if x.get("name") == "duration_secs" - ][0] + ] + if len(prop) == 1: + return floor(prop[0]) + return 0 @staticmethod def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: @@ -393,7 +396,7 @@ def tracks(self) -> List[JSONDict]: index=index, medium_index=index, track_id=raw_item.get("@id"), - length=self.get_duration(raw_item), + length=self.get_duration(raw_item) or None, **self.parse_track_name(name, catalognum), ) track["artist"] = self.get_track_artist( diff --git a/pyproject.toml b/pyproject.toml index cc650e0..f66904c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "beetcamp" -version = "0.10.0" +version = "0.10.1" description = "Bandcamp autotagger source for beets (http://beets.io)." authors = ["Šarūnas Nejus "] readme = "README.md" diff --git a/setup.cfg b/setup.cfg index 5bb0b7b..cfc18b8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -33,6 +33,7 @@ addopts = markers = need_connection: end-to-end tests that require internet connection + jsons: tests that compare parsed releases with json fixtures parsing: parsing tests testpaths = diff --git a/tests/conftest.py b/tests/conftest.py index fd3b8d5..b60c4f9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -690,3 +690,65 @@ def edge_cases() -> ReleaseInfo: mediums=1, ) return info + + +@pytest.fixture +def issues_18() -> ReleaseInfo: + albumartist = "General Dicon" + info = ReleaseInfo( + artist_id="https://54thregiment.bandcamp.com", + album_id="https://54thregiment.bandcamp.com/album/fall-in", + media="Digital Media", + disctitle=None, + ) + tracks = [ + ("54-regi", albumartist, "54 regi", None, None), + ("54-regi-2", albumartist, "54 regi", 227, None), + ( + "mile-high-monsta-ft-nation", + albumartist, + "Mile high monsta ft.nation", + 157, + None, + ), + ("pure-entertainment", albumartist, "Pure entertainment", 166, None), + ("smoke-interlude", albumartist, "Smoke interlude", 69, None), + ( + "ganja-tune-ft-spellbinder", + albumartist, + "Ganja tune ft.spellbinder", + 235, + None, + ), + ("war-ft-spellbinder", albumartist, "War ft.spellbinder", 203, None), + ( + "w-m-m-a-where-my-money-ft-spellbinder", + albumartist, + "w.m.m.a(where my money @) ft.Spellbinder", + 147, + None, + ), + ("due-time", albumartist, "Due time", 257, None), + ("hustle-aint-no-game", albumartist, "Hustle Ain't no game", 197, None), + ( + "its-ova-ft-scrilla-scratch", + albumartist, + "It's ova ft. scrilla scratch", + 201, + None, + ), + ("dont-stop-ft-spellbinder", albumartist, "Don't stop ft.Spellbinder", 244, None), + ] + info.set_albuminfo( + tracks, + album="FALL IN", + albumartist=albumartist, + albumtype="album", + catalognum="", + label="54th Regiment", + release_date=date(2011, 4, 20), + va=False, + country="US", + mediums=1, + ) + return info diff --git a/tests/json/issues/18.json b/tests/json/issues/18.json new file mode 100644 index 0000000..97abe61 --- /dev/null +++ b/tests/json/issues/18.json @@ -0,0 +1,697 @@ +{ + "@context": "https://schema.org", + "@id": "https://54thregiment.bandcamp.com/album/fall-in", + "@type": "MusicAlbum", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "art_id", + "value": 1474409418 + }, + { + "@type": "PropertyValue", + "name": "featured_track_num", + "value": 2 + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + } + ], + "albumRelease": [ + { + "@id": "https://54thregiment.bandcamp.com/album/fall-in#a3640739659", + "@type": [ + "MusicRelease", + "Product" + ], + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "item_id", + "value": 3640739659 + }, + { + "@type": "PropertyValue", + "name": "item_type", + "value": "a" + }, + { + "@type": "PropertyValue", + "name": "selling_band_id", + "value": 732004402 + }, + { + "@type": "PropertyValue", + "name": "type_name", + "value": "Digital" + }, + { + "@type": "PropertyValue", + "name": "art_id", + "value": 1474409418 + } + ], + "description": "Includes high-quality download in MP3, FLAC and more. Paying supporters also get unlimited streaming via the free Bandcamp app.", + "image": [ + "https://f4.bcbits.com/img/a1474409418_10.jpg" + ], + "musicReleaseFormat": "DigitalFormat", + "name": "FALL IN" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/54-regi#t2788523249", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/54-regi-2#t1940656242", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/mile-high-monsta-ft-nation#t2017700002", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/pure-entertainment#t2504455144", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/smoke-interlude#t2191604344", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/ganja-tune-ft-spellbinder#t3808316298", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/war-ft-spellbinder#t1546501549", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/w-m-m-a-where-my-money-ft-spellbinder#t3184302987", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/due-time#t524055908", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/hustle-aint-no-game#t2084081972", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/its-ova-ft-scrilla-scratch#t3829944235", + "@type": "MusicRelease" + }, + { + "@id": "https://54thregiment.bandcamp.com/track/dont-stop-ft-spellbinder#t3094837483", + "@type": "MusicRelease" + } + ], + "albumReleaseType": "AlbumRelease", + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "dateModified": "16 May 2011 03:29:03 GMT", + "datePublished": "20 Apr 2011 00:00:00 GMT", + "image": "https://f4.bcbits.com/img/a1474409418_10.jpg", + "keywords": [ + "Hip-hop", + "Real nigga shit", + "rap", + "Denver" + ], + "name": "FALL IN", + "numTracks": 12, + "publisher": { + "@id": "https://54thregiment.bandcamp.com", + "@type": "MusicGroup", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "band_id", + "value": 732004402 + }, + { + "@type": "PropertyValue", + "name": "currency", + "value": "USD" + }, + { + "@type": "PropertyValue", + "name": "has_any_downloads", + "value": true + } + ], + "foundingLocation": { + "@type": "Place", + "name": "Denver, Colorado" + }, + "name": "54th Regiment", + "subjectOf": [ + { + "@type": "WebPage", + "name": "music", + "url": "https://54thregiment.bandcamp.com/music" + }, + { + "@type": "WebPage", + "name": "community", + "url": "https://54thregiment.bandcamp.com/community" + } + ] + }, + "track": { + "@type": "ItemList", + "itemListElement": [ + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/54-regi", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 2788523249 + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 1 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "name": "54 regi" + }, + "position": 1 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/54-regi-2", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 1940656242 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 227.133 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/3e237aa615688e92b46106c611d15d8b/mp3-128/1940656242?p=0&ts=1631534807&t=4e2cb34b23392f1f9d5804419562921fbd384224&token=1631534807_655c3f230219f021bba3925157ff05f4a51edc83" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 2 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H03M47S", + "name": "54 regi" + }, + "position": 2 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/mile-high-monsta-ft-nation", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 2017700002 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 157.6 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/f3bf4f8c0e53577ae140d1acd27a738d/mp3-128/2017700002?p=0&ts=1631534807&t=98f4ecc70b2fc95d1056d8f8544bc0a6c0fd3410&token=1631534807_2f4034189e091238215720d9939749eb3c66f935" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 3 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H02M37S", + "name": "Mile high monsta ft.nation" + }, + "position": 3 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/pure-entertainment", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 2504455144 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 166.76 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/98d692639a9bffc9b5b3a9af6a0f0cce/mp3-128/2504455144?p=0&ts=1631534807&t=7ae00a5548e00355e72a7730dc9cace4b9d3b0aa&token=1631534807_3fa437819ca5a93f51db41de90890b43f34c1c3a" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 4 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "duration": "P00H02M46S", + "name": "Pure entertainment" + }, + "position": 4 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/smoke-interlude", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 2191604344 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 69.8933 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/336f2a46da8ef4a98ae521904774750c/mp3-128/2191604344?p=0&ts=1631534807&t=06de6a90f07c7fb83c7b074072ca87518264f897&token=1631534807_64cdf287455c108e963c30eded3fb4c95cc0d9cf" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 5 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "duration": "P00H01M09S", + "name": "Smoke interlude" + }, + "position": 5 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/ganja-tune-ft-spellbinder", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 3808316298 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 235.253 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/ae30fdc4c257f2771435c42bb3f6c79e/mp3-128/3808316298?p=0&ts=1631534807&t=9eba37afd1e12c0f0cdf83994891dd5633219bc8&token=1631534807_d3881acdde680dfb03457450525a07679d0179d5" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 6 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H03M55S", + "name": "Ganja tune ft.spellbinder" + }, + "position": 6 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/war-ft-spellbinder", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 1546501549 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 203.907 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/cdf0ed781d4861588f85aa5a89a7e66b/mp3-128/1546501549?p=0&ts=1631534807&t=0b5e049a54e7ec26d860e148df2f2ac0b48082e9&token=1631534807_30cf3b020e4f3c684e0e093ccc6961825f5e3640" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 7 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H03M23S", + "name": "War ft.spellbinder" + }, + "position": 7 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/w-m-m-a-where-my-money-ft-spellbinder", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 3184302987 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 147.44 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/9f207a7854105f69530642b7fc11506b/mp3-128/3184302987?p=0&ts=1631534807&t=66e0593fe53b22aa674b26f4ed7e95f4e75dcd0b&token=1631534807_21e446a9598b35728a2e507e2d4afe4262c454a2" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 8 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H02M27S", + "name": "w.m.m.a(where my money @) ft.Spellbinder" + }, + "position": 8 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/due-time", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 524055908 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 257.453 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/52f3d3eab73893b10cb963263199ab62/mp3-128/524055908?p=0&ts=1631534807&t=ed0f227c2f435c0e223243cc7ef0f303fcdda296&token=1631534807_760ed98b2e2ae9bbaef545d49ed48db1df15b7ae" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 9 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "duration": "P00H04M17S", + "name": "Due time" + }, + "position": 9 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/hustle-aint-no-game", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 2084081972 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 197.733 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/72448b0596b1935b22c347b880417074/mp3-128/2084081972?p=0&ts=1631534807&t=06308d10e1ae7f2491d1ec705b161c31e6566763&token=1631534807_d0c53fffb3f5502d12f2680d3ae155b4be54288e" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 10 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "duration": "P00H03M17S", + "name": "Hustle Ain't no game" + }, + "position": 10 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/its-ova-ft-scrilla-scratch", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 3829944235 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 201.12 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/904ac3e59366444590acdd16894decc7/mp3-128/3829944235?p=0&ts=1631534807&t=2ce322cb65d1dfc138d94869ac189591009dbf1b&token=1631534807_2217b1298b0521f7101bbb668c5ef53f8dc48fd0" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 11 + } + ], + "copyrightNotice": "All Rights Reserved", + "duration": "P00H03M21S", + "name": "It's ova ft. scrilla scratch" + }, + "position": 11 + }, + { + "@type": "ListItem", + "item": { + "@id": "https://54thregiment.bandcamp.com/track/dont-stop-ft-spellbinder", + "@type": "MusicRecording", + "additionalProperty": [ + { + "@type": "PropertyValue", + "name": "track_id", + "value": 3094837483 + }, + { + "@type": "PropertyValue", + "name": "duration_secs", + "value": 244.08 + }, + { + "@type": "PropertyValue", + "name": "file_mp3-128", + "value": "https://t4.bcbits.com/stream/f2e003bda3cbaa4776dc92c9502f4241/mp3-128/3094837483?p=0&ts=1631534807&t=d125ad1f50b8ed02ceda7333dbd0429581c1aff7&token=1631534807_3c5ba711777b9912c392a408c8cbbf78037cb99d" + }, + { + "@type": "PropertyValue", + "name": "license_name", + "value": "all_rights_reserved" + }, + { + "@type": "PropertyValue", + "name": "streaming", + "value": true + }, + { + "@type": "PropertyValue", + "name": "tracknum", + "value": 12 + } + ], + "byArtist": { + "@type": "MusicGroup", + "name": "General Dicon" + }, + "copyrightNotice": "All Rights Reserved", + "duration": "P00H04M04S", + "name": "Don't stop ft.Spellbinder" + }, + "position": 12 + } + ], + "numberOfItems": 12 + } +} diff --git a/tests/test_jsons.py b/tests/test_jsons.py new file mode 100644 index 0000000..0f50074 --- /dev/null +++ b/tests/test_jsons.py @@ -0,0 +1,83 @@ +import re + +import pytest +from beetsplug.bandcamp._metaguru import NEW_BEETS, Metaguru +from pytest_lazyfixture import lazy_fixture + +pytestmark = pytest.mark.jsons + + +@pytest.fixture(name="release") +def _release(request): + """Read the json data and make it span a single line - same like it's found in htmls. + Fixture names map to the testfiles (minus the extension). + """ + info = request.param + fixturename = next(iter(request._parent_request._fixture_defs.keys())) + if fixturename.startswith("issues"): + filename = "tests/json/issues/{}.json".format(fixturename.replace("issues_", "")) + else: + filename = "tests/json/{}.json".format(fixturename) + + if filename: + with open(filename) as file: + return re.sub(r"\n *", "", file.read()), info + + +def check(actual, expected) -> None: + if NEW_BEETS: + assert actual == expected + else: + assert vars(actual) == vars(expected) + + +@pytest.mark.parametrize( + "release", + map(lazy_fixture, ["single_track_release", "single_only_track_name"]), + indirect=["release"], +) +def test_parse_single_track_release(release): + html, expected = release + guru = Metaguru(html) + + check(guru.singleton, expected.singleton) + + +@pytest.mark.parametrize( + "release", + map( + lazy_fixture, + [ + "album", + "album_with_track_alt", + "compilation", + "ep", + "artist_mess", + "description_meta", + "single_with_remixes", + "remix_artists", + "edge_cases", + "issues_18", + ], + ), + indirect=["release"], +) +def test_parse_various_types(release): + html, expected_release = release + guru = Metaguru(html, expected_release.media) + + actual_album = guru.album + expected_album = expected_release.albuminfo + + assert hasattr(actual_album, "tracks") + assert len(actual_album.tracks) == len(expected_album.tracks) + + expected_tracks = sorted(expected_album.tracks, key=lambda t: t.index) + actual_tracks = sorted(actual_album.tracks, key=lambda t: t.index) + + actual_album.tracks = None + expected_album.tracks = None + check(actual_album, expected_album) + + for actual_track, expected_track in zip(actual_tracks, expected_tracks): + check(actual_track, expected_track) diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 1127299..bcc3b4d 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -1,10 +1,8 @@ """Module for tests related to parsing.""" import json -import re import pytest -from beetsplug.bandcamp._metaguru import NEW_BEETS, Helpers, Metaguru, urlify -from pytest_lazyfixture import lazy_fixture +from beetsplug.bandcamp._metaguru import Helpers, Metaguru, urlify from rich.console import Console from rich.panel import Panel from rich.table import Table @@ -348,73 +346,3 @@ def test_bundles_get_excluded(): ] } assert set(Helpers._get_media_index(meta)) == {"Vinyl"} - - -@pytest.fixture(name="release") -def _release(request): - """Read the json data and make it span a single line - same like it's found in htmls. - Fixture names map to the testfiles (minus the extension). - """ - info = request.param - fixturename = next(iter(request._parent_request._fixture_defs.keys())) - filename = "tests/json/{}.json".format(fixturename) - with open(filename) as file: - return re.sub(r"\n *", "", file.read()), info - - -def check(actual, expected) -> None: - if NEW_BEETS: - assert actual == expected - else: - assert vars(actual) == vars(expected) - - -@pytest.mark.parametrize( - "release", - map(lazy_fixture, ["single_track_release", "single_only_track_name"]), - indirect=["release"], -) -def test_parse_single_track_release(release): - html, expected = release - guru = Metaguru(html) - - check(guru.singleton, expected.singleton) - - -@pytest.mark.parametrize( - "release", - map( - lazy_fixture, - [ - "album", - "album_with_track_alt", - "compilation", - "ep", - "artist_mess", - "description_meta", - "single_with_remixes", - "remix_artists", - "edge_cases", - ], - ), - indirect=["release"], -) -def test_parse_various_types(release): - html, expected_release = release - guru = Metaguru(html, expected_release.media) - - actual_album = guru.album - expected_album = expected_release.albuminfo - - assert hasattr(actual_album, "tracks") - assert len(actual_album.tracks) == len(expected_album.tracks) - - expected_tracks = sorted(expected_album.tracks, key=lambda t: t.index) - actual_tracks = sorted(actual_album.tracks, key=lambda t: t.index) - - actual_album.tracks = None - expected_album.tracks = None - check(actual_album, expected_album) - - for actual_track, expected_track in zip(actual_tracks, expected_tracks): - check(actual_track, expected_track) diff --git a/url2json b/url2json index b02c632..a658cf3 100755 --- a/url2json +++ b/url2json @@ -33,13 +33,16 @@ get_human_json () { tracklist_for_tests () { jq < "$1" -r ' - "(" + (.track.itemListElement[].item | [ + "(" + + (.track.itemListElement[].item + | [ "\"" + (.["@id"] | sub(".*/"; "")) + "\"", - "\"" + ((.name | sub(" - .*"; "")) // "albumartist") + "\"", + ((select(.name | test(" - .*")) | "\"" + sub(" - .*"; "") + "\"" | sub("\"\""; "")) // "albumartist"), "\"" + (.name | sub(".* - "; "")) + "\"", - (.additionalProperty[1].value | floor), + (.additionalProperty[] | select(.name == "duration_secs").value | floor) // "None", "None" - ] | join(", ")) + "),"' + ] + | join(", ")) + "),"' } update_test_jsons () { From 72a908a721ffe295fd3586f4d19ce18d03736825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 12 Sep 2021 15:53:39 +0100 Subject: [PATCH 2/7] 18-19 album,title: allow duplicate parens in the middle, like SUNN O))) --- CHANGELOG.md | 9 +++++++++ beetsplug/bandcamp/_metaguru.py | 19 ++++++++++++++----- tests/test_parsing.py | 4 ++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03cf94b..1090cf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ### Fixed - Fixed #18 by handling cases when a track duration is not given. +- Fixed #19 where artist name **SUNN O)))** would get incorrectly damaged by too + aggressive album name cleanup logic. + + Closing parentheses are now deduped from the album name _only if_ + - they follow a space + - or enclose a remix / edit info and are the last characters. + +Thanks @arogl for reporting each of the above! ## [0.10.0] 2021-09-10 @@ -54,6 +62,7 @@ - Improved the way **Various Artists** are cleaned up when catalognum is available - `albumartist`: + - If **various** is specified as the albumartist, make it **Various Artists** - When the label have set their name as the albumartist in every field, and if the actual albumartist could be inferred from the album name, use the inferred name. diff --git a/beetsplug/bandcamp/_metaguru.py b/beetsplug/bandcamp/_metaguru.py index bcf52e4..6be1107 100644 --- a/beetsplug/bandcamp/_metaguru.py +++ b/beetsplug/bandcamp/_metaguru.py @@ -103,8 +103,10 @@ def parse_track_name(name: str, catalognum: str = "") -> Dict[str, Optional[str] track["track_alt"] = track_alt name = name.replace(track_alt, "") + # do not strip a period from the end since it could end with an abbrev + name = name.lstrip(".") # in most cases that's the delimiter between the artist and the title - parts = re.split(r"\s-\s|\s?-\s|\s-\s?", name.strip(",.- ")) + parts = re.split(r"\s-\s|\s?-\s|\s-\s?", name.strip(",- ")) # title is always given track["title"] = parts.pop(-1) @@ -134,17 +136,22 @@ def parse_catalognum(album: str, disctitle: str, description: str, label: str) - 3. Check whether label name is followed by numbers 4. Check album name and disctitle using more flexible rules. """ + label_excl: Tuple[Optional[Pattern], Optional[str]] = (None, None) + if label: + escaped = re.escape(label) + label_excl = (re.compile(rf"({escaped}\s?[0-9]+)"), album) + for pattern, source in [ (PATTERNS["desc_catalognum"], description), (PATTERNS["quick_catalognum"], album), - (rf"({label}\s?[0-9]+)", album) if label else (None, None), + label_excl, (PATTERNS["catalognum"], album), (PATTERNS["catalognum"], disctitle), ]: if not pattern: continue - match = re.search(pattern, re.sub(PATTERNS["catalognum_excl"], "", source)) + match = pattern.search(PATTERNS["catalognum_excl"].sub("", source)) if match: return match.groups()[0] return "" @@ -176,7 +183,9 @@ def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: for pat, repl in [ (r"\s\s+", " "), (r"\(\s+|(- )?\(+", "("), - (r"\s+\)|\)+", ")"), + # Remove duplicate closing parens if they follow a space + # or enclose mix/edit info and are at the end + (r" \)+|(?<=(?i:.mix|edit))\)+$", ")"), (r'"', ""), ]: name = re.sub(pat, repl, name) @@ -209,7 +218,7 @@ def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: def clean(patstr: str, text: str) -> str: return re.sub(patstr, "", text) - return clean(empty_parens, clean(rubbish, name)).strip("/-|([. ") or default + return clean(empty_parens, clean(rubbish, name)).strip("/-|([ ") or default @staticmethod def _get_media_index(meta: JSONDict) -> JSONDict: diff --git a/tests/test_parsing.py b/tests/test_parsing.py index bcc3b4d..c657332 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -163,6 +163,8 @@ def test_convert_title(title, expected): ("Mr. Free - The 4th Room",), (None, "Mr. Free", "The 4th Room", "The 4th Room"), ), + (("O)))Bow 1",), (None, None, "O)))Bow 1", "O)))Bow 1")), + (("H.E.L.L.O.",), (None, None, "H.E.L.L.O.", "H.E.L.L.O.")), ], ) def test_parse_track_name(inputs, expected): @@ -284,6 +286,7 @@ def test_parse_country(name, expected): ("", 'EP 12"', "", "", ""), ("Hope Works 003", "", "", "Hope Works", "Hope Works 003"), ("Counterspell [HMX005]", "", "", "", "HMX005"), + ("3: Flight Of The Behemoth", "", "", "SUNN O)))", ""), ], ) def test_parse_catalognum(album, disctitle, description, label, expected): @@ -332,6 +335,7 @@ def test_parse_catalognum(album, disctitle, description, label, expected): ("RR009 - Various Artist", ["RR009"], "RR009"), ("Diva (Incl. some sort of Remixes)", [], "Diva"), ("HWEP010 - MEZZ - COLOR OF WAR", ["HWEP010", "MEZZ"], "COLOR OF WAR"), + ("O)))Bow 1", [], "O)))Bow 1"), ], ) def test_clean_name(name, extras, expected): From 7580ab07494c795cd9dda07cdd64cdcbd818ae87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 12 Sep 2021 19:00:56 +0100 Subject: [PATCH 3/7] 18-19 album: leave label in album name if followed by an apo --- CHANGELOG.md | 17 ++++++++++++----- beetsplug/bandcamp/_metaguru.py | 2 +- tests/test_parsing.py | 2 ++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1090cf6..875689b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,15 +3,22 @@ ### Fixed - Fixed #18 by handling cases when a track duration is not given. -- Fixed #19 where artist name **SUNN O)))** would get incorrectly damaged by too - aggressive album name cleanup logic. +- Fixed #19 where artist names like **SUNN O)))** would get incorrectly mistreated by + the album name cleanup logic due to multiple consecutive parentheses. The fix involved + adding some rules around it: they are now deduped _only if_ - Closing parentheses are now deduped from the album name _only if_ - - they follow a space - - or enclose a remix / edit info and are the last characters. + - they are preceded with a space + - or they enclose remix / edit info and are the last characters in the album name + +- Fixed #20 where dynamically obtained label names used in further REs caused `re.error` + since they were not appropriately escaped. Thanks @arogl for reporting each of the above! +- `album`: Keep label in the album name if it's immediately followed by an apostrophe. An + example scenario: + - `label: Mike` + - `album: Mike's Creations` ## [0.10.0] 2021-09-10 diff --git a/beetsplug/bandcamp/_metaguru.py b/beetsplug/bandcamp/_metaguru.py index 6be1107..5c6976b 100644 --- a/beetsplug/bandcamp/_metaguru.py +++ b/beetsplug/bandcamp/_metaguru.py @@ -177,7 +177,7 @@ def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: """ # catalognum, album, albumartist for arg in args: - name = name.replace(arg, "") + name = re.sub(rf"{arg}(?=[^'])", "", name) # redundant spaces, duoble quotes, parentheses for pat, repl in [ diff --git a/tests/test_parsing.py b/tests/test_parsing.py index c657332..3947570 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -336,6 +336,8 @@ def test_parse_catalognum(album, disctitle, description, label, expected): ("Diva (Incl. some sort of Remixes)", [], "Diva"), ("HWEP010 - MEZZ - COLOR OF WAR", ["HWEP010", "MEZZ"], "COLOR OF WAR"), ("O)))Bow 1", [], "O)))Bow 1"), + ("hi'Hello", ["hi"], "hi'Hello"), + ("hi]Hello", ["hi"], "]Hello"), ], ) def test_clean_name(name, extras, expected): From f9bf2fa13768ce6e68e4cecd0c9731f6178e9e79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 13 Sep 2021 01:46:36 +0100 Subject: [PATCH 4/7] 18-19 album cleanup: escape parsed label --- CHANGELOG.md | 2 +- beetsplug/bandcamp/_metaguru.py | 1 + tests/test_parsing.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 875689b..c1a31ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ - or they enclose remix / edit info and are the last characters in the album name - Fixed #20 where dynamically obtained label names used in further REs caused `re.error` - since they were not appropriately escaped. + since they were not appropriately escaped (for example, `label: /m\ records`). Thanks @arogl for reporting each of the above! diff --git a/beetsplug/bandcamp/_metaguru.py b/beetsplug/bandcamp/_metaguru.py index 5c6976b..30993a9 100644 --- a/beetsplug/bandcamp/_metaguru.py +++ b/beetsplug/bandcamp/_metaguru.py @@ -177,6 +177,7 @@ def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: """ # catalognum, album, albumartist for arg in args: + arg = re.escape(arg) name = re.sub(rf"{arg}(?=[^'])", "", name) # redundant spaces, duoble quotes, parentheses diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 3947570..5477415 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -338,6 +338,7 @@ def test_parse_catalognum(album, disctitle, description, label, expected): ("O)))Bow 1", [], "O)))Bow 1"), ("hi'Hello", ["hi"], "hi'Hello"), ("hi]Hello", ["hi"], "]Hello"), + ("[CAT001]", "", "", "\m/ records", "CAT001"), ], ) def test_clean_name(name, extras, expected): From 3243f0e6a8b5cf6a8412ef560b04b257a79f976a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 13 Sep 2021 02:28:29 +0100 Subject: [PATCH 5/7] 18-19 fix label escape test --- .github/workflows/build.yml | 3 +-- beetsplug/bandcamp/_metaguru.py | 2 +- tests/test_parsing.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b77f95d..f3329e1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -26,13 +26,12 @@ jobs: - name: Pytest run: | poetry run pytest - poetry run coverage xml - name: Flake8 run: poetry run flake8 . --output-file flake.log --exit-zero - name: Pylint run: poetry run pylint --output pylint.log --exit-zero beetsplug/**/*.py - name: SonarCloud Scan - uses: sonarsource/sonarcloud-github-action@master + uses: SonarSource/sonarcloud-github-action@master env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/beetsplug/bandcamp/_metaguru.py b/beetsplug/bandcamp/_metaguru.py index 30993a9..ecf3886 100644 --- a/beetsplug/bandcamp/_metaguru.py +++ b/beetsplug/bandcamp/_metaguru.py @@ -178,7 +178,7 @@ def clean_name(name: str, *args: str, remove_extra: bool = False) -> str: # catalognum, album, albumartist for arg in args: arg = re.escape(arg) - name = re.sub(rf"{arg}(?=[^'])", "", name) + name = re.sub(rf"{arg}((?=[^'])|$)", "", name) # redundant spaces, duoble quotes, parentheses for pat, repl in [ diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 5477415..d38eefb 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -287,6 +287,7 @@ def test_parse_country(name, expected): ("Hope Works 003", "", "", "Hope Works", "Hope Works 003"), ("Counterspell [HMX005]", "", "", "", "HMX005"), ("3: Flight Of The Behemoth", "", "", "SUNN O)))", ""), + ("[CAT001]", "", "", "\\m/ records", "CAT001"), ], ) def test_parse_catalognum(album, disctitle, description, label, expected): @@ -338,7 +339,6 @@ def test_parse_catalognum(album, disctitle, description, label, expected): ("O)))Bow 1", [], "O)))Bow 1"), ("hi'Hello", ["hi"], "hi'Hello"), ("hi]Hello", ["hi"], "]Hello"), - ("[CAT001]", "", "", "\m/ records", "CAT001"), ], ) def test_clean_name(name, extras, expected): From 3c862079aa7aeffb85bd1773967646fcf01c0853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 13 Sep 2021 03:05:17 +0100 Subject: [PATCH 6/7] 18-19 fix small lints --- beetsplug/bandcamp/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/bandcamp/__init__.py b/beetsplug/bandcamp/__init__.py index c321020..4a8ace2 100644 --- a/beetsplug/bandcamp/__init__.py +++ b/beetsplug/bandcamp/__init__.py @@ -29,7 +29,7 @@ from beets import __version__, config, library, plugins from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.importer import ImportTask -from beetsplug import fetchart +from beetsplug import fetchart # type: ignore[attr-defined] from ._metaguru import DATA_SOURCE, DEFAULT_MEDIA, Metaguru, urlify @@ -266,7 +266,7 @@ def handle(self, guru: Metaguru, attr: str, _id: str) -> Any: except (KeyError, ValueError, AttributeError): self._info("Failed obtaining {}", _id) return None - except Exception: + except Exception: # pylint: disable=broad-except url = "https://github.com/snejus/beetcamp/issues/new" self._exc("Unexpected error obtaining {}, please report at {}", _id, url) return None From a80619c49c808517e38d0a51cd6efbe3502ccea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 13 Sep 2021 03:16:21 +0100 Subject: [PATCH 7/7] 18-19 Add release url to the clog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1a31ed..746df34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## [0.10.1] 2021-09-12 +## [0.10.1] 2021-09-13 ### Fixed @@ -20,6 +20,8 @@ Thanks @arogl for reporting each of the above! - `label: Mike` - `album: Mike's Creations` +[0.10.1]: https://github.com/snejus/beetcamp/releases/tag/0.10.1 + ## [0.10.0] 2021-09-10 ### Fixed