feat(cover-art): Download 500x500 cover art for tracks by default.

Also reduced the timeout for cover art download to 20s.

src/resources/manifest.json

@@ -2,7 +2,7 @@
   "name": "ZoundCloud Downloader",
   "short_name": "ZoundCloud",
   "description": "Adds download buttons to SoundCloud pages.",
-  "version": "1.3.0",
+  "version": "1.3.1",
   "manifest_version": 2,
   "content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",
   "permissions": [

src/ts/download/metadata/id3-metadata-service.ts

@@ -4,7 +4,7 @@ import {ITrackDownloadInfo} from '@src/download/track-download-info';
 import {logger} from '@src/util/logger';
 import {XhrService} from '@src/util/xhr-service';
 import {Observable, of} from 'rxjs';
-import {catchError, map, switchMap, timeout} from 'rxjs/operators';
+import {catchError, flatMap, map, timeout} from 'rxjs/operators';
 
 /**
  * Adds ID3 v2.4 tags and returns downloadInfo with url in downloadOptions set to the updated URL.
@@ -15,15 +15,15 @@ export const ID3MetadataService = {
   addID3V2Metadata$(metadata: ITrackMetadata, downloadInfo: ITrackDownloadInfo): Observable<ITrackDownloadInfo> {
     return XhrService.getArrayBuffer$(downloadInfo.downloadOptions.url).pipe(
       timeout(300000),
-      switchMap(writeMetadata$.bind(null, metadata)),
+      flatMap(writeMetadata$.bind(null, metadata)),
       map(ID3WriterService.getURL),
       map((url: string) => ({
         ...downloadInfo,
         downloadOptions: {
           ...downloadInfo.downloadOptions,
           url
         }
-      })),
+      } as any)),
       catchError((err: Error) => {
         logger.error(`Unable to fetch metadata for track ${downloadInfo.trackInfo.title}`, err);
         return of(downloadInfo);
@@ -35,7 +35,7 @@ export const ID3MetadataService = {
 function writeMetadata$(metadata: ITrackMetadata, arrayBuffer: ArrayBuffer): Observable<IID3Writer> {
   return of(ID3WriterService.createWriter(arrayBuffer)).pipe(
     map(withTextualMetadata.bind(null, metadata)),
-    switchMap(withCoverArt$.bind(null, metadata)),
+    flatMap(withCoverArt$.bind(null, metadata)),
     map(ID3WriterService.addTag)
   );
 }
@@ -58,10 +58,11 @@ function withTextualMetadata(metadata: ITrackMetadata, writer: IID3Writer): IID3
 }
 
 function withCoverArt$(metadata: ITrackMetadata, writer: IID3Writer): Observable<IID3Writer> {
-  if (!metadata.cover_url) {
+  const url = metadata.cover_url;
+  if (!url) {
     return of(writer);
   }
-  return XhrService.getArrayBuffer$(metadata.cover_url).pipe(
+  return XhrService.getArrayBuffer$(url).pipe(
     map((arrayBuffer: ArrayBuffer) =>
       ID3WriterService.setFrame(writer, 'APIC', {
         data: arrayBuffer,
@@ -70,7 +71,7 @@ function withCoverArt$(metadata: ITrackMetadata, writer: IID3Writer): Observable
         useUnicodeEncoding: false
       })
     ),
-    timeout(60000),
+    timeout(20000),
     catchError((err: Error) => {
       logger.error(`Unable to fetch cover art for track ${metadata.title}`, err);
       return of(writer);

src/ts/download/metadata/metadata-adapter.ts

@@ -1,16 +1,38 @@
 import {ID3MetadataService} from '@src/download/metadata/id3-metadata-service';
+import {ITrackMetadata} from '@src/download/metadata/track-metadata';
 import {TrackMetadataFactory} from '@src/download/metadata/track-metadata-factory';
 import {ITrackDownloadInfo} from '@src/download/track-download-info';
+import {XhrService} from '@src/util/xhr-service';
 import {Observable, of} from 'rxjs';
+import {flatMap, map} from 'rxjs/operators';
 
 export const MetadataAdapter = {
   addMetadata$(downloadInfo: ITrackDownloadInfo): Observable<ITrackDownloadInfo> {
-    switch (downloadInfo.downloadOptions.filename.split('.').pop()) {
+    const fileExtension = downloadInfo.downloadOptions.filename.split('.').pop();
+    const metadata$ = of(TrackMetadataFactory.create(downloadInfo.trackInfo)).pipe(
+      flatMap(withUpdatedCoverArtUrl$)
+    );
+
+    switch (fileExtension) {
       case 'mp3':
-        const metadata = TrackMetadataFactory.create(downloadInfo.trackInfo);
-        return ID3MetadataService.addID3V2Metadata$(metadata, downloadInfo);
+        return metadata$.pipe(
+          flatMap((metadata: ITrackMetadata) => ID3MetadataService.addID3V2Metadata$(metadata, downloadInfo))
+        );
       default:
         return of(downloadInfo);
     }
   }
 };
+
+function withUpdatedCoverArtUrl$(metadata: ITrackMetadata): Observable<ITrackMetadata> {
+  const origUrl = metadata.cover_url;
+  const highResUrl = getCoverArtUrlForResolution(origUrl, 500);
+  return XhrService.ping$(highResUrl).pipe(
+    map((status: number) => (status === 200) ? highResUrl : origUrl),
+    map((url: string) => ({...metadata, cover_url: url}))
+  );
+}
+
+function getCoverArtUrlForResolution(url: string, resolution: number): string {
+  return url.replace(/^(.*)-large(\..*)$/, `$1-t${resolution}x${resolution}$2`);
+}

test/ts/download/metadata/id3-metadata-service.spec.ts

@@ -14,17 +14,16 @@ describe('id3 metadata service', () => {
   const rx = useRxTesting();
 
   const fixture = ID3MetadataService;
-  const metadata = createMetadata();
   const downloadInfo = {
     downloadOptions: {url: 'download-options-url'},
     trackInfo: {title: 'track-title'}
   } as ITrackDownloadInfo;
   const writer = {foo: 'bar'} as IID3Writer;
   const metadataAddedURL = 'url-with-metadata-added';
 
+  let metadata: ITrackMetadata;
   let stubGetArrayBuffer$: SinonStub;
   const songData: ArrayBuffer = new Int8Array([1, 2, 3]).buffer;
-  const coverArtData: ArrayBuffer = new Int8Array([4, 5, 6]).buffer;
 
   let stubAddTag: SinonStub;
   let stubSetFrame: SinonStub;
@@ -33,10 +32,10 @@ describe('id3 metadata service', () => {
 
   beforeEach(() => {
     useFakeTimers();
+    metadata = createMetadata();
 
     stubGetArrayBuffer$ = stub(XhrService, 'getArrayBuffer$');
     stubGetArrayBuffer$.withArgs(downloadInfo.downloadOptions.url).returns(of(songData));
-    stubGetArrayBuffer$.withArgs(metadata.cover_url).returns(of(coverArtData));
 
     stubAddTag = stub(ID3WriterService, 'addTag');
     stubAddTag.withArgs(writer).returns(writer);
@@ -92,19 +91,6 @@ describe('id3 metadata service', () => {
       expect(stubAddTag).to.have.been.calledWith(writer).calledAfter(stubSetFrame);
     });
 
-    it('should add cover art metadata', () => {
-      rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
-
-      expect(stubSetFrame).to.have.been.calledWithExactly(writer, 'APIC', {
-        data: coverArtData,
-        description: 'Soundcloud artwork',
-        type: 3,
-        useUnicodeEncoding: false
-      });
-      expect(stubAddTag).to.have.been.calledWith(writer).calledAfter(stubSetFrame);
-      verifyDownloadOptionsEmittedWithUrl(metadataAddedURL);
-    });
-
     it('should not add cover art metadata if cover art url is not defined', () => {
       const metadataWithNoCoverArtUrl = createMetadata({cover_url: undefined});
       rx.subscribeTo(fixture.addID3V2Metadata$(metadataWithNoCoverArtUrl, downloadInfo));
@@ -138,30 +124,60 @@ describe('id3 metadata service', () => {
       verifyDownloadOptionsEmittedWithUrl(downloadInfo.downloadOptions.url);
     });
 
-    it('should add cover art metadata if fetching cover art data takes less than 60 seconds', () => {
-      stubGetArrayBuffer$.withArgs(metadata.cover_url)
-        .returns(timer(59999).pipe(mapTo(coverArtData)));
-      rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
-      clock.tick(60000);
+    describe('adding cover art metadata', () => {
+      const coverArtData: ArrayBuffer = new Int8Array([4, 5, 6]).buffer;
+      const expectedFrame = {
+        data: coverArtData,
+        description: 'Soundcloud artwork',
+        type: 3,
+        useUnicodeEncoding: false
+      };
 
-      expect(stubSetFrame).to.have.been.calledWith(writer, 'APIC', match.any);
-    });
+      beforeEach(() => {
+        metadata = createMetadata({cover_url: 'cover-url'});
+        // Set up so that cover art metadata can be downloaded from the cover art url from the api
+        stubGetArrayBuffer$.withArgs(metadata.cover_url).returns(of(coverArtData));
+      });
 
-    it('should not add cover art metadata if fetching cover art data takes 60 seconds or more', () => {
-      stubGetArrayBuffer$.withArgs(metadata.cover_url)
-        .returns(timer(60000).pipe(mapTo(coverArtData)));
-      rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
-      clock.tick(60001);
+      it('should not add cover art url if there is none', () => {
+        const metadataWithNoCoverArtUrl = {...metadata, cover_url: ''};
+        rx.subscribeTo(fixture.addID3V2Metadata$(metadataWithNoCoverArtUrl, downloadInfo));
+        expect(stubSetFrame).not.to.have.been.calledWith(writer, 'APIC', match.any);
+      });
 
-      expect(stubSetFrame).to.not.have.been.calledWith(writer, 'APIC', match.any);
-    });
+      it('should add cover art metadata', () => {
+        rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
 
-    it('should not add cover art metadata if there is an error while fetching cover art', () => {
-      stubGetArrayBuffer$.withArgs(metadata.cover_url)
-        .returns(throwError('Some error'));
-      rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
+        expect(stubSetFrame).to.have.been.calledWithExactly(writer, 'APIC', expectedFrame);
+        expect(stubAddTag).to.have.been.calledWith(writer).calledAfter(stubSetFrame);
+        verifyDownloadOptionsEmittedWithUrl(metadataAddedURL);
+      });
 
-      expect(stubSetFrame).to.not.have.been.calledWith(writer, 'APIC', match.any);
+      it('should add cover art metadata if fetching cover art data takes less than 20 seconds', () => {
+        stubGetArrayBuffer$.withArgs(metadata.cover_url)
+          .returns(timer(19999).pipe(mapTo(coverArtData)));
+        rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
+        clock.tick(20000);
+
+        expect(stubSetFrame).to.have.been.calledWith(writer, 'APIC', expectedFrame);
+      });
+
+      it('should not add cover art metadata if fetching cover art data takes 20 seconds or more', () => {
+        stubGetArrayBuffer$.withArgs(metadata.cover_url)
+          .returns(timer(20000).pipe(mapTo(coverArtData)));
+        rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
+        clock.tick(20001);
+
+        expect(stubSetFrame).to.not.have.been.calledWith(writer, 'APIC', match.any);
+      });
+
+      it('should not add cover art metadata if there is an error while fetching cover art', () => {
+        stubGetArrayBuffer$.withArgs(metadata.cover_url)
+          .returns(throwError('Some error'));
+        rx.subscribeTo(fixture.addID3V2Metadata$(metadata, downloadInfo));
+
+        expect(stubSetFrame).to.not.have.been.calledWith(writer, 'APIC', match.any);
+      });
     });
   });
 
@@ -184,7 +200,7 @@ describe('id3 metadata service', () => {
       artist_url: 'artist-url',
       audio_source_url: 'audio-source-url',
       bpm: 200,
-      cover_url: 'cover-url',
+      cover_url: '', // No cover art url
       description: 'soundcloud-track-description',
       duration: 12345,
       genres: ['rock', 'classical'],

test/ts/download/metadata/metadata-adapter.spec.ts

@@ -3,8 +3,9 @@ import {MetadataAdapter} from '@src/download/metadata/metadata-adapter';
 import {ITrackMetadata} from '@src/download/metadata/track-metadata';
 import {TrackMetadataFactory} from '@src/download/metadata/track-metadata-factory';
 import {ITrackDownloadInfo} from '@src/download/track-download-info';
+import {XhrService} from '@src/util/xhr-service';
 import {configureChai, useRxTesting} from '@test/test-initializers';
-import {of} from 'rxjs';
+import {EMPTY, of} from 'rxjs';
 import {restore, SinonStub, stub} from 'sinon';
 
 const expect = configureChai();
@@ -14,39 +15,71 @@ describe('metadata adapter', () => {
 
   const fixture = MetadataAdapter;
   let inputDlInfo: ITrackDownloadInfo;
+  let metadata: ITrackMetadata;
+
+  let stubPing$: SinonStub;
   let stubCreateMetadata: SinonStub;
   let stubAddIDV2Metadata: SinonStub;
 
   beforeEach(() => {
     inputDlInfo = {downloadOptions: {}, trackInfo: {}} as ITrackDownloadInfo;
+    metadata = {title: 'foo', cover_url: 'cover-url-large.jpg'} as ITrackMetadata;
+
+    stubPing$ = stub(XhrService, 'ping$');
+    stubPing$.returns(of(404)); // Use original cover art url by default
+
     stubCreateMetadata = stub(TrackMetadataFactory, 'create');
+    stubCreateMetadata.withArgs(inputDlInfo.trackInfo).returns(metadata);
+
     stubAddIDV2Metadata = stub(ID3MetadataService, 'addID3V2Metadata$');
   });
 
   afterEach(() => {
     restore();
   });
 
-  describe('adding metadata', () => {
-    it('should add id3 metadata when downloading a mp3 file', () => {
+  describe('updating the cover art url in the metadata', () => {
+    const expectedHighResUrl = 'cover-url-t500x500.jpg';
+
+    beforeEach(() => {
+      // Set file extension to .mp3 to enable adding metadata for these set of tests
       inputDlInfo.downloadOptions.filename = 'file.name.mp3';
-      const metadata = {title: 'foo'} as ITrackMetadata;
-      const expectedDlInfo = {trackInfo: {title: 'bar'}} as ITrackDownloadInfo;
+    });
 
-      stubCreateMetadata.withArgs(inputDlInfo.trackInfo).returns(metadata);
-      stubAddIDV2Metadata.withArgs(metadata, inputDlInfo).returns(of(expectedDlInfo));
+    it('should use high res url if it is available', () => {
+      stubPing$.withArgs(expectedHighResUrl).returns(of(200));
+      const expectedMetadata = {...metadata, cover_url: expectedHighResUrl};
+      stubAddIDV2Metadata.returns(EMPTY);
 
       rx.subscribeTo(fixture.addMetadata$(inputDlInfo));
-      expect(rx.next).to.be.calledOnceWithExactly(expectedDlInfo);
-      expect(rx.complete).to.be.called;
+      expect(stubAddIDV2Metadata).to.have.been.calledOnceWithExactly(expectedMetadata, inputDlInfo);
     });
 
-    it('should not add metadata when not downloading a mp3 file', () => {
-      inputDlInfo.downloadOptions.filename = 'file.name.wav';
+    it('should fall back to original cover art url if high res version is not available', () => {
+      stubPing$.withArgs(expectedHighResUrl).returns(of(404));
+      const expectedMetadata = {...metadata};
+      stubAddIDV2Metadata.returns(EMPTY);
 
       rx.subscribeTo(fixture.addMetadata$(inputDlInfo));
-      expect(rx.next).to.be.calledOnceWithExactly(inputDlInfo);
-      expect(rx.complete).to.be.called;
+      expect(stubAddIDV2Metadata).to.have.been.calledOnceWithExactly(expectedMetadata, inputDlInfo);
     });
   });
+
+  it('should add id3 metadata when downloading a mp3 file', () => {
+    inputDlInfo.downloadOptions.filename = 'file.name.mp3';
+    const expectedDlInfo = {trackInfo: {title: 'bar'}} as ITrackDownloadInfo;
+    stubAddIDV2Metadata.withArgs(metadata, inputDlInfo).returns(of(expectedDlInfo));
+
+    rx.subscribeTo(fixture.addMetadata$(inputDlInfo));
+    expect(rx.next).to.be.calledOnceWithExactly(expectedDlInfo);
+    expect(rx.complete).to.be.called;
+  });
+
+  it('should not add metadata when not downloading a mp3 file', () => {
+    inputDlInfo.downloadOptions.filename = 'file.name.wav';
+
+    rx.subscribeTo(fixture.addMetadata$(inputDlInfo));
+    expect(rx.next).to.be.calledOnceWithExactly(inputDlInfo);
+    expect(rx.complete).to.be.called;
+  });
 });