Skip to content

Commit 07e44fe

Browse files
authored
Improve playlist import performance (#8062)
1 parent b9599e7 commit 07e44fe

File tree

3 files changed

+106
-65
lines changed

3 files changed

+106
-65
lines changed

src/renderer/components/DataSettings.vue

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import {
8484
showToast,
8585
writeFileWithPicker,
8686
} from '../helpers/utils'
87+
import { processToBeAddedPlaylistVideo } from '../helpers/playlists'
8788
8889
const IMPORT_DIRECTORY_ID = 'data-settings-import'
8990
const START_IN_DIRECTORY = 'downloads'
@@ -1019,6 +1020,8 @@ async function importPlaylists() {
10191020
'videoCount',
10201021
]
10211022
1023+
const knownKeys = [...requiredKeys, ...optionalKeys, ...ignoredKeys]
1024+
10221025
const requiredVideoKeys = [
10231026
'videoId',
10241027
'title',
@@ -1033,16 +1036,19 @@ async function importPlaylists() {
10331036
// 'playlistItemId',
10341037
]
10351038
1039+
const newPlaylists = []
1040+
10361041
playlists.forEach((playlistData) => {
10371042
// We would technically already be done by the time the data is parsed,
10381043
// however we want to limit the possibility of malicious data being sent
10391044
// to the app, so we'll only grab the data we need here.
10401045
10411046
const playlistObject = {}
10421047
const videoIdToBeAddedSet = new Set()
1048+
let countRequiredKeysPresent = 0
10431049
10441050
Object.keys(playlistData).forEach((key) => {
1045-
if ([requiredKeys, optionalKeys, ignoredKeys].every((ks) => !ks.includes(key))) {
1051+
if (!knownKeys.includes(key)) {
10461052
const message = `${t('Settings.Data Settings.Unknown data key')}: ${key}`
10471053
showToast(message)
10481054
} else if (key === 'videos') {
@@ -1057,17 +1063,22 @@ async function importPlaylists() {
10571063
}
10581064
})
10591065
1060-
playlistObject[key] = videoArray
1066+
playlistObject.videos = videoArray
1067+
1068+
if (requiredKeys.includes(key)) {
1069+
countRequiredKeysPresent++
1070+
}
10611071
} else if (!ignoredKeys.includes(key)) {
10621072
// Do nothing for keys to be ignored
10631073
playlistObject[key] = playlistData[key]
1074+
1075+
if (requiredKeys.includes(key)) {
1076+
countRequiredKeysPresent++
1077+
}
10641078
}
10651079
})
10661080
1067-
const playlistObjectKeys = Object.keys(playlistObject)
1068-
const playlistObjectHasAllRequiredKeys = requiredKeys.every((k) => playlistObjectKeys.includes(k))
1069-
1070-
if (!playlistObjectHasAllRequiredKeys) {
1081+
if (countRequiredKeysPresent !== requiredKeys.length) {
10711082
const message = t('Settings.Data Settings.Playlist insufficient data', { playlist: playlistData.playlistName })
10721083
showToast(message)
10731084
return
@@ -1082,47 +1093,58 @@ async function importPlaylists() {
10821093
})
10831094
10841095
if (existingPlaylist === undefined) {
1085-
store.dispatch('addPlaylist', playlistObject)
1096+
newPlaylists.push(playlistObject)
10861097
return
10871098
}
10881099
1089-
const duplicateVideoPresentInToBeAdded = playlistObject.videos.length > videoIdToBeAddedSet.size
1090-
const existingVideoIdSet = existingPlaylist.videos.reduce((video) => videoIdToBeAddedSet.add(video.videoId), new Set())
1091-
const duplicateVideoPresentInExistingPlaylist = existingPlaylist.videos.length > existingVideoIdSet.size
1092-
const shouldAddDuplicateVideos = duplicateVideoPresentInToBeAdded || duplicateVideoPresentInExistingPlaylist
1100+
/** @type {Set<string> | undefined} */
1101+
let existingVideoIdSet
1102+
1103+
let shouldAddDuplicateVideos = playlistObject.videos.length > videoIdToBeAddedSet.size
1104+
1105+
if (!shouldAddDuplicateVideos) {
1106+
existingVideoIdSet = existingPlaylist.videos.reduce((set, video) => set.add(video.videoId), new Set())
1107+
shouldAddDuplicateVideos = existingPlaylist.videos.length > existingVideoIdSet.size
1108+
}
1109+
1110+
const playlistVideos = [...existingPlaylist.videos]
10931111
10941112
playlistObject.videos.forEach((video) => {
10951113
let videoExists = false
10961114
if (shouldAddDuplicateVideos) {
10971115
if (video.playlistItemId != null) {
10981116
// Find by `playlistItemId` if present
1099-
videoExists = existingPlaylist.videos.some((x) => {
1117+
videoExists = playlistVideos.some((x) => {
11001118
// Allow duplicate (by videoId) videos to be added
11011119
return x.videoId === video.videoId && x.playlistItemId === video.playlistItemId
11021120
})
11031121
} else {
11041122
// Older playlist exports have no `playlistItemId` but have `timeAdded`
11051123
// Which might be duplicate for copied playlists with duplicate `videoId`
1106-
videoExists = existingPlaylist.videos.some((x) => {
1124+
videoExists = playlistVideos.some((x) => {
11071125
// Allow duplicate (by videoId) videos to be added
11081126
return x.videoId === video.videoId && x.timeAdded === video.timeAdded
11091127
})
11101128
}
1129+
} else if (existingVideoIdSet !== undefined) {
1130+
// Disallow duplicate (by videoId) videos to be added
1131+
1132+
if (existingVideoIdSet.has(video.videoId)) {
1133+
videoExists = true
1134+
} else {
1135+
existingVideoIdSet.add(video.videoId)
1136+
}
11111137
} else {
1112-
videoExists = existingPlaylist.videos.some((x) => {
1138+
videoExists = playlistVideos.some((x) => {
11131139
// Disallow duplicate (by videoId) videos to be added
11141140
return x.videoId === video.videoId
11151141
})
11161142
}
11171143
11181144
if (!videoExists) {
11191145
// Keep original `timeAdded` value
1120-
const payload = {
1121-
_id: existingPlaylist._id,
1122-
videoData: video,
1123-
}
1124-
1125-
store.dispatch('addVideo', payload)
1146+
processToBeAddedPlaylistVideo(video)
1147+
playlistVideos.push(video)
11261148
}
11271149
})
11281150
// Update playlist's `lastUpdatedAt` & other attributes
@@ -1131,9 +1153,14 @@ async function importPlaylists() {
11311153
// Only these attributes would be updated (besides videos)
11321154
playlistName: playlistObject.playlistName,
11331155
description: playlistObject.description,
1156+
videos: playlistVideos
11341157
})
11351158
})
11361159
1160+
if (newPlaylists.length > 0) {
1161+
store.dispatch('addPlaylists', newPlaylists)
1162+
}
1163+
11371164
showToast(t('Settings.Data Settings.All playlists has been successfully imported'))
11381165
}
11391166

src/renderer/helpers/playlists.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,25 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) {
8484
return 0
8585
}
8686
}
87+
88+
export const generateRandomUniqueId = crypto.randomUUID
89+
? crypto.randomUUID
90+
: () => `id-${Date.now()}-${Math.floor(Math.random() * 10000)}`
91+
92+
/**
93+
* @param {any} videoData
94+
*/
95+
export function processToBeAddedPlaylistVideo(videoData) {
96+
if (videoData.timeAdded == null) {
97+
videoData.timeAdded = Date.now()
98+
}
99+
100+
if (videoData.playlistItemId == null) {
101+
videoData.playlistItemId = generateRandomUniqueId()
102+
}
103+
104+
// For backward compatibility
105+
if (videoData.type == null) {
106+
videoData.type = 'video'
107+
}
108+
}

src/renderer/store/modules/playlists.js

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { DBPlaylistHandlers } from '../../../datastores/handlers/index'
2+
import { generateRandomUniqueId, processToBeAddedPlaylistVideo } from '../../helpers/playlists'
23

34
function generateRandomPlaylistId() {
45
return `ft-playlist--${generateRandomUniqueId()}`
@@ -8,11 +9,6 @@ function generateRandomPlaylistName() {
89
return `Playlist ${new Date().toISOString()}-${Math.floor(Math.random() * 10000)}`
910
}
1011

11-
function generateRandomUniqueId() {
12-
// To avoid importing `crypto` from NodeJS
13-
return crypto.randomUUID ? crypto.randomUUID() : `id-${Date.now()}-${Math.floor(Math.random() * 10000)}`
14-
}
15-
1612
/*
1713
* Function to find the first playlist with 0 videos, or otherwise the most recently accessed.
1814
* This is a good default quick bookmark target if one needs to be set.
@@ -33,6 +29,36 @@ function findEmptyOrLatestPlayedPlaylist(playlists) {
3329
return playlists[maxIndex]
3430
}
3531

32+
/**
33+
* @param {any} playlist
34+
*/
35+
function processNewPlayist(playlist) {
36+
// In case internal id is forgotten, generate one (instead of relying on caller and there being a chance of data corruption)
37+
if (playlist._id == null) {
38+
// {Time now in unix time}-{0-9999}
39+
playlist._id = generateRandomPlaylistId()
40+
}
41+
42+
// Ensure playlist name trimmed
43+
if (typeof playlist.playlistName === 'string') {
44+
playlist.playlistName = playlist.playlistName.trim()
45+
}
46+
47+
// Ensure playlist description trimmed
48+
if (typeof playlist.description === 'string') {
49+
playlist.description = playlist.description.trim()
50+
}
51+
52+
const now = Date.now()
53+
playlist.createdAt = now
54+
playlist.lastUpdatedAt = now
55+
56+
// Ensure all videos have required attributes
57+
if (Array.isArray(playlist.videos)) {
58+
playlist.videos.forEach(processToBeAddedPlaylistVideo)
59+
}
60+
}
61+
3662
const state = {
3763
// Playlist loading takes time on app load (new windows)
3864
// This is necessary to let components to know when to start data loading
@@ -76,35 +102,7 @@ const getters = {
76102

77103
const actions = {
78104
async addPlaylist({ state, commit, rootState, dispatch }, payload) {
79-
// In case internal id is forgotten, generate one (instead of relying on caller and have a chance to cause data corruption)
80-
if (payload._id == null) {
81-
// {Time now in unix time}-{0-9999}
82-
payload._id = generateRandomPlaylistId()
83-
}
84-
// Ensure playlist name trimmed
85-
if (typeof payload.playlistName === 'string') {
86-
payload.playlistName = payload.playlistName.trim()
87-
}
88-
// Ensure playlist description trimmed
89-
if (typeof payload.description === 'string') {
90-
payload.description = payload.description.trim()
91-
}
92-
payload.createdAt = Date.now()
93-
payload.lastUpdatedAt = Date.now()
94-
// Ensure all videos has required attributes
95-
96-
const currentTime = Date.now()
97-
98-
if (Array.isArray(payload.videos)) {
99-
payload.videos.forEach(videoData => {
100-
if (videoData.timeAdded == null) {
101-
videoData.timeAdded = currentTime
102-
}
103-
if (videoData.playlistItemId == null) {
104-
videoData.playlistItemId = generateRandomUniqueId()
105-
}
106-
})
107-
}
105+
processNewPlayist(payload)
108106

109107
try {
110108
await DBPlaylistHandlers.create([payload])
@@ -121,6 +119,8 @@ const actions = {
121119
},
122120

123121
async addPlaylists({ state, commit, rootState, dispatch }, payload) {
122+
payload.forEach(processNewPlayist)
123+
124124
try {
125125
await DBPlaylistHandlers.create(payload)
126126

@@ -172,16 +172,8 @@ const actions = {
172172
async addVideo({ commit }, payload) {
173173
try {
174174
const { _id, videoData } = payload
175-
if (videoData.timeAdded == null) {
176-
videoData.timeAdded = Date.now()
177-
}
178-
if (videoData.playlistItemId == null) {
179-
videoData.playlistItemId = generateRandomUniqueId()
180-
}
181-
// For backward compatibility
182-
if (videoData.type == null) {
183-
videoData.type = 'video'
184-
}
175+
176+
processToBeAddedPlaylistVideo(videoData)
185177

186178
const lastUpdatedAt = Date.now()
187179

@@ -452,7 +444,7 @@ const mutations = {
452444
},
453445

454446
addPlaylists(state, payload) {
455-
state.playlists = state.playlists.concat(payload)
447+
state.playlists.push(...payload)
456448
},
457449

458450
upsertPlaylistToList(state, updatedPlaylist) {

0 commit comments

Comments
 (0)