Skip to content

Commit 98e225d

Browse files
authored
refactor: add JobStatus value object replacing raw string comparisons (#3046)
Replace a few raw string status checks across x2a-common, x2a-node, x2a-backend, and x2a frontend with a flyweight JobStatus class following the Phase pattern from PR #3002. - Add JobStatus class with predicates: isActive(), isFinished(), isPending(), isRunning(), isSuccess(), isError(), isCancelled() - Replace ['pending','running'].includes() with isActive() - Replace 3-way || chains with isFinished() - Keep raw strings for DB writes and API responses (data, not logic) Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>
1 parent 2b30813 commit 98e225d

15 files changed

Lines changed: 415 additions & 69 deletions

File tree

workspaces/x2a/plugins/x2a-backend/src/router/collectArtifacts.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
import {
2525
MigrationPhase,
2626
Artifact,
27-
JobStatusEnum,
27+
JobStatus,
2828
Telemetry,
2929
Phase,
3030
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
@@ -335,20 +335,21 @@ async function processJobCompletion(
335335
logger: RouterDeps['logger'],
336336
job: JobWithToken,
337337
): Promise<{ message: string }> {
338-
let status: JobStatusEnum =
339-
validatedRequest.status === 'success' ? 'success' : 'error';
338+
let jobStatus = JobStatus.from(
339+
validatedRequest.status === 'success' ? 'success' : 'error',
340+
);
340341
let errorDetails = validatedRequest.errorDetails || null;
341342

342-
if (status === 'success') {
343-
status = await executePhaseActionsWithErrorHandling(
343+
if (jobStatus.isSuccess()) {
344+
jobStatus = await executePhaseActionsWithErrorHandling(
344345
phase,
345346
projectId,
346347
validatedRequest,
347348
x2aDatabase,
348349
logger,
349350
);
350351

351-
if (status === 'error') {
352+
if (jobStatus.isError()) {
352353
errorDetails = 'Phase actions failed';
353354
}
354355
}
@@ -357,7 +358,7 @@ async function processJobCompletion(
357358

358359
await x2aDatabase.updateJob({
359360
id: validatedRequest.jobId,
360-
status,
361+
status: jobStatus.value,
361362
finishedAt: new Date(),
362363
errorDetails,
363364
log: logs,
@@ -375,20 +376,20 @@ async function executePhaseActionsWithErrorHandling(
375376
validatedRequest: CollectArtifactsRequestBody,
376377
x2aDatabase: RouterDeps['x2aDatabase'],
377378
logger: RouterDeps['logger'],
378-
): Promise<JobStatusEnum> {
379+
): Promise<JobStatus> {
379380
try {
380381
await executePhaseActions(phase, {
381382
projectId,
382383
artifacts: validatedRequest.artifacts ?? [],
383384
x2aDatabase,
384385
logger,
385386
});
386-
return 'success';
387+
return JobStatus.SUCCESS;
387388
} catch (error) {
388389
logger.error(
389390
`Phase actions failed for job ${validatedRequest.jobId}: ${error instanceof Error ? error.message : String(error)}`,
390391
);
391-
return 'error';
392+
return JobStatus.ERROR;
392393
}
393394
}
394395

workspaces/x2a/plugins/x2a-backend/src/router/jobs.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { InputError, NotFoundError } from '@backstage/errors';
2020
import {
2121
ModulePhase,
2222
Job,
23+
JobStatus,
2324
Phase,
2425
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
2526

@@ -37,11 +38,7 @@ async function sendJobLogs(
3738
const { x2aDatabase, kubeService, logger } = deps;
3839
res.setHeader('Content-Type', 'text/plain; charset=utf-8');
3940

40-
if (
41-
job.status === 'success' ||
42-
job.status === 'error' ||
43-
job.status === 'cancelled'
44-
) {
41+
if (JobStatus.from(job.status).isFinished()) {
4542
logger.info(
4643
`Job ${job.id} is finished (status: ${job.status}), returning logs from database`,
4744
);

workspaces/x2a/plugins/x2a-backend/src/router/modules.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { InputError, NotFoundError } from '@backstage/errors';
2020

2121
import {
2222
type ModulePhase,
23+
JobStatus,
2324
Phase,
2425
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
2526

@@ -261,27 +262,25 @@ export function registerModuleRoutes(
261262
});
262263

263264
// Reconcile jobs that appear active against K8s
265+
const activeJobs = existingJobs.filter(job =>
266+
JobStatus.from(job.status).isActive(),
267+
);
264268
const reconciledJobs = await Promise.all(
265-
existingJobs
266-
.filter(job => ['pending', 'running'].includes(job.status))
267-
.map(job =>
268-
reconcileJobStatus(job, { kubeService, x2aDatabase, logger }),
269-
),
269+
activeJobs.map(job =>
270+
reconcileJobStatus(job, { kubeService, x2aDatabase, logger }),
271+
),
270272
);
271-
const hasActiveJob = reconciledJobs.some(job =>
272-
['pending', 'running'].includes(job.status),
273+
const activeJob = reconciledJobs.find(job =>
274+
JobStatus.from(job.status).isActive(),
273275
);
274276

275-
if (hasActiveJob) {
276-
const activeJob = existingJobs.find(job =>
277-
['pending', 'running'].includes(job.status),
278-
);
277+
if (activeJob) {
279278
return res.status(409).json({
280279
error: 'JobAlreadyRunning',
281-
message: `A ${activeJob!.phase} job is already running for this module`,
280+
message: `A ${activeJob.phase} job is already running for this module`,
282281
details: 'Please wait for the current job to complete or cancel it',
283-
activeJobId: activeJob!.id,
284-
activeJobPhase: activeJob!.phase,
282+
activeJobId: activeJob.id,
283+
activeJobPhase: activeJob.phase,
285284
});
286285
}
287286

@@ -328,7 +327,7 @@ export function registerModuleRoutes(
328327

329328
// Re-read the job to detect cancellation during the K8s creation window
330329
const freshJob = await x2aDatabase.getJob({ id: job.id });
331-
if (freshJob?.status === 'cancelled') {
330+
if (freshJob && JobStatus.from(freshJob.status).isCancelled()) {
332331
try {
333332
await kubeService.deleteJob(k8sJobName);
334333
} catch (e) {
@@ -422,7 +421,7 @@ export function registerModuleRoutes(
422421

423422
const job = jobs[0];
424423

425-
if (!['pending', 'running'].includes(job.status)) {
424+
if (!JobStatus.from(job.status).isActive()) {
426425
return res.status(409).json({
427426
error: 'JobNotCancellable',
428427
message: `The ${phase} job is in "${job.status}" state and cannot be cancelled.`,

workspaces/x2a/plugins/x2a-backend/src/router/projects.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import express from 'express';
1919
import { AuthorizeResult } from '@backstage/plugin-permission-common';
2020
import { InputError, NotAllowedError, NotFoundError } from '@backstage/errors';
2121
import {
22+
JobStatus,
2223
x2aAdminWritePermission,
2324
x2aUserPermission,
2425
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
@@ -209,7 +210,7 @@ export function registerProjectRoutes(
209210
// Cancel any active k8s jobs before deleting DB records
210211
const jobs = await x2aDatabase.listJobsForProject({ projectId });
211212
const activeJobs = jobs.filter(
212-
job => ['pending', 'running'].includes(job.status) && job.k8sJobName,
213+
job => JobStatus.from(job.status).isActive() && job.k8sJobName,
213214
);
214215
await Promise.all(
215216
activeJobs.map(job => {
@@ -308,16 +309,15 @@ export function registerProjectRoutes(
308309
// Check for existing running init job
309310
const existingJobs = await x2aDatabase.listJobsForProject({ projectId });
310311
const activeInitJobs = existingJobs.filter(
311-
job =>
312-
job.phase === 'init' && ['pending', 'running'].includes(job.status),
312+
job => job.phase === 'init' && JobStatus.from(job.status).isActive(),
313313
);
314314
const reconciledInitJobs = await Promise.all(
315315
activeInitJobs.map(job =>
316316
reconcileJobStatus(job, { kubeService, x2aDatabase, logger }),
317317
),
318318
);
319319
const hasActiveInitJob = reconciledInitJobs.some(job =>
320-
['pending', 'running'].includes(job.status),
320+
JobStatus.from(job.status).isActive(),
321321
);
322322

323323
if (hasActiveInitJob) {

workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/projectStatus.ts

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import {
1818
Job,
19+
JobStatus,
1920
Module,
2021
ProjectStatus,
2122
ProjectStatusState,
@@ -48,34 +49,39 @@ export function calculateProjectStatus(
4849
};
4950
}
5051

51-
const error = projectModules.filter(
52-
module => module.status === 'error',
53-
).length;
54-
const finished = projectModules.filter(
55-
module =>
56-
module.status === 'success' && module.publish?.status === 'success',
57-
).length;
58-
const waiting = projectModules.filter(
59-
module =>
60-
module.status === 'success' &&
61-
(!module.publish || module.publish.status === 'cancelled'),
62-
).length;
63-
const pending = projectModules.filter(
64-
module => module.status === 'pending',
52+
const modulesWithStatus = projectModules.map(module => ({
53+
module,
54+
status: module.status ? JobStatus.from(module.status) : undefined,
55+
publishStatus: module.publish?.status
56+
? JobStatus.from(module.publish.status)
57+
: undefined,
58+
}));
59+
60+
const error = modulesWithStatus.filter(m => m.status?.isError()).length;
61+
const finished = modulesWithStatus.filter(
62+
m => m.status?.isSuccess() && m.publishStatus?.isSuccess(),
6563
).length;
66-
const running = projectModules.filter(
67-
module => module.status === 'running',
64+
const waiting = modulesWithStatus.filter(
65+
m =>
66+
m.status?.isSuccess() &&
67+
(!m.module.publish || m.publishStatus?.isCancelled()),
6868
).length;
69-
const cancelled = projectModules.filter(
70-
module => module.status === 'cancelled',
69+
const pending = modulesWithStatus.filter(m => m.status?.isPending()).length;
70+
const running = modulesWithStatus.filter(m => m.status?.isRunning()).length;
71+
const cancelled = modulesWithStatus.filter(m =>
72+
m.status?.isCancelled(),
7173
).length;
7274

75+
const initStatus = initJob?.status
76+
? JobStatus.from(initJob.status)
77+
: undefined;
78+
7379
let state: ProjectStatusState;
7480
if (error > 0) {
7581
state = 'failed'; // At least one module is in error state
76-
} else if (['pending', 'running'].includes(initJob?.status ?? '')) {
82+
} else if (initStatus?.isActive()) {
7783
state = 'initializing'; // Project's init job is running or scheduling
78-
} else if (initJob?.status === 'success') {
84+
} else if (initStatus?.isSuccess()) {
7985
if (total > 0 && finished === total) {
8086
state = 'completed'; // All modules are in success state
8187
} else if (total === 0 || pending + cancelled === total) {

workspaces/x2a/plugins/x2a-common/report.api.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,50 @@ export interface Job {
139139
telemetry?: Telemetry;
140140
}
141141

142+
// @public (undocumented)
143+
export class JobStatus {
144+
// (undocumented)
145+
static activeStatuses(): readonly JobStatus[];
146+
// (undocumented)
147+
static all(): readonly JobStatus[];
148+
// (undocumented)
149+
static readonly CANCELLED: JobStatus;
150+
// (undocumented)
151+
equals(other: JobStatus): boolean;
152+
// (undocumented)
153+
static readonly ERROR: JobStatus;
154+
// (undocumented)
155+
static finishedStatuses(): readonly JobStatus[];
156+
// (undocumented)
157+
static from(raw: string): JobStatus;
158+
// (undocumented)
159+
isActive(): boolean;
160+
// (undocumented)
161+
isCancelled(): boolean;
162+
// (undocumented)
163+
isError(): boolean;
164+
// (undocumented)
165+
isFinished(): boolean;
166+
// (undocumented)
167+
isPending(): boolean;
168+
// (undocumented)
169+
isRunning(): boolean;
170+
// (undocumented)
171+
isSuccess(): boolean;
172+
// (undocumented)
173+
static readonly PENDING: JobStatus;
174+
// (undocumented)
175+
static readonly RUNNING: JobStatus;
176+
// (undocumented)
177+
static readonly SUCCESS: JobStatus;
178+
// (undocumented)
179+
toString(): string;
180+
// (undocumented)
181+
readonly value: JobStatusEnum;
182+
// (undocumented)
183+
static values(): readonly JobStatusEnum[];
184+
}
185+
142186
// @public (undocumented)
143187
export type JobStatusEnum = 'pending' | 'running' | 'success' | 'error' | 'cancelled';
144188

0 commit comments

Comments
 (0)