From 6c78110f56d9ab5d8bc5ed562660bf8deec117c5 Mon Sep 17 00:00:00 2001 From: Guy Harwood Date: Thu, 5 Dec 2019 15:53:04 +0000 Subject: [PATCH] Feature/38405 - simplify SPA queue logic (#1433) * add dev arm templates for service bus * re-organise * deploy and add to key vault * remove obsolete key vault section * raw output * separate sb creation from key vault sync * add queue name to sas token * complete and working --- admin/services/check-start.service.js | 26 +++++-------------- admin/services/sas-token.service.js | 5 ++-- .../service/check-start.service.spec.js | 4 +-- .../service/sas-token.service.spec.js | 1 + .../app/services/azure-queue/queue-names.ts | 6 ----- .../check-complete.service.spec.ts | 2 +- .../check-complete/check-complete.service.ts | 10 ++----- .../check-start/check-start.service.ts | 4 +-- .../app/services/feedback/feedback.service.ts | 4 +-- .../pupil-prefs/pupil-prefs.service.spec.ts | 7 +++-- .../pupil-prefs/pupil-prefs.service.ts | 4 +-- .../{v3.ts => check-receiver.ts} | 4 +-- tslib/src/functions/check-receiver/index.ts | 6 ++--- 13 files changed, 27 insertions(+), 56 deletions(-) delete mode 100644 pupil-spa/src/app/services/azure-queue/queue-names.ts rename tslib/src/functions/check-receiver/{v3.ts => check-receiver.ts} (94%) diff --git a/admin/services/check-start.service.js b/admin/services/check-start.service.js index c7b5742936..25fb645216 100644 --- a/admin/services/check-start.service.js +++ b/admin/services/check-start.service.js @@ -353,18 +353,9 @@ checkStartService.createPupilCheckPayloads = async function (checkIds, schoolId) uuid: o.school_uuid }, tokens: { - checkStarted: { - token: checkStartedSasToken.token, - url: checkStartedSasToken.url - }, - pupilPreferences: { - token: pupilPreferencesSasToken.token, - url: pupilPreferencesSasToken.url - }, - pupilFeedback: { - token: pupilFeedbackSasToken.token, - url: pupilFeedbackSasToken.url - }, + checkStarted: checkStartedSasToken, + pupilPreferences: pupilPreferencesSasToken, + pupilFeedback: pupilFeedbackSasToken, jwt: { token: 'token-disabled' // o.pupil_jwtToken } @@ -375,13 +366,10 @@ checkStartService.createPupilCheckPayloads = async function (checkIds, schoolId) config: pupilConfig } if (o.check_isLiveCheck) { - message.tokens.checkComplete = { - token: checkCompleteSasToken.token, - url: checkCompleteSasToken.url - } - message.tokens.checkSubmit = { - token: checkSubmitSasToken.token, - url: checkCompleteSasToken.url + if (featureToggles.isFeatureEnabled('prepareChecksInRedis')) { + message.tokens.checkComplete = checkSubmitSasToken + } else { + message.tokens.checkComplete = checkCompleteSasToken } } messages.push(message) diff --git a/admin/services/sas-token.service.js b/admin/services/sas-token.service.js index 537bf68aa3..1926c5cebf 100644 --- a/admin/services/sas-token.service.js +++ b/admin/services/sas-token.service.js @@ -14,7 +14,7 @@ const sasTokenService = { * @param queueName * @param {Moment} expiryDate * @param {Object} serviceImplementation - * @return {{token: string, url: string}} + * @return {{token: string, url: string, queueName: string}} */ generateSasToken: function (queueName, expiryDate, serviceImplementation) { if (!serviceImplementation) { @@ -51,7 +51,8 @@ const sasTokenService = { return { token: sasToken, - url: serviceImplementation.getUrl(queueName) + url: serviceImplementation.getUrl(queueName), + queueName: queueName } } } diff --git a/admin/spec/back-end/service/check-start.service.spec.js b/admin/spec/back-end/service/check-start.service.spec.js index 5f2f2ede4e..1427dae93d 100644 --- a/admin/spec/back-end/service/check-start.service.spec.js +++ b/admin/spec/back-end/service/check-start.service.spec.js @@ -266,7 +266,8 @@ describe('check-start.service', () => { spyOn(sasTokenService, 'generateSasToken').and.callFake((s) => { return { token: ' { const res = await checkStartService.createPupilCheckPayloads([1], 1) expect(sasTokenService.generateSasToken).toHaveBeenCalledTimes(5) expect(Object.keys(res[0].tokens)).toContain('checkComplete') - expect(Object.keys(res[0].tokens)).toContain('checkSubmit') }) }) describe('when familiarisation checks are generated', () => { diff --git a/admin/spec/back-end/service/sas-token.service.spec.js b/admin/spec/back-end/service/sas-token.service.spec.js index cbb17a6d6f..6618bbbc8e 100644 --- a/admin/spec/back-end/service/sas-token.service.spec.js +++ b/admin/spec/back-end/service/sas-token.service.spec.js @@ -57,6 +57,7 @@ describe('sas-token.service', () => { expect(queueServiceMock.getUrl).toHaveBeenCalled() expect({}.hasOwnProperty.call(res, 'token')).toBe(true) expect({}.hasOwnProperty.call(res, 'url')).toBe(true) + expect({}.hasOwnProperty.call(res, 'queueName')).toBe(true) }) it('sets the permissions to add only', () => { diff --git a/pupil-spa/src/app/services/azure-queue/queue-names.ts b/pupil-spa/src/app/services/azure-queue/queue-names.ts deleted file mode 100644 index 18ce904ac0..0000000000 --- a/pupil-spa/src/app/services/azure-queue/queue-names.ts +++ /dev/null @@ -1,6 +0,0 @@ -export const queueNames = { - checkStarted: 'check-started', - checkComplete: 'check-complete', - pupilFeedback: 'pupil-feedback', - pupilPreferences: 'pupil-prefs' -}; diff --git a/pupil-spa/src/app/services/check-complete/check-complete.service.spec.ts b/pupil-spa/src/app/services/check-complete/check-complete.service.spec.ts index 13db4884ea..86073fb811 100644 --- a/pupil-spa/src/app/services/check-complete/check-complete.service.spec.ts +++ b/pupil-spa/src/app/services/check-complete/check-complete.service.spec.ts @@ -4,7 +4,7 @@ import { Router } from '@angular/router'; import { AuditService } from '../audit/audit.service'; import { AzureQueueService } from '../azure-queue/azure-queue.service'; import { CheckCompleteService } from './check-complete.service'; -import { AppConfigService, loadConfigMockService, IAppConfig } from '../config/config.service'; +import { AppConfigService, loadConfigMockService } from '../config/config.service'; import { StorageService } from '../storage/storage.service'; import { TestBed } from '@angular/core/testing'; import { TokenService } from '../token/token.service'; diff --git a/pupil-spa/src/app/services/check-complete/check-complete.service.ts b/pupil-spa/src/app/services/check-complete/check-complete.service.ts index a733f79862..6ac017fe58 100644 --- a/pupil-spa/src/app/services/check-complete/check-complete.service.ts +++ b/pupil-spa/src/app/services/check-complete/check-complete.service.ts @@ -10,7 +10,6 @@ import { Injectable } from '@angular/core'; import { Router } from '@angular/router'; import { StorageService } from '../storage/storage.service'; import { TokenService } from '../token/token.service'; -import { queueNames } from '../azure-queue/queue-names'; import { AppUsageService } from '../app-usage/app-usage.service'; import { CompressorService } from '../compressor/compressor.service'; @@ -23,7 +22,6 @@ export class CheckCompleteService { checkSubmissionApiErrorDelay; checkSubmissionAPIErrorMaxAttempts; submissionPendingViewMinDisplay; - submitsToCheckReceiver: boolean; constructor(private auditService: AuditService, private azureQueueService: AzureQueueService, @@ -34,13 +32,11 @@ export class CheckCompleteService { const { checkSubmissionApiErrorDelay, checkSubmissionAPIErrorMaxAttempts, - submissionPendingViewMinDisplay, - submitsToCheckReceiver + submissionPendingViewMinDisplay } = APP_CONFIG; this.checkSubmissionApiErrorDelay = checkSubmissionApiErrorDelay; this.checkSubmissionAPIErrorMaxAttempts = checkSubmissionAPIErrorMaxAttempts; this.submissionPendingViewMinDisplay = submissionPendingViewMinDisplay; - this.submitsToCheckReceiver = submitsToCheckReceiver; } /** @@ -64,9 +60,7 @@ export class CheckCompleteService { if (config.practice) { return this.onSuccess(startTime); } - const queueName = queueNames.checkComplete; - const {url, token} = this.submitsToCheckReceiver ? this.tokenService.getToken('checkSubmit') - : this.tokenService.getToken('checkComplete'); + const {url, token, queueName} = this.tokenService.getToken('checkComplete'); const retryConfig = { errorDelay: this.checkSubmissionApiErrorDelay, errorMaxAttempts: this.checkSubmissionAPIErrorMaxAttempts diff --git a/pupil-spa/src/app/services/check-start/check-start.service.ts b/pupil-spa/src/app/services/check-start/check-start.service.ts index c1b3fb89f9..c0ea4dfd77 100644 --- a/pupil-spa/src/app/services/check-start/check-start.service.ts +++ b/pupil-spa/src/app/services/check-start/check-start.service.ts @@ -9,7 +9,6 @@ import { AzureQueueService } from '../azure-queue/azure-queue.service'; import { AuditService } from '../audit/audit.service'; import { StorageService } from '../storage/storage.service'; import { TokenService } from '../token/token.service'; -import { queueNames } from '../azure-queue/queue-names'; /** * Declaration of check start service @@ -37,8 +36,7 @@ export class CheckStartService { * @returns {Promise.} */ public async submit(): Promise { - const queueName = queueNames.checkStarted; - const { url, token } = this.tokenService.getToken('checkStarted'); + const { url, token, queueName } = this.tokenService.getToken('checkStarted'); // Create a model for the payload const payload = this.storageService.getItem('pupil'); payload.clientCheckStartedAt = new Date(); diff --git a/pupil-spa/src/app/services/feedback/feedback.service.ts b/pupil-spa/src/app/services/feedback/feedback.service.ts index 1b5033a0fd..2a98c68c90 100644 --- a/pupil-spa/src/app/services/feedback/feedback.service.ts +++ b/pupil-spa/src/app/services/feedback/feedback.service.ts @@ -3,7 +3,6 @@ import { APP_CONFIG } from '../config/config.service'; import { StorageService } from '../storage/storage.service'; import { TokenService } from '../token/token.service'; import { AzureQueueService } from '../azure-queue/azure-queue.service'; -import { queueNames } from '../azure-queue/queue-names'; @Injectable() export class FeedbackService { @@ -49,8 +48,7 @@ export class FeedbackService { * @returns {Promise.} */ async queueSubmit(payload) { - const queueName = queueNames.pupilFeedback; - const { url, token } = this.tokenService.getToken('pupilFeedback'); + const { url, token, queueName } = this.tokenService.getToken('pupilFeedback'); // Create a model for the payload const retryConfig = { errorDelay: this.feedbackAPIErrorDelay, diff --git a/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.spec.ts b/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.spec.ts index 70ef47c54a..fa8100c558 100644 --- a/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.spec.ts +++ b/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.spec.ts @@ -10,7 +10,6 @@ import { AuditService } from '../audit/audit.service'; import { QuestionService } from '../question/question.service'; import { QuestionServiceMock } from '../question/question.service.mock'; import { StorageServiceMock } from '../storage/storage.service.mock'; -import { queueNames } from '../azure-queue/queue-names'; import { AccessArrangements } from '../../access-arrangements'; let azureQueueService: AzureQueueService; @@ -59,7 +58,7 @@ describe('PupilPrefsService', () => { it('should call pupil prefs azure queue storage', async () => { const pupil = { checkCode: 'checkCode' }; spyOn(mockQuestionService, 'getConfig').and.returnValue({colourContrast: false}); - spyOn(tokenService, 'getToken').and.returnValue({url: 'url', token: 'token'}); + spyOn(tokenService, 'getToken').and.returnValue({url: 'url', token: 'token', queueName: 'the-queue'}); const addMessageSpy = spyOn(azureQueueService, 'addMessage'); const addEntrySpy = spyOn(auditService, 'addEntry'); spyOn(mockStorageService, 'setItem'); @@ -80,7 +79,7 @@ describe('PupilPrefsService', () => { errorDelay: pupilPrefsService.pupilPrefsAPIErrorDelay, errorMaxAttempts: pupilPrefsService.pupilPrefsAPIErrorMaxAttempts }; - expect(addMessageSpy.calls.all()[0].args[0]).toEqual(queueNames.pupilPreferences); + expect(addMessageSpy.calls.all()[0].args[0]).toEqual('the-queue'); expect(addMessageSpy.calls.all()[0].args[1]).toEqual('url'); expect(addMessageSpy.calls.all()[0].args[2]).toEqual('token'); expect(addMessageSpy.calls.all()[0].args[3]).toEqual(payload); @@ -89,7 +88,7 @@ describe('PupilPrefsService', () => { }); it('should audit log the error when azureQueueService add Message fails', async () => { spyOn(mockQuestionService, 'getConfig').and.returnValue({colourContrast: false}); - spyOn(tokenService, 'getToken').and.returnValue({url: 'url', token: 'token'}); + spyOn(tokenService, 'getToken').and.returnValue({url: 'url', token: 'token', queueName: 'the-queue'}); spyOn(azureQueueService, 'addMessage').and.returnValue(Promise.reject(new Error('error'))); const addEntrySpy = spyOn(auditService, 'addEntry'); spyOn(mockStorageService, 'getItem').and.returnValue(storedPrefs); diff --git a/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.ts b/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.ts index 188e67f267..08a8f07aa2 100644 --- a/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.ts +++ b/pupil-spa/src/app/services/pupil-prefs/pupil-prefs.service.ts @@ -3,7 +3,6 @@ import { APP_CONFIG } from '../config/config.service'; import { AzureQueueService } from '../azure-queue/azure-queue.service'; import { StorageService } from '../storage/storage.service'; import { TokenService } from '../token/token.service'; -import { queueNames } from '../azure-queue/queue-names'; import { accessArrangementsDataKey, AccessArrangementsConfig, AccessArrangements } from '../../access-arrangements'; import { Pupil } from '../../pupil'; import { AuditService } from '../audit/audit.service'; @@ -37,8 +36,7 @@ export class PupilPrefsService { const fontSetting = this.fontSettings.find(f => f.val === accessArrangements.fontSize); const contrastSetting = this.contrastSettings.find(f => f.val === accessArrangements.contrast); const pupil = this.storageService.getItem('pupil') as Pupil; - const queueName = queueNames.pupilPreferences; - const {url, token} = this.tokenService.getToken('pupilPreferences'); + const {url, token, queueName} = this.tokenService.getToken('pupilPreferences'); const retryConfig = { errorDelay: this.pupilPrefsAPIErrorDelay, errorMaxAttempts: this.pupilPrefsAPIErrorMaxAttempts diff --git a/tslib/src/functions/check-receiver/v3.ts b/tslib/src/functions/check-receiver/check-receiver.ts similarity index 94% rename from tslib/src/functions/check-receiver/v3.ts rename to tslib/src/functions/check-receiver/check-receiver.ts index 3d77208a9f..6654c244a2 100644 --- a/tslib/src/functions/check-receiver/v3.ts +++ b/tslib/src/functions/check-receiver/check-receiver.ts @@ -4,7 +4,7 @@ import Moment from 'moment' import * as az from '../../azure/storage-helper' const tableService = new az.AsyncTableService() -class V3 { +class CheckReceiver { async process (context: Context, receivedCheck: SubmittedCheckMessageV3) { const receivedCheckEntity: ReceivedCheck = { PartitionKey: receivedCheck.schoolUUID, @@ -24,4 +24,4 @@ class V3 { } } -export default new V3() +export default new CheckReceiver() diff --git a/tslib/src/functions/check-receiver/index.ts b/tslib/src/functions/check-receiver/index.ts index f104facdf3..945a72a70a 100644 --- a/tslib/src/functions/check-receiver/index.ts +++ b/tslib/src/functions/check-receiver/index.ts @@ -2,18 +2,18 @@ import { AzureFunction, Context } from '@azure/functions' import * as schemas from '../../schemas/models' import { performance } from 'perf_hooks' const functionName = 'check-receiver' -import V3 from './v3' +import CheckReceiver from './check-receiver' const queueTrigger: AzureFunction = async function (context: Context, submittedCheck: schemas.SubmittedCheckMessageV3): Promise { const start = performance.now() const version = submittedCheck.version context.log.info(`${functionName}: version:${version} message received for checkCode ${submittedCheck.checkCode}`) try { - if (version !== 3) { + if (version !== 2) { // dead letter the message as we no longer support below v3 throw new Error(`Message schema version:${version} unsupported`) } - await V3.process(context, submittedCheck) + await CheckReceiver.process(context, submittedCheck) } catch (error) { context.log.error(`${functionName}: ERROR: ${error.message}`) throw error