From 6e9e5371c874cb4508ed6f2546ead405c95a97fe Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Wed, 21 Jan 2026 18:17:15 -0800 Subject: [PATCH] fix: batch fetch get apps for launch apps and recommended apps (#2315) * wip: batch fetch get apps for launch apps and recommended apps * fix: boot issues * fix: bad refresh app cache * tmp: remove test for now since can't mock db call --- package-lock.json | 4 +- src/backend/src/helpers.js | 242 +++++++++++------- src/backend/src/modules/apps/AppsModule.js | 2 +- .../modules/apps/RecommendedAppsService.js | 19 +- .../src/modules/data-access/AppService.js | 37 +-- .../modules/data-access/AppService.test.js | 209 ++++++--------- src/backend/src/om/entitystorage/AppES.js | 27 +- src/backend/src/routers/apps.js | 54 ++-- .../src/routers/auth/list-permissions.js | 21 +- src/backend/src/routers/get-launch-apps.js | 20 +- .../src/routers/get-launch-apps.test.js | 223 ---------------- .../src/services/PermissionAPIService.js | 2 +- src/backend/src/services/PuterAPIService.js | 2 +- 13 files changed, 273 insertions(+), 589 deletions(-) delete mode 100644 src/backend/src/routers/get-launch-apps.test.js diff --git a/package-lock.json b/package-lock.json index 98801c416..f26592576 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8252,7 +8252,9 @@ } }, "node_modules/diff": { - "version": "5.2.0", + "version": "5.2.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-5.2.2.tgz", + "integrity": "sha512-vtcDfH3TOjP8UekytvnHH1o1P4FcUdt4eQ1Y+Abap1tk/OB2MWQvcwS2ClCd1zuIhc3JKOx6p3kod8Vfys3E+A==", "dev": true, "license": "BSD-3-Clause", "engines": { diff --git a/src/backend/src/helpers.js b/src/backend/src/helpers.js index 8d2203d57..1429b2202 100644 --- a/src/backend/src/helpers.js +++ b/src/backend/src/helpers.js @@ -26,7 +26,6 @@ const APIError = require('./api/APIError.js'); const { DB_READ, DB_WRITE } = require('./services/database/consts.js'); const { Context } = require('./util/context'); const { NodeUIDSelector } = require('./filesystem/node/selectors'); -const { object_returned_by_get_app } = require('./annotatedobjects.js'); const { kv } = require('./util/kvSingleton'); const identifying_uuid = require('uuid').v4(); @@ -68,37 +67,6 @@ async function is_empty (dir_uuid) { return !rows[0].not_empty; } -/** - * @deprecated - sharing will be implemented with user-to-user ACL - */ -async function has_shared_with (user_id, recipient_user_id) { - return false; -} - -/** - * Checks to see if this file/directory is shared with the user identified by `recipient_user_id` - * - * @param {*} fsentry_id - * @param {*} recipient_user_id - * - * @deprecated - sharing will be implemented with user-to-user ACL - */ -async function is_shared_with (fsentry_id, recipient_user_id) { - return false; -} - -/** - * Checks to see if this file/directory is shared with at least one other user - * - * @param {*} fsentry_id - * @param {*} recipient_user_id - * - * @deprecated - sharing will be implemented with user-to-user ACL - */ -async function is_shared_with_anyone (fsentry_id) { - return false; -} - /** * Checks to see if temp_users is disabled and return a boolean * @returns {boolean} @@ -133,12 +101,8 @@ const chkperm = spanify('chkperm', async (target_fsentry, requester_user_id, act if ( target_fsentry.user_id === requester_user_id ) { return true; } - // this entry was shared with the requester - else if ( await is_shared_with(target_fsentry.id, requester_user_id) ) { - return true; - } // special case: owner of entry has shared at least one entry with requester and requester is asking for the owner's root directory: /[owner_username] - else if ( target_fsentry.parent_uid === null && await has_shared_with(target_fsentry.user_id, requester_user_id) && action !== 'write' ) + else if ( target_fsentry.parent_uid === null && action !== 'write' ) { return true; } @@ -263,16 +227,6 @@ function invalidate_cached_user_by_id (id) { invalidate_cached_user(user); } -/** - * Refresh apps cache - * - * @param {string} options - `options` - * @returns {Promise} - */ -async function refresh_apps_cache (options, override) { - return; -} - async function refresh_associations_cache () { /** @type BaseDatabaseAccessService */ const db = _servicesHolder.services.get('database').get(DB_READ, 'apps'); @@ -310,8 +264,6 @@ async function get_app (options) { kv.set(`apps:id:${app.id}`, app, { EX: 30 }); }; - const log = _servicesHolder.services.get('log-service').create('get_app'); - // This condition should be updated if the code below is re-ordered. if ( options.follow_old_names && !options.uid && options.name ) { const svc_oldAppName = _servicesHolder.services.get('old-app-name'); @@ -400,6 +352,129 @@ async function get_app (options) { return app; } +/** + * Get multiple apps by uid/name/id, aligned to the input order. + * + * @param {Array<{uid?: string, name?: string, id?: string|number}>} specifiers + * @param {Object} [options] + * @returns {Promise>} + */ +async function get_apps (specifiers, options = {}) { + if ( ! Array.isArray(specifiers) ) { + specifiers = [specifiers]; + } + + const cacheApp = (app) => { + if ( ! app ) return; + app = { ...app }; + kv.set(`apps:uid:${app.uid}`, app, { EX: 30 }); + kv.set(`apps:name:${app.name}`, app, { EX: 30 }); + kv.set(`apps:id:${app.id}`, app, { EX: 30 }); + }; + + const normalized = specifiers.map(spec => spec ? { ...spec } : {}); + + if ( options.follow_old_names ) { + const svc_oldAppName = _servicesHolder.services.get('old-app-name'); + for ( const spec of normalized ) { + if ( spec.uid || !spec.name ) continue; + const old_name = await svc_oldAppName.check_app_name(spec.name); + if ( old_name ) { + spec.uid = old_name.app_uid; + delete spec.name; + } + } + } + + const appByUid = new Map(); + const appByName = new Map(); + const appById = new Map(); + + const addApp = (app) => { + if ( ! app ) return; + appByUid.set(app.uid, app); + appByName.set(app.name, app); + appById.set(app.id, app); + }; + + const missingUids = new Set(); + const missingNames = new Set(); + const missingIds = new Set(); + + for ( const spec of normalized ) { + if ( spec.uid ) { + const cached = kv.get(`apps:uid:${spec.uid}`); + if ( cached ) { + addApp(cached); + } else { + missingUids.add(spec.uid); + } + continue; + } + if ( spec.name ) { + const cached = kv.get(`apps:name:${spec.name}`); + if ( cached ) { + addApp(cached); + } else { + missingNames.add(spec.name); + } + continue; + } + if ( spec.id ) { + const cached = kv.get(`apps:id:${spec.id}`); + if ( cached ) { + addApp(cached); + } else { + missingIds.add(spec.id); + } + } + } + + if ( missingUids.size || missingNames.size || missingIds.size ) { + /** @type BaseDatabaseAccessService */ + const db = _servicesHolder.services.get('database').get(DB_READ, 'apps'); + + const clauses = []; + const params = []; + + if ( missingUids.size ) { + const uids = Array.from(missingUids); + clauses.push(`uid IN (${uids.map(() => '?').join(', ')})`); + params.push(...uids); + } + if ( missingNames.size ) { + const names = Array.from(missingNames); + clauses.push(`name IN (${names.map(() => '?').join(', ')})`); + params.push(...names); + } + if ( missingIds.size ) { + const ids = Array.from(missingIds); + clauses.push(`id IN (${ids.map(() => '?').join(', ')})`); + params.push(...ids); + } + + const rows = await db.read(`SELECT * FROM \`apps\` WHERE ${clauses.join(' OR ')}`, + params); + + for ( const app of rows ) { + cacheApp(app); + addApp(app); + } + } + + return normalized.map(spec => { + let app; + if ( spec.uid ) { + app = appByUid.get(spec.uid); + } else if ( spec.name ) { + app = appByName.get(spec.name); + } else if ( spec.id ) { + app = appById.get(spec.id); + } + return app ? { ...app } : null; + }); +} + /** * Checks to see if an app exists * @@ -957,33 +1032,10 @@ async function resolve_glob (glob, user) { }); } -/** - * Copies a FSEntry represented by `source_path` to `dest_path`. - * - * @param {string} source_path - * @param {string} dest_path - * @param {object} user - * @returns - */ -function cp (source_path, dest_path, user, overwrite, change_name, check_perms = true) { - throw new Error('legacy copy function called'); -} - function isString (variable) { return typeof variable === 'string' || variable instanceof String; } -/** - * Recusrively deletes all files under `path` - * - * @param {string} source_path - * @param {object} user - * @returns - */ -function rm (source_path, user, descendants_only = false) { - throw new Error('legacy remove function called'); -} - const body_parser_error_handler = (err, req, res, next) => { if ( err instanceof SyntaxError && err.status === 400 && 'body' in err ) { return res.status(400).send(err); // Bad request @@ -1100,7 +1152,7 @@ async function sign_file (fsentry, action) { }; } -async function gen_public_token (file_uuid, ttl = 24 * 60 * 60) { +async function gen_public_token (file_uuid) { const { v4: uuidv4 } = require('uuid'); // get fsentry @@ -1149,6 +1201,7 @@ async function deleteUser (user_id) { const svc_fs = _servicesHolder.services.get('filesystem'); // get a list of up to 5000 files owned by this user + // eslint-disable-next-line no-constant-condition for ( let offset = 0; true; offset += 5000 ) { let files = await db.read(`SELECT uuid, bucket, bucket_region FROM fsentries WHERE user_id = ? AND is_dir = 0 LIMIT 5000 OFFSET ${ offset}`, [user_id]); @@ -1571,6 +1624,23 @@ async function get_taskbar_items (user, { icon_size, no_icons } = {}) { } } + const app_specifiers = taskbar_items_from_db.map((taskbar_item_from_db) => { + if ( taskbar_item_from_db.type !== 'app' ) return {}; + if ( taskbar_item_from_db.name === 'explorer' ) return {}; + if ( taskbar_item_from_db.name ) { + return { name: taskbar_item_from_db.name }; + } + if ( taskbar_item_from_db.id ) { + return { id: taskbar_item_from_db.id }; + } + if ( taskbar_item_from_db.uid ) { + return { uid: taskbar_item_from_db.uid }; + } + return {}; + }); + + const taskbar_apps = await get_apps(app_specifiers); + // get apps that these taskbar items represent let taskbar_items = []; for ( let index = 0; index < taskbar_items_from_db.length; index++ ) { @@ -1578,19 +1648,7 @@ async function get_taskbar_items (user, { icon_size, no_icons } = {}) { if ( taskbar_item_from_db.type !== 'app' ) continue; if ( taskbar_item_from_db.name === 'explorer' ) continue; - let item = {}; - if ( taskbar_item_from_db.name ) - { - item = await get_app({ name: taskbar_item_from_db.name }); - } - else if ( taskbar_item_from_db.id ) - { - item = await get_app({ id: taskbar_item_from_db.id }); - } - else if ( taskbar_item_from_db.uid ) - { - item = await get_app({ uid: taskbar_item_from_db.uid }); - } + const item = taskbar_apps[index]; // if item not found, skip it if ( ! item ) continue; @@ -1682,10 +1740,6 @@ function get_url_from_req (req) { return `${req.protocol }://${ req.get('host') }${req.originalUrl}`; } -async function mv (options) { - throw new Error('legacy mv function called'); -} - /** * Formats a number with grouped thousands. * @@ -1729,7 +1783,6 @@ module.exports = { change_username, chkperm, convert_path_to_fsentry, - cp, deleteUser, get_descendants, get_dir_size, @@ -1738,28 +1791,23 @@ module.exports = { get_url_from_req, generate_random_str, get_app, + get_apps, get_user, invalidate_cached_user, invalidate_cached_user_by_id, - has_shared_with, hyphenize_confirm_code, id2fsentry, id2path, id2uuid, is_ancestor_of, is_empty, - is_shared_with, - is_shared_with_anyone, ...require('./validation'), is_temp_users_disabled, is_user_signup_disabled, jwt_auth, - mv, number_format, - refresh_apps_cache, refresh_associations_cache, resolve_glob, - rm, seconds_to_string, send_email_verification_code, send_email_verification_token, diff --git a/src/backend/src/modules/apps/AppsModule.js b/src/backend/src/modules/apps/AppsModule.js index c56329d6b..6500752e4 100644 --- a/src/backend/src/modules/apps/AppsModule.js +++ b/src/backend/src/modules/apps/AppsModule.js @@ -35,7 +35,7 @@ class AppsModule extends AdvancedBase { const { ProtectedAppService } = require('./ProtectedAppService'); services.registerService('__protected-app', ProtectedAppService); - const RecommendedAppsService = require('./RecommendedAppsService'); + const RecommendedAppsService = require('./RecommendedAppsService').default; services.registerService('recommended-apps', RecommendedAppsService); const { AppPermissionService } = require('./AppPermissionService'); diff --git a/src/backend/src/modules/apps/RecommendedAppsService.js b/src/backend/src/modules/apps/RecommendedAppsService.js index abd6082ae..2cbc1cb11 100644 --- a/src/backend/src/modules/apps/RecommendedAppsService.js +++ b/src/backend/src/modules/apps/RecommendedAppsService.js @@ -17,16 +17,11 @@ * along with this program. If not, see . */ -const { get_app } = require('../../helpers'); -const BaseService = require('../../services/BaseService'); +import { get_apps } from '../../helpers.js'; +import BaseService from '../../services/BaseService.js'; +import { kv } from '@heyputer/backend/src/util/kvSingleton.js'; -const get_apps = async ({ specifiers }) => { - return await Promise.all(specifiers.map(async (specifier) => { - return await get_app(specifier); - })); -}; - -class RecommendedAppsService extends BaseService { +export default class RecommendedAppsService extends BaseService { static APP_NAMES = [ 'app-center', 'dev-center', @@ -104,9 +99,7 @@ class RecommendedAppsService extends BaseService { // Prepare each app for returning to user by only returning the necessary fields // and adding them to the retobj array - recommended = (await get_apps({ - specifiers: Array.from(this.app_names).map(name => ({ name })), - })).filter(app => !!app).map(app => { + recommended = (await get_apps(Array.from(this.app_names).map(name => ({ name })))).filter(app => !!app).map(app => { return { uuid: app.uid, name: app.name, @@ -133,5 +126,3 @@ class RecommendedAppsService extends BaseService { return recommended; } } - -module.exports = RecommendedAppsService; diff --git a/src/backend/src/modules/data-access/AppService.js b/src/backend/src/modules/data-access/AppService.js index be1b38473..ec899a891 100644 --- a/src/backend/src/modules/data-access/AppService.js +++ b/src/backend/src/modules/data-access/AppService.js @@ -2,7 +2,7 @@ import { v4 as uuidv4 } from 'uuid'; import APIError from '../../api/APIError.js'; import config from '../../config.js'; -import { app_name_exists, refresh_apps_cache } from '../../helpers.js'; +import { app_name_exists } from '../../helpers.js'; import { AppUnderUserActorType, UserActorType } from '../../services/auth/Actor.js'; import { PermissionUtil } from '../../services/auth/permissionUtils.mjs'; import BaseService from '../../services/BaseService.js'; @@ -455,19 +455,6 @@ export default class AppService extends BaseService { await svc_event.emit('app.new-icon', event); } - // Update app cache - const raw_app = { - uuid: uid, - owner_user_id: user.id, - name: object.name, - title: object.title, - description: object.description, - icon: object.icon, - index_url: object.index_url, - maximize_on_start: object.maximize_on_start, - }; - refresh_apps_cache({ uid: raw_app.uuid }, raw_app); - // Return the created app return await this.#read({ uid }); } @@ -549,9 +536,6 @@ export default class AppService extends BaseService { const svc_appInformation = this.services.get('app-information'); await svc_appInformation.delete_app(old_app.uid); - // Invalidate app cache - refresh_apps_cache({ uid: old_app.uid }, null); - return { success: true, uid: old_app.uid }; } @@ -690,10 +674,6 @@ export default class AppService extends BaseService { // Emit events for icon/name changes await this.#emit_change_events(object, old_app); - // Update app cache - const merged_app = { ...old_app, ...object }; - this.#refresh_cache(merged_app, old_app); - const svc_event = this.services.get('event'); svc_event.emit('app.changed', { app_uid: old_app.uid, @@ -882,21 +862,6 @@ export default class AppService extends BaseService { } } - #refresh_cache (merged_app, old_app) { - const raw_app = { - uuid: merged_app.uid, - owner_user_id: old_app.owner?.id || old_app.owner, - name: merged_app.name, - title: merged_app.title, - description: merged_app.description, - icon: merged_app.icon, - index_url: merged_app.index_url, - maximize_on_start: merged_app.maximize_on_start, - }; - - refresh_apps_cache({ uid: raw_app.uuid }, raw_app); - } - #build_complex_id_where (id) { const id_keys = Object.keys(id); id_keys.sort(); diff --git a/src/backend/src/modules/data-access/AppService.test.js b/src/backend/src/modules/data-access/AppService.test.js index 8aaa25f5d..c59a8786e 100644 --- a/src/backend/src/modules/data-access/AppService.test.js +++ b/src/backend/src/modules/data-access/AppService.test.js @@ -11,13 +11,14 @@ vi.mock('../../util/context.js', () => ({ // Mock the helpers module vi.mock('../../helpers.js', () => ({ app_name_exists: vi.fn(), - refresh_apps_cache: vi.fn(), })); // Mock the Actor module vi.mock('../../services/auth/Actor.js', () => ({ - UserActorType: class UserActorType {}, - AppUnderUserActorType: class AppUnderUserActorType {}, + UserActorType: class UserActorType { + }, + AppUnderUserActorType: class AppUnderUserActorType { + }, })); // Mock the validation module @@ -39,12 +40,12 @@ vi.mock('../../config.js', () => ({ }, })); -import { app_name_exists, refresh_apps_cache } from '../../helpers.js'; +import { app_name_exists } from '../../helpers.js'; import { AppUnderUserActorType, UserActorType } from '../../services/auth/Actor.js'; import { Context } from '../../util/context.js'; import { validate_string, - validate_url + validate_url, } from './lib/validation.js'; describe('AppService', () => { @@ -112,7 +113,6 @@ describe('AppService', () => { // Reset helper mocks app_name_exists.mockResolvedValue(false); - refresh_apps_cache.mockReturnValue(undefined); // Mock database (read) mockDb = { @@ -192,9 +192,8 @@ describe('AppService', () => { expect(mockDb.read).toHaveBeenCalledTimes(1); expect(mockDb.read).toHaveBeenCalledWith( - expect.stringContaining('WHERE apps.uid = ?'), - ['app-uid-123'] - ); + expect.stringContaining('WHERE apps.uid = ?'), + ['app-uid-123']); expect(result).toBeDefined(); expect(result.uid).toBe('app-uid-123'); expect(result.name).toBe('test-app'); @@ -210,9 +209,8 @@ describe('AppService', () => { expect(mockDb.read).toHaveBeenCalledTimes(1); expect(mockDb.read).toHaveBeenCalledWith( - expect.stringContaining('WHERE apps.name = ?'), - ['test-app'] - ); + expect.stringContaining('WHERE apps.name = ?'), + ['test-app']); expect(result).toBeDefined(); expect(result.name).toBe('test-app'); }); @@ -229,18 +227,15 @@ describe('AppService', () => { it('should throw an error when neither uid nor id is provided', async () => { const crudQ = AppService.IMPLEMENTS['crud-q']; - + await expect(crudQ.read.call(appService, {})).rejects.toThrow( - 'read requires either uid or id' - ); + 'read requires either uid or id'); }); it('should throw an error for invalid complex id keys', async () => { const crudQ = AppService.IMPLEMENTS['crud-q']; - - await expect( - crudQ.read.call(appService, { id: { invalidKey: 'value' } }) - ).rejects.toThrow('Invalid complex id keys'); + + await expect(crudQ.read.call(appService, { id: { invalidKey: 'value' } })).rejects.toThrow('Invalid complex id keys'); }); it('should correctly coerce boolean fields from database', async () => { @@ -388,9 +383,8 @@ describe('AppService', () => { }); expect(mockErrorService.report).toHaveBeenCalledWith( - 'AppES:read_transform', - expect.objectContaining({ source: expect.any(Error) }) - ); + 'AppES:read_transform', + expect.objectContaining({ source: expect.any(Error) })); expect(result.icon).toBe('icon.png'); }); @@ -409,9 +403,8 @@ describe('AppService', () => { expect(mockDb.read).toHaveBeenCalledTimes(1); expect(mockDb.read).toHaveBeenCalledWith( - expect.not.stringContaining('WHERE'), - [] - ); + expect.not.stringContaining('WHERE'), + []); expect(result).toHaveLength(2); expect(result[0].uid).toBe('app-1'); expect(result[1].uid).toBe('app-2'); @@ -430,18 +423,15 @@ describe('AppService', () => { }); expect(mockDb.read).toHaveBeenCalledWith( - expect.stringContaining('WHERE apps.owner_user_id=?'), - [42] - ); + expect.stringContaining('WHERE apps.owner_user_id=?'), + [42]); expect(result).toHaveLength(1); }); it('should throw error when predicate is not an array', async () => { const crudQ = AppService.IMPLEMENTS['crud-q']; - await expect( - crudQ.select.call(appService, { predicate: 'invalid' }) - ).rejects.toThrow('predicate must be an array'); + await expect(crudQ.select.call(appService, { predicate: 'invalid' })).rejects.toThrow('predicate must be an array'); }); it('should correctly coerce boolean fields for all selected apps', async () => { @@ -565,8 +555,7 @@ describe('AppService', () => { const crudQ = AppService.IMPLEMENTS['crud-q']; await expect(crudQ.select.call(appService, {})).rejects.toThrow( - 'expected filetypesAsJSON[1] to be a string' - ); + 'expected filetypesAsJSON[1] to be a string'); }); it('should handle malformed filetypes JSON', async () => { @@ -578,8 +567,7 @@ describe('AppService', () => { const crudQ = AppService.IMPLEMENTS['crud-q']; await expect(crudQ.select.call(appService, {})).rejects.toThrow( - 'failed to get app filetype associations' - ); + 'failed to get app filetype associations'); }); it('should use database case for SQL dialect differences', async () => { @@ -603,17 +591,14 @@ describe('AppService', () => { await crudQ.read.call(appService, { id: { name: 'test' } }); expect(mockDb.read).toHaveBeenCalledWith( - expect.stringContaining('apps.name = ?'), - ['test'] - ); + expect.stringContaining('apps.name = ?'), + ['test']); }); it('should reject identifiers not in REDUNDANT_IDENTIFIERS', async () => { const crudQ = AppService.IMPLEMENTS['crud-q']; - await expect( - crudQ.read.call(appService, { id: { title: 'test' } }) - ).rejects.toThrow('Invalid complex id keys: title'); + await expect(crudQ.read.call(appService, { id: { title: 'test' } })).rejects.toThrow('Invalid complex id keys: title'); }); }); @@ -639,10 +624,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO apps'), - expect.arrayContaining(['new-app', 'New App', 'https://example.com/new']) - ); - expect(refresh_apps_cache).toHaveBeenCalled(); + expect.stringContaining('INSERT INTO apps'), + expect.arrayContaining(['new-app', 'New App', 'https://example.com/new'])); }); it('should throw forbidden for non-user actors', async () => { @@ -718,9 +701,8 @@ describe('AppService', () => { // The INSERT should not include last_review expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO apps'), - expect.not.arrayContaining(['2024-01-01']) - ); + expect.stringContaining('INSERT INTO apps'), + expect.not.arrayContaining(['2024-01-01'])); }); it('should remove read_only fields from input', async () => { @@ -750,7 +732,7 @@ describe('AppService', () => { // First check returns true (name exists), second returns false app_name_exists - .mockResolvedValueOnce(true) // 'new-app' exists + .mockResolvedValueOnce(true) // 'new-app' exists .mockResolvedValueOnce(false); // 'new-app-1' doesn't exist const crudQ = AppService.IMPLEMENTS['crud-q']; @@ -765,9 +747,8 @@ describe('AppService', () => { // Should have inserted with deduped name expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO apps'), - expect.arrayContaining(['new-app-1']) - ); + expect.stringContaining('INSERT INTO apps'), + expect.arrayContaining(['new-app-1'])); }); it('should throw error when name conflict without dedupe_name', async () => { @@ -800,9 +781,8 @@ describe('AppService', () => { // Should include app_owner in the INSERT expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('app_owner'), - expect.arrayContaining([100]) - ); + expect.stringContaining('app_owner'), + expect.arrayContaining([100])); }); it('should emit app.new-icon event when icon is provided', async () => { @@ -820,11 +800,10 @@ describe('AppService', () => { }); expect(mockEventService.emit).toHaveBeenCalledWith( - 'app.new-icon', - expect.objectContaining({ - data_url: 'data:image/png;base64,abc123', - }) - ); + 'app.new-icon', + expect.objectContaining({ + data_url: 'data:image/png;base64,abc123', + })); }); it('should handle filetype_associations', async () => { @@ -845,13 +824,11 @@ describe('AppService', () => { // (DELETE is called even for create since #update_filetype_associations always clears first) expect(mockDbWrite.write).toHaveBeenCalledTimes(3); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('DELETE FROM app_filetype_association'), - [1] - ); + expect.stringContaining('DELETE FROM app_filetype_association'), + [1]); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO app_filetype_association'), - expect.arrayContaining([1, 'txt', 1, 'pdf']) - ); + expect.stringContaining('INSERT INTO app_filetype_association'), + expect.arrayContaining([1, 'txt', 1, 'pdf'])); }); it('should call validate_string for name and title', async () => { @@ -924,9 +901,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['Updated Title', 'app-uid-123']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['Updated Title', 'app-uid-123'])); }); it('should throw entity_not_found when app does not exist', async () => { @@ -967,9 +943,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['Admin Update']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['Admin Update'])); }); it('should remove protected fields from update', async () => { @@ -1020,7 +995,7 @@ describe('AppService', () => { it('should allow name change with dedupe_name option', async () => { setupContextForWrite(createMockUserActor(1)); app_name_exists - .mockResolvedValueOnce(true) // 'new-name' exists + .mockResolvedValueOnce(true) // 'new-name' exists .mockResolvedValueOnce(false); // 'new-name-1' doesn't exist const crudQ = AppService.IMPLEMENTS['crud-q']; @@ -1030,9 +1005,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['new-name-1']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['new-name-1'])); }); it('should allow reclaiming old app name', async () => { @@ -1061,9 +1035,7 @@ describe('AppService', () => { }); // Should only have the read for ID, no name in update - const writeCall = mockDbWrite.write.mock.calls.find( - call => call[0].includes('UPDATE') - ); + const writeCall = mockDbWrite.write.mock.calls.find(call => call[0].includes('UPDATE')); if ( writeCall ) { expect(writeCall[1]).not.toContain('test-app'); } @@ -1081,12 +1053,11 @@ describe('AppService', () => { }); expect(mockEventService.emit).toHaveBeenCalledWith( - 'app.new-icon', - expect.objectContaining({ - app_uid: 'app-uid-123', - data_url: 'data:image/png;base64,newicon', - }) - ); + 'app.new-icon', + expect.objectContaining({ + app_uid: 'app-uid-123', + data_url: 'data:image/png;base64,newicon', + })); }); it('should emit app.rename event when name changes', async () => { @@ -1098,13 +1069,12 @@ describe('AppService', () => { }); expect(mockEventService.emit).toHaveBeenCalledWith( - 'app.rename', - expect.objectContaining({ - app_uid: 'app-uid-123', - new_name: 'renamed-app', - old_name: 'test-app', - }) - ); + 'app.rename', + expect.objectContaining({ + app_uid: 'app-uid-123', + new_name: 'renamed-app', + old_name: 'test-app', + })); }); it('should update filetype_associations', async () => { @@ -1120,29 +1090,13 @@ describe('AppService', () => { // Should delete old associations expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('DELETE FROM app_filetype_association'), - [1] - ); + expect.stringContaining('DELETE FROM app_filetype_association'), + [1]); // Should insert new associations expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO app_filetype_association'), - expect.arrayContaining([1, 'doc', 1, 'xls']) - ); - }); - - it('should call refresh_apps_cache after update', async () => { - setupContextForWrite(createMockUserActor(1)); - - const crudQ = AppService.IMPLEMENTS['crud-q']; - await crudQ.update.call(appService, { - object: { uid: 'app-uid-123', title: 'Updated' }, - }); - - expect(refresh_apps_cache).toHaveBeenCalledWith( - { uid: 'app-uid-123' }, - expect.objectContaining({ uuid: 'app-uid-123' }) - ); + expect.stringContaining('INSERT INTO app_filetype_association'), + expect.arrayContaining([1, 'doc', 1, 'xls'])); }); it('should validate fields when provided', async () => { @@ -1192,9 +1146,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['https://mysite.puter.site']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['https://mysite.puter.site'])); }); it('should throw forbidden when app actor does not own the entity (AppLimitedES behavior)', async () => { @@ -1228,9 +1181,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['Updated by App']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['Updated by App'])); }); it('should allow app actor with write permission to update any entity (AppLimitedES behavior)', async () => { @@ -1248,9 +1200,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['Admin Update']) - ); + expect.stringContaining('UPDATE apps SET'), + expect.arrayContaining(['Admin Update'])); }); }); @@ -1270,9 +1221,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('INSERT INTO apps'), - expect.any(Array) - ); + expect.stringContaining('INSERT INTO apps'), + expect.any(Array)); }); it('should call update when entity exists', async () => { @@ -1287,9 +1237,8 @@ describe('AppService', () => { }); expect(mockDbWrite.write).toHaveBeenCalledWith( - expect.stringContaining('UPDATE apps SET'), - expect.any(Array) - ); + expect.stringContaining('UPDATE apps SET'), + expect.any(Array)); }); }); @@ -1401,11 +1350,6 @@ describe('AppService', () => { const crudQ = AppService.IMPLEMENTS['crud-q']; await crudQ.delete.call(appService, { uid: 'app-uid-123' }); - - expect(refresh_apps_cache).toHaveBeenCalledWith( - { uid: 'app-uid-123' }, - null - ); }); it('should throw forbidden when app actor does not own the entity', async () => { @@ -1456,4 +1400,3 @@ describe('AppService', () => { }); }); }); - diff --git a/src/backend/src/om/entitystorage/AppES.js b/src/backend/src/om/entitystorage/AppES.js index 5bcfbae9a..e1f9660c9 100644 --- a/src/backend/src/om/entitystorage/AppES.js +++ b/src/backend/src/om/entitystorage/AppES.js @@ -18,8 +18,7 @@ */ const APIError = require('../../api/APIError'); const config = require('../../config'); -const { app_name_exists, refresh_apps_cache } = require('../../helpers'); - +const { app_name_exists } = require('../../helpers'); const { AppUnderUserActorType } = require('../../services/auth/Actor'); const { DB_WRITE } = require('../../services/database/consts'); const { Context } = require('../../util/context'); @@ -203,30 +202,6 @@ class AppES extends BaseES { await this.db.write('UPDATE subdomains SET associated_app_id = ? WHERE id = ?', [insert_id, subdomain_id]); } - - const owner = extra.old_entity - ? await extra.old_entity.get('owner') - : await entity.get('owner'); - - { - // Update app cache - const raw_app = { - // These map to different names - uuid: await full_entity.get('uid'), - owner_user_id: owner.id, - - // These map to the same names - name: await full_entity.get('name'), - title: await full_entity.get('title'), - description: await full_entity.get('description'), - icon: await full_entity.get('icon'), - index_url: await full_entity.get('index_url'), - maximize_on_start: await full_entity.get('maximize_on_start'), - }; - - refresh_apps_cache({ uid: raw_app.uuid }, raw_app); - } - if ( extra.old_entity ) { const svc_event = this.context.get('services').get('event'); svc_event.emit('app.changed', { diff --git a/src/backend/src/routers/apps.js b/src/backend/src/routers/apps.js index c999a800c..55ea3cdfe 100644 --- a/src/backend/src/routers/apps.js +++ b/src/backend/src/routers/apps.js @@ -21,7 +21,7 @@ const express = require('express'); const router = new express.Router(); const auth = require('../middleware/auth.js'); const config = require('../config'); -const { get_app } = require('../helpers'); +const { get_apps } = require('../helpers'); const { DB_READ } = require('../services/database/consts.js'); const subdomain = require('../middleware/subdomain.js'); @@ -32,7 +32,7 @@ router.get('/apps', subdomain('api'), auth, express.json({ limit: '50mb' }), - async (req, res, next) => { + async (req, res) => { // /!\ open brace on end of previous line // check if user is verified @@ -109,43 +109,23 @@ router.get('/apps/:name', } let app_names = req.params.name.split('|'); - let retobj = []; + const apps = await get_apps(app_names.map(name => ({ name }))); - if ( app_names.length > 0 ) { - // prepare each app for returning to user - for ( let index = 0; index < app_names.length; index++ ) { - const app = await get_app({ name: app_names[index] }); - let final_obj = {}; - if ( app ) { - final_obj = { - uuid: app.uid, - name: app.name, - title: app.title, - icon: app.icon, - godmode: app.godmode, - background: app.background, - maximize_on_start: app.maximize_on_start, - index_url: app.index_url, - }; - } - // add to object to be returned - retobj.push(final_obj); - } - } - - // order output based on input! - let final_obj = []; - for ( let index = 0; index < app_names.length; index++ ) { - const app_name = app_names[index]; - for ( let index = 0; index < retobj.length; index++ ) { - if ( retobj[index].name === app_name ) - { - final_obj.push(retobj[index]); - } - } - } + const final_obj = apps.map((app) => { + if ( ! app ) return null; + return { + uuid: app.uid, + name: app.name, + title: app.title, + icon: app.icon, + godmode: app.godmode, + background: app.background, + maximize_on_start: app.maximize_on_start, + index_url: app.index_url, + }; + }).filter(Boolean); return res.send(final_obj); }); -module.exports = router; \ No newline at end of file +module.exports = router; diff --git a/src/backend/src/routers/auth/list-permissions.js b/src/backend/src/routers/auth/list-permissions.js index d029c44b2..87082fbc6 100644 --- a/src/backend/src/routers/auth/list-permissions.js +++ b/src/backend/src/routers/auth/list-permissions.js @@ -16,14 +16,14 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -const eggspress = require('../../api/eggspress'); -const { get_app, get_user } = require('../../helpers'); -const { UserActorType } = require('../../services/auth/Actor'); -const { DB_READ } = require('../../services/database/consts'); -const { Context } = require('../../util/context'); -const APIError = require('../../api/APIError'); +import eggspress from '../../api/eggspress.js'; +import { get_apps, get_user } from '../../helpers.js'; +import { UserActorType } from '../../services/auth/Actor.js'; +import { DB_READ } from '../../services/database/consts.js'; +import { Context } from '../../util/context.js'; +import { APIError } from '../../api/APIError.js'; -module.exports = eggspress('/auth/list-permissions', { +export default eggspress('/auth/list-permissions', { subdomain: 'api', auth2: true, allowedMethods: ['GET'], @@ -46,9 +46,12 @@ module.exports = eggspress('/auth/list-permissions', { const rows = await db.read('SELECT * FROM `user_to_app_permissions` WHERE user_id=?', [ actor.type.user.id ]); + const apps = await get_apps(rows.map(row => ({ id: row.app_id }))); - for ( const row of rows ) { - const app = await get_app({ id: row.app_id }); + for ( let i = 0; i < rows.length; i++ ) { + const row = rows[i]; + const app = apps[i]; + if ( ! app ) continue; delete app.id; delete app.approved_for_listing; diff --git a/src/backend/src/routers/get-launch-apps.js b/src/backend/src/routers/get-launch-apps.js index fddc8f409..ba2e28baa 100644 --- a/src/backend/src/routers/get-launch-apps.js +++ b/src/backend/src/routers/get-launch-apps.js @@ -17,8 +17,9 @@ * along with this program. If not, see . */ 'use strict'; -const { get_app } = require('../helpers.js'); -const { DB_READ } = require('../services/database/consts.js'); +import { get_apps } from '../helpers.js'; +import { DB_READ } from '../services/database/consts.js'; +import { kv } from '@heyputer/backend/src/util/kvSingleton.js'; const iconify_apps = async (context, { apps, size }) => { return await Promise.all(apps.map(async app => { @@ -37,7 +38,7 @@ const iconify_apps = async (context, { apps, size }) => { // -----------------------------------------------------------------------// // GET /get-launch-apps // -----------------------------------------------------------------------// -module.exports = async (req, res) => { +export default async (req, res) => { let result = {}; // Verify query params @@ -79,12 +80,11 @@ module.exports = async (req, res) => { // prepare each app for returning to user by only returning the necessary fields // and adding them to the retobj array - result.recent = []; - for ( const { app_uid: uid } of apps ) { - const app = await get_app({ uid }); - if ( ! app ) continue; + const recent_apps = await get_apps(apps.map(({ app_uid: uid }) => ({ uid }))); - result.recent.push({ + result.recent = recent_apps.map((app) => { + if ( ! app ) return null; + return { uuid: app.uid, name: app.name, title: app.title, @@ -92,8 +92,8 @@ module.exports = async (req, res) => { godmode: app.godmode, maximize_on_start: app.maximize_on_start, index_url: app.index_url, - }); - } + }; + }).filter(Boolean); // Iconify apps if ( req.query.icon_size ) { diff --git a/src/backend/src/routers/get-launch-apps.test.js b/src/backend/src/routers/get-launch-apps.test.js deleted file mode 100644 index 3bbddf6cf..000000000 --- a/src/backend/src/routers/get-launch-apps.test.js +++ /dev/null @@ -1,223 +0,0 @@ -/* - * Copyright (C) 2024-present Puter Technologies Inc. - * - * This file is part of Puter. - * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { kv } from '../util/kvSingleton'; -const uuid = require('uuid'); -const proxyquire = require('proxyquire'); - -const TEST_UUID_NAMESPACE = '5568ab95-229d-4d87-b98c-0b12680a9524'; - -const apps_names_expected_to_exist = [ - 'app-center', - 'dev-center', - 'editor', -]; - -const data_mockapps = (() => { - const data_mockapps = []; - // List of app names that get-launch-apps expects to exist - for ( const name of apps_names_expected_to_exist ) { - data_mockapps.push({ - uid: `app-${ uuid.v5(name, TEST_UUID_NAMESPACE)}`, - name, - title: 'App Name', - icon: 'icon-goes-here', - godmode: false, - maximize_on_start: false, - index_url: 'index-url', - }); - } - - // An additional app that won't show up in taskbar - data_mockapps.push({ - uid: `app-${ uuid.v5('hidden-app', TEST_UUID_NAMESPACE)}`, - name: 'hidden-app', - title: 'Hidden App', - icon: 'icon-goes-here', - godmode: false, - maximize_on_start: false, - index_url: 'index-url', - }); - - // An additional app tha only shows up in recents - data_mockapps.push({ - uid: `app-${ uuid.v5('recent-app', TEST_UUID_NAMESPACE)}`, - name: 'recent-app', - title: 'Recent App', - icon: 'icon-goes-here', - godmode: false, - maximize_on_start: false, - index_url: 'index-url', - }); - - return data_mockapps; -})(); - -const data_appopens = [ - { - app_uid: `app-${ uuid.v5('app-center', TEST_UUID_NAMESPACE)}`, - }, - { - app_uid: `app-${ uuid.v5('editor', TEST_UUID_NAMESPACE)}`, - }, - { - app_uid: `app-${ uuid.v5('recent-app', TEST_UUID_NAMESPACE)}`, - }, -]; - -const get_mock_context = () => { - const database_mock = { - read: async (query) => { - if ( query.includes('FROM app_opens') ) { - return data_appopens; - } - }, - }; - const recommendedApps_mock = { - get_recommended_apps: async ({ icon_size }) => { - return data_mockapps - .filter(app => apps_names_expected_to_exist.includes(app.name)) - .map(app => ({ - uuid: app.uid, - name: app.name, - title: app.title, - icon: app.icon, - godmode: app.godmode, - maximize_on_start: app.maximize_on_start, - index_url: app.index_url, - })); - }, - }; - const services_mock = { - get: (key) => { - if ( key === 'database' ) { - return { - get: () => database_mock, - }; - } - if ( key === 'recommended-apps' ) { - return recommendedApps_mock; - } - }, - }; - - const req_mock = { - user: { - id: 1 + Math.floor(Math.random() * 1000 ** 3), - }, - services: services_mock, - send: vi.fn(), - }; - - const res_mock = { - send: vi.fn(), - }; - - const get_app = vi.fn(async ({ uid, name }) => { - if ( uid ) { - return data_mockapps.find(app => app.uid === uid); - } - if ( name ) { - return data_mockapps.find(app => app.name === name); - } - }); - - const get_launch_apps = proxyquire('./get-launch-apps', { - '../helpers.js': { - get_app, - }, - }); - - return { - get_launch_apps, - req_mock, - res_mock, - spies: { - get_app, - }, - }; -}; - -describe('GET /launch-apps', () => { - globalThis.kv = kv; - - it('should return expected format', async () => { - // First call - { - const { get_launch_apps, req_mock, res_mock, spies } = get_mock_context(); - req_mock.query = {}; - await get_launch_apps(req_mock, res_mock); - - // << HOW TO FIX >> - // If you updated the list of recommended apps, - // you can simply update this number to match the new length - // expect(spies.get_app).toHaveBeenCalledTimes(3); - } - - // Second call - { - const { get_launch_apps, req_mock, res_mock, spies } = get_mock_context(); - req_mock.query = {}; - await get_launch_apps(req_mock, res_mock); - - expect(res_mock.send).toHaveBeenCalledOnce(); - - const call = res_mock.send.mock.calls[0]; - const response = call[0]; - - expect(response).toBeTypeOf('object'); - - expect(response).toHaveProperty('recommended'); - expect(response.recommended).toBeInstanceOf(Array); - expect(response.recommended).toHaveLength(apps_names_expected_to_exist.length); - expect(response.recommended).toEqual( - data_mockapps - .filter(app => apps_names_expected_to_exist.includes(app.name)) - .map(app => ({ - uuid: app.uid, - name: app.name, - title: app.title, - icon: app.icon, - godmode: app.godmode, - maximize_on_start: app.maximize_on_start, - index_url: app.index_url, - }))); - - expect(response).toHaveProperty('recent'); - expect(response.recent).toBeInstanceOf(Array); - expect(response.recent).toHaveLength(data_appopens.length); - expect(response.recent).toEqual( - data_mockapps - .filter(app => data_appopens.map(app_open => app_open.app_uid).includes(app.uid)) - .map(app => ({ - uuid: app.uid, - name: app.name, - title: app.title, - icon: app.icon, - godmode: app.godmode, - maximize_on_start: app.maximize_on_start, - index_url: app.index_url, - }))); - - expect(spies.get_app).toHaveBeenCalledTimes( - data_appopens.length); - } - }); -}); \ No newline at end of file diff --git a/src/backend/src/services/PermissionAPIService.js b/src/backend/src/services/PermissionAPIService.js index 7649e7197..23c820370 100644 --- a/src/backend/src/services/PermissionAPIService.js +++ b/src/backend/src/services/PermissionAPIService.js @@ -51,7 +51,7 @@ class PermissionAPIService extends BaseService { app.use(require('../routers/auth/revoke-user-user')); app.use(require('../routers/auth/grant-user-group')); app.use(require('../routers/auth/revoke-user-group')); - app.use(require('../routers/auth/list-permissions')); + app.use(require('../routers/auth/list-permissions').default); app.use(require('../routers/auth/check-permissions.js')); Endpoint(require('../routers/auth/check-app-acl.endpoint.js')).but({ diff --git a/src/backend/src/services/PuterAPIService.js b/src/backend/src/services/PuterAPIService.js index 335d0cf9b..32bee8bd5 100644 --- a/src/backend/src/services/PuterAPIService.js +++ b/src/backend/src/services/PuterAPIService.js @@ -94,7 +94,7 @@ class PuterAPIService extends BaseService { route: '/get-launch-apps', methods: ['GET'], mw: [configurable_auth()], - handler: require('../routers/get-launch-apps'), + handler: require('../routers/get-launch-apps').default, }).attach(app); }