fix: Improved behavioral consistency between the database services (#673

) This PR contains a few fixes that where required to make the new compliance tests to pass: - Decimal precision behavior is aligned between the databases when defined - `@cap-js/hana` now properly supports scalar `SELECT` queries in the columns The goal of this PR is to remove as much dependencies as possible now that `cds@8` provides the `cds-test` command. - Removing `jest` by switching to `cds-test` - Removed `jest.config.js` files - Removing `chai` by switching to `cds-test` - Adjust all tests to fit with the `cds-test` `expect` pattern - Removing `HANA` test action - Move `HXE` Github action steps into a reuse file - Added a lock for `HANA` database creation - Added a fallback for `Postgres` during database creation - Removed compliance import tests - Added symbolic link to the compliance test folder --------- Co-authored-by: Daniel Hutzel <daniel.hutzel@sap.com> Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com>
cap-js · Oct 9, 2024 · 5e62096 · 5e62096
1 parent 6a03e8a
commit 5e62096
Show file tree

Hide file tree

Showing 97 changed files with 2,699 additions and 8,695 deletions.
diff --git a/.github/actions/hxe/action.yml b/.github/actions/hxe/action.yml
@@ -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 }}
diff --git a/.github/workflows/hana.yml b/.github/workflows/hana.yml
diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml
@@ -19,56 +19,25 @@ 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
-        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
+        uses: ./.github/actions/hxe
+        with:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - run: npm test -w hana
         if: ${{ steps.release.outputs.releases_created }}
+          TAG: ${{ steps.hxe.outputs.TAG }}
+          IMAGE_ID: ${{ steps.hxe.outputs.IMAGE_ID }}
 
       # Publish packages
       - name: Publish db-service

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -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

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -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
@@ -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 }}
diff --git a/db-service/lib/SQLService.js b/db-service/lib/SQLService.js
@@ -367,6 +367,7 @@ class SQLService extends DatabaseService {
    */
   cqn4sql(q) {
     if (
+      (!q.target || q.target._unresolved) &&
       !cds.env.features.db_strict &&
       !q.SELECT?.from?.join &&
       !q.SELECT?.from?.SELECT &&
@@ -466,6 +467,10 @@ const sqls = new (class extends SQLService {
   get factory() {
     return null
   }
+
+  get model() {
+    return cds.model
+  }
 })()
 cds.extend(cds.ql.Query).with(
   class {

diff --git a/db-service/lib/cql-functions.js b/db-service/lib/cql-functions.js
@@ -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
 
@@ -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

diff --git a/db-service/lib/cqn2sql.js b/db-service/lib/cqn2sql.js
@@ -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
   //}
 })()
 
@@ -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,
@@ -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
@@ -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 ------------------------------------------------
@@ -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}`

diff --git a/db-service/package.json b/db-service/package.json
@@ -22,7 +22,7 @@
     "CHANGELOG.md"
   ],
   "scripts": {
-    "test": "jest --silent"
+    "test": "cds-test"
   },
   "dependencies": {
     "generic-pool": "^3.9.0"

diff --git a/db-service/test/cqn2sql/__snapshots__/create.test.js.snap b/db-service/test/cqn2sql/__snapshots__/create.test.js.snap
diff --git a/db-service/test/cqn2sql/__snapshots__/delete.test.js.snap b/db-service/test/cqn2sql/__snapshots__/delete.test.js.snap
diff --git a/db-service/test/cqn2sql/__snapshots__/drop.test.js.snap b/db-service/test/cqn2sql/__snapshots__/drop.test.js.snap