fix(core): Remove Composition from Select (#86)

* fix(core): Remove Composition from Select * fix(core): Remove Composition from Select * fix(core): change tests to use custom builders * test: add test for composition Co-authored-by: David Sooter <[email protected]> Co-authored-by: Gregor Wolf <[email protected]>
sapmentors · Jan 8, 2021 · 329e44a · 329e44a
1 parent e282429
commit 329e44a
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 26 deletions.
diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml
@@ -1,6 +1,6 @@
 # These are supported funding model platforms
 
-github: [vobu, gregorwolf] # , mikezaschka] Replace with up to 4 GitHub Sponsors-enabled usernames e.g., [user1, user2]
+github: [vobu, gregorwolf, d-sooter] # , mikezaschka] Replace with up to 4 GitHub Sponsors-enabled usernames e.g., [user1, user2]
 patreon: # Replace with a single Patreon username
 open_collective: # Replace with a single Open Collective username
 ko_fi: gregorwolf # Replace with a single Ko-fi username

diff --git a/__tests__/lib/pg/sql-builder.test.js b/__tests__/lib/pg/sql-builder.test.js
@@ -1,20 +1,42 @@
-const CustomBuilder = require('../../../lib/pg/sql-builder')
+const {
+  PGSelectBuilder,
+  PGResourceBuilder,
+  PGExpressionBuilder,
+  PGFunctionBuilder,
+} = require('../../../lib/pg/sql-builder')
 const { sqlFactory } = require('@sap/cds-runtime/lib/db/sql-builder')
 
+const loadModel = async () => {
+  const model = await cds.load('./__tests__/__assets__/cap-proj/db/schema.cds')
+  for (const definition of Object.values(model.definitions)) {
+    if (definition.elements) {
+      for (const [name, element] of Object.entries(definition.elements)) {
+        element.name = name
+      }
+    }
+  }
+  return model
+}
+
 describe('CQN to PostgreSQL', () => {
   beforeAll(async () => {
-    this.csn = await cds.load('./__tests__/__assets__/cap-proj/db/schema.cds')
+    this.csn = await loadModel()
 
     // Helper function
     this.runQuery = (query) =>
       sqlFactory(
         query,
         {
-          user: cds.user,
-          customBuilder: CustomBuilder,
+          user: cds.user || 'ANONYMOUS',
+          customBuilder: {
+            SelectBuilder: PGSelectBuilder,
+            ResourceBuilder: PGResourceBuilder,
+            ExpressionBuilder: PGExpressionBuilder,
+            FunctionBuilder: PGFunctionBuilder,
+          },
           now: { sql: "strftime('%Y-%m-%dT%H:%M:%fZ','now')" }, // '2012-12-03T07:16:23.574Z'
         },
-        cds.model
+        this.csn
       )
   })
 
@@ -44,7 +66,7 @@ describe('CQN to PostgreSQL', () => {
 
       const { sql, values = [] } = this.runQuery(query)
 
-      expect(sql).toMatch('SELECT ID, name FROM BeershopService_Beers')
+      expect(sql).toMatch('SELECT ID AS "ID", name AS "name" FROM BeershopService_Beers')
       expect(values).toEqual([])
     })
 
@@ -59,10 +81,11 @@ describe('CQN to PostgreSQL', () => {
         },
       }
 
-      const { sql, values = [] } = this.runQuery(query)
+      const { sql } = this.runQuery(query)
 
-      expect(sql).toMatch('SELECT ID, name FROM BeershopService_Beers ORDER BY ID ASC LIMIT 1 OFFSET 1')
-      //expect(values).toEqual([1, 1])
+      expect(sql).toMatch(
+        'SELECT ID AS "ID", name AS "name" FROM BeershopService_Beers ORDER BY "ID" ASC LIMIT 1 OFFSET 1'
+      )
     })
 
     test('+ should return valid SELECT statement with given from, columns, groupBy', async () => {
@@ -77,7 +100,7 @@ describe('CQN to PostgreSQL', () => {
 
       const { sql, values = [] } = this.runQuery(query)
 
-      expect(sql).toMatch('SELECT ID, name FROM BeershopService_Beers GROUP BY ID, name')
+      expect(sql).toMatch('SELECT ID AS "ID", name AS "name" FROM BeershopService_Beers GROUP BY ID, name')
       expect(values).toEqual([])
     })
 
@@ -93,10 +116,11 @@ describe('CQN to PostgreSQL', () => {
         },
       }
 
-      const { sql, values = [] } = this.runQuery(query)
+      const { sql } = this.runQuery(query)
 
-      expect(sql).toMatch('SELECT ID, name FROM BeershopService_Beers WHERE ID = 111 ORDER BY ID ASC LIMIT 1 OFFSET 1')
-      //expect(values).toEqual([111, 1, 1])
+      expect(sql).toMatch(
+        'SELECT ID AS "ID", name AS "name" FROM BeershopService_Beers WHERE ID = 111 ORDER BY "ID" ASC LIMIT 1 OFFSET 1'
+      )
     })
 
     test('+ should create a valid count statement', async () => {
@@ -108,16 +132,31 @@ describe('CQN to PostgreSQL', () => {
           columns: [{ func: 'count', args: [{ ref: ['1'] }], as: 'counted' }],
         },
       }
-      const { sql, values = [] } = this.runQuery(query)
+      const { sql } = this.runQuery(query)
 
       expect(sql).toMatch('SELECT count ( 1 ) AS "counted" FROM BeershopService_Beers')
     })
+
+    test('+ should ignore composition fields in select', async () => {
+      const query = {
+        cmd: 'SELECT',
+        SELECT: {
+          from: { ref: ['csw.Brewery'] },
+        },
+      }
+
+      const { sql } = this.runQuery(query)
+      expect(sql.includes('beer')).toBeFalsy()
+      expect(sql).toMatch(
+        'SELECT ID AS "ID", createdAt AS "createdAt", createdBy AS "createdBy", modifiedAt AS "modifiedAt", modifiedBy AS "modifiedBy", name AS "name" FROM csw_Brewery'
+      )
+    })
   })
 
   // Examples taken from: https://cap.cloud.sap/docs/cds/cqn#insert
   describe('InsertBuilder', () => {
     beforeAll(async () => {
-      this.csn = await cds.load('./__tests__/__assets__/cap-proj/db/schema.cds')
+      this.csn = await loadModel()
     })
 
     it('should return a valid INSERT statement with given columns and values', () => {
@@ -132,8 +171,10 @@ describe('CQN to PostgreSQL', () => {
 
       const { sql, values = [] } = this.runQuery(query)
 
-      expect(sql).toMatch(`INSERT INTO csw_Beers ( ID, name ) VALUES ( ?, ? )`)
-      expect(values).toEqual([201, 'MyBeer'])
+      expect(sql).toMatch(
+        `INSERT INTO csw_Beers ( ID, name , createdAt, createdBy, modifiedAt, modifiedBy ) VALUES ( ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ? )`
+      )
+      expect(values).toEqual([201, 'MyBeer', 'ANONYMOUS', 'ANONYMOUS'])
     })
 
     it('should return a valid INSERT statement with given columns and rows', () => {
@@ -151,10 +192,12 @@ describe('CQN to PostgreSQL', () => {
 
       const { sql, values = [] } = this.runQuery(query)
 
-      expect(sql).toMatch(`INSERT INTO csw_Beers ( ID, name ) VALUES ( ?, ? )`)
+      expect(sql).toMatch(
+        `INSERT INTO csw_Beers ( ID, name , createdAt, createdBy, modifiedAt, modifiedBy ) VALUES ( ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ? )`
+      )
       expect(values).toEqual([
-        [201, 'MyBeer'],
-        [202, 'MyOtherBeer'],
+        [201, 'MyBeer', 'ANONYMOUS', 'ANONYMOUS'],
+        [202, 'MyOtherBeer', 'ANONYMOUS', 'ANONYMOUS'],
       ])
     })
 
@@ -172,10 +215,12 @@ describe('CQN to PostgreSQL', () => {
 
       const { sql, values = [] } = this.runQuery(query)
 
-      expect(sql).toMatch(`INSERT INTO csw_Beers ( ID, name ) VALUES ( ?, ? )`)
+      expect(sql).toMatch(
+        `INSERT INTO csw_Beers ( ID, name, createdAt, createdBy, modifiedAt, modifiedBy ) VALUES ( ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ?, strftime('%Y-%m-%dT%H:%M:%fZ','now'), ? )`
+      )
       expect(values).toEqual([
-        [201, 'MyBeer'],
-        [202, 'MyOtherBeer'],
+        [201, 'MyBeer', 'ANONYMOUS', 'ANONYMOUS'],
+        [202, 'MyOtherBeer', 'ANONYMOUS', 'ANONYMOUS'],
       ])
     })
   })

diff --git a/lib/pg/Service.js b/lib/pg/Service.js
@@ -1,6 +1,6 @@
 const { Pool } = require('pg')
 const dateTime = require('@sap/cds-runtime/lib/hana/dateTime.js')
-const { managed, virtual, keys, rewrite } = require('@sap/cds-runtime/lib/db/generic')
+const { rewrite } = require('@sap/cds-runtime/lib/db/generic')
 /*eslint no-undef: "warn"*/
 /*eslint no-unused-vars: "warn"*/
 const cds = global.cds || require('@sap/cds/lib')

diff --git a/lib/pg/sql-builder/SelectBuilder.js b/lib/pg/sql-builder/SelectBuilder.js
@@ -9,6 +9,7 @@ class PGSelectBuilder extends SelectBuilder {
   // Getters and setters have to be overwritten becasue the originals only use the builders passes in the
   // first round of processing after that the refer back to the original builders.
 
+
   get ReferenceBuilder() {
     return this.ReferenceBuilder
   }
@@ -47,6 +48,7 @@ class PGSelectBuilder extends SelectBuilder {
     Object.defineProperty(this, 'ReferenceBuilder', { value: ReferenceBuilder })
     Object.defineProperty(this, 'FunctionBuilder', { value: FunctionBuilder })
     Object.defineProperty(this, 'ExpressionBuilder', { value: ExpressionBuilder })
+    this.excludedColumnTypes = ['cds.Association', 'cds.Composition'];
   }
 
   /**
@@ -76,7 +78,7 @@ class PGSelectBuilder extends SelectBuilder {
         let entity = this._csn.definitions[name]
         if (entity) {
           for (var prop in entity.elements) {
-            if (entity.elements[prop].type !== 'cds.Association') {
+            if (!this.excludedColumnTypes.includes(entity.elements[prop].type)) {
               let column = {
                 ref: [prop],
                 as: prop,