From a762c69f74276c5fdf38f4b4737c3fd27d05f8b9 Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 11:18:57 -0500 Subject: [PATCH 1/6] test(assetlibraryhistory): add a test to find bugs --- .../src/events/events.service.spec.ts | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts diff --git a/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts b/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts new file mode 100644 index 000000000..72e88c7a4 --- /dev/null +++ b/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts @@ -0,0 +1,146 @@ +/********************************************************************************************************************* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. * + * * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance * + * with the License. A copy of the License is located at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * or in the 'license' file accompanying this file. This file is distributed on an 'AS IS' BASIS, WITHOUT WARRANTIES * + * OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions * + * and limitations under the License. * + *********************************************************************************************************************/ +import 'reflect-metadata'; + +import { createMockInstance } from 'jest-create-mock-instance'; +import { CreateAction } from './actions/eventaction.create'; +import { EventActionFactory } from './actions/eventaction.factory'; +import { UnsupportedAction } from './actions/eventaction.unsupported'; +import { UpdateAction } from './actions/eventaction.update'; +import { EventsDao } from './events.dao'; +import { Category, EventModel, EventType } from './events.models'; +import { EventsService } from './events.service'; + +describe('EventsService', () => { + let instance: EventsService; + let mockedEventActionFactory: jest.Mocked; + let mockedEventDao: jest.Mocked; + let updateAction: UpdateAction; + let unsupportedAction: UnsupportedAction; + let createAction: CreateAction; + + beforeEach(() => { + mockedEventActionFactory = createMockInstance(EventActionFactory); + mockedEventDao = createMockInstance(EventsDao); + updateAction = new UpdateAction(mockedEventDao); + unsupportedAction = new UnsupportedAction(); + createAction = new CreateAction(mockedEventDao); + instance = new EventsService(mockedEventActionFactory); + + mockedEventDao.getLatest = jest.fn().mockImplementation((objectId) => { + if (objectId === '/user/test_user_id') { + return Promise.resolve({ + objectId: '/user/test_user_id', + time: 'latest', + event: 'modify', + state: '{"groups":{},"attributes":{"email":"test_email@test.com","lastUserCaptureTime":"1698712913","country":"US","language":"en-us","firstName":"Test","lastName":"Tester"},"name":"test_user_id","templateId":"user","parentPath":"/user","groupPath":"/user/test_user_id","category":"group"}', + type: 'groups', + }); + } else if (objectId === 'test_device_id') { + return Promise.resolve({ + objectId: 'test_device_id', + time: 'latest', + event: 'modify', + state: '{"attributes":{},"groups":{"out":{"manufactured_by":["/supplier/supplier1"],"has_firmware":["/firmware/firmwareId1"],"is_model":["/device/robot/testDevice"]}},"devices":{},"templateId":"testDevice","deviceId":"test_device_id","state":"unprovisioned","category":"device","connected":false}', + type: 'devices', + }); + } else if ((objectId = 'test_undefined_object')) { + return Promise.resolve(undefined); + } else { + return Promise.resolve(undefined); + } + }); + + mockedEventDao.update = jest.fn().mockImplementation(() => { + return Promise.resolve(); + }); + mockedEventDao.create = jest.fn().mockImplementation(() => { + return Promise.resolve(); + }); + + mockedEventActionFactory.getAction = jest.fn().mockImplementation((event) => { + if (event.event === EventType.modify) { + return [updateAction]; + } else if (event.event === EventType.create) { + return [createAction]; + } + return [unsupportedAction]; + }); + }); + + it('happy path update a pre-existing group', async () => { + let testEvent = { + objectId: '/user/test_user_id', + type: Category.groups, + event: EventType.modify, + attributes: { + sourceGroupPath: '/user/test_user_id', + detachedFromGroup: '/client_device/test_device_id', + relationship: 'uses', + }, + time: '2024-02-23T21:00:00.000Z', + }; + await instance.create(testEvent); + + expect(mockedEventDao.create).toBeCalledTimes(1); + expect(mockedEventDao.update).toBeCalledTimes(1); + }); + + it('happy path update a pre-existing device', async () => { + let testEvent: EventModel = { + objectId: 'test_device_id', + type: Category.devices, + event: EventType.modify, + payload: + '{"attributes":{},"groups":{},"devices":{},"deviceId":"test_device_id","connected":false,"templateId":"testDevice","category":"device"}', + time: '2024-02-24T20:00:00.000Z', + attributes: undefined, + }; + await instance.create(testEvent); + + expect(mockedEventDao.create).toBeCalledTimes(1); + expect(mockedEventDao.update).toBeCalledTimes(1); + }); + + it('happy path create a new device', async () => { + let testEvent: EventModel = { + objectId: 'test_device_id', + type: Category.devices, + event: EventType.create, + payload: + '{"attributes":{},"groups":{},"devices":{},"deviceId":"test_device_id","connected":false,"templateId":"testDevice","category":"device"}', + time: '2024-02-24T20:00:00.000Z', + attributes: undefined, + }; + await instance.create(testEvent); + + expect(mockedEventDao.create).toBeCalledTimes(2); + expect(mockedEventDao.update).toBeCalledTimes(0); + }); + + it('getLatest returns an undefined object on a "modify" call', async () => { + let testEvent: EventModel = { + objectId: 'test_undefined_object', + type: Category.devices, + event: EventType.modify, + payload: + '{"attributes":{},"groups":{},"devices":{},"deviceId":"test_undefined_object","connected":false,"templateId":"testDevice","category":"device"}', + time: '2024-02-24T20:00:00.000Z', + attributes: undefined, + }; + await instance.create(testEvent); + + expect(mockedEventDao.create).toBeCalledTimes(1); + expect(mockedEventDao.update).toBeCalledTimes(1); + }); +}); From 0138441d406e518d60a6206eeaa0424e3d615899 Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 11:19:27 -0500 Subject: [PATCH 2/6] fix(assetlibraryhistory): make these parameters optional --- .../services/assetlibraryhistory/src/events/events.models.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/packages/services/assetlibraryhistory/src/events/events.models.ts b/source/packages/services/assetlibraryhistory/src/events/events.models.ts index e6f961ad2..fe682bd96 100644 --- a/source/packages/services/assetlibraryhistory/src/events/events.models.ts +++ b/source/packages/services/assetlibraryhistory/src/events/events.models.ts @@ -16,8 +16,8 @@ export interface EventModel { type: Category; time: string; event: EventType; - user: string; - payload: string; + user?: string; + payload?: string; attributes: { [key: string]: string }; } From 722126dd455850928d0e46119d398cf70934fe12 Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 11:23:36 -0500 Subject: [PATCH 3/6] fix(assetlibraryhistory): fix errors found during testing --- .../src/events/actions/eventaction.update.ts | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts index 5fbedacdb..2673662ee 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts @@ -55,13 +55,18 @@ export class UpdateAction implements EventAction { event.attributes['attachedToGroup'] ); } else if (event.attributes['detachedFromGroup'] !== undefined) { - const newRelationship = mergedState['groups']['out'][ - event.attributes['relationship'] - ].filter((value: string) => { - return value !== event.attributes['detachedFromGroup']; - }); + if (mergedState['groups']['out'] === undefined) { + mergedState['groups']['out'] = {}; + } else { + const newRelationship = mergedState['groups']['out'][ + event.attributes['relationship'] + ].filter((value: string) => { + return value !== event.attributes['detachedFromGroup']; + }); - mergedState['groups']['out'][event.attributes['relationship']] = newRelationship; + mergedState['groups']['out'][event.attributes['relationship']] = + newRelationship; + } } else if (event.attributes['attachedToDevice'] !== undefined) { if (mergedState['devices'] === undefined) { mergedState['devices'] = {}; @@ -78,23 +83,29 @@ export class UpdateAction implements EventAction { event.attributes['attachedToDevice'] ); } else if (event.attributes['detachedFromDevice'] !== undefined) { - const newRelationship = mergedState['devices']['out'][ - event.attributes['relationship'] - ].filter((value: string) => { - return value !== event.attributes['detachedFromDevice']; - }); - - mergedState['groups']['out'][event.attributes['relationship']] = newRelationship; + if (mergedState['devices']['out'] === undefined) { + mergedState['devices']['out'] = {}; + } else { + const newRelationship = mergedState['devices']['out'][ + event.attributes['relationship'] + ].filter((value: string) => { + return value !== event.attributes['detachedFromDevice']; + }); + mergedState['groups']['out'][event.attributes['relationship']] = + newRelationship; + } } } - if (event.event === 'modify' && event.type === 'devices') { - const state = JSON.parse(existingEvent.state); - if (state['groups'] !== undefined) { - mergedState['groups'] = state['groups']; - } - if (state['devices'] !== undefined) { - mergedState['devices'] = state['devices']; + if (existingEvent !== undefined) { + if (event.event === 'modify' && event.type === 'devices') { + const state = JSON.parse(existingEvent.state); + if (state['groups'] !== undefined) { + mergedState['groups'] = state['groups']; + } + if (state['devices'] !== undefined) { + mergedState['devices'] = state['devices']; + } } } From b17159336147d1bc9f03f8e17180658ba9e142e5 Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 11:44:18 -0500 Subject: [PATCH 4/6] fix(assetlibraryhistory): added some more tests and validation --- .../src/events/actions/eventaction.create.ts | 7 +- .../src/events/actions/eventaction.delete.ts | 7 +- .../actions/eventaction.publishTemplate.ts | 7 +- .../src/events/actions/eventaction.update.ts | 18 ++++- .../eventaction.updateComponentParent.ts | 7 +- .../src/events/events.service.spec.ts | 77 ++++++++++++++++++- 6 files changed, 107 insertions(+), 16 deletions(-) diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.create.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.create.ts index 0ce192fea..d63871dd7 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.create.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.create.ts @@ -37,8 +37,11 @@ export class CreateAction implements EventAction { // save the updated job info (1 record for the version, 1 to represent the latest) await this.eventsDao.create(toSave); - toSave.time = 'latest'; - await this.eventsDao.create(toSave); + const toSave2: StateHistoryModel = { + ...toSave, + time: 'latest', + }; + await this.eventsDao.create(toSave2); logger.debug('eventaction.create execute: exit:true'); return event; diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.delete.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.delete.ts index 3335d12ef..50139f5bb 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.delete.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.delete.ts @@ -37,8 +37,11 @@ export class DeleteAction implements EventAction { }; await this.eventsDao.create(toSave); - toSave.time = 'latest'; - await this.eventsDao.update(toSave); + const toUpdate: StateHistoryModel = { + ...toSave, + time: 'latest', + }; + await this.eventsDao.update(toUpdate); return event; } diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.publishTemplate.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.publishTemplate.ts index e78833082..3f030ade3 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.publishTemplate.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.publishTemplate.ts @@ -48,8 +48,11 @@ export class PublishTemplateAction implements EventAction { }; await this.eventsDao.create(toSave); - toSave.time = 'latest'; - await this.eventsDao.update(toSave); + const toUpdate: StateHistoryModel = { + ...toSave, + time: 'latest', + }; + await this.eventsDao.update(toUpdate); return event; } diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts index 2673662ee..4380e6758 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts @@ -60,7 +60,7 @@ export class UpdateAction implements EventAction { } else { const newRelationship = mergedState['groups']['out'][ event.attributes['relationship'] - ].filter((value: string) => { + ]?.filter((value: string) => { return value !== event.attributes['detachedFromGroup']; }); @@ -88,7 +88,7 @@ export class UpdateAction implements EventAction { } else { const newRelationship = mergedState['devices']['out'][ event.attributes['relationship'] - ].filter((value: string) => { + ]?.filter((value: string) => { return value !== event.attributes['detachedFromDevice']; }); mergedState['groups']['out'][event.attributes['relationship']] = @@ -122,9 +122,19 @@ export class UpdateAction implements EventAction { state: event.payload, }; + const toUpdate: StateHistoryModel = { + ...toSave, + time: 'latest', + }; await this.eventsDao.create(toSave); - toSave.time = 'latest'; - await this.eventsDao.update(toSave); + // If there is no "latest" event, then we need to create it...not update it + // This is probably an error getting here (like when the item was initially created, it didn't get the latest entry), + // but we should recover gracefully from it + if (existingEvent === undefined) { + await this.eventsDao.create(toUpdate); + } else { + await this.eventsDao.update(toUpdate); + } return event; } diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.updateComponentParent.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.updateComponentParent.ts index c32d3980a..bb49f503e 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.updateComponentParent.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.updateComponentParent.ts @@ -63,8 +63,11 @@ export class UpdateComponentParentAction implements EventAction { }; await this.eventsDao.create(toSave); - toSave.time = 'latest'; - await this.eventsDao.update(toSave); + const toUpdate: StateHistoryModel = { + ...toSave, + time: 'latest', + }; + await this.eventsDao.update(toUpdate); return event; } diff --git a/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts b/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts index 72e88c7a4..ba09eb29a 100644 --- a/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts +++ b/source/packages/services/assetlibraryhistory/src/events/events.service.spec.ts @@ -20,6 +20,8 @@ import { UpdateAction } from './actions/eventaction.update'; import { EventsDao } from './events.dao'; import { Category, EventModel, EventType } from './events.models'; import { EventsService } from './events.service'; +const TEST_USER_STATE = + '{"groups":{},"attributes":{"email":"test_email@test.com","lastUserCaptureTime":"1698712913","country":"US","language":"en-us","firstName":"Test","lastName":"Tester"},"name":"test_user_id","templateId":"user","parentPath":"/user","groupPath":"/user/test_user_id","category":"group"}'; describe('EventsService', () => { let instance: EventsService; @@ -43,7 +45,7 @@ describe('EventsService', () => { objectId: '/user/test_user_id', time: 'latest', event: 'modify', - state: '{"groups":{},"attributes":{"email":"test_email@test.com","lastUserCaptureTime":"1698712913","country":"US","language":"en-us","firstName":"Test","lastName":"Tester"},"name":"test_user_id","templateId":"user","parentPath":"/user","groupPath":"/user/test_user_id","category":"group"}', + state: TEST_USER_STATE, type: 'groups', }); } else if (objectId === 'test_device_id') { @@ -51,7 +53,7 @@ describe('EventsService', () => { objectId: 'test_device_id', time: 'latest', event: 'modify', - state: '{"attributes":{},"groups":{"out":{"manufactured_by":["/supplier/supplier1"],"has_firmware":["/firmware/firmwareId1"],"is_model":["/device/robot/testDevice"]}},"devices":{},"templateId":"testDevice","deviceId":"test_device_id","state":"unprovisioned","category":"device","connected":false}', + state: '{"attributes":{},"groups":{"out":{"manufactured_by":["/supplier/supplier1"],"has_firmware":["/firmware/firmwareId1"],"is_model":["/device/robot/testDevice"]}},"devices":{},"templateId":"testDevice","deviceId":"test_device_id","state":"unprovisioned","category":"device","connected":true}', type: 'devices', }); } else if ((objectId = 'test_undefined_object')) { @@ -93,7 +95,26 @@ describe('EventsService', () => { await instance.create(testEvent); expect(mockedEventDao.create).toBeCalledTimes(1); + // Calling `create` will add the `out: {}` to the user state in dynamo + let userState = JSON.parse(TEST_USER_STATE); + userState.groups = { out: {} }; + expect(mockedEventDao.create).toHaveBeenCalledWith({ + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: JSON.stringify(userState), + time: testEvent.time, + user: undefined, + }); expect(mockedEventDao.update).toBeCalledTimes(1); + expect(mockedEventDao.update).toHaveBeenCalledWith({ + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: JSON.stringify(userState), + time: 'latest', + user: undefined, + }); }); it('happy path update a pre-existing device', async () => { @@ -109,7 +130,23 @@ describe('EventsService', () => { await instance.create(testEvent); expect(mockedEventDao.create).toBeCalledTimes(1); + expect(mockedEventDao.create).toHaveBeenCalledWith({ + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: testEvent.time, + user: undefined, + }); expect(mockedEventDao.update).toBeCalledTimes(1); + expect(mockedEventDao.update).toHaveBeenCalledWith({ + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: 'latest', + user: undefined, + }); }); it('happy path create a new device', async () => { @@ -125,6 +162,22 @@ describe('EventsService', () => { await instance.create(testEvent); expect(mockedEventDao.create).toBeCalledTimes(2); + expect(mockedEventDao.create).toHaveBeenNthCalledWith(1, { + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: testEvent.time, + user: undefined, + }); + expect(mockedEventDao.create).toHaveBeenNthCalledWith(2, { + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: 'latest', + user: undefined, + }); expect(mockedEventDao.update).toBeCalledTimes(0); }); @@ -140,7 +193,23 @@ describe('EventsService', () => { }; await instance.create(testEvent); - expect(mockedEventDao.create).toBeCalledTimes(1); - expect(mockedEventDao.update).toBeCalledTimes(1); + expect(mockedEventDao.create).toBeCalledTimes(2); + expect(mockedEventDao.create).toHaveBeenNthCalledWith(1, { + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: testEvent.time, + user: undefined, + }); + expect(mockedEventDao.create).toHaveBeenNthCalledWith(2, { + objectId: testEvent.objectId, + type: testEvent.type, + event: testEvent.event, + state: testEvent.payload, + time: 'latest', + user: undefined, + }); + expect(mockedEventDao.update).toBeCalledTimes(0); }); }); From 9951ff0141bb756a9581a08b0fd157480843618d Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 11:48:16 -0500 Subject: [PATCH 5/6] chore: changelogs --- ...ix_assetlibraryhistory_errors_2024-02-26-16-48.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 source/common/changes/@awssolutions/cdf-assetlibrary-history/fix_assetlibraryhistory_errors_2024-02-26-16-48.json diff --git a/source/common/changes/@awssolutions/cdf-assetlibrary-history/fix_assetlibraryhistory_errors_2024-02-26-16-48.json b/source/common/changes/@awssolutions/cdf-assetlibrary-history/fix_assetlibraryhistory_errors_2024-02-26-16-48.json new file mode 100644 index 000000000..ac9f58506 --- /dev/null +++ b/source/common/changes/@awssolutions/cdf-assetlibrary-history/fix_assetlibraryhistory_errors_2024-02-26-16-48.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@awssolutions/cdf-assetlibrary-history", + "comment": "fixed some errors with undefined data and added tests", + "type": "none" + } + ], + "packageName": "@awssolutions/cdf-assetlibrary-history" +} \ No newline at end of file From 0c2c3e178f156303053fb5040d21396d9cc57379 Mon Sep 17 00:00:00 2001 From: Aaron Pittenger Date: Mon, 26 Feb 2024 16:50:27 -0500 Subject: [PATCH 6/6] fix(assetlibraryhistory): fixed group -> devices --- .../src/events/actions/eventaction.update.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts index 4380e6758..ce16fcb0e 100644 --- a/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts +++ b/source/packages/services/assetlibraryhistory/src/events/actions/eventaction.update.ts @@ -91,7 +91,7 @@ export class UpdateAction implements EventAction { ]?.filter((value: string) => { return value !== event.attributes['detachedFromDevice']; }); - mergedState['groups']['out'][event.attributes['relationship']] = + mergedState['devices']['out'][event.attributes['relationship']] = newRelationship; } }