Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improved behavioral consistency between the database services #837

Merged
merged 4 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions .github/actions/hxe/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: 'Start HANA'
description: 'Starts an local HANA Express instance for isolated testing'
inputs:
GITHUB_TOKEN:
description: 'Derivative token for using the GitHub REST API'
required: true
outputs:
TAG:
description: "The Image Tag"
value: ${{ steps.find-hxe.outputs.TAG }}
IMAGE_ID:
description: "The "
value: ${{ steps.find-hxe.outputs.IMAGE_ID }}
runs:
using: "composite"
steps:
- name: Find HXE image
id: find-hxe
shell: bash
# TODO: replace hana/tools/docker/hxe/* with ${{ github.action_path }}
run: |
TAG="$(sha1sum hana/tools/docker/hxe/* | sha1sum --tag | grep '[^ ]*$' -o)";
IMAGE_ID=ghcr.io/${{ github.repository_owner }}/hanaexpress;
IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]');
echo "TAG=${TAG}" >> $GITHUB_OUTPUT;
echo "IMAGE_ID=${IMAGE_ID}" >> $GITHUB_OUTPUT;
GHCR_TOKEN=$(echo ${{ inputs.GITHUB_TOKEN }} | base64);
if
curl -H "Authorization: Bearer ${GHCR_TOKEN}" https://ghcr.io/v2/${{ github.repository_owner }}/hanaexpress/manifests/$TAG | grep "MANIFEST_UNKNOWN";
then
echo "BUILD_HXE=true" >> $GITHUB_OUTPUT
else
echo "BUILD_HXE=false" >> $GITHUB_OUTPUT
fi;
- name: Set up Docker Buildx
if: ${{ steps.find-hxe.outputs.BUILD_HXE == 'true' }}
uses: docker/setup-buildx-action@v3
- name: Build HXE image
if: ${{ steps.find-hxe.outputs.BUILD_HXE == 'true' }}
shell: bash
run: |
echo "${{ inputs.GITHUB_TOKEN }}" | docker login ghcr.io -u $ --password-stdin;
DOCKER_BUILDKIT=1 docker build -t $IMAGE_ID:$TAG ./hana/tools/docker/hxe;
docker push $IMAGE_ID:$TAG;
env:
TAG: ${{ steps.find-hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.find-hxe.outputs.IMAGE_ID }}
- name: Start HXE image
shell: bash
run: |
echo "${{ inputs.GITHUB_TOKEN }}" | docker login ghcr.io -u $ --password-stdin;
{ npm start -w hana; } &
env:
TAG: ${{ steps.find-hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.find-hxe.outputs.IMAGE_ID }}
84 changes: 0 additions & 84 deletions .github/workflows/hana.yml

This file was deleted.

48 changes: 9 additions & 39 deletions .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,56 +19,26 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 18
node-version: 20
registry-url: 'https://registry.npmjs.org'

