Skip to content

Commit

Permalink
feat: pass through of arbitrary client options (#1024)
Browse files Browse the repository at this point in the history
Pass through of arbitrary client options should be possible. The
database services do not interpret this but simply pass through
information to the respective db clients.

```json
{
  "db": {
     "kind": "hana",
     "credentials": { ... },
     "client": { "packetSize": 1048576},
  }
}
```
This sample refers to the [`packetSize` parameter of
`hdb`](https://github.com/SAP/node-hdb?tab=readme-ov-file#controlling-the-maximum-packet-size).

For Hana MT and SQLite the "ugly reuse" of credentials is not possible.
In Hana MT, the credentials for the db connection are not in environment
but provided by service manager.
In SQLite, the credentials and options are different parameters for the
client.

SQLite options:
https://github.com/WiseLibs/better-sqlite3/blob/master/docs/api.md#new-databasepath-options
Postgres options: https://node-postgres.com/apis/client#new-client
hdb options:
https://github.com/SAP/node-hdb?tab=readme-ov-file#cesu-8-encoding-support
hana-client options:
https://help.sap.com/docs/PRODUCT_ID/f1b440ded6144a54ada97ff95dac7adf/4fe9978ebac44f35b9369ef5a4a26f4c.html?state=PRODUCTION&version=latest&locale=en-US

closes #994
closes #920
  • Loading branch information
johannes-vogel authored Feb 26, 2025
1 parent bae7f68 commit b090ccd
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 20 deletions.
4 changes: 2 additions & 2 deletions hana/lib/HANAService.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class HANAService extends SQLService {
get factory() {
const driver = drivers[this.options.driver || this.options.credentials?.driver]?.driver || drivers.default.driver
const service = this
const { credentials, kind } = service.options
const { credentials, kind, client: clientOptions = {} } = service.options
if (!credentials) {
throw new Error(`Database kind "${kind}" configured, but no HDI container or Service Manager instance bound to application.`)
}
Expand All @@ -62,7 +62,7 @@ class HANAService extends SQLService {
const { credentials } = isMultitenant
? await require('@sap/cds-mtxs/lib').xt.serviceManager.get(tenant, { disableCache: false })
: service.options
const dbc = new driver(credentials)
const dbc = new driver({...credentials, ...clientOptions})
await dbc.connect()
HANAVERSION = dbc.server.major
return dbc
Expand Down
4 changes: 2 additions & 2 deletions hana/lib/drivers/hana-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class HANAClientDriver extends driver {
*/
constructor(creds) {
// Enable native @sap/hana-client connection pooling
creds = Object.assign({}, creds, {
creds = Object.assign({
// REVISIT: add pooling related credentials when switching to native pools
// Enables the @sap/hana-client native connection pool implementation
// pooling: true,
Expand All @@ -35,7 +35,7 @@ class HANAClientDriver extends driver {
// compress: true, // TODO: test whether having compression enabled makes things faster
// statement caches come with a side effect when the database structure changes which does not apply to CAP
// statementCacheSize: 100, // TODO: test whether statementCaches make things faster
})
}, creds)

// Retain node-hdb credential mappings to @sap/hana-client credential mapping
for (const m of credentialMappings) {
Expand Down
4 changes: 2 additions & 2 deletions postgres/lib/PostgresService.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class PostgresService extends SQLService {
...this.options.pool,
},
create: async () => {
const cr = this.options.credentials || {}
const { credentials: cr = {}, client: clientOptions = {} } = this.options
const credentials = {
// Cloud Foundry provides the user in the field username the pg npm module expects user
user: cr.username || cr.user,
Expand All @@ -49,7 +49,7 @@ class PostgresService extends SQLService {
ca: cr.sslrootcert,
}),
}
const dbc = new Client(credentials)
const dbc = new Client({...credentials, ...clientOptions})
await dbc.connect()
return dbc
},
Expand Down
2 changes: 1 addition & 1 deletion sqlite/lib/SQLiteService.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class SQLiteService extends SQLService {
options: { max: 1, ...this.options.pool },
create: tenant => {
const database = this.url4(tenant)
const dbc = new sqlite(database)
const dbc = new sqlite(database, this.options.client)
const deterministic = { deterministic: true }
dbc.function('session_context', key => dbc[$session][key])
dbc.function('regexp', deterministic, (re, x) => (RegExp(re).test(x) ? 1 : 0))
Expand Down
15 changes: 15 additions & 0 deletions sqlite/test/deep/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "@test/cds-db-layer",
"version": "1.0.0",
"description": "Base for db layer validations",
"cds": {
"requires": {
"db": {
"impl": "@cap-js/sqlite"
}
},
"features": {
"ieee754compatible": true
}
}
}
4 changes: 2 additions & 2 deletions test/cds.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ cds.test = Object.setPrototypeOf(function () {
const sep = path.sep
const testSource = process.argv[1].split(`${sep}test${sep}`)[0]
const serviceDefinitionPath = `${testSource}/test/service`
cds.env.requires.db = require(serviceDefinitionPath)
cds.env.requires.db = {...cds.env.requires.db, ...require(serviceDefinitionPath)}
require(testSource + '/cds-plugin')
} catch {
// Default to sqlite for packages without their own service
cds.env.requires.db = require('@cap-js/sqlite/test/service')
cds.env.requires.db = {...cds.env.requires.db, ...require('@cap-js/sqlite/test/service')}
require('@cap-js/sqlite/cds-plugin')
}
})
Expand Down
18 changes: 9 additions & 9 deletions test/compliance/CREATE.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,22 @@ describe('CREATE', () => {
model.definitions[n].kind === 'entity'
)

describe('custom entites', () => {
describe('custom entities', () => {
const entityName = 'custom.entity'

afterEach(async () => {
const db = await cds.connect()
const db = await cds.connect.to('db')

const { globals } = cds.entities('basic.literals')

await db.run({ DROP: { entity: globals } }).catch(() => { })
await db.run({ DROP: { entity: globals.name } }).catch(() => { })
await db.run({ DROP: { entity: entityName } }).catch(() => { })
await db.run({ DROP: { table: { ref: [entityName] } } }).catch(() => { })
await db.run({ DROP: { view: { ref: [entityName] } } }).catch(() => { })
})

test('definiton provided', async () => {
const db = await cds.connect()
test('definition provided', async () => {
const db = await cds.connect.to('db')

const { globals } = cds.entities('basic.literals')

Expand All @@ -216,17 +216,17 @@ describe('CREATE', () => {
elements: globals.elements
})
await db.run({ CREATE: { entity } })
// REVISIT: reading from entities not in the model requires additional hanlding in infer
// REVISIT: reading from entities not in the model requires additional handling in infer
// await SELECT.from(entity)
})

test('definiton provided', async () => {
const db = await cds.connect()
test('definition provided', async () => {
const db = await cds.connect.to('db')

const { globals } = cds.entities('basic.literals')

const query = SELECT.from(globals)
// REVISIT: reading from entities not in the model requires additional hanlding in infer
// REVISIT: reading from entities not in the model requires additional handling in infer
/*
const entity = new cds.entity({
kind: 'entity',
Expand Down
22 changes: 22 additions & 0 deletions test/compliance/client-options.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const assert = require('assert')
const cds = require('../cds.js')
cds.test.in(__dirname + '/resources')

const clientOption = cds.env.requires.db.client
let called = 0
Object.defineProperty(cds.env.requires.db, 'client', {
get: () => {
called++
return clientOption
}
})
/**
* Tests explicitely, that all DBs access the specific client options
*/
describe('affected rows', () => {
cds.test()

test('client option is called during bootstrapping', async () => {
assert.strictEqual(called,1)
})
})
4 changes: 2 additions & 2 deletions test/compliance/resources/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"requires": {
"db": {
"impl": "@cap-js/sqlite",
"credentials": {
"url": ":memory:"
"client": {
"xy": 51
}
}
},
Expand Down

0 comments on commit b090ccd

Please sign in to comment.