diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e7e6028e17a..2746f586981 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -42,9 +42,35 @@ This repository contains three interconnected applications: - Write unit tests for new components and services - Follow existing patterns for mocking dependencies -- Use TestEnvironment pattern from existing tests. Use the TestEnvironment class pattern rather than using a `beforeEach`. +- Use TestEnvironment pattern from existing tests. Use the TestEnvironment class pattern rather than using a `beforeEach`. Do not put helper functions outside of TestEnvironment classes; helper functions or setup functions should be in the TestEnvironment class. - Test both online and offline scenarios - Test permission boundaries +- When a TestEnvironment class is being given many optional arguments, prefer using object destructuring with default values, and the argument type definition specified inline rather than as an interface. For example, + ```typescript + class TestEnvironment { + constructor({ + someRequired, + someOptional = 'abc', + }: { + someRequired: string; + someOptional?: string; + }) { + ... + } + } + ``` + Example using `= {}` when all items are optional: + ```typescript + class TestEnvironment { + constructor({ + someOptional = 'abc', + }: { + someOptional?: string; + } = {}) { + ... + } + } + ``` # Code comments @@ -53,14 +79,20 @@ This repository contains three interconnected applications: - Do not add method comments unless the method would be unclear to an experienced developer. - Do put comments into the code to make it more clear what is going on if it would not be obvious to an experienced developer. - Do put comments into the code if the intent is not clear from the code. -- All classes should have a comment to briefly explain why it is there and what its purpose is in the overall system, even if it seems obvious. -- Please do not fail to add a comment to any classes that are created. All classes should have a comment. +- All classes and interfaces should have a comment to briefly explain why it is there and what its purpose is in the overall system, even if it seems obvious. +- Please do not fail to add a comment to any classes or interfaces that are created. All classes and interfaces should have a comment. # TypeScript language - Use explicit true/false/null/undefined rather than truthy/falsy - Never rely on JavaScript's truthy or falsy. Instead, work with actual true, false, null, and undefined values, rather than relying on their interpretation as truthy or falsy. For example, if `count` might be null, or undefined, or zero, don't write code like `if (count)` or `const foo:string = someVariable ? 'a' : 'b'`. Instead, inspect for the null, undefined, or zero rather than letting the value be interpreted as truthy for falsy. For example, use `if (count == null)` or `const foo:string = someVariable != null 'a' : 'b'` or `if (count > 0)`. -- Specify types where not obvious, such as when declaring variables and arguments, and for function return types. +- Specify types when declaring variables, arguments, and for function return types. For example, don't write + `const projectId = this.determineProjectId();` or + `const buildEvents = eventsSorted.filter(...);` or + `const buildEvent = buildEvents[0];`. Instead, write + `const projectId: string | undefined = this.determineProjectId();` and + `const buildEvents: EventMetric[] = eventsSorted.filter(...);` and + `const buildEvent: EventMetric | undefined = buildEvents[0];`. - Use `@if {}` syntax rather than `*ngIf` syntax. - Although interacting with existing code and APIs may necessitate the use of `null`, when writing new code, prefer using `undefined` rather than `null`. - Fields that are of type Subject or BehaviorSubject should have names that end with a `$`. @@ -72,6 +104,7 @@ This repository contains three interconnected applications: - It is better to write code that is verbose and understandable than terse and concise. - It is better to explicitly check for and handle problems, or prevent problems from happening, than to assume problems will not happen. - Corner-cases happen. They should be handled in code. +- Please don't change existing code without good justification. Existing code largely works and changing it will cause work for code review. Leave existing code as is when possible. # Running commands diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.ts index 2cce940907b..a156524e1fd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/app.component.ts @@ -1,8 +1,8 @@ import { BidiModule } from '@angular/cdk/bidi'; import { BreakpointObserver, BreakpointState } from '@angular/cdk/layout'; import { CdkScrollable } from '@angular/cdk/scrolling'; -import { AsyncPipe } from '@angular/common'; -import { Component, DestroyRef, DOCUMENT, HostBinding, Inject, OnDestroy, OnInit } from '@angular/core'; +import { AsyncPipe, DOCUMENT } from '@angular/common'; +import { Component, DestroyRef, HostBinding, Inject, OnDestroy, OnInit } from '@angular/core'; import { MatButton, MatIconAnchor, MatIconButton } from '@angular/material/button'; import { MatDivider } from '@angular/material/divider'; import { MatIcon } from '@angular/material/icon'; diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric.ts index 72f2cb4b3e6..17a473b9a07 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric.ts @@ -9,6 +9,7 @@ export interface EventMetric { timeStamp: string; userId?: string; executionTime?: string; + tags?: { [key: string]: any | undefined }; } export enum EventScope { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.html index 1a6d55a30a7..97cdb720851 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.html @@ -26,6 +26,18 @@ } } +
+
+ Determination + +
+ + Request ID + Timing + +
@if (!isLoading) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.scss index 2d411d6208a..f6345738fac 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.scss @@ -130,6 +130,22 @@ font-weight: 500; } } + + .grouping-toggle { + margin-bottom: 1rem; + align-self: flex-start; + display: flex; + flex-direction: column; + align-items: center; + + > .grouping-toggle-label { + display: flex; + margin-bottom: 0.5rem; + app-info { + margin-left: 0.25rem; + } + } + } } .project-link { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.spec.ts index 645ab4635c5..46e35547b9b 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.spec.ts @@ -7,7 +7,7 @@ import { By } from '@angular/platform-browser'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { ActivatedRoute, provideRouter } from '@angular/router'; import { BehaviorSubject } from 'rxjs'; -import { anything, mock, when } from 'ts-mockito'; +import { anything, capture, mock, resetCalls, verify, when } from 'ts-mockito'; import { AuthService } from 'xforge-common/auth.service'; import { DialogService } from 'xforge-common/dialog.service'; import { I18nService } from 'xforge-common/i18n.service'; @@ -22,6 +22,7 @@ import { SF_TYPE_REGISTRY } from '../core/models/sf-type-registry'; import { SFProjectService } from '../core/sf-project.service'; import { EventMetric, EventScope } from '../event-metrics/event-metric'; import { DraftJob, DraftJobsComponent } from './draft-jobs.component'; +import { JobDetailsDialogComponent } from './job-details-dialog.component'; import sampleEvents from './sample-events.json'; import { ServalAdministrationService } from './serval-administration.service'; @@ -94,7 +95,7 @@ describe('DraftJobsComponent', () => { expect(draftJob!.startEvent!.id).toEqual('event-metric-05'); expect(draftJob!.buildEvent!.id).toEqual('event-metric-04'); expect(draftJob!.cancelEvent).toBeUndefined(); - expect(draftJob!.finishEvent!.id).toEqual('event-metric-03'); + expect(draftJob!.finishEvent!.id).toEqual('event-metric-01'); })); it('cancelled jobs', fakeAsync(() => { @@ -109,7 +110,7 @@ describe('DraftJobsComponent', () => { expect(draftJob!.startEvent!.id).toEqual('event-metric-14'); expect(draftJob!.buildEvent!.id).toEqual('event-metric-13'); expect(draftJob!.cancelEvent!.id).toEqual('event-metric-12'); - expect(draftJob!.finishEvent).toBeUndefined(); + expect(draftJob!.finishEvent!.id).toEqual('event-metric-11'); })); it('faulted jobs', fakeAsync(() => { @@ -142,6 +143,332 @@ describe('DraftJobsComponent', () => { expect(draftJob!.cancelEvent).toBeUndefined(); expect(draftJob!.finishEvent).toBeUndefined(); })); + + it('treats retrieve status events with matching build id as finishing events', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-req', 'StartPreTranslationBuildAsync', { + timeStamp: '2025-01-01T00:00:00.000Z' + }); + const build = env.createEvent('build-req', 'BuildProjectAsync', { + timeStamp: '2025-01-01T00:01:00.000Z', + result: 'build-req' + }); + const retrieve = env.createEvent('retrieve-req', 'RetrievePreTranslationStatusAsync', { + timeStamp: '2025-01-01T00:04:00.000Z', + result: 'build-req' + }); + + componentAny['draftEvents'] = [start, build, retrieve]; + componentAny['processDraftJobs'](); + + expect(env.component.rows.length).toBe(1); + const job = env.component.rows[0].job; + expect(job.finishEvent?.id).toBe('retrieve-req'); + expect(job.status).toBe('success'); + expect(job.duration).toBe(4 * 60 * 1000); + })); + + it('ignores retrieve status events when build id does not match', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-req', 'StartPreTranslationBuildAsync', { + timeStamp: '2025-01-01T00:00:00.000Z' + }); + const build = env.createEvent('build-req', 'BuildProjectAsync', { + timeStamp: '2025-01-01T00:01:00.000Z', + result: 'build-req' + }); + const retrieve = env.createEvent('retrieve-req', 'RetrievePreTranslationStatusAsync', { + timeStamp: '2025-01-01T00:04:00.000Z', + result: 'some-other-build' + }); + + componentAny['draftEvents'] = [start, build, retrieve]; + componentAny['processDraftJobs'](); + + expect(env.component.rows.length).toBe(1); + const job = env.component.rows[0].job; + expect(job.finishEvent).toBeUndefined(); + expect(job.status).toBe('running'); + })); + }); + + describe('associates older events into jobs', () => { + // Older events did not have a draftGenerationRequestId and used another association method. They are stored in the sample events file and have an `A` in the various Ids used. + + it('successful jobs', fakeAsync(() => { + const env = new TestEnvironment(); + env.wait(); + // The Serval build id and event metric ids are written here from looking at the sample data. + const servalBuildId = 'serval-build-A1'; + + const draftJob: DraftJob = env.component.rows.filter(row => row.job.buildId === servalBuildId)[0].job; + // Confirm test setup. + expect(draftJob.status).toEqual('success'); + + expect(draftJob!.startEvent!.id).toEqual('event-metric-A05'); + expect(draftJob!.buildEvent!.id).toEqual('event-metric-A04'); + expect(draftJob!.cancelEvent).toBeUndefined(); + // Notice that the prior event grouping can use ExecuteWebhookAsync for a finish event rather than BuildCompletedAsync. + expect(draftJob!.finishEvent!.id).toEqual('event-metric-A03'); + })); + + it('cancelled jobs', fakeAsync(() => { + const env = new TestEnvironment(); + env.wait(); + const servalBuildId = 'serval-build-A2'; + + const draftJob: DraftJob = env.component.rows.filter(row => row.job.buildId === servalBuildId)[0].job; + // Confirm test setup. + expect(draftJob.status).toEqual('cancelled'); + + expect(draftJob!.startEvent!.id).toEqual('event-metric-A14'); + expect(draftJob!.buildEvent!.id).toEqual('event-metric-A13'); + expect(draftJob!.cancelEvent!.id).toEqual('event-metric-A12'); + // Notice that the prior event grouping did not define a 'finishEvent' for cancelled jobs. + expect(draftJob!.finishEvent).toBeUndefined(); + })); + + it('faulted jobs', fakeAsync(() => { + const env = new TestEnvironment(); + env.wait(); + const servalBuildId = 'serval-build-A4'; + + const draftJob: DraftJob = env.component.rows.filter(row => row.job.buildId === servalBuildId)[0].job; + // Confirm test setup. + expect(draftJob.status).toEqual('failed'); + + expect(draftJob!.startEvent!.id).toEqual('event-metric-A17'); + expect(draftJob!.buildEvent!.id).toEqual('event-metric-A16'); + expect(draftJob!.cancelEvent).toBeUndefined(); + expect(draftJob!.finishEvent!.id).toEqual('event-metric-A15'); + })); + + it('in-progress jobs', fakeAsync(() => { + // This Serval job had StartPreTranslationBuildAsync and BuildProjectAsync but nothing more yet. + const env = new TestEnvironment(); + env.wait(); + const servalBuildId = 'serval-build-A5'; + + const draftJob: DraftJob = env.component.rows.filter(row => row.job.buildId === servalBuildId)[0].job; + // Confirm test setup. + expect(draftJob.status).toEqual('running'); + + expect(draftJob!.startEvent!.id).toEqual('event-metric-A07'); + expect(draftJob!.buildEvent!.id).toEqual('event-metric-A06'); + expect(draftJob!.cancelEvent).toBeUndefined(); + expect(draftJob!.finishEvent).toBeUndefined(); + })); + }); + + describe('job details dialog data', () => { + it('includes draft generation request id in dialog data', fakeAsync(() => { + const env = new TestEnvironment(); + env.wait(); + const jobRow = env.component.rows.find( + row => row.job.draftGenerationRequestId != null && row.job.buildId != null + ); + + expect(jobRow).toBeDefined(); + if (jobRow == null) { + return; + } + + resetCalls(mockedDialogService); + when(mockedDialogService.openMatDialog(anything(), anything())).thenReturn({} as any); + + env.component.openJobDetailsDialog(jobRow.job); + + verify(mockedDialogService.openMatDialog(anything(), anything())).once(); + const [component, config] = capture(mockedDialogService.openMatDialog as any).last(); + + expect(component).toBe(JobDetailsDialogComponent); + const dialogData: any = (config as any)?.data; + expect(dialogData.draftGenerationRequestId).toEqual(jobRow.job.draftGenerationRequestId); + })); + }); + + describe('event group validation', () => { + it('throws when grouped events contain mismatched request ids', fakeAsync(() => { + // Suppose the createJobFromRequestGroup method is called with a set of event metrics that do not have the same draftGenerationRequestId. Throw. + + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const build = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + const finish = env.createEvent('finish-1', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:02:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + const mismatched = env.createEvent('mismatch-1', 'ExecuteWebhookAsync', { + tags: { draftGenerationRequestId: 'req-2' }, + timeStamp: '2025-01-01T00:03:00.000Z' + }); + + expect(() => componentAny['createJobFromRequestGroup']('req-1', [start, build, finish, mismatched])).toThrowError( + /share/i + ); + })); + + it('throws when grouped events contain multiple start events', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const startOne = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const startTwo = env.createEvent('start-2', 'StartPreTranslationBuildAsync', { + timeStamp: '2025-01-01T00:00:30.000Z' + }); + const build = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + + expect(() => componentAny['createJobFromRequestGroup']('req-1', [startOne, startTwo, build])).toThrowError( + /exactly one startpretranslationbuildasync/i + ); + })); + + it('throws when grouped events are missing a start event', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const build = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + + expect(() => componentAny['createJobFromRequestGroup']('req-1', [build])).toThrowError( + /exactly one startpretranslationbuildasync/i + ); + })); + + it('throws when grouped events contain multiple build events', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const buildOne = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + const buildTwo = env.createEvent('build-2', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:30.000Z' }); + + expect(() => componentAny['createJobFromRequestGroup']('req-1', [start, buildOne, buildTwo])).toThrowError( + /more than one buildprojectasync/i + ); + })); + + it('throws when grouped events contain multiple build completed events', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const build = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + const finishOne = env.createEvent('finish-1', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:02:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + const finishTwo = env.createEvent('finish-2', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:03:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + + expect(() => + componentAny['createJobFromRequestGroup']('req-1', [start, build, finishOne, finishTwo]) + ).toThrowError(/more than one buildcompletedasync/i); + })); + + it('throws when there is a cancel event without a BuildCompleted cancel state', fakeAsync(() => { + // When a cancel event happens, the current behavior is that the BuildCompletedAsync event also has a buildState of 'Canceled'. The createJobFromRequestGroup method will expect that. + + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const build = env.createEvent('build-1', 'BuildProjectAsync', { timeStamp: '2025-01-01T00:01:00.000Z' }); + const finish = env.createEvent('finish-1', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:02:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + const cancel = env.createEvent('cancel-1', 'CancelPreTranslationBuildAsync', { + timeStamp: '2025-01-01T00:02:30.000Z' + }); + + expect(() => componentAny['createJobFromRequestGroup']('req-1', [start, build, finish, cancel])).toThrowError( + /Cancel/i + ); + })); + }); + + describe('processDraftJobs', () => { + it('skips request groups missing a start event', fakeAsync(() => { + // Suppose the user selects a dante range such that the start date+time is after a draft generation starts and before it finishes. The list of event metrics will have events about the job being processed, but there will not be a start event. Omit those jobs from the data we present. + // + // If the end date falls after a start event and before a job's finish event, we'll just show that as in progress. + + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + + const buildEvent = env.createEvent('build-1', 'BuildProjectAsync', { + timeStamp: '2025-01-01T00:01:00.000Z', + tags: { draftGenerationRequestId: 'req-1' }, + result: 'build-1' + }); + + env.component['draftEvents'] = [buildEvent]; + env.component['processDraftJobs'](); + // The job was not included in the data. + expect(env.component.rows.length).toBe(0); + })); + }); + + describe('grouping mode selection', () => { + it('uses request grouping when toggle is set', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false, initialGrouping: 'timing' }); + env.wait(); + const componentAny: any = env.component; + + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const build = env.createEvent('build-1', 'BuildProjectAsync', { + timeStamp: '2025-01-01T00:01:00.000Z', + result: 'build-1' + }); + const finish = env.createEvent('finish-1', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:02:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + + componentAny['draftEvents'] = [start, build, finish]; + const requestSpy = spyOn(componentAny, 'createJobFromRequestGroup').and.callThrough(); + const timingSpy = spyOn(componentAny, 'createJobsUsingLegacyCorrelation').and.callThrough(); + + env.component.onGroupingModeChange('requestId'); + + expect(env.component.groupingMode).toBe('requestId'); + expect(requestSpy).toHaveBeenCalledTimes(1); + expect(timingSpy).not.toHaveBeenCalled(); + })); + + it('uses timing grouping when toggle is set', fakeAsync(() => { + const env = new TestEnvironment({ hasEvents: false }); + env.wait(); + const componentAny: any = env.component; + + const start = env.createEvent('start-1', 'StartPreTranslationBuildAsync'); + const build = env.createEvent('build-1', 'BuildProjectAsync', { + timeStamp: '2025-01-01T00:01:00.000Z', + result: 'build-1' + }); + const finish = env.createEvent('finish-1', 'BuildCompletedAsync', { + timeStamp: '2025-01-01T00:02:00.000Z', + payload: { buildState: 'Completed', sfProjectId: 'sf-test-project', buildId: 'build-1' } + }); + + componentAny['draftEvents'] = [start, build, finish]; + const requestSpy = spyOn(componentAny, 'createJobFromRequestGroup').and.callThrough(); + const timingSpy = spyOn(componentAny, 'createJobsUsingLegacyCorrelation').and.callThrough(); + + env.component.onGroupingModeChange('timing'); + + expect(env.component.groupingMode).toBe('timing'); + expect(timingSpy).toHaveBeenCalledTimes(1); + expect(requestSpy).not.toHaveBeenCalled(); + })); }); describe('duration statistics', () => { @@ -335,7 +662,13 @@ class TestEnvironment { readonly localeSubject: BehaviorSubject; private readonly queryParams$ = new BehaviorSubject({}); - constructor({ hasEvents = true }: { hasEvents?: boolean } = {}) { + constructor({ + hasEvents = true, + initialGrouping = 'requestId' + }: { + hasEvents?: boolean; + initialGrouping?: 'requestId' | 'timing'; + } = {}) { const initialLocale: Locale = { canonicalTag: 'en', direction: 'ltr', @@ -358,6 +691,9 @@ class TestEnvironment { this.fixture = TestBed.createComponent(DraftJobsComponent); this.component = this.fixture.componentInstance; this.fixture.detectChanges(); + // The component will default to 'timing' at present. But tests should default to 'requestId' unless 'timing' is + // requested. + this.setGroupingMode(initialGrouping); } setDateRange(start: Date, end: Date): void { @@ -387,6 +723,10 @@ class TestEnvironment { this.fixture.detectChanges(); } + setGroupingMode(mode: 'requestId' | 'timing'): void { + this.component.onGroupingModeChange(mode); + } + createRowWithDuration(duration: number | undefined): any { return { job: { @@ -435,6 +775,34 @@ class TestEnvironment { }); } + createEvent(id: string, eventType: string, overrides: Partial = {}): EventMetric { + const event: EventMetric = { + id, + eventType, + timeStamp: '2025-01-01T00:00:00.000Z', + scope: EventScope.Drafting, + payload: {}, + userId: 'user-1', + projectId: 'sf-test-project', + result: undefined, + executionTime: undefined, + exception: undefined, + tags: { draftGenerationRequestId: 'req-1' } + }; + + if (overrides.timeStamp != null) event.timeStamp = overrides.timeStamp; + if (overrides.scope != null) event.scope = overrides.scope; + if (overrides.payload != null) event.payload = overrides.payload; + if (overrides.userId !== undefined) event.userId = overrides.userId; + if (overrides.projectId !== undefined) event.projectId = overrides.projectId; + if (overrides.result !== undefined) event.result = overrides.result; + if (overrides.executionTime !== undefined) event.executionTime = overrides.executionTime; + if (overrides.exception !== undefined) event.exception = overrides.exception; + if (overrides.tags != null) event.tags = overrides.tags; + + return event; + } + /** * Transforms JSON event data to EventMetric objects. * Transforms "timeStamp":{"$date":"foo"} to just "timeStamp". @@ -450,7 +818,8 @@ class TestEnvironment { projectId: event.projectId, result: event.result, executionTime: event.executionTime, - exception: event.exception + exception: event.exception, + tags: event.tags })); } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts index f9d2bee38e2..d1e24630db3 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts @@ -1,6 +1,7 @@ import { Component, DestroyRef, OnInit } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { MatButton, MatIconButton } from '@angular/material/button'; +import { MatButtonToggle, MatButtonToggleGroup } from '@angular/material/button-toggle'; import { provideNativeDateAdapter } from '@angular/material/core'; import { MatDialogConfig } from '@angular/material/dialog'; import { MatIcon } from '@angular/material/icon'; @@ -28,9 +29,11 @@ import { I18nService } from 'xforge-common/i18n.service'; import { NoticeService } from 'xforge-common/notice.service'; import { OnlineStatusService } from 'xforge-common/online-status.service'; import { OwnerComponent } from 'xforge-common/owner/owner.component'; -import { notNull } from '../../type-utils'; +import { isPopulatedString } from 'xforge-common/utils'; +import { hasElements, notNull, PopulatedArray } from '../../type-utils'; import { SFProjectService } from '../core/sf-project.service'; import { EventMetric } from '../event-metrics/event-metric'; +import { InfoComponent } from '../shared/info/info.component'; import { NoticeComponent } from '../shared/notice/notice.component'; import { projectLabel } from '../shared/utils'; import { DateRangePickerComponent, NormalizedDateRange } from './date-range-picker.component'; @@ -46,6 +49,8 @@ interface ProjectBooks { export interface DraftJob { /** Serval build ID */ buildId: string | undefined; + /** Unique draft job request identifier on event metrics */ + draftGenerationRequestId?: string; projectId: string; /** This is optional since incomplete jobs might not have a start event */ startEvent?: EventMetric; @@ -65,6 +70,7 @@ export interface DraftJob { translationBooks?: ProjectBooks[]; } +/** Data that makes up a row of information in the Draft Jobs table. */ export interface DraftJobsTableRow { job: DraftJob; projectId: string; @@ -80,6 +86,7 @@ export interface DraftJobsTableRow { clearmlUrl?: string; } +/** Names of events that are relevant for pre-translation draft generation requests and processing. */ const DRAFTING_EVENTS = [ 'StartPreTranslationBuildAsync', 'BuildProjectAsync', @@ -109,6 +116,8 @@ const DRAFTING_EVENTS = [ MatHeaderRowDef, MatIcon, MatIconButton, + MatButtonToggle, + MatButtonToggleGroup, MatMenu, MatMenuItem, MatMenuTrigger, @@ -119,7 +128,8 @@ const DRAFTING_EVENTS = [ NoticeComponent, OwnerComponent, RouterLink, - DateRangePickerComponent + DateRangePickerComponent, + InfoComponent ] }) export class DraftJobsComponent extends DataLoadingComponent implements OnInit { @@ -133,6 +143,7 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { 'author' ]; rows: DraftJobsTableRow[] = []; + groupingMode: 'requestId' | 'timing' = 'timing'; currentProjectFilter: string | null = null; @@ -142,8 +153,10 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { private draftEvents?: EventMetric[]; private draftJobs: DraftJob[] = []; - private projectNames = new Map(); // Cache for project names - private projectShortNames = new Map(); // Cache for project short names + /** Cache for project names */ + private projectNames = new Map(); + /** Cache for project short names */ + private projectShortNames = new Map(); filteredProjectName = ''; private currentDateRange: NormalizedDateRange | undefined; private readonly dateRange$ = new BehaviorSubject(undefined); @@ -168,33 +181,26 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { return this.draftEvents == null; } + /** Of jobs in the table. */ get meanDuration(): number | undefined { const durations = this.rows.map(r => r.job.duration).filter((d): d is number => d != null); - if (durations.length === 0) { - return undefined; - } + if (durations.length === 0) return undefined; return durations.reduce((sum, d) => sum + d, 0) / durations.length; } get maxDuration(): number | undefined { const durations = this.rows.map(r => r.job.duration).filter((d): d is number => d != null); - if (durations.length === 0) { - return undefined; - } + if (durations.length === 0) return undefined; return Math.max(...durations); } get meanDurationFormatted(): string | undefined { - if (this.meanDuration == null) { - return undefined; - } + if (this.meanDuration == null) return undefined; return this.formatDurationInHours(this.meanDuration); } get maxDurationFormatted(): string | undefined { - if (this.maxDuration == null) { - return undefined; - } + if (this.maxDuration == null) return undefined; return this.formatDurationInHours(this.maxDuration); } @@ -265,7 +271,8 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { eventBasedDuration, startTime, clearmlUrl, - buildsStartedSince + buildsStartedSince, + draftGenerationRequestId: job.draftGenerationRequestId }; const dialogConfig: MatDialogConfig = { data: dialogData, width: '1000px', maxHeight: '80vh' }; @@ -280,6 +287,15 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { }); } + onGroupingModeChange(mode: 'requestId' | 'timing'): void { + if (this.groupingMode === mode) return; + this.groupingMode = mode; + if (this.draftEvents != null) { + // Re-group events into jobs. + this.processDraftJobs(); + } + } + private async loadDraftJobs( range: NormalizedDateRange, isOnline: boolean, @@ -325,17 +341,16 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { } } - private processDraftJobs(): void { - if (this.draftEvents == null) { - this.draftJobs = []; - this.generateRows(); - return; + /** Group together events using information that was available prior to introducing draftGenerationRequestId. */ + private createJobsUsingLegacyCorrelation(events: EventMetric[]): DraftJob[] { + if (events.length === 0) { + return []; } const jobs: DraftJob[] = []; // Step 1: Find all build events (BuildProjectAsync) - these are our anchors - const buildEvents = this.draftEvents.filter(event => event.eventType === 'BuildProjectAsync'); + const buildEvents = events.filter(event => event.eventType === 'BuildProjectAsync'); // Step 2: For each build event, find the nearest preceding start event for (const buildEvent of buildEvents) { @@ -347,7 +362,7 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { const buildTime = new Date(buildEvent.timeStamp); // Find all StartPreTranslationBuildAsync events for this project that precede the build - const candidateStartEvents = this.draftEvents.filter( + const candidateStartEvents = events.filter( event => event.eventType === 'StartPreTranslationBuildAsync' && event.projectId === buildEvent.projectId && @@ -365,7 +380,7 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { }); // Step 3: Find the first completion event after the build - const candidateCompletionEvents = this.draftEvents.filter(event => { + const candidateCompletionEvents = events.filter(event => { if (event.projectId !== buildEvent.projectId && event.payload.sfProjectId !== buildEvent.projectId) return false; if (new Date(event.timeStamp) <= buildTime) return false; @@ -431,7 +446,7 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { } // Step 4: Collect additional events (any events with same build ID that weren't already included) - const additionalEvents = this.draftEvents.filter(event => { + const additionalEvents = events.filter(event => { // Don't include events we've already tracked if (event === startEvent || event === buildEvent || event === completionEvent) { return false; @@ -455,16 +470,181 @@ export class DraftJobsComponent extends DataLoadingComponent implements OnInit { } // Sort by start time (most recent first) - this.draftJobs = jobs.sort((a, b) => { + return jobs.sort((a, b) => { const aTime = a.startTime?.getTime() ?? 0; const bTime = b.startTime?.getTime() ?? 0; return bTime - aTime; }); + } + + private processDraftJobs(): void { + if (this.draftEvents == null) { + this.draftJobs = []; + this.generateRows(); + return; + } + + const jobs: DraftJob[] = []; - // Generate rows + if (this.groupingMode === 'timing') { + jobs.push(...this.createJobsUsingLegacyCorrelation(this.draftEvents)); + } else { + const eventsGroupedByRequestId = Map.groupBy(this.draftEvents, this.getDraftGenerationRequestId); + for (const [requestId, groupedEvents] of eventsGroupedByRequestId.entries()) { + if (!hasElements(groupedEvents)) continue; + if (requestId != null) { + // Omit jobs that started before the beginning of the date range. + const hasStartEvent: boolean = groupedEvents.some( + event => event.eventType === 'StartPreTranslationBuildAsync' + ); + if (!hasStartEvent) continue; + + jobs.push(this.createJobFromRequestGroup(requestId, groupedEvents)); + } else { + jobs.push(...this.createJobsUsingLegacyCorrelation(groupedEvents)); + } + } + } + this.draftJobs = this.sortJobsByStartTime(jobs); this.generateRows(); } + private createJobFromRequestGroup(requestId: string, groupedEvents: PopulatedArray): DraftJob { + const eventsSorted: EventMetric[] = [...groupedEvents].sort((left, right) => this.compareEventTimes(left, right)); + const [startEvent, buildEvent, finishEvent, cancelEvent, additionalEvents] = this.validateAndExtractEvents( + requestId, + eventsSorted + ); + + const projectId: string | undefined = startEvent.projectId; + if (projectId == null) throw new Error(`Request group ${requestId} is missing projectId on start event.`); + + const books = this.extractBooksFromEvent(startEvent); + const servalBuildId: string | undefined = buildEvent?.result; + // servalBuildId might be undefined if we don't have a BuildProjectAsync event. + + const startTime: Date | undefined = this.createEventDate(startEvent); + const sfUserId: string | undefined = startEvent.userId; + const job: DraftJob = { + projectId, + buildId: servalBuildId, + draftGenerationRequestId: requestId, + startEvent, + buildEvent, + finishEvent, + cancelEvent, + events: eventsSorted, + additionalEvents, + startTime, + userId: sfUserId, + trainingBooks: books.trainingBooks, + translationBooks: books.translationBooks, + status: 'running', + errorMessage: undefined, + finishTime: undefined, + duration: undefined + }; + this.finalizeJobStatus(job); + return job; + } + + /** Validates event group contents and extracts specific events. The received events should all be part of the same draft generation request. */ + private validateAndExtractEvents( + requestId: string, + events: EventMetric[] + ): [EventMetric, EventMetric | undefined, EventMetric | undefined, EventMetric | undefined, EventMetric[]] { + for (const event of events) { + const eventRequestId: string | undefined = this.getDraftGenerationRequestId(event); + if (eventRequestId !== requestId) { + throw new Error( + `Erroneously given a group of events which did not all share draftGenerationRequestId ${requestId}.` + ); + } + } + + const startEvents: EventMetric[] = events.filter(event => event.eventType === 'StartPreTranslationBuildAsync'); + if (startEvents.length !== 1) { + throw new Error(`Request group ${requestId} must include exactly one StartPreTranslationBuildAsync event.`); + } + const startEvent: EventMetric = startEvents[0]; + + const buildEvents: EventMetric[] = events.filter(event => event.eventType === 'BuildProjectAsync'); + if (buildEvents.length > 1) { + throw new Error(`Request group ${requestId} may not have more than one BuildProjectAsync event.`); + } + const buildEvent: EventMetric | undefined = buildEvents[0]; + + const buildCompletedEvents: EventMetric[] = events.filter(event => event.eventType === 'BuildCompletedAsync'); + if (buildCompletedEvents.length > 1) { + throw new Error(`Request group ${requestId} may not have more than one BuildCompletedAsync event.`); + } + const retrieveFinishingEvents: EventMetric[] = events.filter( + event => event.eventType === 'RetrievePreTranslationStatusAsync' && event.result === buildEvent?.result + ); + const finishEvent: EventMetric | undefined = buildCompletedEvents[0] ?? retrieveFinishingEvents[0]; + + const cancelEvents: EventMetric[] = events.filter(event => event.eventType === 'CancelPreTranslationBuildAsync'); + if (cancelEvents.length > 1) { + console.log(`Request group ${requestId} has more than one CancelPreTranslationBuildAsync event.`); + } + const cancelEvent: EventMetric | undefined = cancelEvents[0]; + + if (cancelEvent != null) { + if (finishEvent == null) { + // Probably possible if examining at the wrong moment or the BuildCompletedAsync event never came over the network. + console.log( + `Request group ${requestId} has a CancelPreTranslationBuildAsync event but no BuildCompletedAsync event.` + ); + } else { + const buildState: any = finishEvent.payload?.buildState; + if (buildState !== 'Canceled') { + throw new Error( + `Unexpected data. Request ${requestId} had a CancelPreTranslationBuildAsync event, but BuildCompletedAsync says buildState is ${buildState}.` + ); + } + } + } + + const additionalEvents: EventMetric[] = events.filter( + event => event !== startEvent && event !== buildEvent && event !== finishEvent && event !== cancelEvent + ); + + return [startEvent, buildEvent, finishEvent, cancelEvent, additionalEvents]; + } + + private getDraftGenerationRequestId(event: EventMetric): string | undefined { + if (event.tags == null) return undefined; + const requestId: unknown = event.tags['draftGenerationRequestId']; + if (!isPopulatedString(requestId)) return undefined; + return requestId; + } + + private sortJobsByStartTime(jobs: DraftJob[]): DraftJob[] { + return jobs.sort((left, right) => { + const leftTime = left.startTime != null ? left.startTime.getTime() : 0; + const rightTime = right.startTime != null ? right.startTime.getTime() : 0; + // Newer events first + return rightTime - leftTime; + }); + } + + private compareEventTimes(left: EventMetric, right: EventMetric): number { + const leftTime = this.getEventTimeMs(left) ?? 0; + const rightTime = this.getEventTimeMs(right) ?? 0; + return leftTime - rightTime; + } + + private getEventTimeMs(event: EventMetric): number | undefined { + const timeMs = new Date(event.timeStamp).getTime(); + return Number.isNaN(timeMs) ? undefined : timeMs; + } + + private createEventDate(event: EventMetric): Date | undefined { + const timeMs: number | undefined = this.getEventTimeMs(event); + if (timeMs == null) return undefined; + return new Date(timeMs); + } + private extractBuildIdFromEvent(event: EventMetric): string | null { // First try the result field (common for many event types) if (event.result != null) { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.html index a9c3fca99ac..594dde84b80 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.html @@ -38,6 +38,10 @@

Summary

} +
+ Draft Generation Request ID: + {{ draftGenerationRequestIdDisplay }} +
Project ID: {{ data.projectId }}
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.ts index 89d5edc7153..05ce0f86da7 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-details-dialog.component.ts @@ -13,6 +13,7 @@ import { MatTab, MatTabGroup } from '@angular/material/tabs'; import { MatTooltip } from '@angular/material/tooltip'; import { I18nService } from 'xforge-common/i18n.service'; import { L10nNumberPipe } from 'xforge-common/l10n-number.pipe'; +import { isPopulatedString } from 'xforge-common/utils'; import { EventMetric } from '../event-metrics/event-metric'; import { JsonViewerComponent } from '../shared/json-viewer/json-viewer.component'; import { NoticeComponent } from '../shared/notice/notice.component'; @@ -28,6 +29,7 @@ interface JobDetailsDialogData { startTime?: string; clearmlUrl?: string; buildsStartedSince: number; + draftGenerationRequestId?: string; } const EVENT_TYPE_LABELS: { @@ -191,6 +193,11 @@ export class JobDetailsDialogComponent implements OnInit { } } + get draftGenerationRequestIdDisplay(): string { + if (!isPopulatedString(this.data.draftGenerationRequestId)) return 'none'; + return this.data.draftGenerationRequestId; + } + formatDate(timestamp: string): string { return this.i18n.formatDate(new Date(timestamp), { showTimeZone: true }); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/sample-events.json b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/sample-events.json index a765ab1d778..729dc9749e0 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/sample-events.json +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/sample-events.json @@ -4,7 +4,7 @@ "eventType": "BuildCompletedAsync", "executionTime": "00:01:00.0000000", "payload": { - "sfProjectId": "sf-user-5", + "sfProjectId": "sf-project-15", "buildId": "serval-build-1", "buildState": "Completed", "websiteUrl": "https://example.com/" @@ -13,20 +13,26 @@ "timeStamp": { "$date": "2020-01-04T00:08:00.000Z" }, - "userId": "sf-user-5" + "userId": "sf-project-15", + "tags": { + "draftGenerationRequestId": "draft-gen-req-4" + } }, { "_id": "event-metric-02", "eventType": "RetrievePreTranslationStatusAsync", "executionTime": "00:01:00.0000000", "payload": { - "sfProjectId": "sf-user-5" + "sfProjectId": "sf-project-15" }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "result": "serval-build-1", "scope": "Drafting", "timeStamp": { "$date": "2020-01-04T00:06:00.000Z" + }, + "tags": { + "draftGenerationRequestId": "draft-gen-req-4" } }, { @@ -38,11 +44,14 @@ "event": "TranslationBuildFinished", "translationEngineId": "translation-engine-1" }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "result": "serval-build-1", "scope": "Drafting", "timeStamp": { "$date": "2020-01-04T00:04:00.000Z" + }, + "tags": { + "draftGenerationRequestId": "draft-gen-req-4" } }, { @@ -65,17 +74,20 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-5" + "ProjectId": "sf-project-15" }, "preTranslate": true }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "result": "serval-build-1", "scope": "Drafting", "timeStamp": { "$date": "2020-01-04T00:02:00.000Z" }, - "userId": "sf-user-8" + "userId": "sf-user-8", + "tags": { + "draftGenerationRequestId": "draft-gen-req-4" + } }, { "_id": "event-metric-05", @@ -97,15 +109,18 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-5" + "ProjectId": "sf-project-15" } }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "scope": "Drafting", "timeStamp": { "$date": "2020-01-04T00:00:00.000Z" }, - "userId": "sf-user-8" + "userId": "sf-user-8", + "tags": { + "draftGenerationRequestId": "draft-gen-req-4" + } }, { "_id": "event-metric-06", @@ -127,17 +142,20 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-5" + "ProjectId": "sf-project-15" }, "preTranslate": true }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "result": "serval-build-5", "scope": "Drafting", "timeStamp": { "$date": "2020-01-03T00:02:00.000Z" }, - "userId": "sf-user-8" + "userId": "sf-user-8", + "tags": { + "draftGenerationRequestId": "draft-gen-req-3" + } }, { "_id": "event-metric-07", @@ -159,22 +177,25 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-5" + "ProjectId": "sf-project-15" } }, - "projectId": "sf-user-5", + "projectId": "sf-project-15", "scope": "Drafting", "timeStamp": { "$date": "2020-01-03T00:00:00.000Z" }, - "userId": "sf-user-8" + "userId": "sf-user-8", + "tags": { + "draftGenerationRequestId": "draft-gen-req-3" + } }, { "_id": "event-metric-11", "eventType": "BuildCompletedAsync", "executionTime": "00:01:00.0000000", "payload": { - "sfProjectId": "sf-user-4", + "sfProjectId": "sf-project-14", "buildId": "serval-build-2", "buildState": "Canceled", "websiteUrl": "https://example.com/" @@ -183,7 +204,10 @@ "timeStamp": { "$date": "2020-01-02T00:06:00.000Z" }, - "userId": "sf-user-4" + "userId": "sf-project-14", + "tags": { + "draftGenerationRequestId": "draft-gen-req-2" + } }, { "_id": "event-metric-12", @@ -191,15 +215,18 @@ "executionTime": "00:01:00.0000000", "payload": { "curUserId": "sf-user-2", - "sfProjectId": "sf-user-4" + "sfProjectId": "sf-project-14" }, - "projectId": "sf-user-4", + "projectId": "sf-project-14", "result": "serval-build-2", "scope": "Drafting", "timeStamp": { "$date": "2020-01-02T00:04:00.000Z" }, - "userId": "sf-user-2" + "userId": "sf-user-2", + "tags": { + "draftGenerationRequestId": "draft-gen-req-2" + } }, { "_id": "event-metric-13", @@ -221,17 +248,20 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-4" + "ProjectId": "sf-project-14" }, "preTranslate": true }, - "projectId": "sf-user-4", + "projectId": "sf-project-14", "result": "serval-build-2", "scope": "Drafting", "timeStamp": { "$date": "2020-01-02T00:02:00.000Z" }, - "userId": "sf-user-2" + "userId": "sf-user-2", + "tags": { + "draftGenerationRequestId": "draft-gen-req-2" + } }, { "_id": "event-metric-14", @@ -253,22 +283,25 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-4" + "ProjectId": "sf-project-14" } }, - "projectId": "sf-user-4", + "projectId": "sf-project-14", "scope": "Drafting", "timeStamp": { "$date": "2020-01-02T00:00:00.000Z" }, - "userId": "sf-user-2" + "userId": "sf-user-2", + "tags": { + "draftGenerationRequestId": "draft-gen-req-2" + } }, { "_id": "event-metric-15", "eventType": "BuildCompletedAsync", "executionTime": "00:01:00.0000000", "payload": { - "sfProjectId": "sf-user-1", + "sfProjectId": "sf-project-11", "buildId": "serval-build-4", "buildState": "Faulted", "websiteUrl": "https://example.com/" @@ -277,7 +310,10 @@ "timeStamp": { "$date": "2020-01-01T00:04:00.000Z" }, - "userId": "sf-user-1" + "userId": "sf-project-11", + "tags": { + "draftGenerationRequestId": "draft-gen-req-1" + } }, { "_id": "event-metric-16", @@ -299,17 +335,20 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-1" + "ProjectId": "sf-project-11" }, "preTranslate": true }, - "projectId": "sf-user-1", + "projectId": "sf-project-11", "result": "serval-build-4", "scope": "Drafting", "timeStamp": { "$date": "2020-01-01T00:02:00.000Z" }, - "userId": "sf-user-6" + "userId": "sf-user-6", + "tags": { + "draftGenerationRequestId": "draft-gen-req-1" + } }, { "_id": "event-metric-17", @@ -331,14 +370,359 @@ } ], "SendEmailOnBuildFinished": true, - "ProjectId": "sf-user-1" + "ProjectId": "sf-project-11" + } + }, + "projectId": "sf-project-11", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-01T00:00:00.000Z" + }, + "userId": "sf-user-6", + "tags": { + "draftGenerationRequestId": "draft-gen-req-1" + } + }, + { + "_id": "event-metric-A01", + "eventType": "BuildCompletedAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "sfProjectId": "sf-project-A15", + "buildId": "serval-build-A1", + "buildState": "Completed", + "websiteUrl": "https://example.com/" + }, + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-04T00:08:00.000Z" + }, + "userId": "sf-project-A15" + }, + { + "_id": "event-metric-A02", + "eventType": "RetrievePreTranslationStatusAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "sfProjectId": "sf-project-A15" + }, + "projectId": "sf-project-A15", + "result": "serval-build-A1", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-04T00:06:00.000Z" + } + }, + { + "_id": "event-metric-A03", + "eventType": "ExecuteWebhookAsync", + "payload": { + "buildId": "serval-build-A1", + "buildState": "Completed", + "event": "TranslationBuildFinished", + "translationEngineId": "translation-engine-A1" + }, + "projectId": "sf-project-A15", + "result": "serval-build-A1", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-04T00:04:00.000Z" + } + }, + { + "_id": "event-metric-A04", + "eventType": "BuildProjectAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A8", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A15" + }, + "preTranslate": true + }, + "projectId": "sf-project-A15", + "result": "serval-build-A1", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-04T00:02:00.000Z" + }, + "userId": "sf-user-A8" + }, + { + "_id": "event-metric-A05", + "eventType": "StartPreTranslationBuildAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A8", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A15" + } + }, + "projectId": "sf-project-A15", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-04T00:00:00.000Z" + }, + "userId": "sf-user-A8" + }, + { + "_id": "event-metric-A06", + "eventType": "BuildProjectAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A8", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A15" + }, + "preTranslate": true + }, + "projectId": "sf-project-A15", + "result": "serval-build-A5", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-03T00:02:00.000Z" + }, + "userId": "sf-user-A8" + }, + { + "_id": "event-metric-A07", + "eventType": "StartPreTranslationBuildAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A8", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A1", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A15" + } + }, + "projectId": "sf-project-A15", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-03T00:00:00.000Z" + }, + "userId": "sf-user-A8" + }, + { + "_id": "event-metric-A11", + "eventType": "BuildCompletedAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "sfProjectId": "sf-project-A14", + "buildId": "serval-build-A2", + "buildState": "Canceled", + "websiteUrl": "https://example.com/" + }, + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-02T00:06:00.000Z" + }, + "userId": "sf-project-A14" + }, + { + "_id": "event-metric-A12", + "eventType": "CancelPreTranslationBuildAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A2", + "sfProjectId": "sf-project-A14" + }, + "projectId": "sf-project-A14", + "result": "serval-build-A2", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-02T00:04:00.000Z" + }, + "userId": "sf-user-A2" + }, + { + "_id": "event-metric-A13", + "eventType": "BuildProjectAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A2", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A6", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A6", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A14" + }, + "preTranslate": true + }, + "projectId": "sf-project-A14", + "result": "serval-build-A2", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-02T00:02:00.000Z" + }, + "userId": "sf-user-A2" + }, + { + "_id": "event-metric-A14", + "eventType": "StartPreTranslationBuildAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A2", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A6", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A6", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A14" + } + }, + "projectId": "sf-project-A14", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-02T00:00:00.000Z" + }, + "userId": "sf-user-A2" + }, + { + "_id": "event-metric-A15", + "eventType": "BuildCompletedAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "sfProjectId": "sf-project-A11", + "buildId": "serval-build-A4", + "buildState": "Faulted", + "websiteUrl": "https://example.com/" + }, + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-01T00:04:00.000Z" + }, + "userId": "sf-project-A11" + }, + { + "_id": "event-metric-A16", + "eventType": "BuildProjectAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A6", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A9", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A9", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A11" + }, + "preTranslate": true + }, + "projectId": "sf-project-A11", + "result": "serval-build-A4", + "scope": "Drafting", + "timeStamp": { + "$date": "2020-01-01T00:02:00.000Z" + }, + "userId": "sf-user-A6" + }, + { + "_id": "event-metric-A17", + "eventType": "StartPreTranslationBuildAsync", + "executionTime": "00:01:00.0000000", + "payload": { + "curUserId": "sf-user-A6", + "buildConfig": { + "TrainingScriptureRanges": [ + { + "ProjectId": "sf-project-A9", + "ScriptureRange": "MAT;MRK" + } + ], + "TranslationScriptureRanges": [ + { + "ProjectId": "sf-project-A9", + "ScriptureRange": "GEN;EXO" + } + ], + "SendEmailOnBuildFinished": true, + "ProjectId": "sf-project-A11" } }, - "projectId": "sf-user-1", + "projectId": "sf-project-A11", "scope": "Drafting", "timeStamp": { "$date": "2020-01-01T00:00:00.000Z" }, - "userId": "sf-user-6" + "userId": "sf-user-A6" } ] diff --git a/src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts b/src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts index 157e80211ec..1a7a0386cd4 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts @@ -122,3 +122,12 @@ export type ObjectPaths = Depth extends never * This has a data field that is not null. Such as for a realtime document that has a non-null data field. */ export type WithData = T & { data: NonNullable }; + +/** This type indicates that the array has at least one element; the array is not empty. Briefer but less flexible + * definitions can be seen at https://stackoverflow.com/q/56006111. */ +export type PopulatedArray = [T, ...T[]] | [...T[], T] | [T, ...T[], T]; + +/** The input array has one or more elements; it is not empty. */ +export function hasElements(array: T[]): array is PopulatedArray { + return array.length > 0; +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.spec.ts index cc22914683f..13738fb2004 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.spec.ts @@ -2,7 +2,7 @@ import { Component, DestroyRef, OnDestroy } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { BehaviorSubject } from 'rxjs'; import { quietTakeUntilDestroyed } from './util/rxjs-util'; -import { getAspCultureCookieLanguage, getLinkHTML } from './utils'; +import { getAspCultureCookieLanguage, getLinkHTML, isPopulatedString } from './utils'; describe('xforge-common utils', () => { it('should parse ASP Culture cookie', () => { @@ -27,6 +27,33 @@ describe('xforge-common utils', () => { `example` ); }); + + describe('isPopulatedString', () => { + it('should return true for non-empty strings', () => { + expect(isPopulatedString('a')).toBe(true); + expect(isPopulatedString(' a ')).toBe(true); + expect(isPopulatedString(' ')).toBe(true); + expect(isPopulatedString('0')).toBe(true); + expect(isPopulatedString('false')).toBe(true); + const value: string | null | undefined = 'abc'; + expect(isPopulatedString(value)).toBe(true); + }); + + it('should return false for null, undefined, empty strings, or non-strings', () => { + expect(isPopulatedString(null)).toBe(false); + expect(isPopulatedString(undefined)).toBe(false); + expect(isPopulatedString('')).toBe(false); + let value: string | null | undefined = ''; + expect(isPopulatedString(value)).toBe(false); + value = null; + expect(isPopulatedString(value)).toBe(false); + value = undefined; + expect(isPopulatedString(value)).toBe(false); + expect(isPopulatedString(0)).toBe(false); + expect(isPopulatedString({})).toBe(false); + expect(isPopulatedString([])).toBe(false); + }); + }); }); describe('quietTakeUntilDestroyed', () => { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.ts index ca334f4ff88..59e7405143d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/utils.ts @@ -181,3 +181,10 @@ export function tryParseJSON(data: unknown): unknown { return null; } } + +/** The input is a string, and it is not empty (''). It is not null or undefined. This method could alternatively be known as "isNonEmptyString". + * + * If the negation of this is true, then the input is '' or null or undefined or another non-string. */ +export function isPopulatedString(value: unknown): value is Exclude { + return typeof value === 'string' && value !== ''; +} diff --git a/src/SIL.XForge.Scripture/Services/IMachineProjectService.cs b/src/SIL.XForge.Scripture/Services/IMachineProjectService.cs index 67a740207a1..4a5b625adb3 100644 --- a/src/SIL.XForge.Scripture/Services/IMachineProjectService.cs +++ b/src/SIL.XForge.Scripture/Services/IMachineProjectService.cs @@ -15,6 +15,7 @@ Task BuildProjectForBackgroundJobAsync( string curUserId, BuildConfig buildConfig, bool preTranslate, + string? draftGenerationRequestId, CancellationToken cancellationToken ); Task GetProjectZipAsync(string sfProjectId, Stream outputStream, CancellationToken cancellationToken); diff --git a/src/SIL.XForge.Scripture/Services/MachineApiService.cs b/src/SIL.XForge.Scripture/Services/MachineApiService.cs index 70c4dfc6b33..51e5cd872a2 100644 --- a/src/SIL.XForge.Scripture/Services/MachineApiService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineApiService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Security.Cryptography; using System.Text; @@ -598,6 +599,12 @@ public async Task BuildCompletedAsync(string sfProjectId, string buildId, string { try { + string? draftGenerationRequestId = await GetDraftGenerationRequestIdForBuildAsync(sfProjectId, buildId); + if (!string.IsNullOrEmpty(draftGenerationRequestId)) + { + Activity.Current?.AddTag(MachineProjectService.DraftGenerationRequestIdKey, draftGenerationRequestId); + } + // Retrieve the build started from the event metric. We do this as there may be multiple builds started, // and this ensures that only builds that want to send an email will have one sent. var eventMetrics = await eventMetricService.GetEventMetricsAsync( @@ -694,8 +701,15 @@ await projectSecrets.UpdateAsync( cancellationToken ); + string buildId = translationBuild.Id; + string? draftGenerationRequestId = await GetDraftGenerationRequestIdForBuildAsync(sfProjectId, buildId); + if (!string.IsNullOrEmpty(draftGenerationRequestId)) + { + Activity.Current?.AddTag(MachineProjectService.DraftGenerationRequestIdKey, draftGenerationRequestId); + } + // Return the build id so it can be logged - return translationBuild.Id; + return buildId; } catch (ServalApiException e) when (e.StatusCode == StatusCodes.Status404NotFound) { @@ -779,6 +793,13 @@ public async Task ExecuteWebhookAsync(string json, string signature) return; } + // Add the draftGenerationRequestId to associate with other events. + string? draftGenerationRequestId = await GetDraftGenerationRequestIdForBuildAsync(projectId, buildId); + if (!string.IsNullOrEmpty(draftGenerationRequestId)) + { + Activity.Current?.AddTag(MachineProjectService.DraftGenerationRequestIdKey, draftGenerationRequestId); + } + // Record that the webhook was run successfully var arguments = new Dictionary { @@ -1854,17 +1875,29 @@ await projectSecrets.UpdateAsync( ); // Notify any SignalR clients subscribed to the project + string? buildId = translationBuild.Id; await hubContext.NotifyBuildProgress( sfProjectId, - new ServalBuildState + new ServalBuildState { BuildId = buildId, State = nameof(ServalData.PreTranslationsRetrieved) } + ); + + if (!string.IsNullOrEmpty(buildId)) + { + string? draftGenerationRequestId = await GetDraftGenerationRequestIdForBuildAsync( + sfProjectId, + buildId + ); + if (!string.IsNullOrEmpty(draftGenerationRequestId)) { - BuildId = translationBuild.Id, - State = nameof(ServalData.PreTranslationsRetrieved), + Activity.Current?.AddTag( + MachineProjectService.DraftGenerationRequestIdKey, + draftGenerationRequestId + ); } - ); + } // Return the build id - return translationBuild.Id; + return buildId; } } catch (TaskCanceledException e) when (e.InnerException is not TimeoutException) @@ -1933,6 +1966,7 @@ public async Task StartBuildAsync(string curUserId, string sfProjectId, Cancella curUserId, new BuildConfig { ProjectId = sfProjectId }, false, + null, CancellationToken.None ), null, @@ -1958,6 +1992,8 @@ public async Task StartPreTranslationBuildAsync( CancellationToken cancellationToken ) { + string draftGenerationRequestId = ObjectId.GenerateNewId().ToString(); + Activity.Current?.AddTag(MachineProjectService.DraftGenerationRequestIdKey, draftGenerationRequestId); // Load the project from the realtime service await using IConnection conn = await realtimeService.ConnectAsync(curUserId); IDocument projectDoc = await conn.FetchAsync(buildConfig.ProjectId); @@ -2036,7 +2072,14 @@ .. projectDoc.Data.TranslateConfig.DraftConfig.DraftingSources.Select(s => s.Pro // so that the interceptor functions for BuildProjectAsync(). jobId = backgroundJobClient.ContinueJobWith( jobId, - r => r.BuildProjectForBackgroundJobAsync(curUserId, buildConfig, true, CancellationToken.None) + r => + r.BuildProjectForBackgroundJobAsync( + curUserId, + buildConfig, + true, + draftGenerationRequestId, + CancellationToken.None + ) ); // Set the pre-translation queued date and time, and hang fire job id @@ -2637,6 +2680,31 @@ CancellationToken cancellationToken return project; } + /// + /// Gets the SF-specific draft generation request identifier for a build by looking up the BuildProjectAsync event. + /// + /// The draft generation request identifier, or null if not found. + private async Task GetDraftGenerationRequestIdForBuildAsync(string sfProjectId, string servalBuildId) + { + // BuildProjectAsync events serve as a record of what Serval build id corresponds to what draft generation + // request id. + const int lookupTimeframeDays = 14; + DateTime startDate = DateTime.UtcNow.AddDays(-lookupTimeframeDays); + QueryResults buildProjectEvents = await eventMetricService.GetEventMetricsAsync( + projectId: sfProjectId, + scopes: [EventScope.Drafting], + eventTypes: [nameof(MachineProjectService.BuildProjectAsync)], + fromDate: startDate + ); + EventMetric? buildEvent = buildProjectEvents.Results.FirstOrDefault(e => e.Result?.ToString() == servalBuildId); + return ( + buildEvent?.Tags?.TryGetValue(MachineProjectService.DraftGenerationRequestIdKey, out BsonValue? requestId) + == true + ) + ? requestId?.AsString + : null; + } + private async Task GetTranslationIdAsync( string sfProjectId, bool preTranslate, diff --git a/src/SIL.XForge.Scripture/Services/MachineProjectService.cs b/src/SIL.XForge.Scripture/Services/MachineProjectService.cs index 7f3ad1a8ea4..820871a2191 100644 --- a/src/SIL.XForge.Scripture/Services/MachineProjectService.cs +++ b/src/SIL.XForge.Scripture/Services/MachineProjectService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.IO.Compression; @@ -54,6 +55,11 @@ public class MachineProjectService( IRepository userSecrets ) : IMachineProjectService { + /// + /// The name of the key storing draft generation request id, which may be present in some event metrics in a set of tags. + /// + public static readonly string DraftGenerationRequestIdKey = "draftGenerationRequestId"; + // Supported translation engines (Serval 1.2 format) // Serval 1.2 accepts the translation engine type in 1.1 (PascalCase) and 1.2 (kebab-case) format internal const string Echo = "echo"; @@ -106,6 +112,9 @@ public async Task AddSmtProjectAsync(string sfProjectId, CancellationTok /// The current user identifier. /// The build configuration. /// If true use NMT; otherwise if false use SMT. + /// + /// The draft generation request identifier (NMT only). Pass null for SMT builds. + /// /// The cancellation token. /// An asynchronous task. /// @@ -115,12 +124,18 @@ public async Task BuildProjectForBackgroundJobAsync( string curUserId, BuildConfig buildConfig, bool preTranslate, + string? draftGenerationRequestId, CancellationToken cancellationToken ) { + if (!string.IsNullOrEmpty(draftGenerationRequestId)) + { + Activity.Current?.AddTag(DraftGenerationRequestIdKey, draftGenerationRequestId); + } + try { - await BuildProjectAsync(curUserId, buildConfig, preTranslate, cancellationToken); + await BuildProjectAsync(curUserId, buildConfig, preTranslate, draftGenerationRequestId, cancellationToken); } catch (TaskCanceledException e) when (e.InnerException is not TimeoutException) { @@ -575,8 +590,11 @@ await projectDoc.SubmitJson0OpAsync(op => /// The current user identifier. /// The build configuration. /// If true use NMT; otherwise if false use SMT. + /// + /// The draft generation request identifier (NMT only). Pass null for SMT builds. + /// /// The cancellation token. - /// An asynchronous task. + /// Serval build ID /// The project or project secret could not be found. /// The language of the source project was not specified. /// @@ -593,9 +611,15 @@ public virtual async Task BuildProjectAsync( string curUserId, BuildConfig buildConfig, bool preTranslate, + string? draftGenerationRequestId, CancellationToken cancellationToken ) { + if (!string.IsNullOrEmpty(draftGenerationRequestId)) + { + Activity.Current?.AddTag(DraftGenerationRequestIdKey, draftGenerationRequestId); + } + // Load the target project secrets, so we can get the translation engine ID if ( !(await projectSecrets.TryGetAsync(buildConfig.ProjectId, cancellationToken)).TryResult( diff --git a/src/SIL.XForge.Scripture/Services/SyncService.cs b/src/SIL.XForge.Scripture/Services/SyncService.cs index c78f0ecf7b5..976526dc9fc 100644 --- a/src/SIL.XForge.Scripture/Services/SyncService.cs +++ b/src/SIL.XForge.Scripture/Services/SyncService.cs @@ -198,6 +198,7 @@ await projectSecrets.UpdateAsync( syncConfig.UserId, new BuildConfig { ProjectId = syncConfig.ProjectId }, false, + null, CancellationToken.None ), null, diff --git a/src/SIL.XForge/DataAccess/MemoryRepository.cs b/src/SIL.XForge/DataAccess/MemoryRepository.cs index e1895f4df09..4109df60ecd 100644 --- a/src/SIL.XForge/DataAccess/MemoryRepository.cs +++ b/src/SIL.XForge/DataAccess/MemoryRepository.cs @@ -92,6 +92,9 @@ public void Replace(T entity) public void SetOps(string id, Op[] ops) => _entityOps[id] = ops; + /// + /// Note that 32-bit integers will come back marked as 64-bit integers. + /// public IQueryable Query() => _entities.Select(kvp => DeserializeEntity(kvp.Key, kvp.Value)).AsQueryable(); public Task InsertAsync(T entity, CancellationToken _ = default) diff --git a/src/SIL.XForge/EventMetrics/EventMetric.cs b/src/SIL.XForge/EventMetrics/EventMetric.cs index 2b2968fe843..3dfe1cb1019 100644 --- a/src/SIL.XForge/EventMetrics/EventMetric.cs +++ b/src/SIL.XForge/EventMetrics/EventMetric.cs @@ -36,7 +36,7 @@ public class EventMetric : IIdentifiable public string Id { get; set; } = string.Empty; /// - /// Gets or sets the event payload. + /// Gets or sets the event payload, which contains the arguments given to the method call being recorded. /// /// /// If you are querying by projectId or userId, that will be done here. @@ -73,4 +73,12 @@ public class EventMetric : IIdentifiable /// Gets or sets the event user identifier. /// public string? UserId { get; set; } + + /// + /// Additional event metadata. For example, from items in an Activity Tags. + /// + /// + /// Keys should be normalized to lowerCamelCase. + /// + public Dictionary? Tags { get; set; } } diff --git a/src/SIL.XForge/EventMetrics/EventMetricLogger.cs b/src/SIL.XForge/EventMetrics/EventMetricLogger.cs index f5cbff354da..ae027c4ea1f 100644 --- a/src/SIL.XForge/EventMetrics/EventMetricLogger.cs +++ b/src/SIL.XForge/EventMetrics/EventMetricLogger.cs @@ -56,6 +56,9 @@ public void Intercept(IInvocation invocation) is LogEventMetricAttribute logEventMetricAttribute ) { + // Start an activity so additional information can be logged via tags + Activity activity = new Activity("log_event_metric").Start(); + // Invoke the method, then record its event metrics Task task; Stopwatch stopwatch = Stopwatch.StartNew(); @@ -68,21 +71,21 @@ is LogEventMetricAttribute logEventMetricAttribute task = methodTask.ContinueWith(t => { stopwatch.Stop(); - return SaveEventMetricAsync(stopwatch.Elapsed, t.Exception); + return SaveEventMetricAsync(stopwatch.Elapsed, activity, t.Exception); }); } else { // Save the event metric in another thread after the method has executed stopwatch.Stop(); - task = Task.Run(() => SaveEventMetricAsync(stopwatch.Elapsed)); + task = Task.Run(() => SaveEventMetricAsync(stopwatch.Elapsed, activity)); } } catch (Exception e) { // Save the error in the event metric, as the Proceed() will have faulted stopwatch.Stop(); - task = Task.Run(() => SaveEventMetricAsync(stopwatch.Elapsed, e)); + task = Task.Run(() => SaveEventMetricAsync(stopwatch.Elapsed, activity, e)); // Notify observers of the task of immediate completion TaskStarted?.Invoke(task); @@ -97,7 +100,11 @@ is LogEventMetricAttribute logEventMetricAttribute // Run as a separate task so we do not slow down the method execution // Unless we want the return value, in which case we will not write the metric until the method returns - async Task SaveEventMetricAsync(TimeSpan executionTime, Exception? exception = null) + async Task SaveEventMetricAsync( + TimeSpan executionTime, + Activity currentActivity, + Exception? exception = null + ) { string methodName = invocation.Method.Name; try @@ -194,6 +201,10 @@ await eventMetricService.SaveEventMetricAsync( // Just log any errors rather than throwing logger.LogError(e, "Error logging event metric for {methodName}", methodName); } + finally + { + currentActivity.Dispose(); + } } } diff --git a/src/SIL.XForge/Services/EventMetricService.cs b/src/SIL.XForge/Services/EventMetricService.cs index 66231dd57b6..9472954bd76 100644 --- a/src/SIL.XForge/Services/EventMetricService.cs +++ b/src/SIL.XForge/Services/EventMetricService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -94,14 +95,35 @@ public async Task SaveEventMetricAsync( { // Process the arguments into a MongoDB format for the payload // We should not save the cancellation token as it is not user data - var payload = new Dictionary(); + Dictionary payload = []; foreach (var kvp in argumentsWithNames.Where(kvp => kvp.Value is not CancellationToken)) { payload[kvp.Key] = GetBsonValue(kvp.Value); } + // Collect tags from Activity.Current and all parent activities + // Child activity tags override parent tags with the same key + Dictionary collectedTags = []; + + // Walk up the activity chain collecting tags (child first, so child overrides parent) + Activity activity = Activity.Current; + while (activity is not null) + { + foreach (KeyValuePair tag in activity.TagObjects) + { + collectedTags.TryAdd(tag.Key, tag.Value); + } + activity = activity.Parent; + } + + Dictionary? tags = null; + if (collectedTags.Count > 0) + { + tags = collectedTags.ToDictionary(kvp => kvp.Key, kvp => GetBsonValue(kvp.Value)); + } + // Generate the event metric - var eventMetric = new EventMetric + EventMetric eventMetric = new() { Id = ObjectId.GenerateNewId().ToString(), EventType = eventType, @@ -110,6 +132,7 @@ public async Task SaveEventMetricAsync( ProjectId = projectId, Scope = scope, UserId = userId, + Tags = tags, }; // Do not set Result if it is null, or the document will contain "result: null" diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs index 8e614fd9b94..e060d23790c 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Text; using System.Threading; @@ -42,8 +43,8 @@ public class MachineApiServiceTests private const string Project01 = "project01"; private const string Project02 = "project02"; private const string Project03 = "project03"; - private const string Build01 = "build01"; - private const string Build02 = "build02"; + private const string ServalBuildId01 = "build01"; + private const string ServalBuildId02 = "build02"; private const string ParallelCorpusId01 = "parallelCorpusId01"; private const string TranslationEngine01 = "translationEngine01"; private const string TrainingDataId01 = "trainingDataId01"; @@ -54,11 +55,14 @@ public class MachineApiServiceTests private const string ParatextUserId01 = "paratextUser01"; private const string Segment = "segment"; private const string TargetSegment = "targetSegment"; - private const string JobId = "jobId"; + private const string HangfireJobId = "jobId"; private const string Data01 = "data01"; - private const string JsonPayload = - """{"event":"TranslationBuildFinished","payload":{"build":{"id":"65f0c455682bb17bc4066917","url":"/api/v1/translation/engines/translationEngine01/builds/65f0c455682bb17bc4066917"},"engine":{"id":"translationEngine01","url":"/api/v1/translation/engines/translationEngine01"},"buildState":"Completed","dateFinished":"2024-03-12T21:14:10.789Z"}}"""; + private readonly string JsonPayload = + """{"event":"TranslationBuildFinished","payload":{"build":{"id":"ServalBuildId01","url":"/api/v1/translation/engines/translationEngine01/builds/ServalBuildId01"},"engine":{"id":"translationEngine01","url":"/api/v1/translation/engines/translationEngine01"},"buildState":"Completed","dateFinished":"2024-03-12T21:14:10.789Z"}}""".Replace( + "ServalBuildId01", + ServalBuildId01 + ); private const string TestUsfm = "\\c 1 \\v 1 Verse 1"; private const string TestUsx = @@ -101,7 +105,7 @@ public class MachineApiServiceTests private static readonly TranslationBuild CompletedTranslationBuild = new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = 0, @@ -439,6 +443,7 @@ public async Task BuildCompletedAsync_EventMetricInvalid() { // Set up test environment var env = new TestEnvironment(); + env.SetEmptyDraftGenerationMetricAssociations(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns( Task.FromResult( @@ -450,7 +455,7 @@ public async Task BuildCompletedAsync_EventMetricInvalid() { EventType = nameof(MachineProjectService.BuildProjectAsync), ProjectId = Project01, - Result = new BsonString(Build01), + Result = new BsonString(ServalBuildId01), Scope = EventScope.Drafting, UserId = null, }, @@ -463,7 +468,7 @@ public async Task BuildCompletedAsync_EventMetricInvalid() // SUT await env.Service.BuildCompletedAsync( Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -475,13 +480,14 @@ public async Task BuildCompletedAsync_EventMetricMissing() { // Set up test environment var env = new TestEnvironment(); + env.SetEmptyDraftGenerationMetricAssociations(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(QueryResults.Empty)); // SUT await env.Service.BuildCompletedAsync( Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -494,13 +500,14 @@ public async Task BuildCompletedAsync_Exception() // Set up test environment var env = new TestEnvironment(); ServalApiException ex = ServalApiExceptions.Forbidden; + env.SetEmptyDraftGenerationMetricAssociations(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .ThrowsAsync(ex); // SUT await env.Service.BuildCompletedAsync( Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -513,13 +520,14 @@ public async Task BuildCompletedAsync_Success() { // Set up test environment var env = new TestEnvironment(); + env.SetEmptyDraftGenerationMetricAssociations(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(env.GetEventMetricsForBuildCompleted(true))); // SUT await env.Service.BuildCompletedAsync( Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -528,7 +536,7 @@ await env .SendBuildCompletedEmailAsync( User01, Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -539,13 +547,14 @@ public async Task BuildCompletedAsync_UserDidNotRequestEmail() { // Set up test environment var env = new TestEnvironment(); + env.SetEmptyDraftGenerationMetricAssociations(); env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns(Task.FromResult(env.GetEventMetricsForBuildCompleted(false))); // SUT await env.Service.BuildCompletedAsync( Project01, - Build01, + ServalBuildId01, nameof(JobState.Completed), env.HttpRequestAccessor.SiteRoot ); @@ -560,6 +569,42 @@ await env ); } + [Test] + public async Task BuildCompletedAsync_AddsDraftGenerationRequestIdTag() + { + // Set up test environment + var env = new TestEnvironment(); + const string draftGenerationRequestId = "1234"; + env.SetDraftGenerationMetricAssociation(draftGenerationRequestId); + // Mock for the BuildCompletedAsync email check + env.EventMetricService.GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(env.GetEventMetricsForBuildCompleted(false))); + + Activity? activity = null; + using (new Activity("TestActivity").Start()) + { + // SUT + await env.Service.BuildCompletedAsync( + Project01, + ServalBuildId01, + nameof(JobState.Completed), + env.HttpRequestAccessor.SiteRoot + ); + + activity = Activity.Current; + + // Verify the Activity has the draftGenerationRequestId tag + Assert.IsNotNull(activity, "Activity.Current should be set during execution"); + Assert.IsTrue( + activity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" + ); + } + } + [Test] public void CancelPreTranslationBuildAsync_NoPermission() { @@ -623,7 +668,7 @@ public async Task CancelPreTranslationBuildAsync_NoTranslationEngineAndJobQueued env.Service.CancelPreTranslationBuildAsync(User01, Project01, CancellationToken.None) ); - env.BackgroundJobClient.Received(1).ChangeState(JobId, Arg.Any(), null); // Same as Delete() + env.BackgroundJobClient.Received(1).ChangeState(HangfireJobId, Arg.Any(), null); // Same as Delete() Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationQueuedAt); } @@ -635,17 +680,48 @@ public async Task CancelPreTranslationBuildAsync_Success() var env = new TestEnvironment(); await env.QueueBuildAsync(Project01, preTranslate: true, dateTime: DateTime.UtcNow); env.ConfigureTranslationBuild(); - + env.SetEmptyDraftGenerationMetricAssociations(); // SUT string actual = await env.Service.CancelPreTranslationBuildAsync(User01, Project01, CancellationToken.None); - Assert.AreEqual(Build01, actual); + Assert.AreEqual(ServalBuildId01, actual); await env.TranslationEnginesClient.Received(1).CancelBuildAsync(TranslationEngine01, CancellationToken.None); - env.BackgroundJobClient.Received(1).ChangeState(JobId, Arg.Any(), null); // Same as Delete() + env.BackgroundJobClient.Received(1).ChangeState(HangfireJobId, Arg.Any(), null); // Same as Delete() Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationQueuedAt); } + [Test] + public async Task CancelPreTranslationBuildAsync_AddsDraftGenerationRequestIdTag() + { + // Set up test environment + var env = new TestEnvironment(); + await env.QueueBuildAsync(Project01, preTranslate: true, dateTime: DateTime.UtcNow); + env.ConfigureTranslationBuild(); + const string draftGenerationRequestId = "2345"; + env.SetDraftGenerationMetricAssociation(draftGenerationRequestId); + Activity? activity = null; + using (new Activity("TestActivity").Start()) + { + // SUT + string actual = await env.Service.CancelPreTranslationBuildAsync(User01, Project01, CancellationToken.None); + + activity = Activity.Current; + + Assert.AreEqual(ServalBuildId01, actual); + + // Verify the Activity has the draftGenerationRequestId tag + Assert.IsNotNull(activity, "Activity.Current should be set during execution"); + Assert.IsTrue( + activity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" + ); + } + } + [Test] public void CalculateSignature_Success() { @@ -653,8 +729,11 @@ public void CalculateSignature_Success() var env = new TestEnvironment(); const string expected = "sha256=8C8E8C11165F748AFC6621F1DB213F79CE52759757D9BD6382C94E92C5B31063"; + const string ExamplePayload = + """{"event":"TranslationBuildFinished","payload":{"build":{"id":"65f0c455682bb17bc4066917","url":"/api/v1/translation/engines/translationEngine01/builds/65f0c455682bb17bc4066917"},"engine":{"id":"translationEngine01","url":"/api/v1/translation/engines/translationEngine01"},"buildState":"Completed","dateFinished":"2024-03-12T21:14:10.789Z"}}"""; + // SUT - string actual = env.Service.CalculateSignature(JsonPayload); + string actual = env.Service.CalculateSignature(ExamplePayload); Assert.AreEqual(expected, actual); } @@ -743,20 +822,51 @@ public async Task ExecuteWebhook_Success() // Set up test environment var env = new TestEnvironment(); string signature = env.Service.CalculateSignature(JsonPayload); - + env.SetEmptyDraftGenerationMetricAssociations(); // SUT await env.Service.ExecuteWebhookAsync(JsonPayload, signature); // Two jobs: BuildCompletedAsync & RetrievePreTranslationStatusAsync env.BackgroundJobClient.Received(2).Create(Arg.Any(), Arg.Any()); } + [Test] + public async Task ExecuteWebhook_RecordsDraftGenerationRequestId() + { + // Set up test environment + var env = new TestEnvironment(); + string signature = env.Service.CalculateSignature(JsonPayload); + const string draftGenerationRequestId = "3456"; + env.SetDraftGenerationMetricAssociation(draftGenerationRequestId); + Activity? activity = null; + using (new Activity("TestActivity").Start()) + { + // SUT + await env.Service.ExecuteWebhookAsync(JsonPayload, signature); + activity = Activity.Current; + // Verify the Activity has the draftGenerationRequestId tag + Assert.IsNotNull(activity, "Activity.Current should be set during execution"); + Assert.IsTrue( + activity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" + ); + } + } + [Test] public void GetBuildAsync_BuildEnded() { // Set up test environment var env = new TestEnvironment(); const int minRevision = 0; - env.TranslationEnginesClient.GetBuildAsync(TranslationEngine01, Build01, minRevision, CancellationToken.None) + env.TranslationEnginesClient.GetBuildAsync( + TranslationEngine01, + ServalBuildId01, + minRevision, + CancellationToken.None + ) .Throws(ServalApiExceptions.NotFound); // SUT @@ -764,7 +874,7 @@ public void GetBuildAsync_BuildEnded() env.Service.GetBuildAsync( User01, Project01, - Build01, + ServalBuildId01, minRevision, preTranslate: false, isServalAdmin: false, @@ -778,14 +888,14 @@ public async Task GetBuildAsync_NoBuildRunning() { // Set up test environment var env = new TestEnvironment(); - env.TranslationEnginesClient.GetBuildAsync(TranslationEngine01, Build01, null, CancellationToken.None) + env.TranslationEnginesClient.GetBuildAsync(TranslationEngine01, ServalBuildId01, null, CancellationToken.None) .Throws(ServalApiExceptions.TimeOut); // SUT ServalBuildDto? actual = await env.Service.GetBuildAsync( User01, Project01, - Build01, + ServalBuildId01, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -806,7 +916,7 @@ public void GetBuildAsync_NoPermission() env.Service.GetBuildAsync( User02, Project01, - Build01, + ServalBuildId01, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -826,7 +936,7 @@ public void GetBuildAsync_NoProject() env.Service.GetBuildAsync( User01, "invalid_project_id", - Build01, + ServalBuildId01, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -846,7 +956,7 @@ public void GetBuildAsync_NoTranslationEngine() env.Service.GetBuildAsync( User01, Project03, - Build01, + ServalBuildId01, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -866,7 +976,7 @@ public async Task GetBuildAsync_ServalAdminDoesNotNeedPermission() ServalBuildDto? actual = await env.Service.GetBuildAsync( User02, Project01, - Build01, + ServalBuildId01, minRevision: null, preTranslate: true, isServalAdmin: true, @@ -887,7 +997,7 @@ public async Task GetBuildAsync_Success() ServalBuildDto? actual = await env.Service.GetBuildAsync( User01, Project01, - Build01, + ServalBuildId01, minRevision: null, preTranslate: false, isServalAdmin: false, @@ -1323,7 +1433,7 @@ public async Task GetBuildsAsync_SuccessWithEventMetrics() }, }, ProjectId = Project01, - Result = new BsonString(Build01), + Result = new BsonString(ServalBuildId01), Scope = EventScope.Drafting, }, new EventMetric @@ -1331,7 +1441,7 @@ public async Task GetBuildsAsync_SuccessWithEventMetrics() EventType = nameof(MachineApiService.RetrievePreTranslationStatusAsync), Payload = { { "sfProjectId", Project01 } }, ProjectId = Project01, - Result = new BsonString(Build01), + Result = new BsonString(ServalBuildId01), Scope = EventScope.Drafting, }, ], @@ -1775,7 +1885,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_NoCompletedBuild() TranslationBuild translationBuild = new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = { Id = "engineId", Url = "https://example.com" }, Message = string.Empty, Progress = 0, @@ -1876,6 +1986,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_Success() const double percentCompleted = 0; const int revision = 43; const JobState state = JobState.Completed; + env.SetEmptyDraftGenerationMetricAssociations(); env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) .Returns( Task.FromResult>( @@ -1883,7 +1994,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_Success() new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = percentCompleted, @@ -1908,7 +2019,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_Success() new TranslationBuild { Url = "https://example.com", - Id = Build02, + Id = ServalBuildId02, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, State = JobState.Active, }, @@ -1947,7 +2058,7 @@ await env.Projects.UpdateAsync( new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = percentCompleted, @@ -1995,6 +2106,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_NullScriptureRange_Su const double percentCompleted = 0; const int revision = 43; const JobState state = JobState.Completed; + env.SetEmptyDraftGenerationMetricAssociations(); env.TranslationEnginesClient.GetAllBuildsAsync(TranslationEngine01, CancellationToken.None) .Returns( Task.FromResult>( @@ -2002,7 +2114,7 @@ public async Task GetLastCompletedPreTranslationBuildAsync_NullScriptureRange_Su new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = percentCompleted, @@ -2067,7 +2179,7 @@ public async Task GetLastPreTranslationBuildAsync_LatestByDateFinished_Success() TranslationBuild completedEarlier = new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = 100, @@ -2078,7 +2190,7 @@ public async Task GetLastPreTranslationBuildAsync_LatestByDateFinished_Success() TranslationBuild completedLater = new TranslationBuild { Url = "https://example.com", - Id = Build02, + Id = ServalBuildId02, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = 100, @@ -2097,8 +2209,8 @@ public async Task GetLastPreTranslationBuildAsync_LatestByDateFinished_Success() ); Assert.IsNotNull(actual); - Assert.AreEqual(MachineApi.GetBuildHref(Project01, Build02), actual.Href); - Assert.AreEqual(Build02, actual.Id.Split('.')[1]); + Assert.AreEqual(MachineApi.GetBuildHref(Project01, ServalBuildId02), actual.Href); + Assert.AreEqual(ServalBuildId02, actual.Id.Split('.')[1]); } [Test] @@ -2381,7 +2493,7 @@ public async Task GetPreTranslationRevisionsAsync_NoOps() new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = new ResourceLink { Id = "engineId", Url = "https://example.com" }, Message = MachineApiService.BuildStateCompleted, Progress = 0, @@ -2448,7 +2560,7 @@ public async Task GetPreTranslationRevisionsAsync_ServalAdminDoesNotNeedPermissi // Set up test environment var env = new TestEnvironment(); env.SetupEventMetrics("EXO", "GEN", DateTime.UtcNow.AddMinutes(-30)); - string[] buildIds = [Build01, Build02]; + string[] buildIds = [ServalBuildId01, ServalBuildId02]; env.TranslationEnginesClient.GetAllBuildsAsync(Arg.Any(), CancellationToken.None) .Returns( Task.FromResult>( @@ -2501,7 +2613,7 @@ public async Task GetPreTranslationRevisionsAsync_Success() // Set up test environment var env = new TestEnvironment(); env.SetupEventMetrics("EXO", "GEN", DateTime.UtcNow.AddMinutes(-30)); - string[] buildIds = [Build01, Build02]; + string[] buildIds = [ServalBuildId01, ServalBuildId02]; env.TranslationEnginesClient.GetAllBuildsAsync(Arg.Any(), CancellationToken.None) .Returns( Task.FromResult>( @@ -3717,6 +3829,46 @@ public async Task RetrievePreTranslationStatusAsync_UpdatesPreTranslationStatusI await env.Service.Received().UpdatePreTranslationTextDocumentsAsync(Project01, CancellationToken.None); } + [Test] + public async Task RetrievePreTranslationStatusAsync_SetsDraftGenerationRequestId() + { + // Set up test environment with a completed build + var env = new TestEnvironment(); + env.ConfigureTranslationBuild( + new TranslationBuild + { + Id = ServalBuildId01, + State = JobState.Completed, + Pretranslate = + [ + new PretranslateCorpus { SourceFilters = [new ParallelCorpusFilter { ScriptureRange = "MAT" }] }, + ], + } + ); + env.Service.Configure() + .UpdatePreTranslationTextDocumentsAsync(Project01, CancellationToken.None) + .Returns(Task.CompletedTask); + const string draftGenerationRequestId = "1234"; + env.SetDraftGenerationMetricAssociation(draftGenerationRequestId); + Activity? activity = null; + using (new Activity("TestActivity").Start()) + { + // SUT + await env.Service.RetrievePreTranslationStatusAsync(Project01, CancellationToken.None); + + activity = Activity.Current; + // Verify the Activity has the draftGenerationRequestId tag + Assert.IsNotNull(activity, "Activity.Current should be set during execution"); + Assert.IsTrue( + activity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" + ); + } + } + [Test] public async Task IsLanguageSupportedAsync_LanguageNotSupported() { @@ -3929,7 +4081,7 @@ public async Task StartBuildAsync_Success() await env.ProjectService.Received(1).SyncAsync(User01, Project01); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project01).ServalData!.TranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project01).ServalData!.TranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project01).ServalData?.TranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData?.TranslationErrorMessage); } @@ -3951,7 +4103,7 @@ await env .SyncService.Received(1) .SyncAsync(Arg.Is(s => s.ProjectId == Project03 && s.TargetOnly && s.UserId == User01)); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationErrorMessage); } @@ -3988,7 +4140,7 @@ await env .SyncService.Received(1) .SyncAsync(Arg.Is(s => s.ProjectId == Project01 && s.TargetOnly && s.UserId == User01)); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationErrorMessage); } @@ -4040,7 +4192,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.ProjectService.Received(1).SyncAsync(User01, Project01); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project01).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData?.PreTranslationErrorMessage); Assert.IsEmpty(env.Projects.Get(Project01).TranslateConfig.DraftConfig.LastSelectedTrainingScriptureRanges); @@ -4077,7 +4229,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.ProjectService.Received(1).SyncAsync(User01, Project01); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project01).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData?.PreTranslationErrorMessage); Assert.AreEqual( @@ -4141,7 +4293,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.SyncService.Received(1).SyncAsync(Arg.Any()); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); project = env.Projects.Get(Project02); @@ -4166,7 +4318,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.SyncService.Received(1).SyncAsync(Arg.Any()); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); project = env.Projects.Get(Project02); @@ -4208,7 +4360,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.ProjectService.Received(1).SyncAsync(User01, Project02); await env.SyncService.Received(1).SyncAsync(Arg.Any()); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationErrorMessage); } @@ -4245,7 +4397,7 @@ await env.Service.StartPreTranslationBuildAsync( await env.ProjectService.Received(1).SyncAsync(User01, Project02); await env.SyncService.DidNotReceive().SyncAsync(Arg.Any()); env.BackgroundJobClient.Received(1).Create(Arg.Any(), Arg.Any()); - Assert.AreEqual(JobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); + Assert.AreEqual(HangfireJobId, env.ProjectSecrets.Get(Project02).ServalData!.PreTranslationJobId); Assert.IsNotNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationQueuedAt); Assert.IsNull(env.ProjectSecrets.Get(Project02).ServalData?.PreTranslationErrorMessage); } @@ -4630,7 +4782,7 @@ private class TestEnvironment public TestEnvironment() { BackgroundJobClient = Substitute.For(); - BackgroundJobClient.Create(Arg.Any(), Arg.Any()).Returns(JobId); + BackgroundJobClient.Create(Arg.Any(), Arg.Any()).Returns(HangfireJobId); DeltaUsxMapper = Substitute.For(); EventMetricService = Substitute.For(); ExceptionHandler = Substitute.For(); @@ -4736,14 +4888,14 @@ public TestEnvironment() .HasRight(Arg.Any(), User01, SFProjectDomain.Drafts, Operation.Create) .Returns(true); ProjectService = Substitute.For(); - ProjectService.SyncAsync(User01, Arg.Any()).Returns(Task.FromResult(JobId)); + ProjectService.SyncAsync(User01, Arg.Any()).Returns(Task.FromResult(HangfireJobId)); RealtimeService = new SFMemoryRealtimeService(); RealtimeService.AddRepository("sf_projects", OTType.Json0, Projects); RealtimeService.AddRepository("text_documents", OTType.Json0, TextDocuments); RealtimeService.AddRepository("texts", OTType.RichText, Texts); ServalOptions = Options.Create(new ServalOptions { WebhookSecret = "this_is_a_secret" }); SyncService = Substitute.For(); - SyncService.SyncAsync(Arg.Any()).Returns(Task.FromResult(JobId)); + SyncService.SyncAsync(Arg.Any()).Returns(Task.FromResult(HangfireJobId)); TranslationEnginesClient = Substitute.For(); TranslationEnginesClient .GetAsync(TranslationEngine01, CancellationToken.None) @@ -5086,7 +5238,7 @@ public TranslationBuild ConfigureTranslationBuild(TranslationBuild? translationB translationBuild ??= new TranslationBuild { Url = "https://example.com", - Id = Build01, + Id = ServalBuildId01, Engine = { Id = "engineId", Url = "https://example.com" }, Message = message, Progress = percentCompleted, @@ -5132,7 +5284,7 @@ public QueryResults GetEventMetricsForBuildCompleted(bool sendEmail }, }, ProjectId = Project01, - Result = new BsonString(Build01), + Result = new BsonString(ServalBuildId01), Scope = EventScope.Drafting, UserId = User01, }, @@ -5153,7 +5305,7 @@ await ProjectSecrets.UpdateAsync( { if (preTranslate) { - u.Set(p => p.ServalData.PreTranslationJobId, JobId); + u.Set(p => p.ServalData.PreTranslationJobId, HangfireJobId); u.Set(p => p.ServalData.PreTranslationQueuedAt, dateTime); u.Set(p => p.ServalData.PreTranslationsRetrieved, preTranslationsRetrieved); if (string.IsNullOrWhiteSpace(errorMessage)) @@ -5167,7 +5319,7 @@ await ProjectSecrets.UpdateAsync( } else { - u.Set(p => p.ServalData.TranslationJobId, JobId); + u.Set(p => p.ServalData.TranslationJobId, HangfireJobId); u.Set(p => p.ServalData.TranslationQueuedAt, dateTime); if (string.IsNullOrWhiteSpace(errorMessage)) { @@ -5233,7 +5385,7 @@ public void SetupEventMetrics( DateTime requestedDateTime ) { - string[] buildIds = [Build01, Build02]; + string[] buildIds = [ServalBuildId01, ServalBuildId02]; EventMetricService .GetEventMetricsAsync(Project01, Arg.Any(), Arg.Any()) .Returns( @@ -5298,5 +5450,60 @@ public static void AssertCoreBuildProperties(TranslationBuild translationBuild, Assert.AreEqual(Project01, actual.Engine.Id); Assert.AreEqual(MachineApi.GetEngineHref(Project01), actual.Engine.Href); } + + public void SetEmptyDraftGenerationMetricAssociations() + { + // Mock for GetEventMetricsAsync in GetDraftGenerationRequestIdForBuildAsync + EventMetricService + .GetEventMetricsAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any() + ) + .Returns(Task.FromResult(QueryResults.Empty)); + } + + public void SetDraftGenerationMetricAssociation(string draftGenerationRequestId) + { + // Mock the event metrics service to return a build event with draftGenerationRequestId tag + // This is for GetDraftGenerationRequestIdForBuildAsync + EventMetricService + .GetEventMetricsAsync( + Arg.Any(), + Arg.Is(s => s != null && s.Contains(EventScope.Drafting)), + Arg.Is(t => t.Contains(nameof(Services.MachineProjectService.BuildProjectAsync))), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any() + ) + .Returns( + Task.FromResult( + new QueryResults + { + Results = + [ + new EventMetric + { + EventType = nameof(Services.MachineProjectService.BuildProjectAsync), + Result = ServalBuildId01, + Tags = new Dictionary + { + { + Services.MachineProjectService.DraftGenerationRequestIdKey, + draftGenerationRequestId + }, + }, + }, + ], + UnpagedCount = 1, + } + ) + ); + } } } diff --git a/test/SIL.XForge.Scripture.Tests/Services/MachineProjectServiceTests.cs b/test/SIL.XForge.Scripture.Tests/Services/MachineProjectServiceTests.cs index 826dcc9692d..6a9162f3e08 100644 --- a/test/SIL.XForge.Scripture.Tests/Services/MachineProjectServiceTests.cs +++ b/test/SIL.XForge.Scripture.Tests/Services/MachineProjectServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.IO.Compression; using System.Linq; @@ -111,7 +112,13 @@ public async Task BuildProjectForBackgroundJobAsync_DoesNotRecordBuildInProgress ServalApiException ex = ServalApiExceptions.BuildInProgress; var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -125,6 +132,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -143,7 +151,13 @@ public async Task BuildProjectForBackgroundJobAsync_DoesNotRecordBuildInProgress ServalApiException ex = ServalApiExceptions.BuildInProgress; var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: false, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: false, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // An SMT translation job has been queued @@ -157,6 +171,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: false, + draftGenerationRequestId: null, CancellationToken.None ); @@ -175,7 +190,13 @@ public async Task BuildProjectForBackgroundJobAsync_DoesNotRecordTaskCancellatio var ex = new TaskCanceledException(); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -186,6 +207,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -202,7 +224,13 @@ public async Task BuildProjectForBackgroundJobAsync_DoesNotRecordTaskCancellatio var ex = new TaskCanceledException(); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: false, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: false, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // An SMT translation job has been queued @@ -213,6 +241,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: false, + draftGenerationRequestId: null, CancellationToken.None ); @@ -229,7 +258,13 @@ public async Task BuildProjectForBackgroundJobAsync_RecordsDataNotFoundException var ex = new DataNotFoundException("project not found"); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // SUT @@ -237,6 +272,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -252,7 +288,13 @@ public async Task BuildProjectForBackgroundJobAsync_MissingDirectoryDataNotFound var ex = new DataNotFoundException("directory not found"); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // SUT @@ -260,6 +302,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -274,7 +317,13 @@ public async Task BuildProjectForBackgroundJobAsync_InvalidDataException() var ex = new InvalidDataException("Source project language not specified"); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -288,6 +337,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -306,7 +356,13 @@ public async Task BuildProjectForBackgroundJobAsync_RecordsErrors() ServalApiException ex = ServalApiExceptions.Forbidden; var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -320,6 +376,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -339,7 +396,13 @@ public async Task BuildProjectForBackgroundJobAsync_RecordsErrorsForSmt() ServalApiException ex = ServalApiExceptions.Forbidden; var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: false, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: false, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // An SMT translation job has been queued @@ -353,6 +416,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: false, + draftGenerationRequestId: null, CancellationToken.None ); @@ -371,7 +435,13 @@ public async Task BuildProjectForBackgroundJobAsync_RunsBuildProjectAsync() var env = new TestEnvironment(); var buildConfig = new BuildConfig { ProjectId = Project01 }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .Returns(Task.FromResult(Build01)); // SUT @@ -379,12 +449,19 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); await env .Service.Received(1) - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None); + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ); } [Test] @@ -395,7 +472,13 @@ public async Task BuildProjectForBackgroundJobAsync_SendsEmailForBuildInProgress ServalApiException ex = ServalApiExceptions.BuildInProgress; var buildConfig = new BuildConfig { ProjectId = Project01, SendEmailOnBuildFinished = true }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -409,6 +492,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -423,7 +507,13 @@ public async Task BuildProjectForBackgroundJobAsync_SendsEmailForTaskCancellatio var ex = new TaskCanceledException(); var buildConfig = new BuildConfig { ProjectId = Project01, SendEmailOnBuildFinished = true }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -437,6 +527,7 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); @@ -451,7 +542,13 @@ public async Task BuildProjectForBackgroundJobAsync_SendsEmailForUnexpectedError var ex = new NotSupportedException(); var buildConfig = new BuildConfig { ProjectId = Project01, SendEmailOnBuildFinished = true }; env.Service.Configure() - .BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) .ThrowsAsync(ex); // A pre-translation job has been queued @@ -465,12 +562,58 @@ await env.Service.BuildProjectForBackgroundJobAsync( User01, buildConfig, preTranslate: true, + draftGenerationRequestId: null, CancellationToken.None ); await env.EmailService.Received().SendEmailAsync(Arg.Any(), Arg.Any(), Arg.Any()); } + [Test] + public async Task BuildProjectForBackgroundJobAsync_AddsDraftGenerationRequestIdTag() + { + // Set up test environment + var env = new TestEnvironment(); + var buildConfig = new BuildConfig { ProjectId = Project01 }; + const string draftGenerationRequestId = "5678"; + + // Mock BuildProjectAsync to return successfully + env.Service.Configure() + .BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId, + cancellationToken: CancellationToken.None + ) + .Returns(Task.FromResult(Build01)); + + Activity? activity = null; + using (new Activity("TestActivity").Start()) + { + // SUT + await env.Service.BuildProjectForBackgroundJobAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId, + CancellationToken.None + ); + + activity = Activity.Current; + + // Verify the Activity has the draftGenerationRequestId tag + Assert.IsNotNull(activity, "Activity.Current should be set during execution"); + Assert.IsTrue( + activity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" + ); + } + } + [Test] public async Task BuildProjectAsync_PreTranslationBuild() { @@ -520,7 +663,13 @@ public async Task BuildProjectAsync_PreTranslationBuild() .Returns(translationBuildConfig); // SUT - await env.Service.BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None); + await env.Service.BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationJobId); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.PreTranslationQueuedAt); await env @@ -568,7 +717,13 @@ public async Task BuildProjectAsync_SmtTranslationBuild() .Returns(Task.FromResult>([])); // SUT - await env.Service.BuildProjectAsync(User01, buildConfig, preTranslate: false, CancellationToken.None); + await env.Service.BuildProjectAsync( + User01, + buildConfig, + preTranslate: false, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.TranslationJobId); Assert.IsNull(env.ProjectSecrets.Get(Project01).ServalData!.TranslationQueuedAt); await env @@ -589,6 +744,7 @@ public async Task BuildProjectAsync_ThrowsExceptionWhenProjectMissing() User01, new BuildConfig { ProjectId = Project01 }, preTranslate: false, + draftGenerationRequestId: null, CancellationToken.None ) ); @@ -607,6 +763,7 @@ public async Task BuildProjectAsync_ThrowsExceptionWhenProjectSecretMissing() User01, new BuildConfig { ProjectId = Project01 }, preTranslate: false, + draftGenerationRequestId: null, CancellationToken.None ) ); @@ -624,8 +781,79 @@ public void BuildProjectAsync_ThrowsExceptionWhenSourceProjectMissing() User01, new BuildConfig { ProjectId = Project04 }, preTranslate: false, + draftGenerationRequestId: null, + CancellationToken.None + ) + ); + } + + [Test] + public async Task BuildProjectAsync_CreatesActivityWithDraftGenerationRequestId() + { + var env = new TestEnvironment(); + await env.SetupProjectSecretAsync(Project01, new ServalData { PreTranslationEngineId = TranslationEngine01 }); + var buildConfig = new BuildConfig { ProjectId = Project01 }; + const string draftGenerationRequestId = "test-draft-generation-request-id"; + + env.Service.Configure() + .RemoveLegacyServalDataAsync(Project01, preTranslate: true, CancellationToken.None) + .Returns(Task.CompletedTask); + env.Service.Configure() + .EnsureTranslationEngineExistsAsync( + User01, + Arg.Any>(), + Arg.Any(), + preTranslate: true, + useEcho: false, + CancellationToken.None + ) + .Returns(Task.FromResult(TranslationEngine01)); + env.Service.Configure() + .RecreateOrUpdateTranslationEngineIfRequiredAsync( + TranslationEngine01, + Arg.Any(), + preTranslate: true, + useEcho: false, CancellationToken.None ) + .Returns(Task.CompletedTask); + env.Service.Configure() + .SyncProjectCorporaAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .Returns(Task.FromResult>([])); + + Activity? capturedActivity = null; + env.Service.Configure() + .SyncProjectCorporaAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + .Returns(callInfo => + { + // Capture the Activity during execution to verify it has the tag + capturedActivity = Activity.Current; + return Task.FromResult>([]); + }); + + // Create an Activity for the test (simulating what EventMetricLogger would do) + using var activity = new Activity("test-activity"); + activity.Start(); + + // SUT + await env.Service.BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId, + CancellationToken.None + ); + + activity.Stop(); + + // Verify the Activity was created with the draftGenerationRequestId tag + Assert.IsNotNull(capturedActivity, "Activity.Current should have been set during execution"); + Assert.IsTrue( + capturedActivity!.TagObjects.Any(t => + t.Key == MachineProjectService.DraftGenerationRequestIdKey + && t.Value?.ToString() == draftGenerationRequestId + ), + "Activity should contain draftGenerationRequestId tag with correct value" ); } @@ -664,7 +892,13 @@ public async Task BuildProjectAsync_ThrowsExceptionWhenServalDataMissing() // SUT Assert.ThrowsAsync(() => - env.Service.BuildProjectAsync(User01, buildConfig, preTranslate: true, CancellationToken.None) + env.Service.BuildProjectAsync( + User01, + buildConfig, + preTranslate: true, + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None + ) ); } @@ -684,7 +918,8 @@ await env.Service.BuildProjectAsync( User01, new BuildConfig { ProjectId = Project02 }, preTranslate: true, - CancellationToken.None + draftGenerationRequestId: null, + cancellationToken: CancellationToken.None ); await env .TranslationEnginesClient.Received() diff --git a/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs b/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs index d3c9b1950ab..b960bb0cb81 100644 --- a/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs +++ b/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Threading; @@ -8,8 +9,10 @@ using MongoDB.Bson; using MongoDB.Driver; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using NSubstitute; using NUnit.Framework; +using SIL.Linq; using SIL.XForge.DataAccess; using SIL.XForge.EventMetrics; using SIL.XForge.Models; @@ -393,8 +396,164 @@ public async Task SaveEventMetricAsync_ArraysOfObjects() Assert.AreEqual(BsonNull.Value, eventMetric.Result); } + [Test] + public async Task SaveEventMetricAsync_ActivityTags() + { + var env = new TestEnvironment(); + Dictionary argumentsWithNames = new Dictionary + { + { "projectId", Project01 }, + { "userId", User01 }, + }; + + // Set up Activity tags + using var activity = new Activity("test"); + activity.AddTag(TestEnvironment.SomeActivityTagKey, "abc-123-def-456"); + activity.AddTag("someOtherThing", "xyz-789"); + activity.AddTag("someNumber", 42); + activity.Start(); + + // SUT + await env.Service.SaveEventMetricAsync(Project01, User01, EventType01, EventScope01, argumentsWithNames); + + activity.Stop(); + + EventMetric eventMetric = env.EventMetrics.Query().OrderByDescending(e => e.TimeStamp).First(); + Assert.IsNotNull(eventMetric.Tags); + Assert.AreEqual(3, eventMetric.Tags.Count); + Assert.AreEqual("abc-123-def-456", eventMetric.Tags[TestEnvironment.SomeActivityTagKey]?.AsString); + Assert.AreEqual("xyz-789", eventMetric.Tags["someOtherThing"]?.AsString); + Assert.AreEqual(42, eventMetric.Tags["someNumber"]?.AsInt64); + } + + [Test] + public async Task SaveEventMetricAsync_ActivityTags_FromParentActivity() + { + var env = new TestEnvironment(); + Dictionary argumentsWithNames = new Dictionary { { "projectId", Project01 } }; + + // Set up parent activity with tags + using var parentActivity = new Activity("parent"); + parentActivity.AddTag(TestEnvironment.SomeActivityTagKey, "parent-123"); + parentActivity.AddTag("parentTag", "parent-value"); + parentActivity.AddTag("someNumber", 42); + parentActivity.Start(); + + // Create child activity (simulating what EventMetricLogger does) + using var childActivity = new Activity("child"); + childActivity.AddTag("childTag", "child-value"); + childActivity.AddTag(TestEnvironment.SomeActivityTagKey, "child-override-456"); // Override parent + childActivity.AddTag("someNumber", 99); // Different than parent value. + childActivity.Start(); + + // SUT - should collect tags from both parent and child + await env.Service.SaveEventMetricAsync(Project01, User01, EventType01, EventScope01, argumentsWithNames); + + childActivity.Stop(); + parentActivity.Stop(); + + // Verify the saved event metric + EventMetric eventMetric = env.EventMetrics.Query().OrderByDescending(e => e.TimeStamp).First(); + Assert.IsNotNull(eventMetric.Tags); + Assert.AreEqual(4, eventMetric.Tags.Count); + + // Child tag should be present + Assert.AreEqual("child-value", eventMetric.Tags["childTag"]?.AsString); + + // Parent tag should be present + Assert.AreEqual("parent-value", eventMetric.Tags["parentTag"]?.AsString); + + // Child should override parent for same key + Assert.AreEqual("child-override-456", eventMetric.Tags[TestEnvironment.SomeActivityTagKey]?.AsString); + Assert.AreEqual(99, eventMetric.Tags["someNumber"]?.AsInt64); + } + + [Test] + public async Task SaveEventMetricAsync_ActivityTags_VariousTypes() + { + var env = new TestEnvironment(); + Dictionary argumentsWithNames = new Dictionary { { "projectId", Project01 } }; + + // Suppose various types of object are added as tags. They should be stored and converted as expected. + + const bool boolean = true; + DateTime dateAndTime = DateTime.UtcNow; + const decimal decimalNumber = 12.34M; + const double doubleFloat = 56.78; + const int integer = 1234; + const long longInteger = 5678L; + const float singleFloat = 90.12F; + string[] stringArray = ["string1", "string2"]; + Uri uri = new Uri("https://example.com", UriKind.Absolute); + + TestClass someObject = new() { Records = [new TestRecord { ProjectId = Project01, UserId = User01 }] }; + string someObjectAsJson = JsonConvert.SerializeObject(someObject); + + JObject someJObject = JObject.FromObject(someObject); + string someJObjectAsJson = JsonConvert.SerializeObject(someJObject); + + string serializedJsonInput = someObjectAsJson.ToString(); + string serializedJsonOutput = serializedJsonInput.ToString(); + + Dictionary items = new Dictionary + { + { "projectId", Project01 }, + { "userId", User01 }, + { "boolean", boolean }, + { "dateAndTime", dateAndTime }, + { "decimalNumber", decimalNumber }, + { "doubleFloat", doubleFloat }, + { "integer", integer }, + { "longInteger", longInteger }, + { "singleFloat", singleFloat }, + { "stringArray", stringArray }, + { "uri", uri }, + { "nullValue", null }, + { "someObject", someObject }, + { "someJObject", someJObject }, + { "someJsonString", serializedJsonInput }, + }; + Dictionary expectBsonItems = new Dictionary + { + { "projectId", BsonValue.Create(Project01) }, + { "userId", BsonValue.Create(User01) }, + { "boolean", BsonBoolean.Create(boolean) }, + { "dateAndTime", BsonDateTime.Create(dateAndTime) }, + { "decimalNumber", BsonString.Create(decimalNumber.ToString(CultureInfo.InvariantCulture)) }, // Decimals are stored as strings in JSON + { "doubleFloat", BsonDouble.Create(doubleFloat) }, + { "integer", BsonInt64.Create(integer) }, // Note it is marked as 64-bit + { "longInteger", BsonInt64.Create(longInteger) }, + { "singleFloat", BsonDouble.Create(singleFloat) }, + { "stringArray", BsonArray.Create(stringArray) }, + { "uri", BsonValue.Create(uri.ToString()) }, + { "nullValue", BsonNull.Value }, + { "someObject", BsonDocument.Parse(someObjectAsJson) }, + { "someJObject", BsonDocument.Parse(someJObjectAsJson) }, + { "someJsonString", BsonString.Create(serializedJsonOutput) }, + }; + + using var parentActivity = new Activity("parent"); + items.ForEach(kvp => parentActivity.AddTag(kvp.Key, kvp.Value)); + parentActivity.Start(); + using var childActivity = new Activity("child"); + childActivity.Start(); + + // SUT - Should store tag objects, to be later fetched + await env.Service.SaveEventMetricAsync(Project01, User01, EventType01, EventScope01, argumentsWithNames); + childActivity.Stop(); + parentActivity.Stop(); + + EventMetric eventMetric = env.EventMetrics.Query().OrderByDescending(e => e.TimeStamp).First(); + Assert.IsNotNull(eventMetric.Tags); + Assert.AreEqual(expectBsonItems.Count, eventMetric.Tags.Count); + expectBsonItems.ForEach(kvp => Assert.AreEqual(kvp.Value, eventMetric.Tags[kvp.Key])); + } + private class TestEnvironment { + /// Example name of an activity tag key. + public static readonly string SomeActivityTagKey = "someImportantIdKey"; + public TestEnvironment() { EventMetrics = new MemoryRepository(