From 49b6d2c98392efffa80984d408e2fd03819fc5c9 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Wed, 24 Jul 2024 19:26:31 +0700 Subject: [PATCH 1/3] UBERF-7543: Add low level groupBy api and improve security space lookup Signed-off-by: Andrey Sobolev --- packages/core/src/server.ts | 3 + server/core/src/adapter.ts | 2 + server/core/src/mem.ts | 4 ++ server/core/src/pipeline.ts | 6 ++ server/core/src/server/storage.ts | 4 ++ server/core/src/types.ts | 2 + server/elastic/src/backup.ts | 4 ++ server/middleware/src/base.ts | 15 ++++- server/middleware/src/spaceSecurity.ts | 24 +------- server/mongo/src/storage.ts | 78 +++++++++++++++++++++--- server/server-storage/src/blobStorage.ts | 4 ++ server/ws/src/__tests__/server.test.ts | 2 + 12 files changed, 115 insertions(+), 33 deletions(-) diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts index 1aea148b20..feea3392fb 100644 --- a/packages/core/src/server.ts +++ b/packages/core/src/server.ts @@ -67,6 +67,9 @@ export interface LowLevelStorage { // Remove a list of documents. clean: (ctx: MeasureContext, domain: Domain, docs: Ref[]) => Promise + + // Low level direct group API + groupBy: (ctx: MeasureContext, domain: Domain, field: string) => Promise> } export interface Branding { diff --git a/server/core/src/adapter.ts b/server/core/src/adapter.ts index 43496ae8fd..f8ac693ad0 100644 --- a/server/core/src/adapter.ts +++ b/server/core/src/adapter.ts @@ -113,6 +113,8 @@ export interface DbAdapter { upload: (ctx: MeasureContext, domain: Domain, docs: Doc[]) => Promise clean: (ctx: MeasureContext, domain: Domain, docs: Ref[]) => Promise + groupBy: (ctx: MeasureContext, domain: Domain, field: string) => Promise> + // Bulk update operations update: (ctx: MeasureContext, domain: Domain, operations: Map, DocumentUpdate>) => Promise } diff --git a/server/core/src/mem.ts b/server/core/src/mem.ts index 3d053675f3..f3c0dcd43b 100644 --- a/server/core/src/mem.ts +++ b/server/core/src/mem.ts @@ -74,6 +74,10 @@ export class DummyDbAdapter implements DbAdapter { async clean (ctx: MeasureContext, domain: Domain, docs: Ref[]): Promise {} async update (ctx: MeasureContext, domain: Domain, operations: Map, DocumentUpdate>): Promise {} + + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return new Set() + } } class InMemoryAdapter extends DummyDbAdapter implements DbAdapter { diff --git a/server/core/src/pipeline.ts b/server/core/src/pipeline.ts index cb318b4965..2556893eac 100644 --- a/server/core/src/pipeline.ts +++ b/server/core/src/pipeline.ts @@ -117,6 +117,12 @@ class PipelineImpl implements Pipeline { : await this.storage.findAll(ctx.ctx, _class, query, options) } + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return this.head !== undefined + ? await this.head.groupBy(ctx, domain, field) + : await this.storage.groupBy(ctx, domain, field) + } + async searchFulltext (ctx: SessionContext, query: SearchQuery, options: SearchOptions): Promise { return this.head !== undefined ? await this.head.searchFulltext(ctx, query, options) diff --git a/server/core/src/server/storage.ts b/server/core/src/server/storage.ts index 42ac97aaf9..c97360039d 100644 --- a/server/core/src/server/storage.ts +++ b/server/core/src/server/storage.ts @@ -430,6 +430,10 @@ export class TServerStorage implements ServerStorage { return this.model.filter((it) => it.modifiedOn > lastModelTx) } + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return await this.getAdapter(domain, false).groupBy(ctx, domain, field) + } + async findAll( ctx: MeasureContext, clazz: Ref>, diff --git a/server/core/src/types.ts b/server/core/src/types.ts index 9e29f77fae..a0711efb25 100644 --- a/server/core/src/types.ts +++ b/server/core/src/types.ts @@ -96,6 +96,8 @@ export interface Middleware { query: DocumentQuery, options?: FindOptions ) => Promise> + + groupBy: (ctx: MeasureContext, domain: Domain, field: string) => Promise> handleBroadcast: HandleBroadcastFunc searchFulltext: (ctx: SessionContext, query: SearchQuery, options: SearchOptions) => Promise } diff --git a/server/elastic/src/backup.ts b/server/elastic/src/backup.ts index eadbe947a2..53755ab1a5 100644 --- a/server/elastic/src/backup.ts +++ b/server/elastic/src/backup.ts @@ -61,6 +61,10 @@ class ElasticDataAdapter implements DbAdapter { this.getDocId = (fulltext) => fulltext.slice(0, -1 * (this.workspaceString.length + 1)) as Ref } + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return new Set() + } + async findAll( ctx: MeasureContext, _class: Ref>, diff --git a/server/middleware/src/base.ts b/server/middleware/src/base.ts index 5286e7fcfd..127c61c002 100644 --- a/server/middleware/src/base.ts +++ b/server/middleware/src/base.ts @@ -23,7 +23,9 @@ import { SearchOptions, SearchQuery, SearchResult, - Tx + Tx, + type Domain, + type MeasureContext } from '@hcengineering/core' import { Middleware, SessionContext, TxMiddlewareResult, type ServerStorage } from '@hcengineering/server-core' @@ -45,6 +47,17 @@ export abstract class BaseMiddleware { return await this.provideFindAll(ctx, _class, query, options) } + async providerGroupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + if (this.next !== undefined) { + return await this.next.groupBy(ctx, domain, field) + } + return await this.storage.groupBy(ctx, domain, field) + } + + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return await this.providerGroupBy(ctx, domain, field) + } + async searchFulltext (ctx: SessionContext, query: SearchQuery, options: SearchOptions): Promise { return await this.provideSearchFulltext(ctx, query, options) } diff --git a/server/middleware/src/spaceSecurity.ts b/server/middleware/src/spaceSecurity.ts index 1494f11eb4..ad7ea90879 100644 --- a/server/middleware/src/spaceSecurity.ts +++ b/server/middleware/src/spaceSecurity.ts @@ -450,30 +450,8 @@ export class SpaceSecurityMiddleware extends BaseMiddleware implements Middlewar } async loadDomainSpaces (ctx: MeasureContext, domain: Domain): Promise>> { - const map = new Set>() const field = this.getKey(domain) - while (true) { - const nin = Array.from(map.values()) - const spaces = await this.storage.findAll( - ctx, - core.class.Doc, - nin.length > 0 - ? { - [field]: { $nin: nin } - } - : {}, - { - projection: { [field]: 1 }, - limit: 1000, - domain - } - ) - if (spaces.length === 0) { - break - } - spaces.forEach((p) => map.add((p as any)[field] as Ref)) - } - return map + return await this.storage.groupBy>(ctx, domain, field) } async getDomainSpaces (domain: Domain): Promise>> { diff --git a/server/mongo/src/storage.ts b/server/mongo/src/storage.ts index bf01af5cc7..b2eaa70d05 100644 --- a/server/mongo/src/storage.ts +++ b/server/mongo/src/storage.ts @@ -513,10 +513,18 @@ abstract class MongoAdapterBase implements DbAdapter { let result: WithLookup[] = [] let total = options?.total === true ? 0 : -1 try { - result = await ctx.with('toArray', {}, async (ctx) => await toArray(cursor), { - domain, - pipeline - }) + await ctx.with( + 'toArray', + {}, + async (ctx) => { + result = await toArray(cursor) + }, + () => ({ + size: result.length, + domain, + pipeline + }) + ) } catch (e) { console.error('error during executing cursor in findWithPipeline', clazz, cutObjectArray(query), options, e) throw e @@ -619,6 +627,33 @@ abstract class MongoAdapterBase implements DbAdapter { return false } + @withContext('groupBy') + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + const result = await this.globalCtx.with( + 'groupBy', + { domain }, + async (ctx) => { + const coll = this.collection(domain) + const grResult = await coll + .aggregate([ + { + $group: { + _id: '$' + field + } + } + ]) + .toArray() + return new Set(grResult.map((it) => it._id as unknown as T)) + }, + + () => ({ + findOps: this.findOps, + txOps: this.txOps + }) + ) + return result + } + findOps: number = 0 txOps: number = 0 opIndex: number = 0 @@ -691,6 +726,22 @@ abstract class MongoAdapterBase implements DbAdapter { const coll = this.collection(domain) const mongoQuery = this.translateQuery(_class, query) + if (options?.limit === 1) { + // Skip sort/projection/etc. + return await ctx.with( + 'find-one', + {}, + async (ctx) => { + const doc = await coll.findOne(mongoQuery) + if (doc != null) { + return toFindResult([doc as unknown as T]) + } + return toFindResult([]) + }, + { mongoQuery } + ) + } + let cursor = coll.find(mongoQuery) if (options?.projection !== undefined) { @@ -717,11 +768,20 @@ abstract class MongoAdapterBase implements DbAdapter { // Error in case of timeout try { - const res: T[] = await ctx.with('toArray', {}, async (ctx) => await toArray(cursor), { - mongoQuery, - options, - domain - }) + let res: T[] = [] + await ctx.with( + 'toArray', + {}, + async (ctx) => { + res = await toArray(cursor) + }, + () => ({ + size: res.length, + mongoQuery, + options, + domain + }) + ) if (options?.total === true && options?.limit === undefined) { total = res.length } diff --git a/server/server-storage/src/blobStorage.ts b/server/server-storage/src/blobStorage.ts index ba62756e48..818ded0357 100644 --- a/server/server-storage/src/blobStorage.ts +++ b/server/server-storage/src/blobStorage.ts @@ -53,6 +53,10 @@ class StorageBlobAdapter implements DbAdapter { return await this.blobAdapter.findAll(ctx, _class, query, options) } + async groupBy(ctx: MeasureContext, domain: Domain, field: string): Promise> { + return await this.blobAdapter.groupBy(ctx, domain, field) + } + async tx (ctx: MeasureContext, ...tx: Tx[]): Promise { throw new PlatformError(unknownError('Direct Blob operations are not possible')) } diff --git a/server/ws/src/__tests__/server.test.ts b/server/ws/src/__tests__/server.test.ts index be06d7a935..aff86da089 100644 --- a/server/ws/src/__tests__/server.test.ts +++ b/server/ws/src/__tests__/server.test.ts @@ -72,6 +72,7 @@ describe('server', () => { close: async () => {}, storage: {} as unknown as ServerStorage, domains: async () => [], + groupBy: async () => new Set(), find: (ctx: MeasureContext, domain: Domain) => ({ next: async (ctx: MeasureContext) => undefined, close: async (ctx: MeasureContext) => {} @@ -170,6 +171,7 @@ describe('server', () => { return toFindResult([d as unknown as T]) }, tx: async (ctx: SessionContext, tx: Tx): Promise<[TxResult, Tx[], string[] | undefined]> => [{}, [], undefined], + groupBy: async () => new Set(), close: async () => {}, storage: {} as unknown as ServerStorage, domains: async () => [], From fdc91bda8930d8dfb731aae7c3a0346331f62bd9 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Thu, 25 Jul 2024 13:33:39 +0700 Subject: [PATCH 2/3] Add sort/projection for findOne Signed-off-by: Andrey Sobolev --- server/mongo/src/storage.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/mongo/src/storage.ts b/server/mongo/src/storage.ts index b2eaa70d05..67a48c141e 100644 --- a/server/mongo/src/storage.ts +++ b/server/mongo/src/storage.ts @@ -80,7 +80,8 @@ import { type Filter, type FindCursor, type Sort, - type UpdateFilter + type UpdateFilter, + type FindOptions as MongoFindOptions } from 'mongodb' import { DBCollectionHelper, getMongoClient, getWorkspaceDB, type MongoClientReference } from './utils' @@ -732,7 +733,16 @@ abstract class MongoAdapterBase implements DbAdapter { 'find-one', {}, async (ctx) => { - const doc = await coll.findOne(mongoQuery) + const findOptions: MongoFindOptions = {} + + if (options?.sort !== undefined) { + findOptions.sort = this.collectSort(options, _class) + } + if (options?.projection !== undefined) { + findOptions.projection = this.calcProjection(options, _class) + } + + const doc = await coll.findOne(mongoQuery, findOptions) if (doc != null) { return toFindResult([doc as unknown as T]) } From 2992d34904cba24cf29b9b8c2dd445e7fa732aa0 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Thu, 25 Jul 2024 13:41:54 +0700 Subject: [PATCH 3/3] Add projection to hash if no projection Signed-off-by: Andrey Sobolev --- server/mongo/src/storage.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/mongo/src/storage.ts b/server/mongo/src/storage.ts index 67a48c141e..741e111ffe 100644 --- a/server/mongo/src/storage.ts +++ b/server/mongo/src/storage.ts @@ -740,6 +740,8 @@ abstract class MongoAdapterBase implements DbAdapter { } if (options?.projection !== undefined) { findOptions.projection = this.calcProjection(options, _class) + } else { + findOptions.projection = { '%hash%': 0 } } const doc = await coll.findOne(mongoQuery, findOptions) @@ -759,6 +761,8 @@ abstract class MongoAdapterBase implements DbAdapter { if (projection != null) { cursor = cursor.project(projection) } + } else { + cursor = cursor.project({ '%hash%': 0 }) } let total: number = -1 if (options != null) {