From 3a838a5bd0a6bb87f46ceb988a3648478b8cf7c3 Mon Sep 17 00:00:00 2001 From: Laura Bergoens Date: Wed, 27 Nov 2024 22:26:27 +0100 Subject: [PATCH 1/2] move patch cache entry usecase to the right bounded context --- .../application/learning-content-controller.js | 3 +-- .../domain/usecases/patch-learning-content-cache-entry.js | 0 .../unit/application/learning-content-controller_test.js | 7 +++---- .../usecases/patch-learning-content-cache-entry_test.js | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) rename api/src/{shared => learning-content}/domain/usecases/patch-learning-content-cache-entry.js (100%) rename api/tests/{shared => learning-content}/unit/domain/usecases/patch-learning-content-cache-entry_test.js (93%) diff --git a/api/src/learning-content/application/learning-content-controller.js b/api/src/learning-content/application/learning-content-controller.js index f408549029e..a4f098464c7 100644 --- a/api/src/learning-content/application/learning-content-controller.js +++ b/api/src/learning-content/application/learning-content-controller.js @@ -1,4 +1,3 @@ -import { sharedUsecases } from '../../shared/domain/usecases/index.js'; import { usecases } from '../domain/usecases/index.js'; const createRelease = async function (request, h) { @@ -17,7 +16,7 @@ const patchCacheEntry = async function (request, h) { const updatedRecord = request.payload; const recordId = request.params.id; const modelName = request.params.model; - await sharedUsecases.patchLearningContentCacheEntry({ recordId, updatedRecord, modelName }); + await usecases.patchLearningContentCacheEntry({ recordId, updatedRecord, modelName }); return h.response().code(204); }; diff --git a/api/src/shared/domain/usecases/patch-learning-content-cache-entry.js b/api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js similarity index 100% rename from api/src/shared/domain/usecases/patch-learning-content-cache-entry.js rename to api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js diff --git a/api/tests/learning-content/unit/application/learning-content-controller_test.js b/api/tests/learning-content/unit/application/learning-content-controller_test.js index 7f527c64f91..e95f0796ac8 100644 --- a/api/tests/learning-content/unit/application/learning-content-controller_test.js +++ b/api/tests/learning-content/unit/application/learning-content-controller_test.js @@ -1,9 +1,8 @@ import { learningContentController } from '../../../../src/learning-content/application/learning-content-controller.js'; import { usecases } from '../../../../src/learning-content/domain/usecases/index.js'; -import { sharedUsecases } from '../../../../src/shared/domain/usecases/index.js'; import { expect, hFake, sinon } from '../../../test-helper.js'; -describe('Unit | Controller | learning-content-controller', function () { +describe('Learning Content | Unit | Controller | learning-content-controller', function () { describe('#createRelease', function () { it('should schedule createRelease job', async function () { // given @@ -40,13 +39,13 @@ describe('Unit | Controller | learning-content-controller', function () { it('should call the usecase and return 204', async function () { // given - sinon.stub(sharedUsecases, 'patchLearningContentCacheEntry'); + sinon.stub(usecases, 'patchLearningContentCacheEntry'); // when const response = await learningContentController.patchCacheEntry(request, hFake); // then - expect(sharedUsecases.patchLearningContentCacheEntry).to.have.been.calledWithExactly({ + expect(usecases.patchLearningContentCacheEntry).to.have.been.calledWithExactly({ recordId: 'recId', updatedRecord: { property: 'updatedValue', diff --git a/api/tests/shared/unit/domain/usecases/patch-learning-content-cache-entry_test.js b/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js similarity index 93% rename from api/tests/shared/unit/domain/usecases/patch-learning-content-cache-entry_test.js rename to api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js index 9608864695e..5e53f7c971c 100644 --- a/api/tests/shared/unit/domain/usecases/patch-learning-content-cache-entry_test.js +++ b/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js @@ -1,8 +1,8 @@ -import { patchLearningContentCacheEntry } from '../../../../../src/shared/domain/usecases/patch-learning-content-cache-entry.js'; +import { patchLearningContentCacheEntry } from '../../../../../src/learning-content/domain/usecases/patch-learning-content-cache-entry.js'; import * as LearningContentDatasources from '../../../../../src/shared/infrastructure/datasources/learning-content/index.js'; import { expect, sinon } from '../../../../test-helper.js'; -describe('Unit | Domain | Usecase | Patch learning content cache entry', function () { +describe('Learning Content | Unit | Domain | Usecase | Patch learning content cache entry', function () { describe('#patchLearningContentCacheEntry', function () { context('when entry is already in cache', function () { it('should patch learning content cache with provided updated entry', async function () { From 227e80e80854353e49a5051b69ddf83ec6fb2fbd Mon Sep 17 00:00:00 2001 From: Laura Bergoens Date: Wed, 27 Nov 2024 23:05:11 +0100 Subject: [PATCH 2/2] patch DB when patching new entry in cache --- .../create-learning-content-release.js | 2 +- .../patch-learning-content-cache-entry.js | 78 +++++++++- .../refresh-learning-content-cache.js | 2 +- .../application/lcms-controller_test.js | 58 +++++++- ...patch-learning-content-cache-entry_test.js | 137 +++++++++++++++++- 5 files changed, 269 insertions(+), 8 deletions(-) diff --git a/api/src/learning-content/domain/usecases/create-learning-content-release.js b/api/src/learning-content/domain/usecases/create-learning-content-release.js index 1cefe737e77..11ea87a7fcc 100644 --- a/api/src/learning-content/domain/usecases/create-learning-content-release.js +++ b/api/src/learning-content/domain/usecases/create-learning-content-release.js @@ -9,8 +9,8 @@ export const createLearningContentRelease = withTransaction( competenceRepository, thematicRepository, tubeRepository, - challengeRepository, skillRepository, + challengeRepository, courseRepository, tutorialRepository, missionRepository, diff --git a/api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js b/api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js index 51504f4f02e..483f73cd03f 100644 --- a/api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js +++ b/api/src/learning-content/domain/usecases/patch-learning-content-cache-entry.js @@ -1,7 +1,37 @@ -export async function patchLearningContentCacheEntry({ recordId, updatedRecord, modelName, LearningContentCache }) { +/** @param {import('./dependencies.js').Dependencies} */ +export async function patchLearningContentCacheEntry({ + recordId, + updatedRecord, + modelName, + LearningContentCache, + frameworkRepository, + areaRepository, + competenceRepository, + thematicRepository, + tubeRepository, + skillRepository, + challengeRepository, + courseRepository, + tutorialRepository, + missionRepository, +}) { const currentLearningContent = await LearningContentCache.instance.get(); const patch = generatePatch(currentLearningContent, recordId, updatedRecord, modelName); await LearningContentCache.instance.patch(patch); + await patchDatabase( + modelName, + updatedRecord, + frameworkRepository, + areaRepository, + competenceRepository, + thematicRepository, + tubeRepository, + skillRepository, + challengeRepository, + courseRepository, + tutorialRepository, + missionRepository, + ); } function generatePatch(currentLearningContent, id, newEntry, modelName) { @@ -19,3 +49,49 @@ function generatePatch(currentLearningContent, id, newEntry, modelName) { value: newEntry, }; } + +async function patchDatabase( + modelName, + patchedRecord, + frameworkRepository, + areaRepository, + competenceRepository, + thematicRepository, + tubeRepository, + skillRepository, + challengeRepository, + courseRepository, + tutorialRepository, + missionRepository, +) { + if (modelName === 'frameworks') { + await frameworkRepository.save([patchedRecord]); + } + if (modelName === 'areas') { + await areaRepository.save([patchedRecord]); + } + if (modelName === 'competences') { + await competenceRepository.save([patchedRecord]); + } + if (modelName === 'thematics') { + await thematicRepository.save([patchedRecord]); + } + if (modelName === 'tubes') { + await tubeRepository.save([patchedRecord]); + } + if (modelName === 'skills') { + await skillRepository.save([patchedRecord]); + } + if (modelName === 'challenges') { + await challengeRepository.save([patchedRecord]); + } + if (modelName === 'courses') { + await courseRepository.save([patchedRecord]); + } + if (modelName === 'tutorials') { + await tutorialRepository.save([patchedRecord]); + } + if (modelName === 'missions') { + await missionRepository.save([patchedRecord]); + } +} diff --git a/api/src/learning-content/domain/usecases/refresh-learning-content-cache.js b/api/src/learning-content/domain/usecases/refresh-learning-content-cache.js index 6f05ef1e368..2c45df18508 100644 --- a/api/src/learning-content/domain/usecases/refresh-learning-content-cache.js +++ b/api/src/learning-content/domain/usecases/refresh-learning-content-cache.js @@ -9,8 +9,8 @@ export const refreshLearningContentCache = withTransaction( competenceRepository, thematicRepository, tubeRepository, - challengeRepository, skillRepository, + challengeRepository, courseRepository, tutorialRepository, missionRepository, diff --git a/api/tests/learning-content/acceptance/application/lcms-controller_test.js b/api/tests/learning-content/acceptance/application/lcms-controller_test.js index a1a7cc8abbd..7ed40f07489 100644 --- a/api/tests/learning-content/acceptance/application/lcms-controller_test.js +++ b/api/tests/learning-content/acceptance/application/lcms-controller_test.js @@ -7,6 +7,7 @@ import { databaseBuilder, expect, generateValidRequestAuthorizationHeader, + knex, mockLearningContent, } from '../../../test-helper.js'; @@ -67,16 +68,21 @@ describe('Acceptance | Controller | lcms-controller', function () { LearningContentCache.instance = null; }); - it('should store patches in Redis', async function () { + it('should store patches in Redis and patch the DB for an assign operation', async function () { // given - await mockLearningContent({ frameworks: [{ id: `frameworkId` }] }); + await mockLearningContent({ + frameworks: [ + { id: 'frameworkId', name: 'old name' }, + { id: 'frameworkId_other', name: 'other name' }, + ], + }); const superAdminUserId = databaseBuilder.factory.buildUser.withRole({ role: ROLES.SUPER_ADMIN, }).id; await databaseBuilder.commit(); const payload = { - id: `frameworkId`, - param: `updated framework`, + id: 'frameworkId', + name: 'new name', }; // when @@ -93,6 +99,50 @@ describe('Acceptance | Controller | lcms-controller', function () { expect(await redis.lrange('cache:LearningContent:patches', 0, -1)).to.deep.equal([ JSON.stringify({ operation: 'assign', path: `frameworks[0]`, value: payload }), ]); + const frameworksInDB = await knex.select('*').from('learningcontent.frameworks').orderBy('name'); + expect(frameworksInDB).to.deep.equal([ + { id: 'frameworkId', name: 'new name' }, + { id: 'frameworkId_other', name: 'other name' }, + ]); + }); + + it('should store patches in Redis and patch the DB for a push operation', async function () { + // given + await mockLearningContent({ + frameworks: [ + { id: 'frameworkId1', name: 'name 1' }, + { id: 'frameworkId3', name: 'name 3' }, + ], + }); + const superAdminUserId = databaseBuilder.factory.buildUser.withRole({ + role: ROLES.SUPER_ADMIN, + }).id; + await databaseBuilder.commit(); + const payload = { + id: 'frameworkId2', + name: 'name 2', + }; + + // when + const response = await server.inject({ + method: 'PATCH', + url: `/api/cache/frameworks/frameworkId2`, + headers: { authorization: generateValidRequestAuthorizationHeader(superAdminUserId) }, + payload, + }); + + // then + expect(response.statusCode).to.equal(204); + const redis = new Redis(process.env.TEST_REDIS_URL); + expect(await redis.lrange('cache:LearningContent:patches', 0, -1)).to.deep.equal([ + JSON.stringify({ operation: 'push', path: `frameworks`, value: payload }), + ]); + const frameworksInDB = await knex.select('*').from('learningcontent.frameworks').orderBy('name'); + expect(frameworksInDB).to.deep.equal([ + { id: 'frameworkId1', name: 'name 1' }, + { id: 'frameworkId2', name: 'name 2' }, + { id: 'frameworkId3', name: 'name 3' }, + ]); }); }); }); diff --git a/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js b/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js index 5e53f7c971c..7da6b811496 100644 --- a/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js +++ b/api/tests/learning-content/unit/domain/usecases/patch-learning-content-cache-entry_test.js @@ -3,6 +3,86 @@ import * as LearningContentDatasources from '../../../../../src/shared/infrastru import { expect, sinon } from '../../../../test-helper.js'; describe('Learning Content | Unit | Domain | Usecase | Patch learning content cache entry', function () { + let frameworkRepository, + areaRepository, + competenceRepository, + thematicRepository, + tubeRepository, + skillRepository, + challengeRepository, + courseRepository, + tutorialRepository, + missionRepository; + let repositories; + let repositoriesByModel; + + beforeEach(function () { + frameworkRepository = { + save: sinon.stub(), + }; + frameworkRepository.save.rejects('I should not be called'); + areaRepository = { + save: sinon.stub(), + }; + areaRepository.save.rejects('I should not be called'); + competenceRepository = { + save: sinon.stub(), + }; + competenceRepository.save.rejects('I should not be called'); + thematicRepository = { + save: sinon.stub(), + }; + thematicRepository.save.rejects('I should not be called'); + tubeRepository = { + save: sinon.stub(), + }; + tubeRepository.save.rejects('I should not be called'); + skillRepository = { + save: sinon.stub(), + }; + skillRepository.save.rejects('I should not be called'); + challengeRepository = { + save: sinon.stub(), + }; + challengeRepository.save.rejects('I should not be called'); + courseRepository = { + save: sinon.stub(), + }; + courseRepository.save.rejects('I should not be called'); + tutorialRepository = { + save: sinon.stub(), + }; + tutorialRepository.save.rejects('I should not be called'); + missionRepository = { + save: sinon.stub(), + }; + missionRepository.save.rejects('I should not be called'); + repositories = { + frameworkRepository, + areaRepository, + competenceRepository, + thematicRepository, + tubeRepository, + skillRepository, + challengeRepository, + courseRepository, + tutorialRepository, + missionRepository, + }; + repositoriesByModel = { + frameworks: frameworkRepository, + areas: areaRepository, + competences: competenceRepository, + thematics: thematicRepository, + tubes: tubeRepository, + skills: skillRepository, + challenges: challengeRepository, + courses: courseRepository, + tutorials: tutorialRepository, + missions: missionRepository, + }; + }); + describe('#patchLearningContentCacheEntry', function () { context('when entry is already in cache', function () { it('should patch learning content cache with provided updated entry', async function () { @@ -32,6 +112,7 @@ describe('Learning Content | Unit | Domain | Usecase | Patch learning content ca modelName, LearningContentCache, LearningContentDatasources, + ...repositories, }); // then @@ -64,7 +145,13 @@ describe('Learning Content | Unit | Domain | Usecase | Patch learning content ca LearningContentCache.instance.get.resolves(learningContent); // when - await patchLearningContentCacheEntry({ recordId, updatedRecord, modelName, LearningContentCache }); + await patchLearningContentCacheEntry({ + recordId, + updatedRecord, + modelName, + LearningContentCache, + ...repositories, + }); // then expect(LearningContentCache.instance.patch).to.have.been.calledWithExactly({ @@ -74,5 +161,53 @@ describe('Learning Content | Unit | Domain | Usecase | Patch learning content ca }); }); }); + + // eslint-disable-next-line mocha/no-setup-in-describe + [ + 'frameworks', + 'areas', + 'competences', + 'thematics', + 'tubes', + 'skills', + 'challenges', + 'courses', + 'tutorials', + 'missions', + ].forEach((modelName) => { + it(`should call save on appropriate repository for model ${modelName}`, async function () { + // given + const recordId = 'recId'; + const updatedRecord = Symbol('updated record'); + const learningContent = { + [modelName]: [ + { attr1: 'attr1 value index 0', id: 'otherRecordId' }, + { attr1: 'attr1 value index 1', id: recordId }, + ], + someOtherModelName: [{ other: 'entry', id: recordId }], + }; + const LearningContentCache = { + instance: { + get: sinon.stub().resolves(learningContent), + patch: sinon.stub().resolves(), + }, + }; + repositoriesByModel[modelName].save.withArgs([updatedRecord]).resolves(); + + // when + await patchLearningContentCacheEntry({ + recordId, + updatedRecord, + modelName, + LearningContentCache, + LearningContentDatasources, + ...repositories, + }); + + // then + expect(repositoriesByModel[modelName].save).to.have.been.calledOnce; + expect(repositoriesByModel[modelName].save).to.have.been.calledWithExactly([updatedRecord]); + }); + }); }); });