From d39d4ea16c1eaf9cce6f58bec7dea98059dd2ff4 Mon Sep 17 00:00:00 2001 From: Andrey Sobolev Date: Thu, 6 Feb 2025 23:52:26 +0700 Subject: [PATCH] Fix green reserve (#7951) Signed-off-by: Andrey Sobolev --- pods/green/src/index.ts | 7 +- .../postgres/src/__tests__/conversion.spec.ts | 105 ++++++++++++++++++ server/postgres/src/__tests__/minmodel.ts | 42 ++++++- server/postgres/src/__tests__/utils.ts | 18 +++ server/postgres/src/client.ts | 3 +- server/postgres/src/storage.ts | 2 +- server/server/src/utils.ts | 2 +- server/tool/src/index.ts | 2 +- tests/docker-compose.yaml | 1 - workers/transactor/src/transactor.ts | 5 +- 10 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 server/postgres/src/__tests__/utils.ts diff --git a/pods/green/src/index.ts b/pods/green/src/index.ts index 622c7f16ad..fe52e7c64f 100644 --- a/pods/green/src/index.ts +++ b/pods/green/src/index.ts @@ -40,6 +40,7 @@ async function toResponse (compression: string, data: any, response: http.Server .writeHead(200, { 'content-type': 'application/json', compression: 'snappy', + 'content-encoding': 'snappy', 'keep-alive': 'timeout=5' }) .end(await compress(JSON.stringify(data))) @@ -80,7 +81,7 @@ async function handleSQLFind ( const qid = ++queryId try { const lq = (json.query as string).toLowerCase() - if (lq.includes('begin') || lq.includes('commit') || lq.includes('rollback')) { + if (filterInappropriateQuries(lq)) { console.error('not allowed', json.query) response.writeHead(403).end('Not allowed') return @@ -128,3 +129,7 @@ const reqHandler = (req: http.IncomingMessage, resp: http.ServerResponse): void } http.createServer(reqHandler).listen(port) +function filterInappropriateQuries (lq: string): boolean { + const harmfulPatterns = ['begin', 'commit', 'rollback', 'drop', 'alter', 'truncate'] + return harmfulPatterns.some((pattern) => lq.includes(pattern)) +} diff --git a/server/postgres/src/__tests__/conversion.spec.ts b/server/postgres/src/__tests__/conversion.spec.ts index 62e2f1b07a..f1162791a4 100644 --- a/server/postgres/src/__tests__/conversion.spec.ts +++ b/server/postgres/src/__tests__/conversion.spec.ts @@ -1,4 +1,19 @@ +import core, { + Hierarchy, + MeasureMetricsContext, + ModelDb, + TxFactory, + type DocumentUpdate, + type PersonId, + type Ref, + type Space, + type Tx, + type WorkspaceUuid +} from '@hcengineering/core' +import { PostgresAdapter } from '../storage' import { convertArrayParams, decodeArray } from '../utils' +import { genMinModel, test, type ComplexClass } from './minmodel' +import { createDummyClient, type TypedQuery } from './utils' describe('array conversion', () => { it('should handle undefined parameters', () => { @@ -55,3 +70,93 @@ describe('array decoding', () => { expect(decodeArray('{"first \\"quote\\"","second \\"quote\\""}')).toEqual(['first "quote"', 'second "quote"']) }) }) + +const factory = new TxFactory('email:test' as PersonId) +function upd (id: string, partial: DocumentUpdate): Tx { + return factory.createTxUpdateDoc( + test.class.ComplexClass, + core.space.Workspace, + id as Ref, + partial + ) +} + +describe('query to sql conversion tests', () => { + it('check dummy db client', async () => { + const queries: TypedQuery[] = [] + const c = createDummyClient(queries) + + await c.execute('select now()') + expect(queries[0].query).toEqual('select now()') + }) + it('check simple update', async () => { + const { adapter, ctx, queries } = createTestContext() + + await adapter.tx( + ctx, + upd('obj1', { + stringField: 'test' + }) + ) + expect(queries[0].query).toEqual( + 'UPDATE pg_testing SET "modifiedBy" = update_data."_modifiedBy", "modifiedOn" = update_data."_modifiedOn", "%hash%" = update_data."_%hash%", data = COALESCE(data || update_data._data)\n FROM (values ($2::text, $3::text,$4::bigint,$5::text,$6::jsonb)) AS update_data(__id, "_modifiedBy","_modifiedOn","_%hash%","_data")\n WHERE "workspaceId" = $1::uuid AND "_id" = update_data.__id' + ) + }) + it('check space update', async () => { + const { adapter, ctx, queries } = createTestContext() + + await adapter.tx( + ctx, + upd('obj1', { + space: 'new-space' as Ref + }) + ) + expect(queries[0].query).toEqual( + 'UPDATE pg_testing SET "modifiedBy" = update_data."_modifiedBy", "modifiedOn" = update_data."_modifiedOn", "%hash%" = update_data."_%hash%", "space" = update_data."_space"\n FROM (values ($2::text, $3::text,$4::bigint,$5::text,$6::text)) AS update_data(__id, "_modifiedBy","_modifiedOn","_%hash%","_space")\n WHERE "workspaceId" = $1::uuid AND "_id" = update_data.__id' + ) + }) + it('check few documents update', async () => { + const { adapter, ctx, queries } = createTestContext() + + await adapter.tx( + ctx, + upd('obj1', { + stringField: 'test' + }), + upd('obj2', { + stringField: 'test2' + }), + upd('obj3', { + stringField: 'test' + }) + ) + expect(queries[0].query).toEqual( + 'UPDATE pg_testing SET "modifiedBy" = update_data."_modifiedBy", "modifiedOn" = update_data."_modifiedOn", "%hash%" = update_data."_%hash%", data = COALESCE(data || update_data._data)\n FROM (values ($2::text, $3::text,$4::bigint,$5::text,$6::jsonb),($7::text, $8::text,$9::bigint,$10::text,$11::jsonb),($12::text, $13::text,$14::bigint,$15::text,$16::jsonb)) AS update_data(__id, "_modifiedBy","_modifiedOn","_%hash%","_data")\n WHERE "workspaceId" = $1::uuid AND "_id" = update_data.__id' + ) + }) +}) +function createTestContext (): { adapter: PostgresAdapter, ctx: MeasureMetricsContext, queries: TypedQuery[] } { + const ctx = new MeasureMetricsContext('test', {}) + const queries: TypedQuery[] = [] + const c = createDummyClient(queries) + + const minModel = genMinModel() + const hierarchy = new Hierarchy() + for (const tx of minModel) { + hierarchy.tx(tx) + } + const modelDb = new ModelDb(hierarchy) + modelDb.addTxes(ctx, minModel, true) + const adapter = new PostgresAdapter( + c, + { + url: () => 'test', + close: () => {} + }, + 'workspace' as WorkspaceUuid, + hierarchy, + modelDb, + 'test' + ) + return { adapter, ctx, queries } +} diff --git a/server/postgres/src/__tests__/minmodel.ts b/server/postgres/src/__tests__/minmodel.ts index 80910fce69..3eb179e622 100644 --- a/server/postgres/src/__tests__/minmodel.ts +++ b/server/postgres/src/__tests__/minmodel.ts @@ -14,19 +14,20 @@ // import core, { - type PersonId, type Arr, type AttachedDoc, type Class, ClassifierKind, type Data, type Doc, + type Domain, DOMAIN_DOC_INDEX_STATE, DOMAIN_MODEL, DOMAIN_RELATION, DOMAIN_TX, type Mixin, type Obj, + type PersonId, type Ref, type TxCreateDoc, type TxCUD, @@ -72,15 +73,33 @@ export interface AttachedComment extends AttachedDoc { message: string } +export interface ComplexClass extends Doc { + stringField: string + numberField: number + booleanField: boolean + arrayField: string[] + numberArrayField: number[] +} + +export interface ComplexMixin extends Mixin { + stringField: string + numberField: number + booleanField: boolean + arrayField: string[] + numberArrayField: number[] +} + /** * @public */ export const test = plugin('test' as Plugin, { mixin: { - TestMixin: '' as Ref> + TestMixin: '' as Ref>, + ComplexMixin: '' as Ref> }, class: { - TestComment: '' as Ref> + TestComment: '' as Ref>, + ComplexClass: '' as Ref> } }) @@ -197,6 +216,23 @@ export function genMinModel (): TxCUD[] { kind: ClassifierKind.CLASS }) ) + txes.push( + createClass(test.class.ComplexClass, { + label: 'ComplexClass' as IntlString, + extends: core.class.Doc, + kind: ClassifierKind.CLASS, + domain: 'pg-testing' as Domain + }) + ) + + txes.push( + createClass(test.mixin.ComplexMixin, { + label: 'ComplexMixin' as IntlString, + extends: test.class.ComplexClass, + kind: ClassifierKind.MIXIN, + domain: 'pg-testing' as Domain + }) + ) const u1 = 'User1' as PersonId const u2 = 'User2' as PersonId diff --git a/server/postgres/src/__tests__/utils.ts b/server/postgres/src/__tests__/utils.ts new file mode 100644 index 0000000000..96b2da94ee --- /dev/null +++ b/server/postgres/src/__tests__/utils.ts @@ -0,0 +1,18 @@ +import type { DBClient } from '../client' + +export interface TypedQuery { + query: string + params?: any[] +} +export function createDummyClient (queries: TypedQuery[]): DBClient { + const client: DBClient = { + execute: async (query, params) => { + queries.push({ query, params }) + return Object.assign([], { count: 0 }) + }, + raw: () => jest.fn() as any, + reserve: async () => client, + release: jest.fn() + } + return client +} diff --git a/server/postgres/src/client.ts b/server/postgres/src/client.ts index be9e905f5e..9f54824da9 100644 --- a/server/postgres/src/client.ts +++ b/server/postgres/src/client.ts @@ -83,8 +83,7 @@ class GreenClient implements DBClient { release (): void {} async reserve (): Promise { - // We do reserve of connection, if we need it. - return createGreenDBClient(this.url, this.token, await this.connection.reserve(), this.decoder) + return createDBClient(await this.connection.reserve()) } raw (): postgres.Sql { diff --git a/server/postgres/src/storage.ts b/server/postgres/src/storage.ts index 30a209f3bf..1e89beed16 100644 --- a/server/postgres/src/storage.ts +++ b/server/postgres/src/storage.ts @@ -1714,7 +1714,7 @@ interface OperationBulk { const initRateLimit = new RateLimiter(1) -class PostgresAdapter extends PostgresAdapterBase { +export class PostgresAdapter extends PostgresAdapterBase { async init ( ctx: MeasureContext, contextVars: Record, diff --git a/server/server/src/utils.ts b/server/server/src/utils.ts index 06e40811e9..389fecc7f6 100644 --- a/server/server/src/utils.ts +++ b/server/server/src/utils.ts @@ -42,7 +42,7 @@ export function doSessionOp ( ): void { if (data.session instanceof Promise) { // We need to copy since we will out of protected buffer area - const msgCopy = Buffer.copyBytesFrom(msg) + const msgCopy = Buffer.copyBytesFrom(new Uint8Array(msg)) void data.session .then((_session) => { data.session = _session diff --git a/server/tool/src/index.ts b/server/tool/src/index.ts index cb0fecc8d7..cd30e8d2e6 100644 --- a/server/tool/src/index.ts +++ b/server/tool/src/index.ts @@ -111,7 +111,7 @@ export async function initModel ( } try { - logger.log('creating database...', workspaceId) + logger.log('creating database...', { workspaceId }) const firstTx: Tx = { _class: core.class.Tx, _id: 'first-tx' as Ref, diff --git a/tests/docker-compose.yaml b/tests/docker-compose.yaml index 73162d5129..85ff38005f 100644 --- a/tests/docker-compose.yaml +++ b/tests/docker-compose.yaml @@ -12,7 +12,6 @@ services: image: cockroachdb/cockroach:latest-v24.2 ports: - '26258:26257' - - '8089:8080' command: start-single-node --insecure restart: unless-stopped minio: diff --git a/workers/transactor/src/transactor.ts b/workers/transactor/src/transactor.ts index 0d478d3eef..3d8116d973 100644 --- a/workers/transactor/src/transactor.ts +++ b/workers/transactor/src/transactor.ts @@ -219,13 +219,14 @@ export class Transactor extends DurableObject { const st = Date.now() const r = this.sessionManager.handleRequest(this.measureCtx, s.session, cs, request, this.workspace) void r.finally(() => { + const time = Date.now() - st console.log({ - message: 'handle-request', + message: 'handle-request: ' + time, method: request.method, params: request.params, workspace: s.workspaceId, user: s.session.getUser(), - time: Date.now() - st + time }) }) this.ctx.waitUntil(r)