fix(scheduler): eliminate read/write concurrency hazzard while updating predicate status

packages/scheduler/src/Domain/UseCase/UpdatePredicateStatus/UpdatePredicateStatus.spec.ts

@@ -1,8 +1,7 @@
-import { PredicateAuthority, PredicateName, PredicateVerificationResult } from '@standardnotes/predicates'
 import 'reflect-metadata'
 
-import { JobDoneInterpreterInterface } from '../../Job/JobDoneInterpreterInterface'
+import { PredicateAuthority, PredicateName, PredicateVerificationResult } from '@standardnotes/predicates'
-import { JobRepositoryInterface } from '../../Job/JobRepositoryInterface'
+
 import { Predicate } from '../../Predicate/Predicate'
 import { PredicateRepositoryInterface } from '../../Predicate/PredicateRepositoryInterface'
 import { PredicateStatus } from '../../Predicate/PredicateStatus'
@@ -11,12 +10,10 @@ import { UpdatePredicateStatus } from './UpdatePredicateStatus'
 
 describe('UpdatePredicateStatus', () => {
   let predicateRepository: PredicateRepositoryInterface
-  let jobRepository: JobRepositoryInterface
-  let jobDoneInterpreter: JobDoneInterpreterInterface
   let predicateComplete: Predicate
   let predicateIncomplete: Predicate
 
-  const createUseCase = () => new UpdatePredicateStatus(predicateRepository, jobRepository, jobDoneInterpreter)
+  const createUseCase = () => new UpdatePredicateStatus(predicateRepository)
 
   beforeEach(() => {
     predicateComplete = {
@@ -33,15 +30,9 @@ describe('UpdatePredicateStatus', () => {
     predicateRepository = {} as jest.Mocked<PredicateRepositoryInterface>
     predicateRepository.findByJobUuid = jest.fn().mockReturnValue([predicateComplete, predicateIncomplete])
     predicateRepository.save = jest.fn()
-
-    jobRepository = {} as jest.Mocked<JobRepositoryInterface>
-    jobRepository.markJobAsDone = jest.fn()
-
-    jobDoneInterpreter = {} as jest.Mocked<JobDoneInterpreterInterface>
-    jobDoneInterpreter.interpret = jest.fn()
   })
 
-  it('should mark a predicate as complete and update job as done', async () => {
+  it('should mark a predicate as complete', async () => {
     expect(
       await createUseCase().execute({
         predicate: { name: PredicateName.EmailBackupsEnabled, jobUuid: '1-2-3', authority: PredicateAuthority.Auth },
@@ -49,7 +40,6 @@ describe('UpdatePredicateStatus', () => {
       }),
     ).toEqual({
       success: true,
-      allPredicatesChecked: true,
     })
 
     expect(predicateRepository.save).toHaveBeenCalledWith({
@@ -57,37 +47,5 @@ describe('UpdatePredicateStatus', () => {
       name: 'email-backups-enabled',
       status: 'denied',
     })
-
-    expect(jobRepository.markJobAsDone).toHaveBeenCalled()
-    expect(jobDoneInterpreter.interpret).toHaveBeenCalled()
-  })
-
-  it('should mark a predicate as complete and not update job as done if there are still incomplete predicates', async () => {
-    predicateRepository.findByJobUuid = jest
-      .fn()
-      .mockReturnValue([
-        predicateComplete,
-        predicateIncomplete,
-        { uuid: '3-4-5', status: PredicateStatus.Pending } as jest.Mocked<Predicate>,
-      ])
-
-    expect(
-      await createUseCase().execute({
-        predicate: { name: PredicateName.EmailBackupsEnabled, jobUuid: '1-2-3', authority: PredicateAuthority.Auth },
-        predicateVerificationResult: PredicateVerificationResult.Denied,
-      }),
-    ).toEqual({
-      success: true,
-      allPredicatesChecked: false,
-    })
-
-    expect(predicateRepository.save).toHaveBeenCalledWith({
-      uuid: '2-3-4',
-      name: 'email-backups-enabled',
-      status: 'denied',
-    })
-
-    expect(jobRepository.markJobAsDone).not.toHaveBeenCalled()
-    expect(jobDoneInterpreter.interpret).not.toHaveBeenCalled()
   })
 })

packages/scheduler/src/Domain/UseCase/UpdatePredicateStatus/UpdatePredicateStatus.ts

@@ -1,8 +1,6 @@
 import { inject, injectable } from 'inversify'
 
 import TYPES from '../../../Bootstrap/Types'
-import { JobDoneInterpreterInterface } from '../../Job/JobDoneInterpreterInterface'
-import { JobRepositoryInterface } from '../../Job/JobRepositoryInterface'
 import { PredicateRepositoryInterface } from '../../Predicate/PredicateRepositoryInterface'
 import { PredicateStatus } from '../../Predicate/PredicateStatus'
 import { UseCaseInterface } from '../UseCaseInterface'
@@ -12,35 +10,20 @@ import { UpdatePredicateStatusResponse } from './UpdatePredicateStatusResponse'
 
 @injectable()
 export class UpdatePredicateStatus implements UseCaseInterface {
-  constructor(
+  constructor(@inject(TYPES.PredicateRepository) private predicateRepository: PredicateRepositoryInterface) {}
-    @inject(TYPES.PredicateRepository) private predicateRepository: PredicateRepositoryInterface,
-    @inject(TYPES.JobRepository) private jobRepository: JobRepositoryInterface,
-    @inject(TYPES.JobDoneInterpreter) private jobDoneInterpreter: JobDoneInterpreterInterface,
-  ) {}
 
   async execute(dto: UpdatePredicateStatusDTO): Promise<UpdatePredicateStatusResponse> {
     const predicates = await this.predicateRepository.findByJobUuid(dto.predicate.jobUuid)
 
-    let allPredicatesChecked = true
     for (const predicate of predicates) {
       if (predicate.name === dto.predicate.name) {
         predicate.status = dto.predicateVerificationResult as unknown as PredicateStatus
         await this.predicateRepository.save(predicate)
       }
-      if (predicate.status === PredicateStatus.Pending) {
-        allPredicatesChecked = false
-      }
-    }
-
-    if (allPredicatesChecked) {
-      await this.jobDoneInterpreter.interpret(dto.predicate.jobUuid)
-
-      await this.jobRepository.markJobAsDone(dto.predicate.jobUuid)
     }
 
     return {
       success: true,
-      allPredicatesChecked,
     }
   }
 }

packages/scheduler/src/Domain/UseCase/UpdatePredicateStatus/UpdatePredicateStatusResponse.ts

@@ -1,6 +1,3 @@
-export type UpdatePredicateStatusResponse =
+export type UpdatePredicateStatusResponse = {
-  | {
+  success: boolean
-      success: true
+}
-      allPredicatesChecked: boolean
-    }
-  | { success: false }