From 2bb803a9f04f23d72e45412d7cbc4a470eb56b42 Mon Sep 17 00:00:00 2001 From: Nikkel Mollenhauer <57323886+NikkelM@users.noreply.github.com> Date: Sat, 5 Aug 2023 14:04:14 +0200 Subject: [PATCH] Updated code to work with new DB rules, better tests (#214) --- CHANGELOG.md | 9 ++++-- package.json | 2 +- src/background.js | 11 +++++-- src/content.js | 5 ++-- src/shuffleVideo.js | 26 ++++++++++++---- static/manifest.json | 2 +- test/playlistPermutations.js | 14 ++++----- test/shuffleVideo.test.js | 57 +++++++++++++++++++++++++++++++----- 8 files changed, 99 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f3da2e..d4ffc0f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,16 @@ # Changelog -## v2.2.2 +## v2.2.3 +- Error messages now include the current channel ID to help with reporting an issue. +- Some changes in how data that is sent to the database is handled, to prepare for a future security update. + + +## v2.2.2 + - Fixed a bug where there would be no video IDs stored in the database, leading to an error. - Fixed the 'Changelog' button on the changelog page not doing anything. - ## v2.2.1 diff --git a/package.json b/package.json index d5cb339e..d69694ca 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "random-youtube-video", - "version": "2.2.2", + "version": "2.2.3", "description": "Play a random video uploaded on the current YouTube channel.", "scripts": { "dev": "concurrently \"npm run dev:chromium\" \"npm run dev:firefox\"", diff --git a/src/background.js b/src/background.js index 10b604a5..3a55d217 100644 --- a/src/background.js +++ b/src/background.js @@ -138,7 +138,7 @@ chrome.runtime.onMessage.addListener((request, sender, sendResponse) => { // Updates (overwriting videos) a playlist in Firebase case "overwritePlaylistInfoInDB": updatePlaylistInfoInDB('uploadsPlaylists/' + request.data.key, request.data.val, true).then(sendResponse); - break; + break; // Before v1.0.0 the videos were stored in an array without upload times, so they need to all be refetched case 'updateDBPlaylistToV1.0.0': updateDBPlaylistToV1_0_0('uploadsPlaylists/' + request.data.key).then(sendResponse); @@ -189,7 +189,14 @@ const app = initializeApp(firebaseConfig); const db = getDatabase(app); async function updatePlaylistInfoInDB(playlistId, playlistInfo, overwriteVideos) { - if (overwriteVideos) { + // Find out if the playlist already exists in the database + // We only need to send this request if we don't already have to overwrite the entry + let playlistExists = true; + if (!overwriteVideos) { + playlistExists = await readDataOnce(playlistId); + } + + if (overwriteVideos || !playlistExists) { console.log("Setting playlistInfo in the database..."); // Update the entire object. Due to the way Firebase works, this will overwrite the existing 'videos' object, as it is nested within the playlist update(ref(db, playlistId), playlistInfo); diff --git a/src/content.js b/src/content.js index 3c7f6c0e..11a0bec0 100644 --- a/src/content.js +++ b/src/content.js @@ -120,9 +120,10 @@ function resetShuffleButtonText() { // ---------- Shuffle ---------- // Called when the 'Shuffle' button is clicked async function shuffleVideos() { + let channelId; try { // Get the saved channelId from the button - const channelId = shuffleButton?.children[0]?.children[0]?.children[0]?.children?.namedItem('channelId')?.innerText; + channelId = shuffleButton?.children[0]?.children[0]?.children[0]?.children?.namedItem('channelId')?.innerText; // If the channelId somehow wasn't saved, throw an error if (!channelId) { @@ -179,7 +180,7 @@ The page will reload and you can try again.`) } // Alert the user about the error - window.alert(`Random YouTube Video:\n\n${displayText}${error.message ? "\n" + error.message : ""}${error.reason ? "\n" + error.reason : ""}${error.solveHint ? "\n" + error.solveHint : ""}${error.showTrace !== false ? "\n\n" + error.stack : ""}`); + window.alert(`Random YouTube Video:\n\nChannel ${channelId}\n${displayText}${error.message ? "\n" + error.message : ""}${error.reason ? "\n" + error.reason : ""}${error.solveHint ? "\n" + error.solveHint : ""}${error.showTrace !== false ? "\n\n" + error.stack : ""}`); // Immediately display the error shuffleButtonTextElement.innerText = `\xa0${displayText}`; diff --git a/src/shuffleVideo.js b/src/shuffleVideo.js index 80620a15..8afade71 100644 --- a/src/shuffleVideo.js +++ b/src/shuffleVideo.js @@ -131,10 +131,10 @@ export async function chooseRandomVideo(channelId, firedFromPopup, progressTextE } else { // Otherwise, we want to only upload new videos. If there are no "newVideos", we upload all videos, as this is the first time we are uploading the playlist console.log("Uploading new video IDs to the database..."); - if(getLength(playlistInfo["newVideos"] ?? {}) > 0) { + if (getLength(playlistInfo["newVideos"] ?? {}) > 0) { videosToDatabase = playlistInfo["newVideos"]; } else { - videosToDatabase = playlistInfo["videos"] ?? 0; + videosToDatabase = playlistInfo["videos"]; } } @@ -155,7 +155,7 @@ export async function chooseRandomVideo(channelId, firedFromPopup, progressTextE // Remember the last time the playlist was accessed locally (==now) "lastAccessedLocally": new Date().toISOString(), "lastFetchedFromDB": playlistInfo["lastFetchedFromDB"] ?? new Date(0).toISOString(), - "lastVideoPublishedAt": playlistInfo["lastVideoPublishedAt"] ?? new Date(0).toISOString(), + "lastVideoPublishedAt": playlistInfo["lastVideoPublishedAt"] ?? new Date(0).toISOString().slice(0, 19) + 'Z', "videos": playlistInfo["videos"] ?? {} }; @@ -225,10 +225,24 @@ async function uploadPlaylistToDatabase(playlistInfo, videosToDatabase, uploadsP // Only upload the wanted keys const playlistInfoForDatabase = { "lastUpdatedDBAt": playlistInfo["lastUpdatedDBAt"] ?? new Date().toISOString(), - "lastVideoPublishedAt": playlistInfo["lastVideoPublishedAt"] ?? new Date(0).toISOString(), + "lastVideoPublishedAt": playlistInfo["lastVideoPublishedAt"] ?? new Date(0).toISOString().slice(0, 19) + 'Z', "videos": videosToDatabase }; + // Make sure the data is in the correct format + if (playlistInfoForDatabase["lastUpdatedDBAt"].length !== 24) { + alert(`Random YouTube Video:\nPlease send this information to the developer:\n\nlastUpdatedDBAt has the wrong format (got ${playlistInfoForDatabase["lastVideoPublishedAt"]}).\nChannelId: ${uploadsPlaylistId}.`); + return; + } + if (playlistInfoForDatabase["lastVideoPublishedAt"].length !== 20) { + alert(`Random YouTube Video:\nPlease send this information to the developer:\n\nlastVideoPublishedAt has the wrong format (got ${playlistInfoForDatabase["lastVideoPublishedAt"]}).\nChannelId: ${uploadsPlaylistId}.`); + return; + } + if (getLength(playlistInfoForDatabase["videos"]) < 1) { + alert(`Random YouTube Video:\nPlease send this information to the developer:\n\nNo videos object was found.\nChannelId: ${uploadsPlaylistId}.`); + return; + } + // Send the playlist info to the database const msg = { command: encounteredDeletedVideos ? 'overwritePlaylistInfoInDB' : 'updatePlaylistInfoInDB', @@ -916,12 +930,14 @@ async function aprilFoolsJoke() { async function tryGetPlaylistFromLocalStorage(playlistId) { return await chrome.storage.local.get([playlistId]).then(async (result) => { if (result[playlistId]) { + /* c8 ignore start */ // To fix a bug introduced in v2.2.1 - if(!result[playlistId]["videos"]) { + if (!result[playlistId]["videos"]) { // Remove from localStorage await chrome.storage.local.remove([playlistId]); return {} } + /* c8 ignore stop */ return result[playlistId]; } return {}; diff --git a/static/manifest.json b/static/manifest.json index 8e484f05..60cab42a 100644 --- a/static/manifest.json +++ b/static/manifest.json @@ -1,7 +1,7 @@ { "name": "Random YouTube Video", "description": "Play a random video uploaded on the current YouTube channel.", - "version": "2.2.2", + "version": "2.2.3", "manifest_version": 3, "content_scripts": [ { diff --git a/test/playlistPermutations.js b/test/playlistPermutations.js index 395f1081..ce6d7e06 100644 --- a/test/playlistPermutations.js +++ b/test/playlistPermutations.js @@ -272,19 +272,19 @@ const defaultDBDeletedVideos = { // The YT API returns a full-length timestamp const oneNewYTAPIVideo = { - "YT_V_000001": zeroDaysAgo + "YT_V_000001": zeroDaysAgo.slice(0, 19) + 'Z' }; // Get over the 50 per page API limit, and get to more than one additional page for the inner while loop const multipleNewYTAPIVideos = {}; for (let i = 1; i <= 70; i++) { const key = `YT_V_${String(i).padStart(6, '0')}`; - multipleNewYTAPIVideos[key] = zeroDaysAgo; + multipleNewYTAPIVideos[key] = zeroDaysAgo.slice(0, 19) + 'Z'; } // Add another 35 shorts, with _S_ in the key for (let i = 71; i <= 105; i++) { const key = `YT_S_${String(i).padStart(6, '0')}`; - multipleNewYTAPIVideos[key] = zeroDaysAgo; + multipleNewYTAPIVideos[key] = zeroDaysAgo.slice(0, 19) + 'Z'; } // ---------- Permutations for testing ---------- @@ -379,7 +379,7 @@ for (let i = 0; i < playlistModifiers[0].length; i++) { } // When was the last locally known video published - localLastVideoPublishedAt = threeDaysAgo; + localLastVideoPublishedAt = threeDaysAgo.slice(0, 19) + 'Z'; if (playlistModifiers[3][l] === "LocalPlaylistContainsDeletedVideos") { localVideos = deepCopy(defaultLocalVideos); @@ -401,7 +401,7 @@ for (let i = 0; i < playlistModifiers[0].length; i++) { if (playlistModifiers[5][n] === "DBContainsVideosNotInLocalPlaylist") { dbVideos = deepCopy({ ...defaultLocalVideos, ...defaultDBVideos }); dbDeletedVideos = null; - dbLastVideoPublishedAt = twoDaysAgo; + dbLastVideoPublishedAt = twoDaysAgo.slice(0, 19) + 'Z'; } else if (playlistModifiers[5][n] === "DBContainsNoVideosNotInLocalPlaylist") { dbVideos = playlistModifiers[3][l] === "LocalPlaylistContainsOnlyShorts" ? deepCopy(defaultLocalShorts) @@ -421,10 +421,10 @@ for (let i = 0; i < playlistModifiers[0].length; i++) { if (playlistModifiers[1][j] !== "DBEntryIsUpToDate") { if (playlistModifiers[4][m] === "OneNewVideoUploaded") { newUploadedVideos = deepCopy(oneNewYTAPIVideo); - newLastVideoPublishedAt = zeroDaysAgo; + newLastVideoPublishedAt = zeroDaysAgo.slice(0, 19) + 'Z'; } else if (playlistModifiers[4][m] === "MultipleNewVideosUploaded") { newUploadedVideos = deepCopy(multipleNewYTAPIVideos); - newLastVideoPublishedAt = zeroDaysAgo; + newLastVideoPublishedAt = zeroDaysAgo.slice(0, 19) + 'Z'; } else if (playlistModifiers[4][m] === "NoNewVideoUploaded") { newUploadedVideos = null; newLastVideoPublishedAt = dbLastVideoPublishedAt; diff --git a/test/shuffleVideo.test.js b/test/shuffleVideo.test.js index a9c7ef09..a7779395 100644 --- a/test/shuffleVideo.test.js +++ b/test/shuffleVideo.test.js @@ -42,6 +42,28 @@ function setUpMockResponses(mockResponses) { }); } +// Checks that a set of messages contains the correct data format for the database +function checkPlaylistsUploadedToDB(messages, input) { + messages.forEach((message) => { + const data = message[0].data; + + expect(message.length).to.be(1); + + expect(data.key).to.be(input.playlistId); + expect(Object.keys(data.val)).to.contain('lastUpdatedDBAt'); + expect(data.val.lastUpdatedDBAt.length).to.be(24); + expect(Object.keys(data.val)).to.contain('lastVideoPublishedAt'); + expect(data.val.lastVideoPublishedAt.length).to.be(20); + expect(Object.keys(data.val)).to.contain('videos'); + expect(Object.keys(data.val.videos).length).to.be.greaterThan(0); + // Check the format of the videos + for (const [videoId, publishTime] of Object.entries(data.val.videos)) { + expect(videoId.length).to.be(11); + expect(publishTime.length).to.be(10); + } + }); +} + describe('shuffleVideo', function () { beforeEach(function () { @@ -481,7 +503,8 @@ describe('shuffleVideo', function () { if (videoId.startsWith('DEL_LOC')) { delete allVideos[videoId]; } else { - allVideos[videoId] = publishTime; + // The YT API returns the publishTime in ISO 8601 format, so we need to convert it, as the localStorage and DB use a different format + allVideos[videoId] = new Date(publishTime).toISOString().slice(0, 19) + 'Z'; } } allVideos = Object.fromEntries(Object.entries(allVideos).sort((a, b) => b[1].localeCompare(a[1]))); @@ -856,7 +879,8 @@ describe('shuffleVideo', function () { if (videoId.startsWith('DEL_LOC')) { delete allVideos[videoId]; } else { - allVideos[videoId] = publishTime; + // The YT API returns the publishTime in ISO 8601 format, so we need to convert it, as the localStorage and DB use a different format + allVideos[videoId] = new Date(publishTime).toISOString().slice(0, 19) + 'Z'; } } allVideos = Object.fromEntries(Object.entries(allVideos).sort((a, b) => b[1].localeCompare(a[1]))); @@ -963,7 +987,8 @@ describe('shuffleVideo', function () { it('should only interact with the database to remove deleted videos', async function () { await chooseRandomVideo(input.channelId, false, domElement); - const commands = chrome.runtime.sendMessage.args.map(arg => arg[0].command); + const messages = chrome.runtime.sendMessage.args; + const commands = messages.map(arg => arg[0].command); // callCount is 3 if we didn't choose a deleted video, 4 else expect(chrome.runtime.sendMessage.callCount).to.be.within(3, 4); @@ -974,6 +999,9 @@ describe('shuffleVideo', function () { if (chrome.runtime.sendMessage.callCount === 4) { expect(commands).to.contain('overwritePlaylistInfoInDB'); + // One entry should contain a valid DB update + const overwriteMessages = messages.filter(arg => arg[0].command === 'overwritePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(overwriteMessages, input); } }); @@ -982,7 +1010,8 @@ describe('shuffleVideo', function () { await chooseRandomVideo(input.channelId, false, domElement); const playlistInfoAfter = await getKeyFromLocalStorage(input.playlistId); - const commands = chrome.runtime.sendMessage.args.map(arg => arg[0].command); + const messages = chrome.runtime.sendMessage.args; + const commands = messages.map(arg => arg[0].command); if (needsYTAPIInteraction(input)) { expect(chrome.runtime.sendMessage.callCount).to.be(6); @@ -1008,14 +1037,21 @@ describe('shuffleVideo', function () { break; case 5: expect(commands).to.contain('overwritePlaylistInfoInDB'); + const overwriteMessages = messages.filter(arg => arg[0].command === 'overwritePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(overwriteMessages, input); + expect(numDeletedVideosBefore).to.be.greaterThan(numDeletedVideosAfter); break; case 6: expect(commands).to.contain('getAPIKey'); if (numDeletedVideosBefore > numDeletedVideosAfter) { expect(commands).to.contain('overwritePlaylistInfoInDB'); + const overwriteMessages = messages.filter(arg => arg[0].command === 'overwritePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(overwriteMessages, input); } else { expect(commands).to.contain('updatePlaylistInfoInDB'); + const updateMessages = messages.filter(arg => arg[0].command === 'updatePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(updateMessages, input); } break; default: @@ -1027,7 +1063,8 @@ describe('shuffleVideo', function () { await chooseRandomVideo(input.channelId, false, domElement); const playlistInfoAfter = await getKeyFromLocalStorage(input.playlistId); - const commands = chrome.runtime.sendMessage.args.map(arg => arg[0].command); + const messages = chrome.runtime.sendMessage.args; + const commands = messages.map(arg => arg[0].command); expect(commands).to.contain('connectionTest'); expect(commands).to.contain('getPlaylistFromDB'); @@ -1041,6 +1078,8 @@ describe('shuffleVideo', function () { expect(chrome.runtime.sendMessage.callCount).to.be(6); expect(commands).to.contain('getAPIKey'); expect(commands).to.contain('updatePlaylistInfoInDB'); + const updateMessages = messages.filter(arg => arg[0].command === 'updatePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(updateMessages, input); } else { expect(chrome.runtime.sendMessage.callCount).to.be(4); } @@ -1053,7 +1092,8 @@ describe('shuffleVideo', function () { it('should only fetch data from the database', async function () { await chooseRandomVideo(input.channelId, false, domElement); - const commands = chrome.runtime.sendMessage.args.map(arg => arg[0].command); + const messages = chrome.runtime.sendMessage.args; + const commands = messages.map(arg => arg[0].command); // 4 because we only need to fetch from the DB expect(chrome.runtime.sendMessage.callCount).to.be(4); @@ -1068,7 +1108,8 @@ describe('shuffleVideo', function () { it('should update the database', async function () { await chooseRandomVideo(input.channelId, false, domElement); - const commands = chrome.runtime.sendMessage.args.map(arg => arg[0].command); + const messages = chrome.runtime.sendMessage.args; + const commands = messages.map(arg => arg[0].command); // 6 because we need to fetch from the DB, fetch from the YT API, and update the DB expect(chrome.runtime.sendMessage.callCount).to.be(6); @@ -1079,6 +1120,8 @@ describe('shuffleVideo', function () { expect(commands).to.contain('getCurrentTabId'); expect(commands).to.contain('getAPIKey'); expect(commands).to.contain('updatePlaylistInfoInDB'); + const updateMessages = messages.filter(arg => arg[0].command === 'updatePlaylistInfoInDB'); + checkPlaylistsUploadedToDB(updateMessages, input); }); } });