From e1b607d9fa5ea553f4678f57cf405011e1f33859 Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Thu, 31 Dec 2020 13:06:51 +0530
Subject: [PATCH 1/6] Refactor LastFMImporter Modal Code
---
.../static/js/src/LastFMImporter.tsx | 336 +++++++++++++++++-
1 file changed, 318 insertions(+), 18 deletions(-)
diff --git a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
index e78668d8eb..3b3e6c0ec8 100644
--- a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
+++ b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
@@ -1,6 +1,10 @@
import * as ReactDOM from "react-dom";
import * as React from "react";
-import Importer from "./Importer";
+import { faSpinner, faCheck } from "@fortawesome/free-solid-svg-icons";
+import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
+import { IconProp } from "@fortawesome/fontawesome-svg-core";
+import APIService from "./APIService";
+import Scrobble from "./Scrobble";
import LastFMImporterModal from "./LastFMImporterModal";
@@ -20,14 +24,59 @@ export type ImporterState = {
show: boolean;
canClose: boolean;
lastfmUsername: string;
- msg: string;
+ msg?: React.ReactElement;
};
export default class LastFmImporter extends React.Component<
ImporterProps,
ImporterState
> {
- importer: any;
+ static encodeScrobbles(scrobbles: LastFmScrobblePage): any {
+ const rawScrobbles = scrobbles.recenttracks.track;
+ const parsedScrobbles = LastFmImporter.map((rawScrobble: any) => {
+ const scrobble = new Scrobble(rawScrobble);
+ return scrobble.asJSONSerializable();
+ }, rawScrobbles);
+ return parsedScrobbles;
+ }
+
+ static map(applicable: (collection: any) => Listen, collection: any) {
+ const newCollection = [];
+ for (let i = 0; i < collection.length; i += 1) {
+ const result = applicable(collection[i]);
+ if (result.listened_at > 0) {
+ // If the 'listened_at' attribute is -1 then either the listen is invalid or the
+ // listen is currently playing. In both cases we need to skip the submission.
+ newCollection.push(result);
+ }
+ }
+ return newCollection;
+ }
+
+ APIService: APIService;
+ private lastfmURL: string;
+ private lastfmKey: string;
+
+ private userName: string;
+ private userToken: string;
+
+ private page = 1;
+ private totalPages = 0;
+
+ private playCount = -1; // the number of scrobbles reported by Last.FM
+ private countReceived = 0; // number of scrobbles the Last.FM API sends us, this can be diff from playCount
+
+ private latestImportTime = 0; // the latest timestamp that we've imported earlier
+ private maxTimestampForImport = 0; // the latest listen found in this import
+ private incrementalImport = false;
+
+ private numCompleted = 0; // number of pages completed till now
+
+ // Variables used to honor LB's rate limit
+ private rlRemain = -1;
+ private rlReset = -1;
+
+ private rlOrigin = -1;
constructor(props: ImporterProps) {
super(props);
@@ -36,22 +85,138 @@ export default class LastFmImporter extends React.Component<
show: false,
canClose: true,
lastfmUsername: "",
- msg: "",
+ msg: undefined,
};
+
+ this.APIService = new APIService(
+ props.apiUrl || `${window.location.origin}/1`
+ ); // Used to access LB API
+
+ this.lastfmURL = props.lastfmApiUrl;
+ this.lastfmKey = props.lastfmApiKey;
+
+ this.userName = props.user.name;
+ this.userToken = props.user.auth_token || "";
}
- handleChange = (event: React.ChangeEvent) => {
- this.setState({ lastfmUsername: event.target.value });
- };
+ async getTotalNumberOfScrobbles() {
+ /*
+ * Get the total play count reported by Last.FM for user
+ */
+ const { lastfmUsername } = this.state;
+ const url = `${this.lastfmURL}?method=user.getinfo&user=${lastfmUsername}&api_key=${this.lastfmKey}&format=json`;
+ try {
+ const response = await fetch(encodeURI(url));
+ const data = await response.json();
+ if ("playcount" in data.user) {
+ return Number(data.user.playcount);
+ }
+ return -1;
+ } catch {
+ this.updateModalAction(
+ An error occurred, please try again. :(
,
+ true
+ );
+ const error = new Error();
+ error.message = "Something went wrong";
+ throw error;
+ }
+ }
- handleSubmit = (event: React.FormEvent) => {
+ async getNumberOfPages() {
+ /*
+ * Get the total pages of data from last import
+ */
const { lastfmUsername } = this.state;
- this.toggleModal();
- event.preventDefault();
- this.importer = new Importer(lastfmUsername, this.props);
- setInterval(this.updateMessage, 100);
- setInterval(this.setClose, 100);
- this.importer.startImport();
+
+ const url = `${
+ this.lastfmURL
+ }?method=user.getrecenttracks&user=${lastfmUsername}&api_key=${
+ this.lastfmKey
+ }&from=${this.latestImportTime + 1}&format=json`;
+ try {
+ const response = await fetch(encodeURI(url));
+ const data = await response.json();
+ if ("recenttracks" in data) {
+ return Number(data.recenttracks["@attr"].totalPages);
+ }
+ return 0;
+ } catch (error) {
+ this.updateModalAction(
+ An error occurred, please try again. :(
,
+ true
+ );
+ return -1;
+ }
+ }
+
+ async getPage(page: number) {
+ /*
+ * Fetch page from Last.fm
+ */
+ const { lastfmUsername } = this.state;
+
+ const retry = (reason: string) => {
+ // console.warn(`${reason} while fetching last.fm page=${page}, retrying in 3s`);
+ setTimeout(() => this.getPage(page), 3000);
+ };
+
+ const url = `${
+ this.lastfmURL
+ }?method=user.getrecenttracks&user=${lastfmUsername}&api_key=${
+ this.lastfmKey
+ }&from=${this.latestImportTime + 1}&page=${page}&format=json`;
+ try {
+ const response = await fetch(encodeURI(url));
+ if (response.ok) {
+ const data = await response.json();
+ // Set latest import time
+ if ("date" in data.recenttracks.track[0]) {
+ this.maxTimestampForImport = Math.max(
+ data.recenttracks.track[0].date.uts,
+ this.maxTimestampForImport
+ );
+ } else {
+ this.maxTimestampForImport = Math.floor(Date.now() / 1000);
+ }
+
+ // Encode the page so that it can be submitted
+ const payload = LastFmImporter.encodeScrobbles(data);
+ this.countReceived += payload.length;
+ return payload;
+ }
+ if (/^5/.test(response.status.toString())) {
+ retry(`Got ${response.status}`);
+ } else {
+ // ignore 40x
+ // console.warn(`Got ${response.status} while fetching page last.fm page=${page}, skipping`);
+ }
+ } catch {
+ // Retry if there is a network error
+ retry("Error");
+ }
+ return null;
+ }
+
+ getRateLimitDelay() {
+ /* Get the amount of time we should wait according to LB rate limits before making a request to LB */
+ let delay = 0;
+ const current = new Date().getTime() / 1000;
+ if (this.rlReset < 0 || current > this.rlOrigin + this.rlReset) {
+ delay = 0;
+ } else if (this.rlRemain > 0) {
+ delay = Math.max(0, Math.ceil((this.rlReset * 1000) / this.rlRemain));
+ } else {
+ delay = Math.max(0, Math.ceil(this.rlReset * 1000));
+ }
+ return delay;
+ }
+
+ updateModalAction = (msg: React.ReactElement, canClose: boolean) => {
+ this.setState({
+ msg,
+ canClose,
+ });
};
toggleModal = () => {
@@ -60,14 +225,149 @@ export default class LastFmImporter extends React.Component<
});
};
- setClose = () => {
- this.setState({ canClose: this.importer.canClose });
+ setClose = (canClose: boolean) => {
+ this.setState({ canClose });
+ };
+
+ updateMessage = (msg: React.ReactElement) => {
+ this.setState({ msg });
};
- updateMessage = () => {
- this.setState({ msg: this.importer.msg });
+ handleChange = (event: React.ChangeEvent) => {
+ this.setState({ lastfmUsername: event.target.value });
+ };
+
+ handleSubmit = (event: React.FormEvent) => {
+ const { lastfmUsername } = this.state;
+ this.toggleModal();
+ event.preventDefault();
+ this.startImport();
};
+ async submitPage(payload: Array) {
+ const delay = this.getRateLimitDelay();
+ // Halt execution for some time
+ await new Promise((resolve) => {
+ setTimeout(resolve, delay);
+ });
+
+ const response = await this.APIService.submitListens(
+ this.userToken,
+ "import",
+ payload
+ );
+ this.updateRateLimitParameters(response);
+ }
+
+ async startImport() {
+ this.updateModalAction(Your import from Last.fm is starting!
, false);
+ this.playCount = await this.getTotalNumberOfScrobbles();
+ this.latestImportTime = await this.APIService.getLatestImport(
+ this.userName
+ );
+ this.incrementalImport = this.latestImportTime > 0;
+ this.totalPages = await this.getNumberOfPages();
+ this.page = this.totalPages; // Start from the last page so that oldest scrobbles are imported first
+
+ while (this.page > 0) {
+ // Fixing no-await-in-loop will require significant changes to the code, ignoring for now
+ const payload = await this.getPage(this.page); // eslint-disable-line
+ if (payload) {
+ // Submit only if response is valid
+ this.submitPage(payload);
+ }
+
+ this.page -= 1;
+ this.numCompleted += 1;
+
+ // Update message
+ const msg = (
+
+ Sending page{" "}
+ {this.numCompleted} of {this.totalPages} to ListenBrainz
+
+ {this.incrementalImport && (
+
+ Note: This import will stop at the starting point of your last
+ import. :)
+
+
+ )}
+ Please don't close this page while this is running
+
+
+ );
+ this.updateMessage(msg);
+ }
+
+ // Update latest import time on LB server
+ try {
+ this.maxTimestampForImport = Math.max(
+ Number(this.maxTimestampForImport),
+ this.latestImportTime
+ );
+ this.APIService.setLatestImport(
+ this.userToken,
+ this.maxTimestampForImport
+ );
+ } catch {
+ // console.warn("Error setting latest import timestamp, retrying in 3s");
+ setTimeout(
+ () =>
+ this.APIService.setLatestImport(
+ this.userToken,
+ this.maxTimestampForImport
+ ),
+ 3000
+ );
+ }
+ const { profileUrl } = this.props;
+ const finalMsg = (
+
+ Import finished
+
+
+ Successfully submitted {this.countReceived} listens to ListenBrainz
+
+
+ {/* if the count received is different from the api count, show a message accordingly
+ * also don't show this message if it's an incremental import, because countReceived
+ * and playCount will be different by definition in incremental imports
+ */}
+ {!this.incrementalImport &&
+ this.playCount !== -1 &&
+ this.countReceived !== this.playCount && (
+
+
+ The number submitted listens is different from the{" "}
+ {this.playCount} that Last.fm reports due to an inconsistency in
+ their API, sorry!
+
+
+
+ )}
+
+ Thank you for using ListenBrainz!
+
+
+
+
+
+ Close and go to your ListenBrainz profile
+
+
+
+ );
+ this.setState({ canClose: true, msg: finalMsg });
+ }
+
+ updateRateLimitParameters(response: Response) {
+ /* Update the variables we use to honor LB's rate limits */
+ this.rlRemain = Number(response.headers.get("X-RateLimit-Remaining"));
+ this.rlReset = Number(response.headers.get("X-RateLimit-Reset-In"));
+ this.rlOrigin = new Date().getTime() / 1000;
+ }
+
render() {
const { show, canClose, lastfmUsername, msg } = this.state;
From 4e21f25241483b04f1f5cf4467d060af014456d3 Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Mon, 4 Jan 2021 21:43:21 +0530
Subject: [PATCH 2/6] Update playcount if non incremental import
---
listenbrainz/webserver/static/js/src/LastFMImporter.tsx | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
index 3b3e6c0ec8..16a9c15e42 100644
--- a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
+++ b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
@@ -261,11 +261,12 @@ export default class LastFmImporter extends React.Component<
async startImport() {
this.updateModalAction(Your import from Last.fm is starting!
, false);
- this.playCount = await this.getTotalNumberOfScrobbles();
this.latestImportTime = await this.APIService.getLatestImport(
this.userName
);
this.incrementalImport = this.latestImportTime > 0;
+ if (!this.incrementalImport)
+ this.playCount = await this.getTotalNumberOfScrobbles();
this.totalPages = await this.getNumberOfPages();
this.page = this.totalPages; // Start from the last page so that oldest scrobbles are imported first
From bbf9f93bdc6f3107a23ecd30551f3bca2150490c Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Tue, 5 Jan 2021 00:00:07 +0530
Subject: [PATCH 3/6] Refactor LastFMImporter.test.tsx
---
.../static/js/src/LastFMImporter.test.tsx | 261 +++++++++++++++++-
1 file changed, 260 insertions(+), 1 deletion(-)
diff --git a/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx b/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
index 954d9a14a0..f4a7ae318a 100644
--- a/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
+++ b/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
@@ -1,8 +1,14 @@
import * as React from "react";
import { mount, shallow } from "enzyme";
-
import LastFmImporter from "./LastFMImporter";
+// Mock data to test functions
+import * as page from "./__mocks__/page.json";
+import * as getInfo from "./__mocks__/getInfo.json";
+import * as getInfoNoPlayCount from "./__mocks__/getInfoNoPlayCount.json";
+// Output for the mock data
+import * as encodeScrobbleOutput from "./__mocks__/encodeScrobbleOutput.json";
+jest.useFakeTimers();
const props = {
user: {
id: "id",
@@ -15,6 +21,259 @@ const props = {
lastfmApiKey: "foobar",
};
+describe("encodeScrobbles", () => {
+ it("encodes the given scrobbles correctly", () => {
+ expect(LastFmImporter.encodeScrobbles(page)).toEqual(encodeScrobbleOutput);
+ });
+});
+
+let instance: LastFmImporter;
+
+describe("getNumberOfPages", () => {
+ beforeEach(() => {
+ const wrapper = shallow();
+ instance = wrapper.instance();
+ instance.setState({ lastfmUsername: "dummyUser" });
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: true,
+ json: () => Promise.resolve(page),
+ });
+ });
+ });
+
+ it("should call with the correct url", () => {
+ instance.getNumberOfPages();
+
+ expect(window.fetch).toHaveBeenCalledWith(
+ `${props.lastfmApiUrl}?method=user.getrecenttracks&user=${instance.state.lastfmUsername}&api_key=${props.lastfmApiKey}&from=1&format=json`
+ );
+ });
+
+ it("should return number of pages", async () => {
+ const num = await instance.getNumberOfPages();
+ expect(num).toBe(1);
+ });
+
+ it("should return -1 if there is an error", async () => {
+ // Mock function for failed fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: false,
+ });
+ });
+
+ const num = await instance.getNumberOfPages();
+ expect(num).toBe(-1);
+ });
+});
+
+describe("getTotalNumberOfScrobbles", () => {
+ beforeEach(() => {
+ const wrapper = shallow();
+ instance = wrapper.instance();
+ instance.setState({ lastfmUsername: "dummyUser" });
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: true,
+ json: () => Promise.resolve(getInfo),
+ });
+ });
+ });
+
+ it("should call with the correct url", () => {
+ instance.getTotalNumberOfScrobbles();
+
+ expect(window.fetch).toHaveBeenCalledWith(
+ `${props.lastfmApiUrl}?method=user.getinfo&user=${instance.state.lastfmUsername}&api_key=${props.lastfmApiKey}&format=json`
+ );
+ });
+
+ it("should return number of pages", async () => {
+ const num = await instance.getTotalNumberOfScrobbles();
+ expect(num).toBe(1026);
+ });
+
+ it("should return -1 if playcount is not available", async () => {
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: true,
+ json: () => Promise.resolve(getInfoNoPlayCount),
+ });
+ });
+
+ const num = await instance.getTotalNumberOfScrobbles();
+ expect(num).toBe(-1);
+ });
+
+ it("should throw an error when fetch fails", async () => {
+ // Mock function for failed fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: false,
+ });
+ });
+ await expect(instance.getTotalNumberOfScrobbles()).rejects.toThrowError();
+ });
+});
+
+describe("getPage", () => {
+ beforeEach(() => {
+ const wrapper = shallow();
+ instance = wrapper.instance();
+ instance.setState({ lastfmUsername: "dummyUser" });
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: true,
+ json: () => Promise.resolve(page),
+ });
+ });
+ });
+
+ it("should call with the correct url", () => {
+ instance.getPage(1);
+
+ expect(window.fetch).toHaveBeenCalledWith(
+ `${props.lastfmApiUrl}?method=user.getrecenttracks&user=${instance.state.lastfmUsername}&api_key=${props.lastfmApiKey}&from=1&page=1&format=json`
+ );
+ });
+
+ it("should call encodeScrobbles", async () => {
+ // Mock function for encodeScrobbles
+ LastFmImporter.encodeScrobbles = jest.fn(() => ["foo", "bar"]);
+
+ const data = await instance.getPage(1);
+ expect(LastFmImporter.encodeScrobbles).toHaveBeenCalledTimes(1);
+ expect(data).toEqual(["foo", "bar"]);
+ });
+
+ it("should retry if 50x error is recieved", async () => {
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: false,
+ status: 503,
+ });
+ });
+
+ await instance.getPage(1);
+ // There is no direct way to check if retry has been called
+ expect(setTimeout).toHaveBeenCalledTimes(1);
+
+ jest.runAllTimers();
+ });
+
+ it("should skip the page if 40x is recieved", async () => {
+ // Mock function for failed fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: false,
+ status: 404,
+ });
+ });
+
+ await instance.getPage(1);
+ expect(setTimeout).not.toHaveBeenCalled();
+ });
+
+ it("should retry if there is any other error", async () => {
+ // Mock function for fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: true,
+ json: () => Promise.reject(),
+ });
+ });
+
+ await instance.getPage(1);
+ // There is no direct way to check if retry has been called
+ expect(setTimeout).toHaveBeenCalledTimes(1);
+ jest.runAllTimers();
+ });
+});
+
+describe("submitPage", () => {
+ beforeEach(() => {
+ const wrapper = shallow();
+ instance = wrapper.instance();
+ instance.setState({ lastfmUsername: "dummyUser" });
+ instance.getRateLimitDelay = jest.fn().mockImplementation(() => 0);
+ instance.updateRateLimitParameters = jest.fn();
+ });
+
+ it("calls submitListens once", async () => {
+ // window.fetch = jest.fn().mockImplementation(() => {
+ // return Promise.resolve({
+ // ok: true,
+ // json: () => Promise.resolve({ status: 200 }),
+ // });
+ // });
+ instance.APIService.submitListens = jest.fn().mockImplementation(() => {
+ return Promise.resolve({ status: 200 });
+ });
+ instance.submitPage([
+ {
+ listened_at: 1000,
+ track_metadata: {
+ artist_name: "foobar",
+ track_name: "bazfoo",
+ },
+ },
+ ]);
+
+ jest.runAllTimers();
+
+ // Flush all promises
+ // https://stackoverflow.com/questions/51126786/jest-fake-timers-with-promises
+ await new Promise((resolve) => setImmediate(resolve));
+
+ expect(instance.APIService.submitListens).toHaveBeenCalledTimes(1);
+ });
+
+ it("calls updateRateLimitParameters once", async () => {
+ instance.APIService.submitListens = jest.fn().mockImplementation(() => {
+ return Promise.resolve({ status: 200 });
+ });
+ instance.submitPage([
+ {
+ listened_at: 1000,
+ track_metadata: {
+ artist_name: "foobar",
+ track_name: "bazfoo",
+ },
+ },
+ ]);
+
+ jest.runAllTimers();
+
+ // Flush all promises
+ // https://stackoverflow.com/questions/51126786/jest-fake-timers-with-promises
+ await new Promise((resolve) => setImmediate(resolve));
+
+ expect(instance.updateRateLimitParameters).toHaveBeenCalledTimes(1);
+ expect(instance.updateRateLimitParameters).toHaveBeenCalledWith({
+ status: 200,
+ });
+ });
+
+ it("calls getRateLimitDelay once", async () => {
+ instance.submitPage([
+ {
+ listened_at: 1000,
+ track_metadata: {
+ artist_name: "foobar",
+ track_name: "bazfoo",
+ },
+ },
+ ]);
+ expect(instance.getRateLimitDelay).toHaveBeenCalledTimes(1);
+ });
+});
+
describe("LastFmImporter Page", () => {
it("renders", () => {
const wrapper = mount();
From 76c126dd7df68e559e2caf3f8d42107066278835 Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Thu, 7 Jan 2021 00:10:55 +0530
Subject: [PATCH 4/6] Importer reports error if user is invalid
---
listenbrainz/webserver/static/js/src/LastFMImporter.tsx | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
index 16a9c15e42..731935621a 100644
--- a/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
+++ b/listenbrainz/webserver/static/js/src/LastFMImporter.tsx
@@ -229,10 +229,6 @@ export default class LastFmImporter extends React.Component<
this.setState({ canClose });
};
- updateMessage = (msg: React.ReactElement) => {
- this.setState({ msg });
- };
-
handleChange = (event: React.ChangeEvent) => {
this.setState({ lastfmUsername: event.target.value });
};
@@ -265,8 +261,7 @@ export default class LastFmImporter extends React.Component<
this.userName
);
this.incrementalImport = this.latestImportTime > 0;
- if (!this.incrementalImport)
- this.playCount = await this.getTotalNumberOfScrobbles();
+ this.playCount = await this.getTotalNumberOfScrobbles();
this.totalPages = await this.getNumberOfPages();
this.page = this.totalPages; // Start from the last page so that oldest scrobbles are imported first
@@ -298,7 +293,7 @@ export default class LastFmImporter extends React.Component<
);
- this.updateMessage(msg);
+ this.setState({ msg });
}
// Update latest import time on LB server
From 923935a1aa39ad607a1a7fe34b17c2073a2f3646 Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Thu, 7 Jan 2021 00:39:27 +0530
Subject: [PATCH 5/6] Add test for showing error message in importer
---
.../webserver/static/js/src/LastFMImporter.test.tsx | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx b/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
index f4a7ae318a..8fd3ffba70 100644
--- a/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
+++ b/listenbrainz/webserver/static/js/src/LastFMImporter.test.tsx
@@ -118,6 +118,18 @@ describe("getTotalNumberOfScrobbles", () => {
});
await expect(instance.getTotalNumberOfScrobbles()).rejects.toThrowError();
});
+ it("should show the error message in importer", async () => {
+ // Mock function for failed fetch
+ window.fetch = jest.fn().mockImplementation(() => {
+ return Promise.resolve({
+ ok: false,
+ });
+ });
+ await expect(instance.getTotalNumberOfScrobbles()).rejects.toThrowError();
+ expect(instance.state.msg?.props.children).toMatch(
+ "An error occurred, please try again. :("
+ );
+ });
});
describe("getPage", () => {
From ed0cf9323bc96b3ad209830e052bbac1b98326f6 Mon Sep 17 00:00:00 2001
From: Kshitiz Kumar
Date: Fri, 8 Jan 2021 20:57:14 +0530
Subject: [PATCH 6/6] Delete redundant Importer.tsx, Importer.test.ts
---
.../webserver/static/js/src/Importer.test.ts | 256 ---------------
.../webserver/static/js/src/Importer.tsx | 307 ------------------
2 files changed, 563 deletions(-)
delete mode 100644 listenbrainz/webserver/static/js/src/Importer.test.ts
delete mode 100644 listenbrainz/webserver/static/js/src/Importer.tsx
diff --git a/listenbrainz/webserver/static/js/src/Importer.test.ts b/listenbrainz/webserver/static/js/src/Importer.test.ts
deleted file mode 100644
index b03dca3b40..0000000000
--- a/listenbrainz/webserver/static/js/src/Importer.test.ts
+++ /dev/null
@@ -1,256 +0,0 @@
-import Importer from "./Importer";
-
-// Mock data to test functions
-import * as page from "./__mocks__/page.json";
-import * as getInfo from "./__mocks__/getInfo.json";
-import * as getInfoNoPlayCount from "./__mocks__/getInfoNoPlayCount.json";
-// Output for the mock data
-import * as encodeScrobbleOutput from "./__mocks__/encodeScrobbleOutput.json";
-
-jest.useFakeTimers();
-const props = {
- user: {
- id: "id",
- name: "dummyUser",
- auth_token: "foobar",
- },
- profileUrl: "http://profile",
- apiUrl: "apiUrl",
- lastfmApiUrl: "http://ws.audioscrobbler.com/2.0/",
- lastfmApiKey: "foobar",
-};
-const lastfmUsername = "dummyUser";
-const importer = new Importer(lastfmUsername, props);
-
-describe("encodeScrobbles", () => {
- it("encodes the given scrobbles correctly", () => {
- expect(Importer.encodeScrobbles(page)).toEqual(encodeScrobbleOutput);
- });
-});
-
-describe("getNumberOfPages", () => {
- beforeEach(() => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: true,
- json: () => Promise.resolve(page),
- });
- });
- });
-
- it("should call with the correct url", () => {
- importer.getNumberOfPages();
-
- expect(window.fetch).toHaveBeenCalledWith(
- `${props.lastfmApiUrl}?method=user.getrecenttracks&user=${lastfmUsername}&api_key=${props.lastfmApiKey}&from=1&format=json`
- );
- });
-
- it("should return number of pages", async () => {
- const num = await importer.getNumberOfPages();
- expect(num).toBe(1);
- });
-
- it("should return -1 if there is an error", async () => {
- // Mock function for failed fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: false,
- });
- });
-
- const num = await importer.getNumberOfPages();
- expect(num).toBe(-1);
- });
-});
-
-describe("getTotalNumberOfScrobbles", () => {
- beforeEach(() => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: true,
- json: () => Promise.resolve(getInfo),
- });
- });
- });
-
- it("should call with the correct url", () => {
- importer.getTotalNumberOfScrobbles();
-
- expect(window.fetch).toHaveBeenCalledWith(
- `${props.lastfmApiUrl}?method=user.getinfo&user=${lastfmUsername}&api_key=${props.lastfmApiKey}&format=json`
- );
- });
-
- it("should return number of pages", async () => {
- const num = await importer.getTotalNumberOfScrobbles();
- expect(num).toBe(1026);
- });
-
- it("should return -1 if playcount is not available", async () => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: true,
- json: () => Promise.resolve(getInfoNoPlayCount),
- });
- });
-
- const num = await importer.getTotalNumberOfScrobbles();
- expect(num).toBe(-1);
- });
-
- it("should throw an error when fetch fails", async () => {
- // Mock function for failed fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: false,
- });
- });
- await expect(importer.getTotalNumberOfScrobbles()).rejects.toThrowError();
- });
-});
-
-describe("getPage", () => {
- beforeEach(() => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: true,
- json: () => Promise.resolve(page),
- });
- });
- });
-
- it("should call with the correct url", () => {
- importer.getPage(1);
-
- expect(window.fetch).toHaveBeenCalledWith(
- `${props.lastfmApiUrl}?method=user.getrecenttracks&user=${lastfmUsername}&api_key=${props.lastfmApiKey}&from=1&page=1&format=json`
- );
- });
-
- it("should call encodeScrobbles", async () => {
- // Mock function for encodeScrobbles
- Importer.encodeScrobbles = jest.fn(() => ["foo", "bar"]);
-
- const data = await importer.getPage(1);
- expect(Importer.encodeScrobbles).toHaveBeenCalledTimes(1);
- expect(data).toEqual(["foo", "bar"]);
- });
-
- it("should retry if 50x error is recieved", async () => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: false,
- status: 503,
- });
- });
-
- await importer.getPage(1);
- // There is no direct way to check if retry has been called
- expect(setTimeout).toHaveBeenCalledTimes(1);
-
- jest.runAllTimers();
- });
-
- it("should skip the page if 40x is recieved", async () => {
- // Mock function for failed fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: false,
- status: 404,
- });
- });
-
- await importer.getPage(1);
- expect(setTimeout).not.toHaveBeenCalled();
- });
-
- it("should retry if there is any other error", async () => {
- // Mock function for fetch
- window.fetch = jest.fn().mockImplementation(() => {
- return Promise.resolve({
- ok: true,
- json: () => Promise.reject(),
- });
- });
-
- await importer.getPage(1);
- // There is no direct way to check if retry has been called
- expect(setTimeout).toHaveBeenCalledTimes(1);
- jest.runAllTimers();
- });
-});
-
-describe("submitPage", () => {
- beforeEach(() => {
- importer.getRateLimitDelay = jest.fn().mockImplementation(() => 0);
- importer.updateRateLimitParameters = jest.fn();
- });
-
- it("calls submitListens once", async () => {
- importer.APIService.submitListens = jest.fn().mockImplementation(() => {
- return Promise.resolve({ status: 200 });
- });
- importer.submitPage([
- {
- listened_at: 1000,
- track_metadata: {
- artist_name: "foobar",
- track_name: "bazfoo",
- },
- },
- ]);
-
- jest.runAllTimers();
-
- // Flush all promises
- // https://stackoverflow.com/questions/51126786/jest-fake-timers-with-promises
- await new Promise((resolve) => setImmediate(resolve));
-
- expect(importer.APIService.submitListens).toHaveBeenCalledTimes(1);
- });
-
- it("calls updateRateLimitParameters once", async () => {
- importer.APIService.submitListens = jest.fn().mockImplementation(() => {
- return Promise.resolve({ status: 200 });
- });
- importer.submitPage([
- {
- listened_at: 1000,
- track_metadata: {
- artist_name: "foobar",
- track_name: "bazfoo",
- },
- },
- ]);
-
- jest.runAllTimers();
-
- // Flush all promises
- // https://stackoverflow.com/questions/51126786/jest-fake-timers-with-promises
- await new Promise((resolve) => setImmediate(resolve));
-
- expect(importer.updateRateLimitParameters).toHaveBeenCalledTimes(1);
- expect(importer.updateRateLimitParameters).toHaveBeenCalledWith({
- status: 200,
- });
- });
-
- it("calls getRateLimitDelay once", async () => {
- importer.submitPage([
- {
- listened_at: 1000,
- track_metadata: {
- artist_name: "foobar",
- track_name: "bazfoo",
- },
- },
- ]);
- expect(importer.getRateLimitDelay).toHaveBeenCalledTimes(1);
- });
-});
diff --git a/listenbrainz/webserver/static/js/src/Importer.tsx b/listenbrainz/webserver/static/js/src/Importer.tsx
deleted file mode 100644
index e1c7335641..0000000000
--- a/listenbrainz/webserver/static/js/src/Importer.tsx
+++ /dev/null
@@ -1,307 +0,0 @@
-import * as React from "react";
-import { faSpinner, faCheck } from "@fortawesome/free-solid-svg-icons";
-import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
-import { IconProp } from "@fortawesome/fontawesome-svg-core";
-import Scrobble from "./Scrobble";
-import APIService from "./APIService";
-import { ImporterProps } from "./LastFMImporter";
-
-export default class Importer {
- APIService: APIService;
-
- private lastfmUsername: string;
- private lastfmURL: string;
- private lastfmKey: string;
-
- private userName: string;
- private userToken: string;
-
- private page = 1;
- private totalPages = 0;
-
- private playCount = -1; // the number of scrobbles reported by Last.FM
- private countReceived = 0; // number of scrobbles the Last.FM API sends us, this can be diff from playCount
-
- private latestImportTime = 0; // the latest timestamp that we've imported earlier
- private maxTimestampForImport = 0; // the latest listen found in this import
- private incrementalImport = false;
-
- private numCompleted = 0; // number of pages completed till now
-
- // Variables used to honor LB's rate limit
- private rlRemain = -1;
- private rlReset = -1;
- private rlOrigin = -1;
-
- public msg?: React.ReactElement; // Message to be displayed in modal
- public canClose = true;
-
- constructor(lastfmUsername: string, private props: ImporterProps) {
- this.APIService = new APIService(
- props.apiUrl || `${window.location.origin}/1`
- ); // Used to access LB API
-
- this.lastfmUsername = lastfmUsername;
- this.lastfmURL = props.lastfmApiUrl;
- this.lastfmKey = props.lastfmApiKey;
-
- this.userName = props.user.name;
- this.userToken = props.user.auth_token || "";
- }
-
- async startImport() {
- this.canClose = false; // Disable the close button
- this.updateMessage(Your import from Last.fm is starting!
);
- this.playCount = await this.getTotalNumberOfScrobbles();
- this.latestImportTime = await this.APIService.getLatestImport(
- this.userName
- );
- this.incrementalImport = this.latestImportTime > 0;
- this.totalPages = await this.getNumberOfPages();
- this.page = this.totalPages; // Start from the last page so that oldest scrobbles are imported first
-
- while (this.page > 0) {
- // Fixing no-await-in-loop will require significant changes to the code, ignoring for now
- const payload = await this.getPage(this.page); // eslint-disable-line
- if (payload) {
- // Submit only if response is valid
- this.submitPage(payload);
- }
-
- this.page -= 1;
- this.numCompleted += 1;
-
- // Update message
- const msg = (
-
- Sending page{" "}
- {this.numCompleted} of {this.totalPages} to ListenBrainz
-
- {this.incrementalImport && (
-
- Note: This import will stop at the starting point of your last
- import. :)
-
-
- )}
- Please don't close this page while this is running
-
-
- );
- this.updateMessage(msg);
- }
-
- // Update latest import time on LB server
- try {
- this.maxTimestampForImport = Math.max(
- Number(this.maxTimestampForImport),
- this.latestImportTime
- );
- this.APIService.setLatestImport(
- this.userToken,
- this.maxTimestampForImport
- );
- } catch {
- // console.warn("Error setting latest import timestamp, retrying in 3s");
- setTimeout(
- () =>
- this.APIService.setLatestImport(
- this.userToken,
- this.maxTimestampForImport
- ),
- 3000
- );
- }
- const finalMsg = (
-
- Import finished
-
-
- Successfully submitted {this.countReceived} listens to ListenBrainz
-
-
- {/* if the count received is different from the api count, show a message accordingly
- * also don't show this message if it's an incremental import, because countReceived
- * and playCount will be different by definition in incremental imports
- */}
- {!this.incrementalImport &&
- this.playCount !== -1 &&
- this.countReceived !== this.playCount && (
-
-
- The number submitted listens is different from the{" "}
- {this.playCount} that Last.fm reports due to an inconsistency in
- their API, sorry!
-
-
-
- )}
-
- Thank you for using ListenBrainz!
-
-
-
-
-
- Close and go to your ListenBrainz profile
-
-
-
- );
- this.updateMessage(finalMsg);
- this.canClose = true;
- }
-
- async getTotalNumberOfScrobbles() {
- /*
- * Get the total play count reported by Last.FM for user
- */
-
- const url = `${this.lastfmURL}?method=user.getinfo&user=${this.lastfmUsername}&api_key=${this.lastfmKey}&format=json`;
- try {
- const response = await fetch(encodeURI(url));
- const data = await response.json();
- if ("playcount" in data.user) {
- return Number(data.user.playcount);
- }
- return -1;
- } catch {
- this.updateMessage(An error occurred, please try again. :(
);
- this.canClose = true; // Enable the close button
- const error = new Error();
- error.message = "Something went wrong";
- throw error;
- }
- }
-
- async getNumberOfPages() {
- /*
- * Get the total pages of data from last import
- */
-
- const url = `${this.lastfmURL}?method=user.getrecenttracks&user=${
- this.lastfmUsername
- }&api_key=${this.lastfmKey}&from=${this.latestImportTime + 1}&format=json`;
- try {
- const response = await fetch(encodeURI(url));
- const data = await response.json();
- if ("recenttracks" in data) {
- return Number(data.recenttracks["@attr"].totalPages);
- }
- return 0;
- } catch (error) {
- this.updateMessage(An error occurred, please try again. :(
);
- this.canClose = true; // Enable the close button
- return -1;
- }
- }
-
- async getPage(page: number) {
- /*
- * Fetch page from Last.fm
- */
-
- const retry = (reason: string) => {
- // console.warn(`${reason} while fetching last.fm page=${page}, retrying in 3s`);
- setTimeout(() => this.getPage(page), 3000);
- };
-
- const url = `${this.lastfmURL}?method=user.getrecenttracks&user=${
- this.lastfmUsername
- }&api_key=${this.lastfmKey}&from=${
- this.latestImportTime + 1
- }&page=${page}&format=json`;
- try {
- const response = await fetch(encodeURI(url));
- if (response.ok) {
- const data = await response.json();
- // Set latest import time
- if ("date" in data.recenttracks.track[0]) {
- this.maxTimestampForImport = Math.max(
- data.recenttracks.track[0].date.uts,
- this.maxTimestampForImport
- );
- } else {
- this.maxTimestampForImport = Math.floor(Date.now() / 1000);
- }
-
- // Encode the page so that it can be submitted
- const payload = Importer.encodeScrobbles(data);
- this.countReceived += payload.length;
- return payload;
- }
- if (/^5/.test(response.status.toString())) {
- retry(`Got ${response.status}`);
- } else {
- // ignore 40x
- // console.warn(`Got ${response.status} while fetching page last.fm page=${page}, skipping`);
- }
- } catch {
- // Retry if there is a network error
- retry("Error");
- }
- return null;
- }
-
- async submitPage(payload: Array) {
- const delay = this.getRateLimitDelay();
- // Halt execution for some time
- await new Promise((resolve) => {
- setTimeout(resolve, delay);
- });
-
- const response = await this.APIService.submitListens(
- this.userToken,
- "import",
- payload
- );
- this.updateRateLimitParameters(response);
- }
-
- static encodeScrobbles(scrobbles: LastFmScrobblePage): any {
- const rawScrobbles = scrobbles.recenttracks.track;
- const parsedScrobbles = Importer.map((rawScrobble: any) => {
- const scrobble = new Scrobble(rawScrobble);
- return scrobble.asJSONSerializable();
- }, rawScrobbles);
- return parsedScrobbles;
- }
-
- static map(applicable: (collection: any) => Listen, collection: any) {
- const newCollection = [];
- for (let i = 0; i < collection.length; i += 1) {
- const result = applicable(collection[i]);
- if (result.listened_at > 0) {
- // If the 'listened_at' attribute is -1 then either the listen is invalid or the
- // listen is currently playing. In both cases we need to skip the submission.
- newCollection.push(result);
- }
- }
- return newCollection;
- }
-
- updateMessage = (msg: React.ReactElement) => {
- this.msg = msg;
- };
-
- getRateLimitDelay() {
- /* Get the amount of time we should wait according to LB rate limits before making a request to LB */
- let delay = 0;
- const current = new Date().getTime() / 1000;
- if (this.rlReset < 0 || current > this.rlOrigin + this.rlReset) {
- delay = 0;
- } else if (this.rlRemain > 0) {
- delay = Math.max(0, Math.ceil((this.rlReset * 1000) / this.rlRemain));
- } else {
- delay = Math.max(0, Math.ceil(this.rlReset * 1000));
- }
- return delay;
- }
-
- updateRateLimitParameters(response: Response) {
- /* Update the variables we use to honor LB's rate limits */
- this.rlRemain = Number(response.headers.get("X-RateLimit-Remaining"));
- this.rlReset = Number(response.headers.get("X-RateLimit-Reset-In"));
- this.rlOrigin = new Date().getTime() / 1000;
- }
-}