fix: name clashes with CDS elements named nodes and totalCount

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

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)
 }

lib/resolvers/crud/read.js

@@ -3,15 +3,14 @@ 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) => {
   const selections = selection.selectionSet.selections
   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)
 }

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)
 }

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,29 +31,31 @@ 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),
       ..._aliasFieldsWithValue(selections, CONNECTION_FIELDS.totalCount, result.$count)
     }
   }
 
-  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

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 : ['*']
     }

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 }

test/resources/edge-cases/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@cap-js/graphql": "*"
+  }
+}

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
+  }
 }

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;
+  }
+}

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"]
   }
 ]

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
+}