« get me outta code hell

data, test: track: inherit album props more declaratively - hsmusic-wiki - HSMusic - static wiki software cataloguing collaborative creation
about summary refs log tree commit diff
diff options
context:
space:
mode:
author(quasar) nebula <qznebula@protonmail.com>2023-08-21 10:58:55 -0300
committer(quasar) nebula <qznebula@protonmail.com>2023-09-05 21:02:48 -0300
commit218a99a3164e8ae6967335190b72fd36275d1892 (patch)
tree952373949aa36434398d60a8d3ca8d627b255c7b
parentd194fc4f537ee79b0558b54ff2e1fdc3e9cbf4d9 (diff)
data, test: track: inherit album props more declaratively
-rw-r--r--src/data/things/track.js225
-rw-r--r--src/data/yaml.js6
-rw-r--r--test/unit/data/things/track.js55
3 files changed, 131 insertions, 155 deletions
diff --git a/src/data/things/track.js b/src/data/things/track.js
index e176acb4..39c2930f 100644
--- a/src/data/things/track.js
+++ b/src/data/things/track.js
@@ -44,59 +44,58 @@ export class Track extends Thing {
     sampledTracksByRef: Thing.common.referenceList(Track),
     artTagsByRef: Thing.common.referenceList(ArtTag),
 
-    hasCoverArt: {
+    // Disables presenting the track as though it has its own unique artwork.
+    // This flag should only be used in select circumstances, i.e. to override
+    // an album's trackCoverArtists. This flag supercedes that property, as well
+    // as the track's own coverArtists.
+    disableUniqueCoverArt: Thing.common.flag(),
+
+    // File extension for track's corresponding media file. This represents the
+    // track's unique cover artwork, if any, and does not inherit the cover's
+    // main artwork. (It does inherit `trackCoverArtFileExtension` if present
+    // on the album.)
+    coverArtFileExtension: {
       flags: {update: true, expose: true},
 
-      update: {
-        validate(value) {
-          if (value !== false) {
-            throw new TypeError(`Expected false or null`);
-          }
+      update: {validate: isFileExtension},
 
-          return true;
-        },
-      },
+      expose: Track.withAlbumProperties(['trackCoverArtistContribsByRef', 'trackCoverArtFileExtension'], {
+        dependencies: ['coverArtistContribsByRef', 'disableUniqueCoverArt'],
 
-      expose: {
-        dependencies: ['albumData', 'coverArtistContribsByRef'],
-        transform: (hasCoverArt, {
-          albumData,
+        transform(coverArtFileExtension, {
           coverArtistContribsByRef,
-          [Track.instance]: track,
-        }) =>
-          Track.hasCoverArt(
-            track,
-            albumData,
-            coverArtistContribsByRef,
-            hasCoverArt
-          ),
-      },
+          disableUniqueCoverArt,
+          album: {trackCoverArtistContribsByRef, trackCoverArtFileExtension},
+        }) {
+          if (disableUniqueCoverArt) return null;
+          if (empty(coverArtistContribsByRef) && empty(trackCoverArtistContribsByRef)) return null;
+          return coverArtFileExtension ?? trackCoverArtFileExtension ?? 'jpg';
+        },
+      }),
     },
 
-    coverArtFileExtension: {
+    // Date of cover art release. Like coverArtFileExtension, this represents
+    // only the track's own unique cover artwork, if any. This exposes only as
+    // the track's own coverArtDate or its album's trackArtDate, so if neither
+    // is specified, this value is null.
+    coverArtDate: {
       flags: {update: true, expose: true},
 
-      update: {validate: isFileExtension},
+      update: {validate: isDate},
 
-      expose: {
-        dependencies: ['albumData', 'coverArtistContribsByRef'],
-        transform: (coverArtFileExtension, {
-          albumData,
+      expose: Track.withAlbumProperties(['trackArtDate', 'trackCoverArtistContribsByRef'], {
+        dependencies: ['coverArtistContribsByRef', 'disableUniqueCoverArt'],
+
+        transform(coverArtDate, {
           coverArtistContribsByRef,
-          hasCoverArt,
-          [Track.instance]: track,
-        }) =>
-          coverArtFileExtension ??
-          (Track.hasCoverArt(
-            track,
-            albumData,
-            coverArtistContribsByRef,
-            hasCoverArt
-          )
-            ? Track.findAlbum(track, albumData)?.trackCoverArtFileExtension
-            : Track.findAlbum(track, albumData)?.coverArtFileExtension) ??
-          'jpg',
-      },
+          disableUniqueCoverArt,
+          album: {trackArtDate, trackCoverArtistContribsByRef},
+        }) {
+          if (disableUniqueCoverArt) return null;
+          if (empty(coverArtistContribsByRef) && empty(trackCoverArtistContribsByRef)) return null;
+          return coverArtDate ?? trackArtDate;
+        },
+      }),
     },
 
     originalReleaseTrackByRef: Thing.common.singleReference(Track),
@@ -170,53 +169,29 @@ export class Track extends Thing {
       },
     },
 
-    coverArtDate: {
-      flags: {update: true, expose: true},
-
-      update: {validate: isDate},
-
-      expose: {
-        dependencies: [
-          'albumData',
-          'coverArtistContribsByRef',
-          'dateFirstReleased',
-          'hasCoverArt',
-        ],
-        transform: (coverArtDate, {
-          albumData,
-          coverArtistContribsByRef,
-          dateFirstReleased,
-          hasCoverArt,
-          [Track.instance]: track,
-        }) =>
-          (Track.hasCoverArt(track, albumData, coverArtistContribsByRef, hasCoverArt)
-            ? coverArtDate ??
-              dateFirstReleased ??
-              Track.findAlbum(track, albumData)?.trackArtDate ??
-              Track.findAlbum(track, albumData)?.date ??
-              null
-            : null),
-      },
-    },
-
+    // Whether or not the track has "unique" cover artwork - a cover which is
+    // specifically associated with this track in particular, rather than with
+    // the track's album as a whole. This is typically used to select between
+    // displaying the track artwork and a fallback, such as the album artwork
+    // or a placeholder. (This property is named hasUniqueCoverArt instead of
+    // the usual hasCoverArt to emphasize that it does not inherit from the
+    // album.)
     hasUniqueCoverArt: {
       flags: {expose: true},
 
-      expose: {
-        dependencies: ['albumData', 'coverArtistContribsByRef', 'hasCoverArt'],
-        compute: ({
-          albumData,
+      expose: Track.withAlbumProperties(['trackCoverArtistContribsByRef'], {
+        dependencies: ['coverArtistContribsByRef', 'disableUniqueCoverArt'],
+        compute({
           coverArtistContribsByRef,
-          hasCoverArt,
-          [Track.instance]: track,
-        }) =>
-          Track.hasUniqueCoverArt(
-            track,
-            albumData,
-            coverArtistContribsByRef,
-            hasCoverArt
-          ),
-      },
+          disableUniqueCoverArt,
+          album: {trackCoverArtistContribsByRef},
+        }) {
+          if (disableUniqueCoverArt) return false;
+          if (!empty(coverArtistContribsByRef)) true;
+          if (!empty(trackCoverArtistContribsByRef)) return true;
+          return false;
+        },
+      }),
     },
 
     originalReleaseTrack: Thing.common.dynamicThingFromSingleReference(
@@ -342,53 +317,6 @@ export class Track extends Thing {
     ),
   });
 
-  // This is a quick utility function for now, since the same code is reused in
-  // several places. Ideally it wouldn't be - we'd just reuse the `album`
-  // property - but support for that hasn't been coded yet :P
-  static findAlbum = (track, albumData) =>
-    albumData?.find((album) => album.tracks.includes(track));
-
-  // Another reused utility function. This one's logic is a bit more complicated.
-  static hasCoverArt(
-    track,
-    albumData,
-    coverArtistContribsByRef,
-    hasCoverArt
-  ) {
-    if (!empty(coverArtistContribsByRef)) {
-      return true;
-    }
-
-    const album = Track.findAlbum(track, albumData);
-    if (album && !empty(album.trackCoverArtistContribsByRef)) {
-      return true;
-    }
-
-    return false;
-  }
-
-  static hasUniqueCoverArt(
-    track,
-    albumData,
-    coverArtistContribsByRef,
-    hasCoverArt
-  ) {
-    if (!empty(coverArtistContribsByRef)) {
-      return true;
-    }
-
-    if (hasCoverArt === false) {
-      return false;
-    }
-
-    const album = Track.findAlbum(track, albumData);
-    if (album && !empty(album.trackCoverArtistContribsByRef)) {
-      return true;
-    }
-
-    return false;
-  }
-
   static inheritFromOriginalRelease(
     originalProperty,
     originalMissingValue,
@@ -423,6 +351,39 @@ export class Track extends Thing {
     };
   }
 
+  static withAlbumProperties(albumProperties, oldExpose) {
+    const applyAlbumDependency = dependencies => {
+      const track = dependencies[Track.instance];
+      const album =
+        dependencies.albumData
+          ?.find((album) => album.tracks.includes(track));
+
+      const filteredAlbum = Object.create(null);
+      for (const property of albumProperties) {
+        filteredAlbum[property] =
+          (album
+            ? album[property]
+            : null);
+      }
+
+      return {...dependencies, album: filteredAlbum};
+    };
+
+    const newExpose = {dependencies: [...oldExpose.dependencies, 'albumData']};
+
+    if (oldExpose.compute) {
+      newExpose.compute = dependencies =>
+        oldExpose.compute(applyAlbumDependency(dependencies));
+    }
+
+    if (oldExpose.transform) {
+      newExpose.transform = (value, dependencies) =>
+        oldExpose.transform(value, applyAlbumDependency(dependencies));
+    }
+
+    return newExpose;
+  }
+
   [inspect.custom]() {
     const base = Thing.prototype[inspect.custom].apply(this);
 
diff --git a/src/data/yaml.js b/src/data/yaml.js
index 35943199..13412f17 100644
--- a/src/data/yaml.js
+++ b/src/data/yaml.js
@@ -316,6 +316,10 @@ export const processTrackDocument = makeProcessDocument(T.Track, {
 
     'Date First Released': (value) => new Date(value),
     'Cover Art Date': (value) => new Date(value),
+    'Has Cover Art': (value) =>
+      (value === true ? false :
+       value === false ? true :
+       value),
 
     'Artists': parseContributors,
     'Contributors': parseContributors,
@@ -336,7 +340,7 @@ export const processTrackDocument = makeProcessDocument(T.Track, {
     dateFirstReleased: 'Date First Released',
     coverArtDate: 'Cover Art Date',
     coverArtFileExtension: 'Cover Art File Extension',
-    hasCoverArt: 'Has Cover Art',
+    disableCoverArt: 'Has Cover Art', // This gets transformed to flip true/false.
 
     lyrics: 'Lyrics',
     commentary: 'Commentary',
diff --git a/test/unit/data/things/track.js b/test/unit/data/things/track.js
index 383e3e3f..d0e50c7f 100644
--- a/test/unit/data/things/track.js
+++ b/test/unit/data/things/track.js
@@ -3,6 +3,7 @@ import thingConstructors from '#things';
 
 const {
   Album,
+  Artist,
   Thing,
   Track,
   TrackGroup,
@@ -20,55 +21,65 @@ function stubAlbum(tracks) {
 }
 
 t.test(`Track.coverArtDate`, t => {
-  t.plan(5);
+  t.plan(6);
 
-  // Priority order is as follows, with the last (trackCoverArtDate) being
-  // greatest priority.
-  const albumDate = new Date('2010-10-10');
   const albumTrackArtDate = new Date('2012-12-12');
-  const trackDateFirstReleased = new Date('2008-08-08');
   const trackCoverArtDate = new Date('2009-09-09');
+  const dummyContribs = [{who: 'Test Artist', what: null}]
 
   const track = new Track();
   track.directory = 'foo';
+  track.coverArtistContribsByRef = dummyContribs;
 
   const album = stubAlbum([track]);
 
+  const artist = new Artist();
+  artist.name = 'Test Artist';
+
   track.albumData = [album];
+  track.artistData = [artist];
+
+  const XXX_CLEAR_TRACK_ALBUM_CACHE = () => {
+    // XXX clear cache so change in album's property is reflected
+    track.albumData = [];
+    track.albumData = [album];
+  };
 
   // 1. coverArtDate defaults to null
 
   t.equal(track.coverArtDate, null);
 
-  // 2. coverArtDate inherits album release date
+  // 2. coverArtDate inherits album trackArtDate
 
-  album.date = albumDate;
+  album.trackArtDate = albumTrackArtDate;
 
-  // XXX clear cache so change in album's property is reflected
-  track.albumData = [];
-  track.albumData = [album];
+  XXX_CLEAR_TRACK_ALBUM_CACHE();
 
-  t.equal(track.coverArtDate, albumDate);
+  t.equal(track.coverArtDate, albumTrackArtDate);
 
-  // 3. coverArtDate inherits album trackArtDate
+  // 3. coverArtDate is own value
 
-  album.trackArtDate = albumTrackArtDate;
+  track.coverArtDate = trackCoverArtDate;
 
-  // XXX clear cache again
-  track.albumData = [];
-  track.albumData = [album];
+  t.equal(track.coverArtDate, trackCoverArtDate);
 
-  t.equal(track.coverArtDate, albumTrackArtDate);
+  // 4. coverArtDate is null if track is missing coverArtists
 
-  // 4. coverArtDate is overridden dateFirstReleased
+  track.coverArtistContribsByRef = [];
 
-  track.dateFirstReleased = trackDateFirstReleased;
+  t.equal(track.coverArtDate, null);
 
-  t.equal(track.coverArtDate, trackDateFirstReleased);
+  // 5. coverArtDate is not null if album specifies trackCoverArtistContribs
 
-  // 5. coverArtDate is overridden coverArtDate
+  album.trackCoverArtistContribsByRef = dummyContribs;
 
-  track.coverArtDate = trackCoverArtDate;
+  XXX_CLEAR_TRACK_ALBUM_CACHE();
 
   t.equal(track.coverArtDate, trackCoverArtDate);
+
+  // 6. coverArtDate is null if track disables unique cover artwork
+
+  track.disableUniqueCoverArt = true;
+
+  t.equal(track.coverArtDate, null);
 });