# Run tests
- run: npm ci
if: ${{ steps.release.outputs.releases_created }}
# test sqlite/postgres/db-service
- run: npm test -w db-service -w sqlite -w postgres -- --maxWorkers=1
- run: npm test -w db-service -w sqlite -w postgres
if: ${{ steps.release.outputs.releases_created }}
# test hana
# TODO: Factor this setup script out
- name: Find HXE image
- id: hxe
if: ${{ steps.release.outputs.releases_created }}
id: find-hxe
run: |
TAG="$(sha1sum hana/tools/docker/hxe/* | sha1sum --tag | grep '[^ ]*$' -o)";
IMAGE_ID=ghcr.io/${{ github.repository_owner }}/hanaexpress;
IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]');
echo "TAG=${TAG}" >> $GITHUB_OUTPUT;
echo "IMAGE_ID=${IMAGE_ID}" >> $GITHUB_OUTPUT;
GHCR_TOKEN=$(echo ${{ secrets.GITHUB_TOKEN }} | base64);
if
curl -H "Authorization: Bearer ${GHCR_TOKEN}" https://ghcr.io/v2/${{ github.repository_owner }}/hanaexpress/manifests/$TAG | grep "MANIFEST_UNKNOWN";
then
echo "BUILD_HXE=true" >> $GITHUB_OUTPUT
else
echo "BUILD_HXE=false" >> $GITHUB_OUTPUT
fi;
- name: Set up Docker Buildx
if: ${{ steps.release.outputs.releases_created && steps.find-hxe.outputs.BUILD_HXE == 'true' }}
uses: docker/setup-buildx-action@v3
- name: Build HXE image
if: ${{ steps.release.outputs.releases_created && steps.find-hxe.outputs.BUILD_HXE == 'true' }}
run: |
echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u $ --password-stdin;
DOCKER_BUILDKIT=1 docker build -t $IMAGE_ID:$TAG ./hana/tools/docker/hxe;
docker push $IMAGE_ID:$TAG;
env:
TAG: ${{ steps.find-hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.find-hxe.outputs.IMAGE_ID }}
- name: Start HXE image
uses: ./.github/actions/hxe
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- run: npm test -w hana
if: ${{ steps.release.outputs.releases_created }}
run: |
echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u $ --password-stdin;
{ npm start -w hana; } &
env:
TAG: ${{ steps.find-hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.find-hxe.outputs.IMAGE_ID }}
- run: npm test -w hana -- --maxWorkers=1
if: ${{ steps.release.outputs.releases_created }}
TAG: ${{ steps.hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.hxe.outputs.IMAGE_ID }}

# Publish packages
- name: Publish db-service
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 18
node-version: 20
registry-url: https://registry.npmjs.org/
- run: npm i
- run: npm test -w db-service -w sqlite -w postgres -- --maxWorkers=1
- run: npm test -w db-service -w sqlite -w postgres
- name: get-version # this takes the version of the monorepo root
id: package-version
# v1.3.1
Expand Down
17 changes: 13 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ concurrency:
jobs:
test:
runs-on: ubuntu-latest
timeout-minutes: 5
name: Node.js ${{ matrix.node }}
timeout-minutes: 20
name: Tests
permissions:
packages: write

strategy:
fail-fast: true
matrix:
node: [18]
node: [22]

steps:
- uses: actions/checkout@v4
Expand All @@ -31,6 +33,13 @@ jobs:

