diff --git a/src/backend/src/helpers.js b/src/backend/src/helpers.js index b70c11c4d..da2862220 100644 --- a/src/backend/src/helpers.js +++ b/src/backend/src/helpers.js @@ -1862,7 +1862,7 @@ async function get_taskbar_items (user, { icon_size, no_icons } = {}) { { name: 'camera', type: 'app' }, { name: 'recorder', type: 'app' }, ]; - await db.write('UPDATE user SET taskbar_items = ? WHERE id = ?', + db.write('UPDATE user SET taskbar_items = ? WHERE id = ?', [ JSON.stringify(taskbar_items_from_db), user.id, diff --git a/src/backend/src/modules/data-access/AppService.js b/src/backend/src/modules/data-access/AppService.js index 90c0eeb27..35e874673 100644 --- a/src/backend/src/modules/data-access/AppService.js +++ b/src/backend/src/modules/data-access/AppService.js @@ -109,27 +109,31 @@ export default class AppService extends BaseService { const userCanEditOnly = Array.prototype.includes.call(predicate, 'user-can-edit'); - const sql_associatedFiletypes = this.db.case({ - mysql: 'COALESCE(JSON_ARRAYAGG(afa.type), JSON_ARRAY())', - sqlite: "COALESCE(json_group_array(afa.type), json('[]'))", - }); - const stmt = 'SELECT apps.*, ' + 'owner_user.username AS owner_user_username, ' + 'owner_user.uuid AS owner_user_uuid, ' + - 'app_owner.uid AS app_owner_uid, ' + - `${sql_associatedFiletypes} AS filetypes ` + + 'app_owner.uid AS app_owner_uid ' + 'FROM apps ' + 'LEFT JOIN user owner_user ON apps.owner_user_id = owner_user.id ' + 'LEFT JOIN apps app_owner ON apps.app_owner = app_owner.id ' + - 'LEFT JOIN app_filetype_association afa ON apps.id = afa.app_id ' + `${userCanEditOnly ? 'WHERE apps.owner_user_id=?' : ''} ` + - 'GROUP BY apps.id ' + 'LIMIT 5000'; const values = userCanEditOnly ? [Context.get('user').id] : []; const rows = await db.read(stmt, values); - const client_safe_apps = []; + const shouldFetchFiletypes = rows.some(row => typeof row.filetypes !== 'string'); + const filetypesByAppId = shouldFetchFiletypes + ? await this.#getFiletypeAssociationsByAppIds(rows.map(row => row.id)) + : new Map(); + + const svc_appIcon = params.icon_size + ? this.context.get('services').get('app-icon') + : null; + const svc_error = params.icon_size + ? this.context.get('services').get('error-service') + : null; + + const appAndOwnerIds = []; for ( const row of rows ) { const app = {}; @@ -166,29 +170,20 @@ export default class AppService extends BaseService { app.owner = user_to_client(owner_user); } - if ( typeof row.filetypes === 'string' ) { - try { - let filetypesAsJSON = JSON.parse(row.filetypes); - filetypesAsJSON = filetypesAsJSON.filter(ft => ft !== null); - for ( let i = 0 ; i < filetypesAsJSON.length ; i++ ) { - if ( typeof filetypesAsJSON[i] !== 'string' ) { - throw new Error(`expected filetypesAsJSON[${i}] to be a string, got: ${filetypesAsJSON[i]}`); - } - if ( String.prototype.startsWith.call(filetypesAsJSON[i], '.') ) { - filetypesAsJSON[i] = filetypesAsJSON[i].slice(1); - } - } - app.filetype_associations = filetypesAsJSON; - } catch (e) { - throw new Error(`failed to get app filetype associations: ${e.message}`, { cause: e }); + try { + if ( typeof row.filetypes === 'string' ) { + app.filetype_associations = this.#parseFiletypeAssociationsJson(row.filetypes); + } else { + app.filetype_associations = this.#normalizeFiletypeAssociations(filetypesByAppId.get(row.id) ?? []); } + } catch (e) { + throw new Error(`failed to get app filetype associations: ${e.message}`, { cause: e }); } // REFINED BY OTHER DATA // app.icon; - if ( params.icon_size ) { + if ( params.icon_size && svc_appIcon ) { const icon_size = params.icon_size; - const svc_appIcon = this.context.get('services').get('app-icon'); try { const iconPath = svc_appIcon.getAppIconPath({ appUid: row.uid, @@ -198,21 +193,25 @@ export default class AppService extends BaseService { app.icon = iconPath; } } catch (e) { - const svc_error = this.context.get('services').get('error-service'); - svc_error.report('AppES:read_transform', { source: e }); + svc_error?.report('AppES:read_transform', { source: e }); } } - // Check protected app access before adding to results - if ( await this.#check_protected_app_access(app, row.owner_user_id) ) { - // App should be filtered out (not accessible) - continue; - } - - client_safe_apps.push(app); + appAndOwnerIds.push({ + app, + ownerUserId: row.owner_user_id, + }); } - return client_safe_apps; + // Check protected app access in parallel for faster large selections. + const allowed_apps = await Promise.all(appAndOwnerIds.map(async ({ app, ownerUserId }) => { + if ( await this.#check_protected_app_access(app, ownerUserId) ) { + return null; + } + return app; + })); + + return allowed_apps.filter(Boolean); } async #read ({ uid, id, params = {}, backend_only_options = {} }) { @@ -222,11 +221,6 @@ export default class AppService extends BaseService { throw new Error('read requires either uid or id'); } - const sql_associatedFiletypes = this.db.case({ - mysql: 'COALESCE(JSON_ARRAYAGG(afa.type), JSON_ARRAY())', - sqlite: "COALESCE(json_group_array(afa.type), json('[]'))", - }); - // Build WHERE clause based on identifier type let whereClause; let whereValues; @@ -247,14 +241,11 @@ export default class AppService extends BaseService { const stmt = 'SELECT apps.*, ' + 'owner_user.username AS owner_user_username, ' + 'owner_user.uuid AS owner_user_uuid, ' + - 'app_owner.uid AS app_owner_uid, ' + - `${sql_associatedFiletypes} AS filetypes ` + + 'app_owner.uid AS app_owner_uid ' + 'FROM apps ' + 'LEFT JOIN user owner_user ON apps.owner_user_id = owner_user.id ' + 'LEFT JOIN apps app_owner ON apps.app_owner = app_owner.id ' + - 'LEFT JOIN app_filetype_association afa ON apps.id = afa.app_id ' + `WHERE ${whereClause} ` + - 'GROUP BY apps.id ' + 'LIMIT 1'; const rows = await db.read(stmt, whereValues); @@ -296,22 +287,28 @@ export default class AppService extends BaseService { else app.owner = user_to_client(owner_user); } - if ( typeof row.filetypes === 'string' ) { - try { - let filetypesAsJSON = JSON.parse(row.filetypes); - filetypesAsJSON = filetypesAsJSON.filter(ft => ft !== null); - for ( let i = 0 ; i < filetypesAsJSON.length ; i++ ) { - if ( typeof filetypesAsJSON[i] !== 'string' ) { - throw new Error(`expected filetypesAsJSON[${i}] to be a string, got: ${filetypesAsJSON[i]}`); - } - if ( String.prototype.startsWith.call(filetypesAsJSON[i], '.') ) { - filetypesAsJSON[i] = filetypesAsJSON[i].slice(1); - } - } - app.filetype_associations = filetypesAsJSON; - } catch (e) { - throw new Error(`failed to get app filetype associations: ${e.message}`, { cause: e }); + let protectedAccessPromise; + try { + if ( typeof row.filetypes === 'string' ) { + app.filetype_associations = this.#parseFiletypeAssociationsJson(row.filetypes); + } else { + protectedAccessPromise = this.#check_protected_app_access(app, row.owner_user_id); + const filetypeAssociations = await this.#getFiletypeAssociationsByAppId(row.id); + app.filetype_associations = this.#normalizeFiletypeAssociations(filetypeAssociations); } + } catch (e) { + throw new Error(`failed to get app filetype associations: ${e.message}`, { cause: e }); + } + + // Check protected app access as soon as dependent fields are resolved. + if ( ! protectedAccessPromise ) { + protectedAccessPromise = this.#check_protected_app_access(app, row.owner_user_id); + } + if ( await protectedAccessPromise ) { + // App should not be accessible + throw APIError.create('entity_not_found', null, { + identifier: uid || JSON.stringify(id), + }); } if ( params.icon_size ) { @@ -331,15 +328,64 @@ export default class AppService extends BaseService { } } - // Check protected app access - if ( await this.#check_protected_app_access(app, row.owner_user_id) ) { - // App should not be accessible - throw APIError.create('entity_not_found', null, { - identifier: uid || JSON.stringify(id), - }); + return app; + } + + #parseFiletypeAssociationsJson (filetypes) { + return this.#normalizeFiletypeAssociations(JSON.parse(filetypes)); + } + + async #getFiletypeAssociationsByAppId (appId) { + if ( appId === undefined || appId === null ) return []; + + const rows = await this.db.read('SELECT type FROM app_filetype_association WHERE app_id = ?', + [appId]); + return rows + .map(row => row.type) + .filter(type => typeof type === 'string' || type === null); + } + + #normalizeFiletypeAssociations (filetypesAsJSON) { + filetypesAsJSON = Array.isArray(filetypesAsJSON) + ? filetypesAsJSON + : []; + filetypesAsJSON = filetypesAsJSON.filter(ft => ft !== null); + for ( let i = 0 ; i < filetypesAsJSON.length ; i++ ) { + if ( typeof filetypesAsJSON[i] !== 'string' ) { + throw new Error(`expected filetypesAsJSON[${i}] to be a string, got: ${filetypesAsJSON[i]}`); + } + if ( String.prototype.startsWith.call(filetypesAsJSON[i], '.') ) { + filetypesAsJSON[i] = filetypesAsJSON[i].slice(1); + } + } + return filetypesAsJSON; + } + + async #getFiletypeAssociationsByAppIds (appIds) { + appIds = [...new Set(appIds.filter(appId => appId !== undefined && appId !== null))]; + if ( appIds.length === 0 ) return new Map(); + + const filetypesByAppId = new Map(); + for ( const appId of appIds ) { + filetypesByAppId.set(appId, []); } - return app; + // SQLite has a low bind-parameter limit; chunk to avoid oversized IN lists. + const chunkSize = 500; + for ( let i = 0 ; i < appIds.length ; i += chunkSize ) { + const chunk = appIds.slice(i, i + chunkSize); + const placeholders = chunk.map(() => '?').join(', '); + const rows = await this.db.read(`SELECT app_id, type FROM app_filetype_association WHERE app_id IN (${placeholders})`, + chunk); + for ( const row of rows ) { + if ( ! filetypesByAppId.has(row.app_id) ) { + filetypesByAppId.set(row.app_id, []); + } + filetypesByAppId.get(row.app_id).push(row.type); + } + } + + return filetypesByAppId; } async #create ({ object, options }) { diff --git a/src/backend/src/modules/data-access/AppService.test.js b/src/backend/src/modules/data-access/AppService.test.js index b918940a3..f859e6ba4 100644 --- a/src/backend/src/modules/data-access/AppService.test.js +++ b/src/backend/src/modules/data-access/AppService.test.js @@ -133,6 +133,7 @@ describe('AppService', () => { // Mock permission service mockPermissionService = { check: vi.fn().mockResolvedValue(false), + scan: vi.fn().mockResolvedValue([]), }; // Mock puter-site service @@ -185,13 +186,13 @@ describe('AppService', () => { describe('#read', () => { it('should read an app by uid', async () => { const mockRow = createMockAppRow(); - mockDb.read.mockResolvedValue([mockRow]); + mockDb.read.mockResolvedValueOnce([mockRow]); const crudQ = AppService.IMPLEMENTS['crud-q']; const result = await crudQ.read.call(appService, { uid: 'app-uid-123' }); expect(mockDb.read).toHaveBeenCalledTimes(1); - expect(mockDb.read).toHaveBeenCalledWith( + expect(mockDb.read).toHaveBeenNthCalledWith(1, expect.stringContaining('WHERE apps.uid = ?'), ['app-uid-123']); expect(result).toBeDefined(); @@ -202,13 +203,13 @@ describe('AppService', () => { it('should read an app by complex id (name)', async () => { const mockRow = createMockAppRow(); - mockDb.read.mockResolvedValue([mockRow]); + mockDb.read.mockResolvedValueOnce([mockRow]); const crudQ = AppService.IMPLEMENTS['crud-q']; const result = await crudQ.read.call(appService, { id: { name: 'test-app' } }); expect(mockDb.read).toHaveBeenCalledTimes(1); - expect(mockDb.read).toHaveBeenCalledWith( + expect(mockDb.read).toHaveBeenNthCalledWith(1, expect.stringContaining('WHERE apps.name = ?'), ['test-app']); expect(result).toBeDefined(); @@ -266,24 +267,47 @@ describe('AppService', () => { const mockRow = createMockAppRow({ filetypes: '[".txt", ".doc", "pdf"]', }); - mockDb.read.mockResolvedValue([mockRow]); + mockDb.read.mockResolvedValueOnce([mockRow]); const crudQ = AppService.IMPLEMENTS['crud-q']; const result = await crudQ.read.call(appService, { uid: 'app-uid-123' }); expect(result.filetype_associations).toEqual(['txt', 'doc', 'pdf']); + expect(mockDb.read).toHaveBeenCalledTimes(1); }); it('should filter out null values in filetypes array', async () => { const mockRow = createMockAppRow({ filetypes: '[".txt", null, "pdf"]', }); - mockDb.read.mockResolvedValue([mockRow]); + mockDb.read.mockResolvedValueOnce([mockRow]); const crudQ = AppService.IMPLEMENTS['crud-q']; const result = await crudQ.read.call(appService, { uid: 'app-uid-123' }); expect(result.filetype_associations).toEqual(['txt', 'pdf']); + expect(mockDb.read).toHaveBeenCalledTimes(1); + }); + + it('should query filetype associations table when filetypes JSON is missing', async () => { + const mockRow = createMockAppRow({ filetypes: null }); + mockDb.read + .mockResolvedValueOnce([mockRow]) + .mockResolvedValueOnce([ + { type: '.txt' }, + { type: null }, + { type: 'pdf' }, + ]); + + const crudQ = AppService.IMPLEMENTS['crud-q']; + const result = await crudQ.read.call(appService, { uid: 'app-uid-123' }); + + expect(result.filetype_associations).toEqual(['txt', 'pdf']); + expect(mockDb.read).toHaveBeenCalledTimes(2); + expect(mockDb.read).toHaveBeenNthCalledWith( + 2, + 'SELECT type FROM app_filetype_association WHERE app_id = ?', + [mockRow.id]); }); it('should have owner parameter', async () => { @@ -567,16 +591,13 @@ describe('AppService', () => { 'failed to get app filetype associations'); }); - it('should use database case for SQL dialect differences', async () => { + it('should not require dialect-specific JSON aggregation for app selection', async () => { mockDb.read.mockResolvedValue([]); const crudQ = AppService.IMPLEMENTS['crud-q']; await crudQ.select.call(appService, {}); - expect(mockDb.case).toHaveBeenCalledWith({ - mysql: expect.stringContaining('JSON_ARRAYAGG'), - sqlite: expect.stringContaining('json_group_array'), - }); + expect(mockDb.case).not.toHaveBeenCalled(); }); }); @@ -612,7 +633,7 @@ describe('AppService', () => { })]); const crudQ = AppService.IMPLEMENTS['crud-q']; - const result = await crudQ.create.call(appService, { + await crudQ.create.call(appService, { object: { name: 'new-app', title: 'New App', @@ -920,7 +941,7 @@ describe('AppService', () => { setupContextForWrite(createMockUserActor(1)); const crudQ = AppService.IMPLEMENTS['crud-q']; - const result = await crudQ.update.call(appService, { + await crudQ.update.call(appService, { object: { uid: 'app-uid-123', title: 'Updated Title' }, }); diff --git a/src/backend/src/services/PuterAPIService.js b/src/backend/src/services/PuterAPIService.js index 96b168f6d..612a40a55 100644 --- a/src/backend/src/services/PuterAPIService.js +++ b/src/backend/src/services/PuterAPIService.js @@ -39,7 +39,9 @@ class PuterAPIService extends BaseService { * It does not return a value as it configures the server directly. */ async ['__on_install.routes'] () { - const { app } = this.services.get('web-server'); + const svc_web = this.services.get('web-server'); + const { app } = svc_web; + svc_web.allow_undefined_origin('/healthcheck'); app.use(require('../routers/apps')); app.use(require('../routers/query/app'));