diff --git a/CHANGELOG.md b/CHANGELOG.md index 3710c583..70a6de5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Name clashes when CDS elements are named `nodes` or `totalCount` + ### Removed ## Version 0.9.0 - 2023-11-16 diff --git a/lib/resolvers/crud/create.js b/lib/resolvers/crud/create.js index 2fa88252..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) => { @@ -15,7 +14,6 @@ module.exports = async ({ req, res }, service, entity, selection) => { query.entries(entries) const result = await service.dispatch(new GraphQLRequest({ req, res, query })) - const resultInArray = isPlainObject(result) ? [result] : result - return formatResult(selection, resultInArray, true) + return formatResult(entity, selection, result, false) } diff --git a/lib/resolvers/crud/read.js b/lib/resolvers/crud/read.js index bdcc8b1f..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) => { @@ -11,7 +10,7 @@ module.exports = async ({ req, res }, 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) { @@ -30,7 +29,5 @@ module.exports = async ({ req, res }, service, entity, selection) => { const result = await service.dispatch(new GraphQLRequest({ req, res, query })) - const resultInArray = isPlainObject(result) ? [result] : result - - return formatResult(selection, resultInArray) + return formatResult(entity, selection, result, true) } diff --git a/lib/resolvers/crud/update.js b/lib/resolvers/crud/update.js index 4fffdc2a..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) => { @@ -13,7 +12,7 @@ module.exports = async ({ req, res }, 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)) @@ -40,7 +39,5 @@ module.exports = async ({ req, res }, service, entity, selection) => { mergedResults = resultBeforeUpdate.map(original => ({ ...original, ...result })) } - const resultInArray = isPlainObject(mergedResults) ? [mergedResults] : mergedResults - - return formatResult(selection, resultInArray, true) + return formatResult(entity, selection, mergedResults, false) } diff --git a/lib/resolvers/parse/ast/result.js b/lib/resolvers/parse/ast/result.js index 18f85c7c..22e15138 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 { isPlainObject } = require('../../utils') const { getPotentiallyNestedNodesSelections } = require('../util') -const formatResult = (field, result, skipTopLevelConnection) => { - const _formatObject = (selections, object) => { +const formatResult = (entity, field, result, isConnection) => { + const _formatObject = (type, selections, object) => { const result = {} for (const key in object) { @@ -12,8 +12,10 @@ 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 elementType = element._target || element const responseKey = field.alias?.value || field.name.value - result[responseKey] = _formatByType(field, value) + result[responseKey] = _formatByType(elementType, field, value, element.is2many) } } @@ -29,13 +31,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) - const result = array.map(e => _formatObject(potentiallyNestedSelections, e)) + const potentiallyNestedSelections = getPotentiallyNestedNodesSelections(selections, isConnection) + const result = array.map(e => _formatObject(type, potentiallyNestedSelections, e)) // REVISIT: requires differentiation for support of configurable schema flavors - if (skipTopLevelConnection) return result + if (!isConnection) return result return { ..._aliasFieldsWithValue(selections, CONNECTION_FIELDS.nodes, result), @@ -43,15 +45,17 @@ const formatResult = (field, result, skipTopLevelConnection) => { } } - const _formatByType = (field, value, skipTopLevelConnection) => { - if (Array.isArray(value)) return _formatArray(field, value, skipTopLevelConnection) + const _formatByType = (type, field, value, isConnection) => { + if (Array.isArray(value)) return _formatArray(type, field, value, isConnection) - if (isPlainObject(value)) return _formatObject(field.selectionSet.selections, value) + if (isPlainObject(value)) return _formatObject(type, field.selectionSet.selections, value) return value } - return _formatByType(field, result, skipTopLevelConnection) + // TODO: if singleton, don't wrap in array + const resultInArray = isPlainObject(result) ? [result] : result + return _formatByType(entity, field, resultInArray, isConnection) } 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 } 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/srv/field-named-localized.cds b/test/resources/edge-cases/srv/field-named-localized.cds index 2cb150ad..5d93a53d 100644 --- a/test/resources/edge-cases/srv/field-named-localized.cds +++ b/test/resources/edge-cases/srv/field-named-localized.cds @@ -1,12 +1,5 @@ -using {managed} from '@sap/cds/common'; - +@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 @@ -15,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 + } } 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..41fbdf0d --- /dev/null +++ b/test/resources/edge-cases/srv/fields-with-connection-names.cds @@ -0,0 +1,14 @@ +@graphql +service FieldsWithConnectionNamesService { + entity Root { + key ID : UUID; + nodes : Composition of Nodes; + totalCount : String; + } + + entity Nodes { + key ID : UUID; + nodes : String; + totalCount : String; + } +} diff --git a/test/resources/models.json b/test/resources/models.json index 36a0900a..6eee3046 100644 --- a/test/resources/models.json +++ b/test/resources/models.json @@ -18,5 +18,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..951a1144 --- /dev/null +++ b/test/schemas/edge-cases/fields-with-connection-names.gql @@ -0,0 +1,126 @@ +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 { + 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 { + nodes: FieldsWithConnectionNamesService_Nodes_C + 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 +} \ No newline at end of file diff --git a/test/tests/edge-cases.test.js b/test/tests/edge-cases.test.js new file mode 100644 index 00000000..aa06f593 --- /dev/null +++ b/test/tests/edge-cases.test.js @@ -0,0 +1,83 @@ +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 + + beforeEach(async () => { + await data.reset() + }) + + 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 }) + }) +})