- run: npm ci
- run: npm run lint
- run: npm test -w db-service -w sqlite -w postgres -- --maxWorkers=1
- id: hxe
uses: ./.github/actions/hxe
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# testing
- run: npm test -ws
env:
FORCE_COLOR: true
TAG: ${{ steps.hxe.outputs.TAG }}
IMAGE_ID: ${{ steps.hxe.outputs.IMAGE_ID }}
4 changes: 4 additions & 0 deletions db-service/lib/SQLService.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ const sqls = new (class extends SQLService {
get factory() {
return null
}

get model() {
return cds.model
}
})()
cds.extend(cds.ql.Query).with(
class {
Expand Down
4 changes: 3 additions & 1 deletion db-service/lib/cql-functions.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const cds = require("@sap/cds")

const StandardFunctions = {
// OData: https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_CanonicalFunctions

Expand Down Expand Up @@ -59,7 +61,7 @@ const StandardFunctions = {
* @param {string} x
* @returns {string}
*/
countdistinct: x => `count(distinct ${x || '*'})`,
countdistinct: x => `count(distinct ${x || cds.error`countdistinct requires a ref to be counted`})`,
/**
* Generates SQL statement that produces the index of the first occurrence of the second string in the first string
* @param {string} x
Expand Down
21 changes: 14 additions & 7 deletions db-service/lib/cqn2sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const DEBUG = (() => {
return cds.debug('sql|sqlite')
//if (DEBUG) {
// return DEBUG
// (sql, ...more) => DEBUG (sql.replace(/(?:SELECT[\n\r\s]+(json_group_array\()?[\n\r\s]*json_insert\((\n|\r|.)*?\)[\n\r\s]*\)?[\n\r\s]+as[\n\r\s]+_json_[\n\r\s]+FROM[\n\r\s]*\(|\)[\n\r\s]*(\)[\n\r\s]+AS )|\)$)/gim,(a,b,c,d) => d || ''), ...more)
// FIXME: looses closing ) on INSERT queries
// (sql, ...more) => DEBUG (sql.replace(/(?:SELECT[\n\r\s]+(json_group_array\()?[\n\r\s]*json_insert\((\n|\r|.)*?\)[\n\r\s]*\)?[\n\r\s]+as[\n\r\s]+_json_[\n\r\s]+FROM[\n\r\s]*\(|\)[\n\r\s]*(\)[\n\r\s]+AS )|\)$)/gim,(a,b,c,d) => d || ''), ...more)
// FIXME: looses closing ) on INSERT queries
//}
})()

Expand Down Expand Up @@ -88,6 +88,7 @@ class CQN2SQLRenderer {
this.values = [] // prepare values, filled in by subroutines
this[kind]((this.cqn = q)) // actual sql rendering happens here
if (vars?.length && !this.values?.length) this.values = vars
if (vars && Object.keys(vars).length && !this.values?.length) this.values = vars
const sanitize_values = process.env.NODE_ENV === 'production' && cds.env.log.sanitize_values !== false
DEBUG?.(
this.sql,
Expand Down Expand Up @@ -116,8 +117,13 @@ class CQN2SQLRenderer {
* @param {import('./infer/cqn').CREATE} q
*/
CREATE(q) {
const { target } = q,
{ query } = target
let { target } = q
let query = target?.query || q.CREATE.as
if (!target || target._unresolved) {
const entity = q.CREATE.entity
target = typeof entity === 'string' ? { name: entity } : q.CREATE.entity
}

const name = this.name(target.name)
// Don't allow place holders inside views
delete this.values
Expand Down Expand Up @@ -203,8 +209,9 @@ class CQN2SQLRenderer {
*/
DROP(q) {
const { target } = q
const isView = target.query || target.projection
return (this.sql = `DROP ${isView ? 'VIEW' : 'TABLE'} IF EXISTS ${this.quote(this.name(target.name))}`)
const isView = target?.query || target?.projection || q.DROP.view
const name = target?.name || q.DROP.table?.ref?.[0] || q.DROP.view?.ref?.[0]
return (this.sql = `DROP ${isView ? 'VIEW' : 'TABLE'} IF EXISTS ${this.quote(this.name(name))}`)
}

// SELECT Statements ------------------------------------------------
Expand All @@ -223,7 +230,7 @@ class CQN2SQLRenderer {

// REVISIT: When selecting from an entity that is not in the model the from.where are not normalized (as cqn4sql is skipped)
if (!where && from?.ref?.length === 1 && from.ref[0]?.where) where = from.ref[0]?.where
let columns = this.SELECT_columns(q)
const columns = this.SELECT_columns(q)
let sql = `SELECT`
if (distinct) sql += ` DISTINCT`
if (!_empty(columns)) sql += ` ${columns}`
Expand Down
2 changes: 1 addition & 1 deletion db-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"CHANGELOG.md"
],
"scripts": {
"test": "jest --silent"
"test": "cds-test"
},
"dependencies": {
"generic-pool": "^3.9.0"
Expand Down
7 changes: 0 additions & 7 deletions db-service/test/cqn2sql/__snapshots__/create.test.js.snap

This file was deleted.

15 changes: 0 additions & 15 deletions db-service/test/cqn2sql/__snapshots__/delete.test.js.snap

This file was deleted.

13 changes: 0 additions & 13 deletions db-service/test/cqn2sql/__snapshots__/drop.test.js.snap

This file was deleted.

Loading