From ee0f23718ba2c28f9fe895c6c33adce6d592d569 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 25 May 2021 20:29:59 +0200 Subject: [PATCH] Server: Fixed Item and Log page when using Postgres --- packages/server/src/models/BaseModel.ts | 5 ++- packages/server/src/models/ChangeModel.ts | 33 +++++++++++-------- packages/server/src/models/ItemModel.test.ts | 12 +++++++ packages/server/src/models/ItemModel.ts | 17 ++++++---- .../server/src/utils/testing/testUtils.ts | 2 +- 5 files changed, 48 insertions(+), 21 deletions(-) diff --git a/packages/server/src/models/BaseModel.ts b/packages/server/src/models/BaseModel.ts index c4cd587e31..8c41aef342 100644 --- a/packages/server/src/models/BaseModel.ts +++ b/packages/server/src/models/BaseModel.ts @@ -97,7 +97,10 @@ export default abstract class BaseModel { } if (mainTable) { - output = output.map(f => `${mainTable}.${f}`); + output = output.map(f => { + if (f.includes(`${mainTable}.`)) return f; + return `${mainTable}.${f}`; + }); } return output; diff --git a/packages/server/src/models/ChangeModel.ts b/packages/server/src/models/ChangeModel.ts index 6f2990c33c..351b4cedf9 100644 --- a/packages/server/src/models/ChangeModel.ts +++ b/packages/server/src/models/ChangeModel.ts @@ -68,7 +68,7 @@ export default class ChangeModel extends BaseModel { return results; } - private changesForUserQuery(userId: Uuid): Knex.QueryBuilder { + private changesForUserQuery(userId: Uuid, count: boolean): Knex.QueryBuilder { // When need to get: // // - All the CREATE and DELETE changes associated with the user @@ -78,15 +78,8 @@ export default class ChangeModel extends BaseModel { // UPDATE changes do not have the user_id set because they are specific // to the item, not to a particular user. - return this + const query = this .db('changes') - .select([ - 'id', - 'item_id', - 'item_name', - 'type', - 'updated_time', - ]) .where(function() { void this.whereRaw('((type = ? OR type = ?) AND user_id = ?)', [ChangeType.Create, ChangeType.Delete, userId]) // Need to use a RAW query here because Knex has a "not a @@ -96,6 +89,20 @@ export default class ChangeModel extends BaseModel { // https://github.com/knex/knex/issues/1851 .orWhereRaw('type = ? AND item_id IN (SELECT item_id FROM user_items WHERE user_id = ?)', [ChangeType.Update, userId]); }); + + if (count) { + void query.countDistinct('id', { as: 'total' }); + } else { + void query.select([ + 'id', + 'item_id', + 'item_name', + 'type', + 'updated_time', + ]); + } + + return query; } public async allByUser(userId: Uuid, pagination: Pagination = null): Promise { @@ -106,9 +113,9 @@ export default class ChangeModel extends BaseModel { ...pagination, }; - const query = this.changesForUserQuery(userId); - const countQuery = query.clone(); - const itemCount = (await countQuery.countDistinct('id', { as: 'total' }))[0].total; + const query = this.changesForUserQuery(userId, false); + const countQuery = this.changesForUserQuery(userId, true); + const itemCount = (await countQuery.first()).total; void query .orderBy(pagination.order[0].by, pagination.order[0].dir) @@ -140,7 +147,7 @@ export default class ChangeModel extends BaseModel { if (!changeAtCursor) throw new ErrorResyncRequired(); } - const query = this.changesForUserQuery(userId); + const query = this.changesForUserQuery(userId, false); // If a cursor was provided, apply it to the query. if (changeAtCursor) { diff --git a/packages/server/src/models/ItemModel.test.ts b/packages/server/src/models/ItemModel.test.ts index e342849689..fce1e3312c 100644 --- a/packages/server/src/models/ItemModel.test.ts +++ b/packages/server/src/models/ItemModel.test.ts @@ -130,4 +130,16 @@ describe('ItemModel', function() { } }); + test('should count items', async function() { + const { user: user1 } = await createUserAndSession(1, true); + + await createItemTree(user1.id, '', { + '000000000000000000000000000000F1': { + '00000000000000000000000000000001': null, + }, + }); + + expect(await models().item().childrenCount(user1.id)).toBe(2); + }); + }); diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 6a84265174..2b48f788ef 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -349,13 +349,18 @@ export default class ItemModel extends BaseModel { } - private childrenQuery(userId: Uuid, pathQuery: string = '', options: LoadOptions = {}): Knex.QueryBuilder { + private childrenQuery(userId: Uuid, pathQuery: string = '', count: boolean = false, options: LoadOptions = {}): Knex.QueryBuilder { const query = this .db('user_items') .leftJoin('items', 'user_items.item_id', 'items.id') - .select(this.selectFields(options, ['id', 'name', 'updated_time'], 'items')) .where('user_items.user_id', '=', userId); + if (count) { + void query.countDistinct('items.id', { as: 'total' }); + } else { + void query.select(this.selectFields(options, ['id', 'name', 'updated_time'], 'items')); + } + if (pathQuery) { // We support /* as a prefix only. Anywhere else would have // performance issue or requires a revert index. @@ -376,14 +381,14 @@ export default class ItemModel extends BaseModel { public async children(userId: Uuid, pathQuery: string = '', pagination: Pagination = null, options: LoadOptions = {}): Promise { pagination = pagination || defaultPagination(); - const query = this.childrenQuery(userId, pathQuery, options); + const query = this.childrenQuery(userId, pathQuery, false, options); return paginateDbQuery(query, pagination, 'items'); } public async childrenCount(userId: Uuid, pathQuery: string = ''): Promise { - const query = this.childrenQuery(userId, pathQuery); - const r = await query.countDistinct('items.id', { as: 'total' }); - return r[0].total; + const query = this.childrenQuery(userId, pathQuery, true); + const r = await query.first(); + return r ? r.total : 0; } private async joplinItemPath(jopId: string): Promise { diff --git a/packages/server/src/utils/testing/testUtils.ts b/packages/server/src/utils/testing/testUtils.ts index bca55c7bed..400ad21865 100644 --- a/packages/server/src/utils/testing/testUtils.ts +++ b/packages/server/src/utils/testing/testUtils.ts @@ -68,7 +68,7 @@ export async function beforeAllDb(unitName: string) { // initConfig({ // DB_CLIENT: 'pg', - // POSTGRES_DATABASE: createdDbPath_, + // POSTGRES_DATABASE: unitName, // POSTGRES_USER: 'joplin', // POSTGRES_PASSWORD: 'joplin', // }, {