From ec7501fa72454a9c9dfc45009b2768e6bd0a8637 Mon Sep 17 00:00:00 2001 From: vkozyura Date: Wed, 17 Jan 2024 11:07:23 +0100 Subject: [PATCH 1/6] feat: SELECT in Sqlite return binaries as Buffers --- sqlite/lib/SQLiteService.js | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/sqlite/lib/SQLiteService.js b/sqlite/lib/SQLiteService.js index 5ecf1da38..af93f012c 100644 --- a/sqlite/lib/SQLiteService.js +++ b/sqlite/lib/SQLiteService.js @@ -5,6 +5,11 @@ const $session = Symbol('dbc.session') const convStrm = require('stream/consumers') const { Readable } = require('stream') +const BINARY_TYPES = { + 'cds.Binary': 1, + 'cds.hana.BINARY': 1 +} + class SQLiteService extends SQLService { init() { return super.init(...arguments) @@ -137,6 +142,38 @@ class SQLiteService extends SQLService { return (await ps.run(vals)).changes } + // change each binary to Buffer except of large binaries + _changeToBuffers(columns, rows, one, compat) { + if (!rows || !columns) return + if (!Array.isArray(rows)) rows = [rows] + if (!rows.length || !Object.keys(rows[0]).length) return + if (compat) return + + for (let col of columns) { + const name = col.as || col.ref?.[col.ref.length - 1] || (typeof col === 'string' && col) + if (col.element?.isAssociation) { + if (one) this._changeToBuffers(col.SELECT.columns, rows[0][name], false, compat) + else + rows.forEach(row => { + this._changeToBuffers(col.SELECT.columns, row[name], false, compat) + }) + } else if (col.element?.type in BINARY_TYPES) { + if (one) rows[0][name] = Buffer.from(rows[0][name], 'base64') + else + rows.forEach(row => { + row[name] = Buffer.from(row[name], 'base64') + }) + } + } + } + + async onSELECT(req) { + const rows = await super.onSELECT(req) + this._changeToBuffers(req.query.SELECT.columns, rows, req.query.SELECT.one, cds.env.features.stream_compat) + + return rows + } + onPlainSQL({ query, data }, next) { if (typeof query === 'string') { // REVISIT: this is a hack the target of $now might not be a timestamp or date time From 32f81ed699f031175b2a58bc68fae0e4b7adc4b2 Mon Sep 17 00:00:00 2001 From: vkozyura Date: Wed, 17 Jan 2024 13:35:46 +0100 Subject: [PATCH 2/6] add binary tests --- sqlite/lib/SQLiteService.js | 11 +++++-- test/compliance/CREATE.test.js | 21 +++++++++++-- .../resources/db/basic/literals.cds | 5 +++ .../basic/literals/basic.literals.binaries.js | 31 +++++++++++++++++++ 4 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 test/compliance/resources/db/basic/literals/basic.literals.binaries.js diff --git a/sqlite/lib/SQLiteService.js b/sqlite/lib/SQLiteService.js index af93f012c..c53b76bc8 100644 --- a/sqlite/lib/SQLiteService.js +++ b/sqlite/lib/SQLiteService.js @@ -142,6 +142,11 @@ class SQLiteService extends SQLService { return (await ps.run(vals)).changes } + _buffer(val) { + if (val === null) return null + return Buffer.from(val, 'base64') + } + // change each binary to Buffer except of large binaries _changeToBuffers(columns, rows, one, compat) { if (!rows || !columns) return @@ -158,10 +163,10 @@ class SQLiteService extends SQLService { this._changeToBuffers(col.SELECT.columns, row[name], false, compat) }) } else if (col.element?.type in BINARY_TYPES) { - if (one) rows[0][name] = Buffer.from(rows[0][name], 'base64') + if (one) rows[0][name] = this._buffer(rows[0][name]) else rows.forEach(row => { - row[name] = Buffer.from(row[name], 'base64') + row[name] = this._buffer(row[name]) }) } } @@ -169,7 +174,7 @@ class SQLiteService extends SQLService { async onSELECT(req) { const rows = await super.onSELECT(req) - this._changeToBuffers(req.query.SELECT.columns, rows, req.query.SELECT.one, cds.env.features.stream_compat) + this._changeToBuffers(this.cqn4sql(req.query).SELECT.columns, rows, req.query.SELECT.one, cds.env.features.stream_compat) return rows } diff --git a/test/compliance/CREATE.test.js b/test/compliance/CREATE.test.js index 1006b891b..bfd3f9653 100644 --- a/test/compliance/CREATE.test.js +++ b/test/compliance/CREATE.test.js @@ -1,4 +1,6 @@ const assert = require('assert') +const { Readable } = require('stream') +const { buffer } = require('stream/consumers') const cds = require('../cds.js') const fspath = require('path') // Add the test names you want to run as only @@ -126,6 +128,9 @@ describe('CREATE', () => { if (Buffer.isBuffer(b) || b?.type === 'Buffer') { return `Buffer(${b.byteLength || b.data?.length})` } + if (b instanceof Readable) { + return 'Readable' + } if (typeof b === 'function') return `${b}` return b }, @@ -167,18 +172,30 @@ describe('CREATE', () => { if (throws !== false) assert.equal('resolved', throws, 'Ensure that the correct error message is being thrown.') + const columns = [] + for (let col in entity.elements) { + columns.push({ ref: [col] }) + } + // Extract data set const sel = await tx.run({ SELECT: { from: { ref: [table] }, + columns }, }) // TODO: Can we expect all Database to respond in insert order ? const result = sel[sel.length - 1] - Object.keys(expect).forEach(k => { + await Promise.all(Object.keys(expect).map(async k => { const msg = `Ensure that the Database echos correct data back, property ${k} does not match expected result.` + if (result[k] instanceof Readable) { + result[k] = await buffer(result[k]) + } + if (expect[k] instanceof Readable) { + expect[k] = await buffer(expect[k]) + } if (result[k] instanceof Buffer && expect[k] instanceof Buffer) { assert.equal(result[k].compare(expect[k]), 0, `${msg} (Buffer contents are different)`) } else if (typeof expect[k] === 'object' && expect[k]) { @@ -186,7 +203,7 @@ describe('CREATE', () => { } else { assert.equal(result[k], expect[k], msg) } - }) + })) }) }, ) diff --git a/test/compliance/resources/db/basic/literals.cds b/test/compliance/resources/db/basic/literals.cds index a594857ca..79d2d324a 100644 --- a/test/compliance/resources/db/basic/literals.cds +++ b/test/compliance/resources/db/basic/literals.cds @@ -51,3 +51,8 @@ entity array { string : array of String; integer : array of Integer; } + +entity binaries { + binary : Binary; + largebinary : LargeBinary; +} diff --git a/test/compliance/resources/db/basic/literals/basic.literals.binaries.js b/test/compliance/resources/db/basic/literals/basic.literals.binaries.js new file mode 100644 index 000000000..52684ad29 --- /dev/null +++ b/test/compliance/resources/db/basic/literals/basic.literals.binaries.js @@ -0,0 +1,31 @@ +const { Readable } = require('stream') + +const generator = function*() { + yield Buffer.from('Simple Large Binary') +} + +module.exports = [ + { + binary: null, + largebinary: null, + }, + { + binary: Buffer.from('Simple Binary') + }, + { + binary: Buffer.from('Simple Binary').toString('base64'), + '=binary': Buffer.from('Simple Binary') + }, + { + largebinary: Buffer.from('Simple Large Binary'), + '=largebinary': () => Readable.from(generator()) + }, + { + largebinary: Buffer.from('Simple Large Binary').toString('base64'), + '=largebinary': () => Readable.from(generator()) + }, + { + largebinary: Readable.from(generator()), + '=largebinary': () => Readable.from(generator()) + } +] From 85cfb8f9bfa87cd9dc9d7a9f2b94ef38fe1cc979 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Wed, 17 Jan 2024 14:32:45 +0100 Subject: [PATCH 3/6] Move buffer converter to SQLService --- db-service/lib/SQLService.js | 20 +++++++++++++++-- sqlite/lib/SQLiteService.js | 42 ------------------------------------ 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js index eac8fb311..1889cace0 100644 --- a/db-service/lib/SQLService.js +++ b/db-service/lib/SQLService.js @@ -5,6 +5,11 @@ const { resolveView } = require('@sap/cds/libx/_runtime/common/utils/resolveView const DatabaseService = require('./common/DatabaseService') const cqn4sql = require('./cqn4sql') +const BINARY_TYPES = { + 'cds.Binary': 1, + 'cds.hana.BINARY': 1 +} + /** @typedef {import('@sap/cds/apis/services').Request} Request */ /** @@ -54,6 +59,12 @@ class SQLService extends DatabaseService { rows.forEach(row => { row[name] = this._stream(row[name]) }) + } else if (col.element?.type in BINARY_TYPES) { + if (one) rows[0][name] = this._buffer(rows[0][name]) + else + rows.forEach(row => { + row[name] = this._buffer(row[name]) + }) } } } @@ -72,6 +83,11 @@ class SQLService extends DatabaseService { }) } + _buffer(val) { + if (val === null) return null + return Buffer.from(val, 'base64') + } + /** * Handler for SELECT * @type {Handler} @@ -177,8 +193,8 @@ class SQLService extends DatabaseService { } else if (visited.includes(c._target.name)) throw new Error( `Transitive circular composition detected: \n\n` + - ` ${visited.join(' > ')} > ${c._target.name} \n\n` + - `These are not supported by deep delete.`, + ` ${visited.join(' > ')} > ${c._target.name} \n\n` + + `These are not supported by deep delete.`, ) // Prepare and run deep query, à la CQL`DELETE from Foo[pred]:comp1.comp2...` const query = DELETE.from({ ref: [...from.ref, c.name] }) diff --git a/sqlite/lib/SQLiteService.js b/sqlite/lib/SQLiteService.js index c53b76bc8..5ecf1da38 100644 --- a/sqlite/lib/SQLiteService.js +++ b/sqlite/lib/SQLiteService.js @@ -5,11 +5,6 @@ const $session = Symbol('dbc.session') const convStrm = require('stream/consumers') const { Readable } = require('stream') -const BINARY_TYPES = { - 'cds.Binary': 1, - 'cds.hana.BINARY': 1 -} - class SQLiteService extends SQLService { init() { return super.init(...arguments) @@ -142,43 +137,6 @@ class SQLiteService extends SQLService { return (await ps.run(vals)).changes } - _buffer(val) { - if (val === null) return null - return Buffer.from(val, 'base64') - } - - // change each binary to Buffer except of large binaries - _changeToBuffers(columns, rows, one, compat) { - if (!rows || !columns) return - if (!Array.isArray(rows)) rows = [rows] - if (!rows.length || !Object.keys(rows[0]).length) return - if (compat) return - - for (let col of columns) { - const name = col.as || col.ref?.[col.ref.length - 1] || (typeof col === 'string' && col) - if (col.element?.isAssociation) { - if (one) this._changeToBuffers(col.SELECT.columns, rows[0][name], false, compat) - else - rows.forEach(row => { - this._changeToBuffers(col.SELECT.columns, row[name], false, compat) - }) - } else if (col.element?.type in BINARY_TYPES) { - if (one) rows[0][name] = this._buffer(rows[0][name]) - else - rows.forEach(row => { - row[name] = this._buffer(row[name]) - }) - } - } - } - - async onSELECT(req) { - const rows = await super.onSELECT(req) - this._changeToBuffers(this.cqn4sql(req.query).SELECT.columns, rows, req.query.SELECT.one, cds.env.features.stream_compat) - - return rows - } - onPlainSQL({ query, data }, next) { if (typeof query === 'string') { // REVISIT: this is a hack the target of $now might not be a timestamp or date time From 4c849bac7e847f53fd744962b7c01eded99929d6 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Thu, 18 Jan 2024 09:05:51 +0100 Subject: [PATCH 4/6] Switch HANA Service to using hex encoding --- db-service/lib/cqn2sql.js | 8 ++++---- hana/lib/HANAService.js | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/db-service/lib/cqn2sql.js b/db-service/lib/cqn2sql.js index 56de86ed4..4482f6a53 100644 --- a/db-service/lib/cqn2sql.js +++ b/db-service/lib/cqn2sql.js @@ -440,7 +440,7 @@ class CQN2SQLRenderer { }) SELECT ${extraction} FROM json_each(?)`) } - async *INSERT_entries_stream(entries) { + async *INSERT_entries_stream(entries, binaryEncoding = 'base64') { const bufferLimit = 1 << 16 let buffer = '[' @@ -459,7 +459,7 @@ class CQN2SQLRenderer { buffer += `${keyJSON}"` // TODO: double check that it works - val.setEncoding('base64') + val.setEncoding(binaryEncoding) for await (const chunk of val) { buffer += chunk if (buffer.length > bufferLimit) { @@ -484,7 +484,7 @@ class CQN2SQLRenderer { yield buffer } - async *INSERT_rows_stream(entries) { + async *INSERT_rows_stream(entries, binaryEncoding = 'base64') { const bufferLimit = 1 << 16 let buffer = '[' @@ -500,7 +500,7 @@ class CQN2SQLRenderer { buffer += `${sepsub}"` // TODO: double check that it works - val.setEncoding('base64') + val.setEncoding(binaryEncoding) for await (const chunk of val) { buffer += chunk if (buffer.length > bufferLimit) { diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index db67434eb..a9b0a9aee 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -1,6 +1,6 @@ const fs = require('fs') const path = require('path') -const { Readable } = require('stream') +const { Readable } = require('stream') const { SQLService } = require('@cap-js/db-service') const drivers = require('./drivers') @@ -527,12 +527,12 @@ class HANAService extends SQLService { // HANA Express does not process large JSON documents // The limit is somewhere between 64KB and 128KB if (HANAVERSION <= 2) { - this.entries = INSERT.entries.map(e => (e instanceof Readable ? [e] : [Readable.from(this.INSERT_entries_stream([e]))])) + this.entries = INSERT.entries.map(e => (e instanceof Readable ? [e] : [Readable.from(this.INSERT_entries_stream([e], 'hex'))])) } else { this.entries = [ INSERT.entries[0] instanceof Readable ? INSERT.entries[0] - : Readable.from(this.INSERT_entries_stream(INSERT.entries)) + : Readable.from(this.INSERT_entries_stream(INSERT.entries, 'hex')) ] } @@ -859,7 +859,7 @@ class HANAService extends SQLService { // REVISIT: BASE64_DECODE has stopped working // Unable to convert NVARCHAR to UTF8 // Not encoded string with CESU-8 or some UTF-8 except a surrogate pair at "base64_decode" function - Binary: e => `CONCAT('base64,',${e})`, + Binary: e => `HEXTOBIN(${e})`, Boolean: e => `CASE WHEN ${e} = 'true' THEN TRUE WHEN ${e} = 'false' THEN FALSE END`, } @@ -1013,7 +1013,7 @@ const createContainerDatabase = fs.readFileSync(path.resolve(__dirname, 'scripts const createContainerTenant = fs.readFileSync(path.resolve(__dirname, 'scripts/container-tenant.sql'), 'utf-8') Buffer.prototype.toJSON = function () { - return this.toString('base64') + return this.toString('hex') } const is_regexp = x => x?.constructor?.name === 'RegExp' // NOTE: x instanceof RegExp doesn't work in repl From 13364ec0635b47565c4c752dcf07fe6d22239f25 Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Thu, 18 Jan 2024 09:07:43 +0100 Subject: [PATCH 5/6] Fix upsert for entities with multiple key columns --- hana/lib/HANAService.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index a9b0a9aee..0a606dc0d 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -603,7 +603,7 @@ class HANAService extends SQLService { Object.keys(keys) .filter(k => !keys[k].isAssociation) .map(k => `NEW.${this.quote(k)}=OLD.${this.quote(k)}`) - .join('AND') + .join(' AND ') return (this.sql = `UPSERT ${this.quote(entity)} (${this.columns.map(c => this.quote(c), From e8184206f4e84b48d6c9e878f4b7d9ae968f95be Mon Sep 17 00:00:00 2001 From: BobdenOs Date: Thu, 18 Jan 2024 11:44:41 +0100 Subject: [PATCH 6/6] transform base64 strings for HANA to hex --- db-service/lib/cqn2sql.js | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/db-service/lib/cqn2sql.js b/db-service/lib/cqn2sql.js index 4482f6a53..57fa1b2a8 100644 --- a/db-service/lib/cqn2sql.js +++ b/db-service/lib/cqn2sql.js @@ -2,6 +2,12 @@ const cds = require('@sap/cds/lib') const cds_infer = require('./infer') const cqn4sql = require('./cqn4sql') +const BINARY_TYPES = { + 'cds.Binary': 1, + 'cds.LargeBinary': 1, + 'cds.hana.BINARY': 1, +} + const { Readable } = require('stream') const DEBUG = (() => { @@ -441,7 +447,11 @@ class CQN2SQLRenderer { } async *INSERT_entries_stream(entries, binaryEncoding = 'base64') { - const bufferLimit = 1 << 16 + const elements = this.cqn.target?.elements || {} + const transformBase64 = binaryEncoding === 'base64' + ? a => a + : a => a != null ? Buffer.from(a, 'base64').toString(binaryEncoding) : a + const bufferLimit = 65536 // 1 << 16 let buffer = '[' let sep = '' @@ -454,7 +464,7 @@ class CQN2SQLRenderer { const keyJSON = `${sepsub}${JSON.stringify(key)}:` if (!sepsub) sepsub = ',' - const val = row[key] + let val = row[key] if (val instanceof Readable) { buffer += `${keyJSON}"` @@ -470,6 +480,9 @@ class CQN2SQLRenderer { buffer += '"' } else { + if (elements[key]?.type in BINARY_TYPES) { + val = transformBase64(val) + } buffer += `${keyJSON}${val === undefined ? 'null' : JSON.stringify(val)}` } } @@ -485,7 +498,11 @@ class CQN2SQLRenderer { } async *INSERT_rows_stream(entries, binaryEncoding = 'base64') { - const bufferLimit = 1 << 16 + const elements = this.cqn.target?.elements || {} + const transformBase64 = binaryEncoding === 'base64' + ? a => a + : a => a != null ? Buffer.from(a, 'base64').toString(binaryEncoding) : a + const bufferLimit = 65536 // 1 << 16 let buffer = '[' let sep = '' @@ -495,7 +512,7 @@ class CQN2SQLRenderer { let sepsub = '' for (let key = 0; key < row.length; key++) { - const val = row[key] + let val = row[key] if (val instanceof Readable) { buffer += `${sepsub}"` @@ -511,6 +528,9 @@ class CQN2SQLRenderer { buffer += '"' } else { + if (elements[this.columns[key]]?.type in BINARY_TYPES) { + val = transformBase64(val) + } buffer += `${sepsub}${val === undefined ? 'null' : JSON.stringify(val)}` } @@ -656,7 +676,7 @@ class CQN2SQLRenderer { * @returns {string} SQL */ UPDATE(q) { - const { entity, with: _with, data, where } = q.UPDATE + const { entity, with: _with, data, where } = q.UPDATE const elements = q.target?.elements let sql = `UPDATE ${this.name(entity.ref?.[0] || entity)}` if (entity.as) sql += ` AS ${entity.as}` @@ -813,8 +833,8 @@ class CQN2SQLRenderer { case 'object': if (val === null) return 'NULL' if (val instanceof Date) return `'${val.toISOString()}'` - if (val instanceof Readable) ; // go on with default below - else if (Buffer.isBuffer(val)) ; // go on with default below + if (val instanceof Readable); // go on with default below + else if (Buffer.isBuffer(val)); // go on with default below else if (is_regexp(val)) val = val.source else val = JSON.stringify(val) case 'string': // eslint-disable-line no-fallthrough