Data item modification result refactor (#483)

* refactor: DataItemModificationResult * refactor: new wixdataitem function * refactor: review changes
wix · Dec 10, 2023 · dbd7ab0 · dbd7ab0
1 parent 16ea92b
commit dbd7ab0
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 67 deletions.
diff --git a/apps/velo-external-db/test/e2e/app_data.e2e.spec.ts b/apps/velo-external-db/test/e2e/app_data.e2e.spec.ts
@@ -73,7 +73,7 @@ describe(`Velo External DB Data REST API: ${currentDbImplementationName()}`,  ()
 
         const response = await axiosInstance.post('/items/insert', data.insertRequest(ctx.collectionName, ctx.items), authAdmin)
 
-        expect(response.data).toEqual({ results: expect.toIncludeSameMembers(ctx.items) })
+        expect(response.data).toEqual({ results: expect.toIncludeSameMembers(ctx.items.map(item => ({ item }))) })
 
         await expect(data.queryCollectionAsArray(ctx.collectionName, [], undefined, authOwner)).resolves.toEqual({
             items: expect.toIncludeSameMembers(ctx.items),
@@ -87,14 +87,10 @@ describe(`Velo External DB Data REST API: ${currentDbImplementationName()}`,  ()
         const response = await axiosInstance.post('/items/insert', data.insertRequest(ctx.collectionName, ctx.items), authAdmin)
 
 
-        expect(response.data.results).toEqual([
-            ctx.items[0],
-            {
-                errorCode: 409,
-                errorMessage: expect.toInclude('Item already exists'),
-                data: expect.any(Object)
-            },
-            ...ctx.items.slice(2, ctx.items.length),
+        expect(response.data.results).toStrictEqual([
+            { item: ctx.items[0] },
+            { error: { errorCode: 409, errorMessage: expect.toInclude('Item already exists'), data: expect.any(Object) } },
+            ...(ctx.items.slice(2, ctx.items.length).map( item => ({ item }) )),
         ])
 
         await expect(data.queryCollectionAsArray(ctx.collectionName, [], undefined, authOwner)).resolves.toEqual({
@@ -144,12 +140,12 @@ describe(`Velo External DB Data REST API: ${currentDbImplementationName()}`,  ()
     testIfSupportedOperationsIncludes(supportedOperations, [ DeleteImmediately ])('bulk delete api', async() => {
         await schema.givenCollection(ctx.collectionName, [ctx.column], authOwner)
         await data.givenItems(ctx.items, ctx.collectionName, authAdmin)
-await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
+        await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
         const response = await axiosInstance.post('/items/remove', { 
             collectionId: ctx.collectionName, itemIds: ctx.items.map(i => i._id) 
         }, authAdmin)
 
-        expect(response.data.results).toEqual(expect.toIncludeSameMembers(ctx.items))
+        expect(response.data.results).toEqual(expect.toIncludeSameMembers(ctx.items.map( item => ({ item }) ) ))
         await expect(data.queryCollectionAsArray(ctx.collectionName, [], undefined, authOwner)).resolves.toEqual({
             items: [],
             pagingMetadata: data.pagingMetadata(0, 0)
@@ -164,12 +160,14 @@ await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
             collectionId: ctx.collectionName, itemIds: ctx.items.map(i => i._id) 
         }, authAdmin)
 
-        expect(response.data.results).toEqual([
-            ctx.items[0],
+        expect(response.data.results).toStrictEqual([
+            { item: ctx.items[0] },
             ...ctx.items.slice(1, ctx.items.length).map(_i => ({
-                errorCode: 404,
-                errorMessage: expect.toInclude('Item doesn\'t exists'),
-                data: expect.any(Object)
+                error: {
+                    errorCode: 404,
+                    errorMessage: expect.toInclude('Item doesn\'t exists'),
+                    data: expect.any(Object)
+                }
             }))
         ])
 
@@ -229,7 +227,7 @@ await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
         await data.givenItems(ctx.items, ctx.collectionName, authAdmin)
         const response = await axiosInstance.post('/items/update', data.updateRequest(ctx.collectionName, ctx.modifiedItems),  authAdmin)
 
-        expect(response.data.results).toEqual(ctx.modifiedItems)
+        expect(response.data.results).toEqual(ctx.modifiedItems.map( item => ({ item }) ))
 
         await expect(data.queryCollectionAsArray(ctx.collectionName, [], undefined, authOwner)).resolves.toEqual({
                 items: expect.toIncludeSameMembers(ctx.modifiedItems),
@@ -243,11 +241,13 @@ await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
         const response = await axiosInstance.post('/items/update', data.updateRequest(ctx.collectionName, ctx.modifiedItems),  authAdmin)
 
         expect(response.data.results).toEqual([
-            ...ctx.modifiedItems.slice(0, ctx.items.length - 1),
-            {
-                errorCode: 404,
-                errorMessage: expect.toInclude('Item doesn\'t exists'),
-                data: expect.any(Object)
+            ...(ctx.modifiedItems.slice(0, ctx.items.length - 1).map( item => ({ item }))),
+            { 
+                error: { 
+                    errorCode: 404, 
+                    errorMessage: expect.toInclude('Item doesn\'t exists'), 
+                    data: expect.any(Object) 
+                } 
             }
         ])
 
@@ -337,11 +337,14 @@ await data.givenItems([ ctx.items[1] ], ctx.collectionName, authAdmin)
             const response = await axiosInstance.post('/items/insert', data.insertRequest(ctx.collectionName, [ctx.item]), authAdmin)
 
             expect(response.data).toEqual(expect.objectContaining({
-                results: [{
-                    errorCode: 409,
-                    errorMessage: expect.toInclude('Item already exists'),
-                    data: expect.any(Object)
-                }
+                results: [
+                    {
+                        error: {
+                            errorCode: 409,
+                            errorMessage: expect.toInclude('Item already exists'),
+                            data: expect.any(Object)
+                        }
+                    }
                 ]
             }))
         })

diff --git a/apps/velo-external-db/test/e2e/app_data_hooks.e2e.spec.ts b/apps/velo-external-db/test/e2e/app_data_hooks.e2e.spec.ts
@@ -1,8 +1,8 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
 import { authOwner, errorResponseWith } from '@wix-velo/external-db-testkit'
 import { testIfSupportedOperationsIncludes, testSupportedOperations } from '@wix-velo/test-commons'
-import { dataSpi, types as coreTypes, collectionSpi } from '@wix-velo/velo-external-db-core'
-import { DataOperation, InputField, ItemWithId, SchemaOperations } from '@wix-velo/velo-external-db-types'
+import { dataSpi, types as coreTypes, collectionSpi, dataConvertUtils } from '@wix-velo/velo-external-db-core'
+import { DataOperation, InputField, Item, ItemWithId, SchemaOperations } from '@wix-velo/velo-external-db-types'
 import { Uninitialized, gen as genCommon } from '@wix-velo/test-commons'
 import { initApp, teardownApp, dbTeardown, setupDb, currentDbImplementationName, env, supportedOperations } from '../resources/e2e_resources'
 import gen = require('../gen')
@@ -187,7 +187,7 @@ describe(`Velo External DB Data Hooks: ${currentDbImplementationName()}`, () =>
                             beforeAll: (payload: dataSpi.InsertRequest | dataSpi.UpdateRequest, requestContext: coreTypes.RequestContext, _serviceContext) => {
                                 if (requestContext.operation !== DataOperation.query) {
                                     return {
-                                        ...payload, items: payload.items.map(item => ({
+                                        ...payload, items: payload.items.map( item => ({
                                             ...item,
                                             [ctx.afterAllColumn.name]: true,
                                             [ctx.afterWriteColumn.name]: false,
@@ -460,29 +460,35 @@ describe(`Velo External DB Data Hooks: ${currentDbImplementationName()}`, () =>
                         afterAll: (payload: coreTypes.InsertResponse | coreTypes.UpdateResponse | coreTypes.RemoveResponse, requestContext: coreTypes.RequestContext, _serviceContext) => {
                             if (requestContext.operation !== DataOperation.query) {
                                 return {
-                                    ...payload, results: payload.results.map(item => ({
-                                        ...item,
-                                        [ctx.afterAllColumn.name]: true,
-                                        [ctx.afterWriteColumn.name]: false,
-                                        [ctx.afterHookColumn.name]: false,
+                                    ...payload, results: payload.results.map(({ item }: { item: Item }) => ({
+                                        item: {
+                                            ...item,
+                                            [ctx.afterAllColumn.name]: true,
+                                            [ctx.afterWriteColumn.name]: false,
+                                            [ctx.afterHookColumn.name]: false,
+                                        }
                                     }))
                                 }
                             }
                         },
                         afterWrite: (payload: coreTypes.InsertResponse | coreTypes.UpdateResponse | coreTypes.RemoveResponse, _requestContext, _serviceContext) => {
                             return {
-                                ...payload, results: payload.results.map(item => ({
-                                    ...item,
-                                    [ctx.afterWriteColumn.name]: true,
-                                    [ctx.afterHookColumn.name]: false,
+                                ...payload, results: payload.results.map(({ item }: { item: Item }) => ({
+                                    item: {
+                                        ...item,
+                                        [ctx.afterWriteColumn.name]: true,
+                                        [ctx.afterHookColumn.name]: false,
+                                    }
                                 }))
                             }
                         },
                         [hookName]: (payload, _requestContext, _serviceContext) => {
                             return {
-                                ...payload, results: payload.results.map(item => ({
-                                    ...item,
-                                    [ctx.afterHookColumn.name]: true,
+                                ...payload, results: payload.results.map(({ item }: { item: Item }) => ({
+                                    item: {
+                                        ...item,
+                                        [ctx.afterHookColumn.name]: true,
+                                    }
                                 }))
                             }
                         }
@@ -493,10 +499,12 @@ describe(`Velo External DB Data Hooks: ${currentDbImplementationName()}`, () =>
 
                 await expect(response.data).toEqual({
                     results: [{
-                        ...ctx.item,
-                        [ctx.afterAllColumn.name]: true,
-                        [ctx.afterWriteColumn.name]: true,
-                        [ctx.afterHookColumn.name]: true,
+                        item: { 
+                            ...ctx.item, 
+                            [ctx.afterAllColumn.name]: true, 
+                            [ctx.afterWriteColumn.name]: true, 
+                            [ctx.afterHookColumn.name]: true, 
+                        }
                     }]
                 })
             })

diff --git a/libs/velo-external-db-core/src/converters/data_utils.spec.ts b/libs/velo-external-db-core/src/converters/data_utils.spec.ts
@@ -1,4 +1,4 @@
-import { asWixData, generateIdsIfNeeded } from './data_utils'
+import { asWixData, asWixDataItem, generateIdsIfNeeded } from './data_utils'
 import { Uninitialized, gen } from '@wix-velo/test-commons'
 import * as dateTimeProvider from '../../test/drivers/date_time_provider_test_support'
 import Chance = require('chance')
@@ -17,6 +17,10 @@ describe('Converters', () => {
                                                                           [ctx.anotherProperty]: { $date: dateTimeProvider.currentDateTime().toISOString() } } )
     })
 
+    test('should correctly convert the object to a WixDataItem', async() => {
+        expect(asWixDataItem(ctx.obj)).toMatchObject({ item: ctx.obj })
+    })
+
     test('if _id field exists do nothing', async() => {
         expect(generateIdsIfNeeded({ ...ctx.obj, _id: 'something' })).toEqual( { ...ctx.obj, _id: 'something' } )
     })

diff --git a/libs/velo-external-db-core/src/converters/data_utils.ts b/libs/velo-external-db-core/src/converters/data_utils.ts
@@ -7,6 +7,10 @@ export const asWixData = (item: Item) => {
     return generateIdsIfNeeded(packDates(item))
 }
 
+export const asWixDataItem = (item: Item) => { 
+    return { item: asWixData(item) }
+}
+
 export const generateIdsIfNeeded = (item: Item): ItemWithId => {
     if ('_id' in item)
         return item as ItemWithId

diff --git a/libs/velo-external-db-core/src/index.ts b/libs/velo-external-db-core/src/index.ts
@@ -74,6 +74,7 @@ export * as types from './types'
 export * as dataSpi from './spi-model/data_source'
 export * as collectionSpi from './spi-model/collection'
 export * as schemaUtils from '../src/utils/schema_utils'
+export * as dataConvertUtils from './converters/data_utils'
 export * as convertersUtils from './converters/utils'
 export { config } from './roles-config.json'
 export { DataService, SchemaService, OperationService, CacheableSchemaInformation, FilterTransformer, AggregationTransformer, QueryValidator, SchemaAwareDataService, ItemTransformer, Hooks, ServiceContext, CollectionCapability, decodeBase64 }
diff --git a/libs/velo-external-db-core/src/service/data.spec.ts b/libs/velo-external-db-core/src/service/data.spec.ts
@@ -4,6 +4,7 @@ import * as driver from '../../test/drivers/data_provider_test_support'
 import { SystemFields } from '@wix-velo/velo-external-db-commons'
 import Chance = require('chance')
 import { getByIdFilterFor } from '../utils/data_utils'
+import { asWixDataItem } from '../converters/data_utils'
 import { HttpStatusCode } from '../spi-model/errors'
 const chance = new Chance()
 
@@ -70,13 +71,13 @@ describe('Data Service', () => {
     test('bulk insert will insert data into db', async() => {
         driver.expectInsertFor(ctx.entities, ctx.collectionName, ctx.defaultProjection)
 
-        return expect(env.dataService.bulkInsert(ctx.collectionName, ctx.entities, ctx.defaultProjection)).resolves.toEqual({ items: ctx.entities })
+        return expect(env.dataService.bulkInsert(ctx.collectionName, ctx.entities, ctx.defaultProjection)).resolves.toEqual({ items: ctx.entities.map(asWixDataItem) })
     })
 
     test('insert already item will return error object', async() => {        
         driver.expectInsertAlreadyExistsFor([ctx.entity], ctx.collectionName,  ctx.defaultProjection)
 
-        return expect(env.dataService.insert(ctx.collectionName, ctx.entity, ctx.defaultProjection)).resolves.toEqual({ item: { errorCode: HttpStatusCode.ALREADY_EXISTS, errorMessage: expect.any(String), data: expect.any(Object)  } })
+        return expect(env.dataService.insert(ctx.collectionName, ctx.entity, ctx.defaultProjection)).resolves.toEqual({ error: { errorCode: HttpStatusCode.ALREADY_EXISTS, errorMessage: expect.any(String), data: expect.any(Object)  } })
     })
 
     test('update will update data into db', async() => {
@@ -88,7 +89,7 @@ describe('Data Service', () => {
     test('bulk update will update data into db', async() => {
         driver.expectUpdateFor(ctx.entities, ctx.collectionName)
 
-        return expect(env.dataService.bulkUpdate(ctx.collectionName, ctx.entities)).resolves.toEqual({ items: ctx.entities })
+        return expect(env.dataService.bulkUpdate(ctx.collectionName, ctx.entities)).resolves.toEqual({ items: ctx.entities.map(asWixDataItem) })
     })
 
     test('delete by item id', async() => {
@@ -101,7 +102,7 @@ describe('Data Service', () => {
         driver.givenItemsById(ctx.entities, ctx.collectionName, '', 0, 1, ctx.defaultProjection)
         driver.expectDeleteFor(ctx.itemIds, ctx.collectionName)
 
-        return expect(env.dataService.bulkDelete(ctx.collectionName, ctx.entities.map((e: any) => e._id), ctx.defaultProjection)).resolves.toEqual({ items: ctx.entities })
+        return expect(env.dataService.bulkDelete(ctx.collectionName, ctx.entities.map((e: any) => e._id), ctx.defaultProjection)).resolves.toEqual({ items: ctx.entities.map(asWixDataItem) })
     })
 
     // eslint-disable-next-line jest/expect-expect