feat: improve handling of various return types from custom handlers

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Improved consistency of handling results of different types returned by custom handlers in CRUD resolvers:
+  + Wrap only objects (i.e. not primitive types or arrays) returned by custom handlers in arrays in create, read, and update resolvers
+  + Delete mutations return the length of an array that is returned by a `DELETE` custom handler or 1 if a single object is returned
+
 ### Fixed
 
 ### Removed

lib/resolvers/crud/create.js

@@ -2,6 +2,7 @@ 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) => {
@@ -12,7 +13,7 @@ module.exports = async (service, entity, selection) => {
   query.entries(entries)
 
   const result = await service.run(query)
-  const resultInArray = Array.isArray(result) ? result : [result]
+  const resultInArray = isPlainObject(result) ? [result] : result
 
   return formatResult(selection, resultInArray, true)
 }

lib/resolvers/crud/delete.js

@@ -1,6 +1,7 @@
 const { DELETE } = require('@sap/cds/lib').ql
 const { ARGS } = require('../../constants')
 const { getArgumentByName, astToWhere } = require('../parse/ast2cqn')
+const { isPlainObject } = require('../utils')
 
 module.exports = async (service, entity, selection) => {
   let query = DELETE.from(entity)
@@ -16,5 +17,10 @@ module.exports = async (service, entity, selection) => {
     else throw e
   }
 
+  // The CDS delete query returns the number of deleted entries
+  // However, custom handlers can return non-numeric results for delete
+  if (isPlainObject(result)) return 1
+  if (Array.isArray(result)) return result.length
+
   return result
 }

lib/resolvers/crud/read.js

@@ -2,6 +2,7 @@ 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
@@ -27,5 +28,7 @@ module.exports = async (service, entity, selection) => {
 
   const result = await service.run(query)
 
-  return formatResult(selection, result)
+  const resultInArray = isPlainObject(result) ? [result] : result
+
+  return formatResult(selection, resultInArray)
 }

lib/resolvers/crud/update.js

@@ -2,6 +2,7 @@ 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) => {
@@ -26,12 +27,17 @@ module.exports = async (service, entity, selection) => {
   const result = await service.tx(async tx => {
     // read needs to be done before the update, otherwise the where clause might become invalid (case that properties in where clause are updated by the mutation)
     resultBeforeUpdate = await tx.run(queryBeforeUpdate)
-    if (resultBeforeUpdate.length === 0) return []
+    if (resultBeforeUpdate.length === 0) return {}
     return tx.run(query)
   })
 
-  // Merge selected fields with updated data
-  const mergedResults = resultBeforeUpdate.map(original => ({ ...original, ...result }))
+  let mergedResults = result
+  if (Array.isArray(resultBeforeUpdate)) {
+    // Merge selected fields with updated data
+    mergedResults = resultBeforeUpdate.map(original => ({ ...original, ...result }))
+  }
 
-  return formatResult(selection, mergedResults, true)
+  const resultInArray = isPlainObject(mergedResults) ? [mergedResults] : mergedResults
+
+  return formatResult(selection, resultInArray, true)
 }

lib/resolvers/parse/ast/fromObject.js

@@ -1,5 +1,5 @@
 const { Kind } = require('graphql')
-const { isPlainObject } = require('./util')
+const { isPlainObject } = require('../../utils')
 
 const _nullValue = { kind: Kind.NULL }
 

lib/resolvers/parse/ast/result.js

@@ -1,6 +1,6 @@
 const { CONNECTION_FIELDS } = require('../../../constants')
+const { isPlainObject } = require('../../utils')
 const { getPotentiallyNestedNodesSelections } = require('../util')
-const { isPlainObject } = require('./util')
 
 const formatResult = (field, result, skipTopLevelConnection) => {
   const _formatObject = (selections, object) => {

lib/resolvers/utils/index.js

@@ -1,3 +1,6 @@
+const isPlainObject = value =>
+  value !== null && typeof value === 'object' && !Array.isArray(value) && !Buffer.isBuffer(value)
+
 const _ensureError = error => (error instanceof Error ? error : new Error(error))
 
 const setResponse = async (response, key, value) => {
@@ -8,4 +11,4 @@ const setResponse = async (response, key, value) => {
   }
 }
 
-module.exports = { setResponse }
+module.exports = { isPlainObject, setResponse }

test/resources/custom-handlers/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') }
+}

test/resources/custom-handlers/srv/return-types.cds

@@ -0,0 +1,21 @@
+service ReturnTypesService {
+  entity Integer {
+    key id     : UUID;
+        string : cds.String;
+  }
+
+  entity String {
+    key id     : UUID;
+        string : cds.String;
+  }
+
+  entity Object {
+    key id     : UUID;
+        string : cds.String;
+  }
+
+  entity Array {
+    key id     : UUID;
+        string : cds.String;
+  }
+}

test/resources/custom-handlers/srv/return-types.js

@@ -0,0 +1,9 @@
+module.exports = srv => {
+  const string = 'foo'
+  const id = '0557a188-326e-4dcb-999b-e1acf7979fa3'
+
+  srv.on('*', 'Integer', () => 999)
+  srv.on('*', 'String', () => string)
+  srv.on('*', 'Object', () => ({ id, string }))
+  srv.on('*', 'Array', () => [{ id, string }, { id, string }])
+}

test/tests/concurrency.test.js

@@ -1,4 +1,4 @@
-describe('graphql resolver concurrency', () => {
+describe('graphql - resolver concurrency', () => {
   const cds = require('@sap/cds/lib')
   const path = require('path')
   const { gql } = require('../util')