From 06ca29b1525a48c028b08a85ed3db4a2b6b53587 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 16 Jan 2023 15:57:14 +0100 Subject: [PATCH 01/22] Add test querying only totalCount --- test/tests/queries/totalCount.test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/tests/queries/totalCount.test.js b/test/tests/queries/totalCount.test.js index acb6df73..92d8d0bc 100644 --- a/test/tests/queries/totalCount.test.js +++ b/test/tests/queries/totalCount.test.js @@ -37,6 +37,27 @@ describe('graphql - queries with totalCount', () => { expect(response.data).toEqual({ data }) }) + test('query selecting only totalCount', async () => { + const query = `#graphql + { + AdminService { + Books { + totalCount + } + } + } + ` + const data = { + AdminService: { + Books: { + totalCount: 5 + } + } + } + const response = await POST('/graphql', { query }) + expect(response.data).toEqual({ data }) + }) + test('query with totalCount and simple filter', async () => { const query = `#graphql { From 8bbd16899f788ca5ee27280a7016aaf7281223e2 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 16 Jan 2023 16:07:53 +0100 Subject: [PATCH 02/22] Check if is connection from selected field names --- lib/resolvers/parse/util/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/resolvers/parse/util/index.js b/lib/resolvers/parse/util/index.js index 645e78ad..6eaa608f 100644 --- a/lib/resolvers/parse/util/index.js +++ b/lib/resolvers/parse/util/index.js @@ -16,9 +16,9 @@ const _filterOutDuplicateColumnsSelections = selections => { } const getPotentiallyNestedNodesSelections = selections => { - const nodesSelections = selections.filter(selection => selection.name.value === CONNECTION_FIELDS.nodes) - if (nodesSelections.length === 0) return selections - return _filterOutDuplicateColumnsSelections(selections) + const isConnection = selections.some(selection => Object.values(CONNECTION_FIELDS).includes(selection.name.value)) + if (isConnection) return _filterOutDuplicateColumnsSelections(selections) + return selections } module.exports = { getPotentiallyNestedNodesSelections } From 0cdfbec5c7d6206a4d3b19ab5ff4a890bc92837d Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 16 Jan 2023 17:15:49 +0100 Subject: [PATCH 03/22] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffbcf3c8..8ee66484 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed a server crash that occourred if an entity property is named `localized`. +- A bug where the field `totalCount` could not be queried on its own ### Removed From 7a1675dec2d3b76102617c50e78ba914a006dec8 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Thu, 19 Jan 2023 17:51:01 +0100 Subject: [PATCH 04/22] Use gql tag in test --- test/tests/queries/totalCount.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/queries/totalCount.test.js b/test/tests/queries/totalCount.test.js index e8fc1fcc..dc4fd946 100644 --- a/test/tests/queries/totalCount.test.js +++ b/test/tests/queries/totalCount.test.js @@ -39,7 +39,7 @@ describe('graphql - queries with totalCount', () => { }) test('query selecting only totalCount', async () => { - const query = `#graphql + const query = gql` { AdminService { Books { From e8efc2b13a8f6ed71d725e558d7cba9b5c19ada5 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 23 Jan 2023 18:11:19 +0100 Subject: [PATCH 05/22] Use ternary conditional operator --- lib/resolvers/parse/util/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/resolvers/parse/util/index.js b/lib/resolvers/parse/util/index.js index 6eaa608f..878676f1 100644 --- a/lib/resolvers/parse/util/index.js +++ b/lib/resolvers/parse/util/index.js @@ -17,8 +17,7 @@ const _filterOutDuplicateColumnsSelections = selections => { const getPotentiallyNestedNodesSelections = selections => { const isConnection = selections.some(selection => Object.values(CONNECTION_FIELDS).includes(selection.name.value)) - if (isConnection) return _filterOutDuplicateColumnsSelections(selections) - return selections + return isConnection ? _filterOutDuplicateColumnsSelections(selections) : selections } module.exports = { getPotentiallyNestedNodesSelections } From 65c09638b0b8ab949ef9a8a4bd84d99cf3b682c4 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 23 Jan 2023 19:56:58 +0100 Subject: [PATCH 06/22] Fix CDS name with nodes and totalCount --- lib/resolvers/crud/create.js | 2 +- lib/resolvers/crud/read.js | 4 ++-- lib/resolvers/crud/update.js | 4 ++-- lib/resolvers/parse/ast/result.js | 23 ++++++++++++----------- lib/resolvers/parse/ast2cqn/columns.js | 7 ++++--- lib/resolvers/parse/util/index.js | 8 ++------ 6 files changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/resolvers/crud/create.js b/lib/resolvers/crud/create.js index 3b98a451..8f18400d 100644 --- a/lib/resolvers/crud/create.js +++ b/lib/resolvers/crud/create.js @@ -14,5 +14,5 @@ module.exports = async (service, entity, selection) => { const result = await service.run(query) const resultInArray = Array.isArray(result) ? result : [result] - return formatResult(selection, resultInArray, true) + return formatResult(entity, selection, resultInArray, false) } diff --git a/lib/resolvers/crud/read.js b/lib/resolvers/crud/read.js index c005e8d3..071f9086 100644 --- a/lib/resolvers/crud/read.js +++ b/lib/resolvers/crud/read.js @@ -8,7 +8,7 @@ module.exports = async (service, entity, selection) => { const args = selection.arguments let query = SELECT.from(entity) - query.columns(astToColumns(selection.selectionSet.selections)) + query.columns(astToColumns(entity, selection.selectionSet.selections, true)) const filter = getArgumentByName(args, ARGS.filter) if (filter) query.where(astToWhere(filter)) @@ -24,5 +24,5 @@ module.exports = async (service, entity, selection) => { const result = await service.run(query) - return formatResult(selection, result) + return formatResult(entity, selection, result, true) } diff --git a/lib/resolvers/crud/update.js b/lib/resolvers/crud/update.js index c9ea4f15..254cca68 100644 --- a/lib/resolvers/crud/update.js +++ b/lib/resolvers/crud/update.js @@ -10,7 +10,7 @@ module.exports = async (service, entity, selection) => { const filter = getArgumentByName(args, ARGS.filter) const queryBeforeUpdate = SELECT.from(entity) - queryBeforeUpdate.columns(astToColumns(selection.selectionSet.selections)) + queryBeforeUpdate.columns(astToColumns(entity, selection.selectionSet.selections, false)) if (filter) queryBeforeUpdate.where(astToWhere(filter)) @@ -33,5 +33,5 @@ module.exports = async (service, entity, selection) => { // Merge selected fields with updated data const mergedResults = resultBeforeUpdate.map(original => ({ ...original, ...result })) - return formatResult(selection, mergedResults, true) + return formatResult(entity, selection, mergedResults, false) } diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index 7819b85c..1640e7f0 100644 --- a/lib/resolvers/parse/ast/result.js +++ b/lib/resolvers/parse/ast/result.js @@ -2,8 +2,8 @@ const { CONNECTION_FIELDS } = require('../../../constants') const { getPotentiallyNestedNodesSelections } = require('../util') const { isPlainObject } = require('./util') -const formatResult = (field, result, skipTopLevelConnection) => { - const _formatObject = (selections, object) => { +const formatResult = (entity, field, result, isTopLevelConnection) => { + const _formatObject = (type, selections, object) => { for (const key in object) { const value = object[key] delete object[key] @@ -11,8 +11,9 @@ const formatResult = (field, result, skipTopLevelConnection) => { const fields = selections.filter(s => s.alias?.value === key || s.name.value === key) for (const field of fields) { + const element = type.elements[field.name.value] const responseKey = field.alias?.value || field.name.value - object[responseKey] = _formatByType(field, value) + object[responseKey] = _formatByType(element._target || element, field, value, element.is2many) } } return object @@ -27,13 +28,13 @@ const formatResult = (field, result, skipTopLevelConnection) => { return acc }, {}) - const _formatArray = (field, array, skipTopLevelConnection) => { + const _formatArray = (type, field, array, isConnection) => { const selections = field.selectionSet.selections - const potentiallyNestedSelections = getPotentiallyNestedNodesSelections(selections) - array.map(e => _formatObject(potentiallyNestedSelections, e)) + const potentiallyNestedSelections = getPotentiallyNestedNodesSelections(selections, isConnection) + array.map(e => _formatObject(type, potentiallyNestedSelections, e)) // REVISIT: requires differentiation for support of configurable schema flavors - if (skipTopLevelConnection) return array + if (!isConnection) return array return { ..._aliasFieldsWithValue(selections, CONNECTION_FIELDS.nodes, array), @@ -41,17 +42,17 @@ const formatResult = (field, result, skipTopLevelConnection) => { } } - const _formatByType = (field, value, skipTopLevelConnection) => { + const _formatByType = (type, field, value, isConnection) => { if (Array.isArray(value)) { - return _formatArray(field, value, skipTopLevelConnection) + return _formatArray(type, field, value, isConnection) } else if (isPlainObject(value)) { - return _formatObject(field.selectionSet.selections, value) + return _formatObject(type, field.selectionSet.selections, value) } else { return value } } - return _formatByType(field, result, skipTopLevelConnection) + return _formatByType(entity, field, result, isTopLevelConnection) } module.exports = formatResult diff --git a/lib/resolvers/parse/ast2cqn/columns.js b/lib/resolvers/parse/ast2cqn/columns.js index 5ecd3a49..d0e0fff4 100644 --- a/lib/resolvers/parse/ast2cqn/columns.js +++ b/lib/resolvers/parse/ast2cqn/columns.js @@ -5,18 +5,19 @@ const astToOrderBy = require('./orderBy') const astToLimit = require('./limit') const { ARGS } = require('../../../constants') -const astToColumns = selections => { +const astToColumns = (entity, selections, isConnection) => { let columns = [] - for (const selection of getPotentiallyNestedNodesSelections(selections)) { + for (const selection of getPotentiallyNestedNodesSelections(selections, isConnection)) { const args = selection.arguments const fieldName = selection.name.value + const element = entity.elements[fieldName] const column = { ref: [fieldName] } if (selection.alias) column.as = selection.alias.value if (selection.selectionSet?.selections) { - const columns = astToColumns(selection.selectionSet.selections) + const columns = astToColumns(element._target, selection.selectionSet.selections, element.is2many) // columns is empty if only __typename was selected (which was filtered out in the enriched AST) column.expand = columns.length > 0 ? columns : ['*'] } diff --git a/lib/resolvers/parse/util/index.js b/lib/resolvers/parse/util/index.js index 878676f1..f2616bd7 100644 --- a/lib/resolvers/parse/util/index.js +++ b/lib/resolvers/parse/util/index.js @@ -1,5 +1,3 @@ -const { CONNECTION_FIELDS } = require('../../../constants') - const _filterOutDuplicateColumnsSelections = selections => { const mergedSelectionsMap = new Map() @@ -15,9 +13,7 @@ const _filterOutDuplicateColumnsSelections = selections => { return Array.from(mergedSelectionsMap.values()) } -const getPotentiallyNestedNodesSelections = selections => { - const isConnection = selections.some(selection => Object.values(CONNECTION_FIELDS).includes(selection.name.value)) - return isConnection ? _filterOutDuplicateColumnsSelections(selections) : selections -} +const getPotentiallyNestedNodesSelections = (selections, isConnection) => + isConnection ? _filterOutDuplicateColumnsSelections(selections) : selections module.exports = { getPotentiallyNestedNodesSelections } From 73f7c86455b974bf200011206fed37af4edb40c8 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 23 Jan 2023 19:57:12 +0100 Subject: [PATCH 07/22] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 385a7756..03285e24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a server crash that occourred if an entity property is named `localized`. - A bug where the field `totalCount` could not be queried on its own +- Name clashes when CDS elements are named `nodes` or `totalCount` ### Removed From 150ef92fd80b200f5d8970fa95ab35cc52a268b8 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 23 Jan 2023 20:33:00 +0100 Subject: [PATCH 08/22] Test to reproduce name clashes --- test/resources/edge-cases/server.js | 6 + .../srv/fields-with-connection-names.cds | 13 ++ test/resources/models.json | 4 + .../fields-with-connection-names.gql | 152 ++++++++++++++++++ test/tests/edge-cases.test.js | 80 +++++++++ 5 files changed, 255 insertions(+) create mode 100644 test/resources/edge-cases/server.js create mode 100644 test/resources/edge-cases/srv/fields-with-connection-names.cds create mode 100644 test/schemas/edge-cases/fields-with-connection-names.gql create mode 100644 test/tests/edge-cases.test.js diff --git a/test/resources/edge-cases/server.js b/test/resources/edge-cases/server.js new file mode 100644 index 00000000..6808b2bb --- /dev/null +++ b/test/resources/edge-cases/server.js @@ -0,0 +1,6 @@ +const cds = require('@sap/cds') +const path = require('path') + +cds.env.protocols = { + graphql: { path: '/graphql', impl: path.join(__dirname, '../../../index.js') } +} \ No newline at end of file diff --git a/test/resources/edge-cases/srv/fields-with-connection-names.cds b/test/resources/edge-cases/srv/fields-with-connection-names.cds new file mode 100644 index 00000000..6555341d --- /dev/null +++ b/test/resources/edge-cases/srv/fields-with-connection-names.cds @@ -0,0 +1,13 @@ +service FieldsWithConnectionNamesService { + entity Nodes { + key ID : UUID; + nodes : String; + totalCount : String; + } + + entity Root { + key ID : UUID; + nodes : Composition of Nodes; + totalCount : String; + } +} diff --git a/test/resources/models.json b/test/resources/models.json index d4de2274..5888dd4d 100644 --- a/test/resources/models.json +++ b/test/resources/models.json @@ -17,5 +17,9 @@ { "name": "edge-cases/field-named-localized", "files": ["./edge-cases/srv/field-named-localized.cds"] + }, + { + "name": "edge-cases/fields-with-connection-names", + "files": ["./edge-cases/srv/fields-with-connection-names.cds"] } ] diff --git a/test/schemas/edge-cases/fields-with-connection-names.gql b/test/schemas/edge-cases/fields-with-connection-names.gql new file mode 100644 index 00000000..7ee44df7 --- /dev/null +++ b/test/schemas/edge-cases/fields-with-connection-names.gql @@ -0,0 +1,152 @@ +type FieldsWithConnectionNamesService { + Nodes( + filter: [FieldsWithConnectionNamesService_Nodes_filter] + orderBy: [FieldsWithConnectionNamesService_Nodes_orderBy] + skip: Int + top: Int + ): FieldsWithConnectionNamesService_Nodes_connection + Root( + filter: [FieldsWithConnectionNamesService_Root_filter] + orderBy: [FieldsWithConnectionNamesService_Root_orderBy] + skip: Int + top: Int + ): FieldsWithConnectionNamesService_Root_connection +} + +type FieldsWithConnectionNamesService_Nodes { + ID: ID + nodes: String + totalCount: String +} + +input FieldsWithConnectionNamesService_Nodes_C { + ID: ID + nodes: String + totalCount: String +} + +input FieldsWithConnectionNamesService_Nodes_U { + ID: ID + nodes: String + totalCount: String +} + +type FieldsWithConnectionNamesService_Nodes_connection { + nodes: [FieldsWithConnectionNamesService_Nodes] + totalCount: Int +} + +input FieldsWithConnectionNamesService_Nodes_filter { + ID: [ID_filter] + nodes: [String_filter] + totalCount: [String_filter] +} + +type FieldsWithConnectionNamesService_Nodes_input { + create( + input: [FieldsWithConnectionNamesService_Nodes_C]! + ): [FieldsWithConnectionNamesService_Nodes] + delete( + filter: [FieldsWithConnectionNamesService_Nodes_filter] + ): Int + update( + filter: [FieldsWithConnectionNamesService_Nodes_filter] + input: FieldsWithConnectionNamesService_Nodes_U! + ): [FieldsWithConnectionNamesService_Nodes] +} + +input FieldsWithConnectionNamesService_Nodes_orderBy { + ID: SortDirection + nodes: SortDirection + totalCount: SortDirection +} + +type FieldsWithConnectionNamesService_Root { + ID: ID + nodes: FieldsWithConnectionNamesService_Nodes + nodes_ID: ID + totalCount: String +} + +input FieldsWithConnectionNamesService_Root_C { + ID: ID + nodes: FieldsWithConnectionNamesService_Nodes_C + nodes_ID: ID + totalCount: String +} + +input FieldsWithConnectionNamesService_Root_U { + ID: ID + nodes: FieldsWithConnectionNamesService_Nodes_U + nodes_ID: ID + totalCount: String +} + +type FieldsWithConnectionNamesService_Root_connection { + nodes: [FieldsWithConnectionNamesService_Root] + totalCount: Int +} + +input FieldsWithConnectionNamesService_Root_filter { + ID: [ID_filter] + nodes_ID: [ID_filter] + totalCount: [String_filter] +} + +type FieldsWithConnectionNamesService_Root_input { + create( + input: [FieldsWithConnectionNamesService_Root_C]! + ): [FieldsWithConnectionNamesService_Root] + delete( + filter: [FieldsWithConnectionNamesService_Root_filter] + ): Int + update( + filter: [FieldsWithConnectionNamesService_Root_filter] + input: FieldsWithConnectionNamesService_Root_U! + ): [FieldsWithConnectionNamesService_Root] +} + +input FieldsWithConnectionNamesService_Root_orderBy { + ID: SortDirection + nodes_ID: SortDirection + totalCount: SortDirection +} + +type FieldsWithConnectionNamesService_input { + Nodes: FieldsWithConnectionNamesService_Nodes_input + Root: FieldsWithConnectionNamesService_Root_input +} + +input ID_filter { + eq: ID + ge: ID + gt: ID + le: ID + lt: ID + ne: ID +} + +type Mutation { + FieldsWithConnectionNamesService: FieldsWithConnectionNamesService_input +} + +type Query { + FieldsWithConnectionNamesService: FieldsWithConnectionNamesService +} + +enum SortDirection { + asc + desc +} + +input String_filter { + contains: String + endswith: String + eq: String + ge: String + gt: String + le: String + lt: String + ne: String + startswith: String +} diff --git a/test/tests/edge-cases.test.js b/test/tests/edge-cases.test.js new file mode 100644 index 00000000..8470d32b --- /dev/null +++ b/test/tests/edge-cases.test.js @@ -0,0 +1,80 @@ +describe('graphql - edge cases', () => { + const cds = require('@sap/cds/lib') + const path = require('path') + const { gql } = require('../util') + + const { axios, POST, data } = cds.test(path.join(__dirname, '../resources/edge-cases')) + // Prevent axios from throwing errors for non 2xx status codes + axios.defaults.validateStatus = false + data.autoReset(true) + + test('no name clashes occur between CDS names and connection fields with 2 one', async () => { + const queryCreate = gql` + mutation { + FieldsWithConnectionNamesService { + Root { + create(input: { totalCount: "foo", nodes: { totalCount: "bar", nodes: "baz" } }) { + totalCount + nodes { + totalCount + nodes + } + } + } + } + } + ` + const dataCreate = { + FieldsWithConnectionNamesService: { + Root: { + create: [ + { + totalCount: 'foo', + nodes: { + totalCount: 'bar', + nodes: 'baz' + } + } + ] + } + } + } + const responseCreate = await POST('/graphql', { query: queryCreate }) + expect(responseCreate.data).toEqual({ data: dataCreate }) + + const queryRead = gql` + { + FieldsWithConnectionNamesService { + Root { + totalCount + nodes { + totalCount + nodes { + totalCount + nodes + } + } + } + } + } + ` + const dataRead = { + FieldsWithConnectionNamesService: { + Root: { + totalCount: 1, + nodes: [ + { + totalCount: 'foo', + nodes: { + totalCount: 'bar', + nodes: 'baz' + } + } + ] + } + } + } + const responseRead = await POST('/graphql', { query: queryRead }) + expect(responseRead.data).toEqual({ data: dataRead }) + }) +}) From 5edbe822aba7b1103cd92b7f614761414a91480e Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Tue, 18 Apr 2023 13:48:29 +0200 Subject: [PATCH 09/22] Regenerate schemas --- .../fields-with-connection-names.gql | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/test/schemas/edge-cases/fields-with-connection-names.gql b/test/schemas/edge-cases/fields-with-connection-names.gql index 7ee44df7..454c616e 100644 --- a/test/schemas/edge-cases/fields-with-connection-names.gql +++ b/test/schemas/edge-cases/fields-with-connection-names.gql @@ -26,7 +26,6 @@ input FieldsWithConnectionNamesService_Nodes_C { } input FieldsWithConnectionNamesService_Nodes_U { - ID: ID nodes: String totalCount: String } @@ -76,8 +75,7 @@ input FieldsWithConnectionNamesService_Root_C { } input FieldsWithConnectionNamesService_Root_U { - ID: ID - nodes: FieldsWithConnectionNamesService_Nodes_U + nodes: FieldsWithConnectionNamesService_Nodes_C nodes_ID: ID totalCount: String } @@ -118,12 +116,12 @@ type FieldsWithConnectionNamesService_input { } input ID_filter { - eq: ID - ge: ID - gt: ID - le: ID - lt: ID - ne: ID + eq: [ID] + ge: [ID] + gt: [ID] + le: [ID] + lt: [ID] + ne: [ID] } type Mutation { @@ -140,13 +138,13 @@ enum SortDirection { } input String_filter { - contains: String - endswith: String - eq: String - ge: String - gt: String - le: String - lt: String - ne: String - startswith: String + contains: [String] + endswith: [String] + eq: [String] + ge: [String] + gt: [String] + le: [String] + lt: [String] + ne: [String] + startswith: [String] } From dea239014d6fafdd746508a552e9cb6b7ca765d8 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Tue, 18 Apr 2023 13:49:49 +0200 Subject: [PATCH 10/22] Remove duplicate changelog entry --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19a7e90a..765a714f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A bug where the field `totalCount` could not be queried on its own - Name clashes when CDS elements are named `nodes` or `totalCount` ### Removed From 792e1a037270b832ee5866c139c6779726e8d3db Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Tue, 18 Apr 2023 14:01:40 +0200 Subject: [PATCH 11/22] Move resultInArray into formatResult --- lib/resolvers/crud/create.js | 4 +--- lib/resolvers/crud/read.js | 5 +---- lib/resolvers/crud/update.js | 5 +---- lib/resolvers/parse/ast/result.js | 3 ++- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/resolvers/crud/create.js b/lib/resolvers/crud/create.js index a156d66f..1d2101ab 100644 --- a/lib/resolvers/crud/create.js +++ b/lib/resolvers/crud/create.js @@ -2,7 +2,6 @@ const { INSERT } = require('@sap/cds/lib').ql const { ARGS } = require('../../constants') const formatResult = require('../parse/ast/result') const { getArgumentByName, astToEntries } = require('../parse/ast2cqn') -const { isPlainObject } = require('../utils') const { entriesStructureToEntityStructure } = require('./utils') module.exports = async (service, entity, selection) => { @@ -13,7 +12,6 @@ module.exports = async (service, entity, selection) => { query.entries(entries) const result = await service.run(query) - const resultInArray = isPlainObject(result) ? [result] : result - return formatResult(entity, selection, resultInArray, false) + return formatResult(entity, selection, result, false) } diff --git a/lib/resolvers/crud/read.js b/lib/resolvers/crud/read.js index d33a6ced..cb23e143 100644 --- a/lib/resolvers/crud/read.js +++ b/lib/resolvers/crud/read.js @@ -2,7 +2,6 @@ const { SELECT } = require('@sap/cds/lib').ql const { ARGS, CONNECTION_FIELDS } = require('../../constants') const { getArgumentByName, astToColumns, astToWhere, astToOrderBy, astToLimit } = require('../parse/ast2cqn') const formatResult = require('../parse/ast/result') -const { isPlainObject } = require('../utils') module.exports = async (service, entity, selection) => { const selections = selection.selectionSet.selections @@ -28,7 +27,5 @@ module.exports = async (service, entity, selection) => { const result = await service.run(query) - const resultInArray = isPlainObject(result) ? [result] : result - - return formatResult(entity, selection, resultInArray, true) + return formatResult(entity, selection, result, true) } diff --git a/lib/resolvers/crud/update.js b/lib/resolvers/crud/update.js index e96cdc4d..5d0a169c 100644 --- a/lib/resolvers/crud/update.js +++ b/lib/resolvers/crud/update.js @@ -2,7 +2,6 @@ const { SELECT, UPDATE } = require('@sap/cds/lib').ql const { ARGS } = require('../../constants') const formatResult = require('../parse/ast/result') const { getArgumentByName, astToColumns, astToWhere, astToEntries } = require('../parse/ast2cqn') -const { isPlainObject } = require('../utils') const { entriesStructureToEntityStructure } = require('./utils') module.exports = async (service, entity, selection) => { @@ -37,7 +36,5 @@ module.exports = async (service, entity, selection) => { mergedResults = resultBeforeUpdate.map(original => ({ ...original, ...result })) } - const resultInArray = isPlainObject(mergedResults) ? [mergedResults] : mergedResults - - return formatResult(entity, selection, resultInArray, false) + return formatResult(entity, selection, mergedResults, false) } diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index 90d3d9ba..13a1a87d 100644 --- a/lib/resolvers/parse/ast/result.js +++ b/lib/resolvers/parse/ast/result.js @@ -51,7 +51,8 @@ const formatResult = (entity, field, result, isTopLevelConnection) => { return value } - return _formatByType(entity, field, result, isTopLevelConnection) + const resultInArray = isPlainObject(result) ? [result] : result + return _formatByType(entity, field, resultInArray, isTopLevelConnection) } module.exports = formatResult From ba0302dac87f39df0ca53727b94f6f4195d2f0ae Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:30:52 +0200 Subject: [PATCH 12/22] Annotate edge case services with `@graphql` --- test/resources/edge-cases/srv/field-named-localized.cds | 1 + test/resources/edge-cases/srv/fields-with-connection-names.cds | 1 + 2 files changed, 2 insertions(+) diff --git a/test/resources/edge-cases/srv/field-named-localized.cds b/test/resources/edge-cases/srv/field-named-localized.cds index 2cb150ad..acc96591 100644 --- a/test/resources/edge-cases/srv/field-named-localized.cds +++ b/test/resources/edge-cases/srv/field-named-localized.cds @@ -1,5 +1,6 @@ using {managed} from '@sap/cds/common'; +@graphql service FieldNamedLocalizedService { entity localized { key ID : Integer; diff --git a/test/resources/edge-cases/srv/fields-with-connection-names.cds b/test/resources/edge-cases/srv/fields-with-connection-names.cds index 6555341d..30950353 100644 --- a/test/resources/edge-cases/srv/fields-with-connection-names.cds +++ b/test/resources/edge-cases/srv/fields-with-connection-names.cds @@ -1,3 +1,4 @@ +@graphql service FieldsWithConnectionNamesService { entity Nodes { key ID : UUID; From 548e8c64a9e2d27b0494019b0ae4bd70bfeccb12 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:31:18 +0200 Subject: [PATCH 13/22] Regenerate schemas --- .../fields-with-connection-names.gql | 66 ++++++------------- 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/test/schemas/edge-cases/fields-with-connection-names.gql b/test/schemas/edge-cases/fields-with-connection-names.gql index 454c616e..951a1144 100644 --- a/test/schemas/edge-cases/fields-with-connection-names.gql +++ b/test/schemas/edge-cases/fields-with-connection-names.gql @@ -1,16 +1,6 @@ type FieldsWithConnectionNamesService { - Nodes( - filter: [FieldsWithConnectionNamesService_Nodes_filter] - orderBy: [FieldsWithConnectionNamesService_Nodes_orderBy] - skip: Int - top: Int - ): FieldsWithConnectionNamesService_Nodes_connection - Root( - filter: [FieldsWithConnectionNamesService_Root_filter] - orderBy: [FieldsWithConnectionNamesService_Root_orderBy] - skip: Int - top: Int - ): FieldsWithConnectionNamesService_Root_connection + Nodes(filter: [FieldsWithConnectionNamesService_Nodes_filter], orderBy: [FieldsWithConnectionNamesService_Nodes_orderBy], skip: Int, top: Int): FieldsWithConnectionNamesService_Nodes_connection + Root(filter: [FieldsWithConnectionNamesService_Root_filter], orderBy: [FieldsWithConnectionNamesService_Root_orderBy], skip: Int, top: Int): FieldsWithConnectionNamesService_Root_connection } type FieldsWithConnectionNamesService_Nodes { @@ -42,16 +32,9 @@ input FieldsWithConnectionNamesService_Nodes_filter { } type FieldsWithConnectionNamesService_Nodes_input { - create( - input: [FieldsWithConnectionNamesService_Nodes_C]! - ): [FieldsWithConnectionNamesService_Nodes] - delete( - filter: [FieldsWithConnectionNamesService_Nodes_filter] - ): Int - update( - filter: [FieldsWithConnectionNamesService_Nodes_filter] - input: FieldsWithConnectionNamesService_Nodes_U! - ): [FieldsWithConnectionNamesService_Nodes] + create(input: [FieldsWithConnectionNamesService_Nodes_C]!): [FieldsWithConnectionNamesService_Nodes] + delete(filter: [FieldsWithConnectionNamesService_Nodes_filter]!): Int + update(filter: [FieldsWithConnectionNamesService_Nodes_filter]!, input: FieldsWithConnectionNamesService_Nodes_U!): [FieldsWithConnectionNamesService_Nodes] } input FieldsWithConnectionNamesService_Nodes_orderBy { @@ -92,16 +75,9 @@ input FieldsWithConnectionNamesService_Root_filter { } type FieldsWithConnectionNamesService_Root_input { - create( - input: [FieldsWithConnectionNamesService_Root_C]! - ): [FieldsWithConnectionNamesService_Root] - delete( - filter: [FieldsWithConnectionNamesService_Root_filter] - ): Int - update( - filter: [FieldsWithConnectionNamesService_Root_filter] - input: FieldsWithConnectionNamesService_Root_U! - ): [FieldsWithConnectionNamesService_Root] + create(input: [FieldsWithConnectionNamesService_Root_C]!): [FieldsWithConnectionNamesService_Root] + delete(filter: [FieldsWithConnectionNamesService_Root_filter]!): Int + update(filter: [FieldsWithConnectionNamesService_Root_filter]!, input: FieldsWithConnectionNamesService_Root_U!): [FieldsWithConnectionNamesService_Root] } input FieldsWithConnectionNamesService_Root_orderBy { @@ -116,11 +92,11 @@ type FieldsWithConnectionNamesService_input { } input ID_filter { - eq: [ID] - ge: [ID] - gt: [ID] - le: [ID] - lt: [ID] + eq: ID + ge: ID + gt: ID + le: ID + lt: ID ne: [ID] } @@ -139,12 +115,12 @@ enum SortDirection { input String_filter { contains: [String] - endswith: [String] - eq: [String] - ge: [String] - gt: [String] - le: [String] - lt: [String] + endswith: String + eq: String + ge: String + gt: String + le: String + lt: String ne: [String] - startswith: [String] -} + startswith: String +} \ No newline at end of file From c48c6ce97d8bac82e11c57009f9121d5ad2beaed Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:33:07 +0200 Subject: [PATCH 14/22] Reorder entity definitions --- .../edge-cases/srv/fields-with-connection-names.cds | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/resources/edge-cases/srv/fields-with-connection-names.cds b/test/resources/edge-cases/srv/fields-with-connection-names.cds index 30950353..41fbdf0d 100644 --- a/test/resources/edge-cases/srv/fields-with-connection-names.cds +++ b/test/resources/edge-cases/srv/fields-with-connection-names.cds @@ -1,14 +1,14 @@ @graphql service FieldsWithConnectionNamesService { - entity Nodes { + entity Root { key ID : UUID; - nodes : String; + nodes : Composition of Nodes; totalCount : String; } - entity Root { + entity Nodes { key ID : UUID; - nodes : Composition of Nodes; + nodes : String; totalCount : String; } } From 8e99f67d944f38bc54cc98214c4751332dbe0c11 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:33:34 +0200 Subject: [PATCH 15/22] Remove unused managed import --- test/resources/edge-cases/srv/field-named-localized.cds | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/resources/edge-cases/srv/field-named-localized.cds b/test/resources/edge-cases/srv/field-named-localized.cds index acc96591..cb3f71d7 100644 --- a/test/resources/edge-cases/srv/field-named-localized.cds +++ b/test/resources/edge-cases/srv/field-named-localized.cds @@ -1,5 +1,3 @@ -using {managed} from '@sap/cds/common'; - @graphql service FieldNamedLocalizedService { entity localized { From 9395a3ac0299f0e8040777d693b7a04ff554661b Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:37:18 +0200 Subject: [PATCH 16/22] Replace config in `server.js` with `package.json` --- test/resources/edge-cases/package.json | 5 +++++ test/resources/edge-cases/server.js | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) create mode 100644 test/resources/edge-cases/package.json delete mode 100644 test/resources/edge-cases/server.js diff --git a/test/resources/edge-cases/package.json b/test/resources/edge-cases/package.json new file mode 100644 index 00000000..a713a269 --- /dev/null +++ b/test/resources/edge-cases/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "@cap-js/graphql": "*" + } +} diff --git a/test/resources/edge-cases/server.js b/test/resources/edge-cases/server.js deleted file mode 100644 index 6808b2bb..00000000 --- a/test/resources/edge-cases/server.js +++ /dev/null @@ -1,6 +0,0 @@ -const cds = require('@sap/cds') -const path = require('path') - -cds.env.protocols = { - graphql: { path: '/graphql', impl: path.join(__dirname, '../../../index.js') } -} \ No newline at end of file From 188eae4534de08ee167df26dda4bf8302012323b Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:47:10 +0200 Subject: [PATCH 17/22] Remove unused imports --- lib/resolvers/crud/create.js | 1 - lib/resolvers/crud/read.js | 1 - lib/resolvers/crud/update.js | 1 - 3 files changed, 3 deletions(-) diff --git a/lib/resolvers/crud/create.js b/lib/resolvers/crud/create.js index 1f37514e..e050eac3 100644 --- a/lib/resolvers/crud/create.js +++ b/lib/resolvers/crud/create.js @@ -4,7 +4,6 @@ const { ARGS } = require('../../constants') const { getArgumentByName, astToEntries } = require('../parse/ast2cqn') const { entriesStructureToEntityStructure } = require('./utils') const GraphQLRequest = require('../GraphQLRequest') -const { isPlainObject } = require('../utils') const formatResult = require('../parse/ast/result') module.exports = async ({ req, res }, service, entity, selection) => { diff --git a/lib/resolvers/crud/read.js b/lib/resolvers/crud/read.js index a9eedff0..4c8c8b96 100644 --- a/lib/resolvers/crud/read.js +++ b/lib/resolvers/crud/read.js @@ -3,7 +3,6 @@ const { SELECT } = cds.ql const { ARGS, CONNECTION_FIELDS } = require('../../constants') const { getArgumentByName, astToColumns, astToWhere, astToOrderBy, astToLimit } = require('../parse/ast2cqn') const GraphQLRequest = require('../GraphQLRequest') -const { isPlainObject } = require('../utils') const formatResult = require('../parse/ast/result') module.exports = async ({ req, res }, service, entity, selection) => { diff --git a/lib/resolvers/crud/update.js b/lib/resolvers/crud/update.js index 356e0989..ae4d76fd 100644 --- a/lib/resolvers/crud/update.js +++ b/lib/resolvers/crud/update.js @@ -4,7 +4,6 @@ const { ARGS } = require('../../constants') const { getArgumentByName, astToColumns, astToWhere, astToEntries } = require('../parse/ast2cqn') const { entriesStructureToEntityStructure } = require('./utils') const GraphQLRequest = require('../GraphQLRequest') -const { isPlainObject } = require('../utils') const formatResult = require('../parse/ast/result') module.exports = async ({ req, res }, service, entity, selection) => { From bd6f00857e47960eab587b04ec7d5e7f8e01f1d9 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:48:43 +0200 Subject: [PATCH 18/22] Fix argument name --- lib/resolvers/parse/ast/result.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index 356969b5..e8439abc 100644 --- a/lib/resolvers/parse/ast/result.js +++ b/lib/resolvers/parse/ast/result.js @@ -44,8 +44,8 @@ const formatResult = (entity, field, result, isConnection) => { } } - const _formatByType = (type, field, value, skipTopLevelConnection) => { - if (Array.isArray(value)) return _formatArray(type, field, value, skipTopLevelConnection) + const _formatByType = (type, field, value, isConnection) => { + if (Array.isArray(value)) return _formatArray(type, field, value, isConnection) if (isPlainObject(value)) return _formatObject(type, field.selectionSet.selections, value) From e30b704a485bbc46539b638089d6fad10373b700 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:51:12 +0200 Subject: [PATCH 19/22] Reorder entity definitions --- .../edge-cases/srv/field-named-localized.cds | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/resources/edge-cases/srv/field-named-localized.cds b/test/resources/edge-cases/srv/field-named-localized.cds index cb3f71d7..5d93a53d 100644 --- a/test/resources/edge-cases/srv/field-named-localized.cds +++ b/test/resources/edge-cases/srv/field-named-localized.cds @@ -1,11 +1,5 @@ @graphql service FieldNamedLocalizedService { - entity localized { - key ID : Integer; - root : Association to Root; - localized : String; // to test that a property only named 'localized' is not confused with localized keyword - } - entity Root { key ID : Integer; // The resulting GraphQL schema should contain a field named @@ -14,4 +8,10 @@ service FieldNamedLocalizedService { localized : Association to many localized on localized.root = $self; } + + entity localized { + key ID : Integer; + root : Association to Root; + localized : String; // to test that a property only named 'localized' is not confused with localized keyword + } } From 0456d60a52967e200620b315b894c1997175413e Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 13:52:36 +0200 Subject: [PATCH 20/22] Replace `data.autoReset()` with `data.reset()` --- test/tests/edge-cases.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/tests/edge-cases.test.js b/test/tests/edge-cases.test.js index 8470d32b..aa06f593 100644 --- a/test/tests/edge-cases.test.js +++ b/test/tests/edge-cases.test.js @@ -6,7 +6,10 @@ describe('graphql - edge cases', () => { const { axios, POST, data } = cds.test(path.join(__dirname, '../resources/edge-cases')) // Prevent axios from throwing errors for non 2xx status codes axios.defaults.validateStatus = false - data.autoReset(true) + + beforeEach(async () => { + await data.reset() + }) test('no name clashes occur between CDS names and connection fields with 2 one', async () => { const queryCreate = gql` From cccb665d078cb2bce506dde84f18b5d4539acf4a Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 14:35:54 +0200 Subject: [PATCH 21/22] Extract element type to variable --- lib/resolvers/parse/ast/result.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index e8439abc..7e67a47b 100644 --- a/lib/resolvers/parse/ast/result.js +++ b/lib/resolvers/parse/ast/result.js @@ -13,8 +13,9 @@ const formatResult = (entity, field, result, isConnection) => { for (const field of fields) { const element = type.elements[field.name.value] + const elementType = element._target || element const responseKey = field.alias?.value || field.name.value - result[responseKey] = _formatByType(element._target || element, field, value, element.is2many) + result[responseKey] = _formatByType(elementType, field, value, element.is2many) } } From 7b1074f8508a421084aa31bb6d581203a88978b0 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Wed, 18 Oct 2023 14:37:58 +0200 Subject: [PATCH 22/22] Add TODO to not wrap singletons in arrays --- lib/resolvers/parse/ast/result.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index 7e67a47b..22e15138 100644 --- a/lib/resolvers/parse/ast/result.js +++ b/lib/resolvers/parse/ast/result.js @@ -53,6 +53,7 @@ const formatResult = (entity, field, result, isConnection) => { return value } + // TODO: if singleton, don't wrap in array const resultInArray = isPlainObject(result) ? [result] : result return _formatByType(entity, field, resultInArray, isConnection) }