Skip to content

Commit dbcc8c6

Browse files
feat(#3529): move ThresholdConfig from MetricProvider to individual Metrics
Refactor the scorecard MetricProvider interface to move threshold configuration from provider-level methods to individual Metric objects. Core interface changes: - Add `threshold: ThresholdConfig` field to the `Metric` type - Remove `getMetricType()`, `getMetric()`, `getMetricThresholds()`, `calculateMetric()`, and `getMetricIds()` from MetricProvider - Make `getMetrics()` and `calculateMetrics()` required on MetricProvider - Consolidate single/batch provider code paths into batch-only Provider migrations: - All providers (dependabot, filecheck, github, jira, openssf, sonarqube) updated to the new interface with thresholds on metric objects - ThresholdResolver API: `resolveProviderThresholds(provider)` → `resolveMetricThresholds(metric, providerId)` - ThresholdResolver API: `resolveEntityThresholds(entity, provider)` → `resolveEntityThresholds(entity, metric, providerId)` - mergeEntityAndProviderThresholds signature updated similarly - PullMetricsByProviderTask consolidated to always use calculateMetrics() - MetricProvidersRegistry uses provider.getMetrics() for all operations All tests updated. API reports regenerated. Note: DatabaseMetricValues tests could not run (missing better-sqlite3 native binding in sandbox). All other 91 test suites (976 tests) passed. Closes #3529 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0186e63 commit dbcc8c6

41 files changed

Lines changed: 643 additions & 880 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.test.ts

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('DependabotMetricProvider', () => {
6969
});
7070
});
7171

72-
describe('getProviderId / getMetric', () => {
72+
describe('getProviderId / getMetrics', () => {
7373
it.each([
7474
['critical', 'dependabot.alerts_critical', 'Dependabot Critical Alerts'],
7575
['high', 'dependabot.alerts_high', 'Dependabot High Alerts'],
@@ -84,40 +84,21 @@ describe('DependabotMetricProvider', () => {
8484
severity,
8585
);
8686
expect(provider.getProviderId()).toBe(expectedId);
87-
const metric = provider.getMetric();
87+
const metrics = provider.getMetrics();
88+
expect(metrics).toHaveLength(1);
89+
const metric = metrics[0];
8890
expect(metric.id).toBe(expectedId);
8991
expect(metric.title).toBe(expectedTitle);
9092
expect(metric.description).toBe(
9193
DEPENDABOT_SEVERITY_METRIC[severity].description,
9294
);
9395
expect(metric.type).toBe('number');
96+
expect(metric.threshold).toEqual(DEPENDABOT_THRESHOLDS);
9497
expect(metric.history).toBe(true);
9598
},
9699
);
97100
});
98101

99-
describe('getMetricType', () => {
100-
it('returns number', () => {
101-
const provider = new DependabotMetricProvider(
102-
mockConfig,
103-
mockLogger,
104-
'critical',
105-
);
106-
expect(provider.getMetricType()).toBe('number');
107-
});
108-
});
109-
110-
describe('getMetricThresholds', () => {
111-
it('returns default thresholds', () => {
112-
const provider = new DependabotMetricProvider(
113-
mockConfig,
114-
mockLogger,
115-
'critical',
116-
);
117-
expect(provider.getMetricThresholds()).toEqual(DEPENDABOT_THRESHOLDS);
118-
});
119-
});
120-
121102
describe('getCatalogFilter', () => {
122103
it('requires project-slug and dependabot annotation value true', () => {
123104
const provider = new DependabotMetricProvider(
@@ -184,7 +165,7 @@ describe('DependabotMetricProvider', () => {
184165
);
185166
});
186167

187-
describe('calculateMetric', () => {
168+
describe('calculateMetrics', () => {
188169
it.each(['critical', 'high', 'medium', 'low'] as const)(
189170
'calls getAlerts with target from getEntitySourceLocation and returns count',
190171
async severity => {
@@ -196,9 +177,9 @@ describe('DependabotMetricProvider', () => {
196177
);
197178
const ent = entity();
198179

199-
const result = await provider.calculateMetric(ent);
180+
const results = await provider.calculateMetrics(ent);
200181

201-
expect(result).toBe(2);
182+
expect(results.get(provider.getProviderId())).toBe(2);
202183
// target comes from getEntitySourceLocation(entity), not hardcoded
203184
expect(mockGetAlerts).toHaveBeenCalledWith(
204185
'https://github.com/owner/repo',
@@ -219,7 +200,8 @@ describe('DependabotMetricProvider', () => {
219200
mockLogger,
220201
'critical',
221202
);
222-
expect(await provider.calculateMetric(entity())).toBe(0);
203+
const results = await provider.calculateMetrics(entity());
204+
expect(results.get(provider.getProviderId())).toBe(0);
223205
});
224206

225207
it('propagates errors when getAlerts fails', async () => {
@@ -230,7 +212,7 @@ describe('DependabotMetricProvider', () => {
230212
'critical',
231213
);
232214

233-
await expect(provider.calculateMetric(entity())).rejects.toThrow(
215+
await expect(provider.calculateMetrics(entity())).rejects.toThrow(
234216
'dependabot unavailable',
235217
);
236218
expect(mockGetAlerts).toHaveBeenCalledWith(

workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProvider.ts

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import {
18-
Metric,
19-
ThresholdConfig,
20-
} from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
17+
import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
2118
import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node';
2219
import type { LoggerService } from '@backstage/backend-plugin-api';
2320
import type { Config } from '@backstage/config';
@@ -63,23 +60,18 @@ export class DependabotMetricProvider implements MetricProvider<'number'> {
6360
return DEPENDABOT_SEVERITY_METRIC[this.severity].id;
6461
}
6562

66-
getMetricType(): 'number' {
67-
return 'number';
68-
}
69-
70-
getMetric(): Metric<'number'> {
63+
getMetrics(): Metric<'number'>[] {
7164
const meta = DEPENDABOT_SEVERITY_METRIC[this.severity];
72-
return {
73-
id: meta.id,
74-
title: meta.title,
75-
description: meta.description,
76-
type: this.getMetricType(),
77-
history: true,
78-
};
79-
}
80-
81-
getMetricThresholds(): ThresholdConfig {
82-
return DEPENDABOT_THRESHOLDS;
65+
return [
66+
{
67+
id: meta.id,
68+
title: meta.title,
69+
description: meta.description,
70+
type: 'number',
71+
threshold: DEPENDABOT_THRESHOLDS,
72+
history: true,
73+
},
74+
];
8375
}
8476

8577
getCatalogFilter(): Record<string, string | symbol | (string | symbol)[]> {
@@ -112,14 +104,16 @@ export class DependabotMetricProvider implements MetricProvider<'number'> {
112104
return { owner, repo };
113105
}
114106

115-
async calculateMetric(entity: Entity): Promise<number> {
107+
async calculateMetrics(entity: Entity): Promise<Map<string, number>> {
116108
const repository = this.getRepository(entity);
117109
const { target } = getEntitySourceLocation(entity);
118110
const alerts = await this.dependabotClient.getAlerts(
119111
target,
120112
repository,
121113
this.severity,
122114
);
123-
return alerts.length;
115+
const results = new Map<string, number>();
116+
results.set(this.getProviderId(), alerts.length);
117+
return results;
124118
}
125119
}

workspaces/scorecard/plugins/scorecard-backend-module-dependabot/src/metricProviders/DependabotMetricProviderFactory.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ describe('createDependabotMetricProvider', () => {
3636
);
3737
expect(provider.getProviderId()).toBe('dependabot.alerts_high');
3838
expect(provider.getProviderDatasourceId()).toBe('dependabot');
39-
expect(provider.getMetricType()).toBe('number');
40-
expect(provider.getMetricThresholds()).toBe(DEPENDABOT_THRESHOLDS);
39+
const metrics = provider.getMetrics();
40+
expect(metrics).toHaveLength(1);
41+
expect(metrics[0].type).toBe('number');
42+
expect(metrics[0].threshold).toBe(DEPENDABOT_THRESHOLDS);
4143
});
4244
});
4345

workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.test.ts

Lines changed: 10 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ describe('FilecheckMetricProvider', () => {
135135
);
136136

137137
expect(provider).toBeDefined();
138-
expect(provider?.getMetricIds()).toEqual([
138+
expect(provider?.getMetrics().map(m => m.id)).toEqual([
139139
'filecheck.readme',
140140
'filecheck.license',
141141
]);
@@ -254,22 +254,17 @@ describe('FilecheckMetricProvider', () => {
254254
expect(provider?.getProviderDatasourceId()).toBe('filecheck');
255255
});
256256

257-
it('should return correct metric type', () => {
258-
expect(provider?.getMetricType()).toBe('boolean');
259-
});
260-
261-
it('should return all metric IDs', () => {
262-
expect(provider?.getMetricIds()).toEqual([
257+
it('should return all metrics with correct type and thresholds', () => {
258+
const metrics = provider?.getMetrics();
259+
expect(metrics?.map(m => m.id)).toEqual([
263260
'filecheck.readme',
264261
'filecheck.codeowners',
265262
'filecheck.dockerfile',
266263
]);
267-
});
268-
269-
it('should return default file check thresholds', () => {
270-
expect(provider?.getMetricThresholds()).toEqual(
271-
DEFAULT_FILECHECK_THRESHOLDS,
272-
);
264+
metrics?.forEach(m => {
265+
expect(m.type).toBe('boolean');
266+
expect(m.threshold).toEqual(DEFAULT_FILECHECK_THRESHOLDS);
267+
});
273268
});
274269

275270
it('should return correct catalog filter', () => {
@@ -288,25 +283,15 @@ describe('FilecheckMetricProvider', () => {
288283
title: 'File: README.md',
289284
description: 'Checks if README.md exists in the repository.',
290285
type: 'boolean',
286+
threshold: DEFAULT_FILECHECK_THRESHOLDS,
291287
history: true,
292288
});
293289
expect(metrics?.[1]).toEqual({
294290
id: 'filecheck.codeowners',
295291
title: 'File: CODEOWNERS',
296292
description: 'Checks if CODEOWNERS exists in the repository.',
297293
type: 'boolean',
298-
history: true,
299-
});
300-
});
301-
302-
it('should return first metric for backward compatibility via getMetric()', () => {
303-
const metric = provider?.getMetric();
304-
305-
expect(metric).toEqual({
306-
id: 'filecheck.readme',
307-
title: 'File: README.md',
308-
description: 'Checks if README.md exists in the repository.',
309-
type: 'boolean',
294+
threshold: DEFAULT_FILECHECK_THRESHOLDS,
310295
history: true,
311296
});
312297
});
@@ -398,53 +383,6 @@ describe('FilecheckMetricProvider', () => {
398383
);
399384
});
400385

401-
it('should return first metric result for legacy calculateMetric()', async () => {
402-
const existingFiles = new Set(['README.md']);
403-
const mockUrlReader = createMockUrlReader(existingFiles);
404-
405-
const config = new ConfigReader({
406-
scorecard: {
407-
plugins: {
408-
filecheck: {
409-
files: { readme: 'README.md', license: 'LICENSE' },
410-
},
411-
},
412-
},
413-
});
414-
const provider = createFilecheckMetricProvider(
415-
config,
416-
mockUrlReader,
417-
createMockCacheService(),
418-
);
419-
420-
const result = await provider?.calculateMetric(mockEntity);
421-
422-
expect(result).toBe(true);
423-
});
424-
425-
it('should return false when metric result is not found in legacy calculateMetric()', async () => {
426-
const mockUrlReader = createMockUrlReader(new Set());
427-
428-
const config = new ConfigReader({
429-
scorecard: {
430-
plugins: {
431-
filecheck: {
432-
files: { readme: 'README.md' },
433-
},
434-
},
435-
},
436-
});
437-
const provider = createFilecheckMetricProvider(
438-
config,
439-
mockUrlReader,
440-
createMockCacheService(),
441-
);
442-
443-
const result = await provider?.calculateMetric(mockEntity);
444-
445-
expect(result).toBe(false);
446-
});
447-
448386
it('should handle bare repo source locations without branch ref', async () => {
449387
const { getEntitySourceLocation } = jest.requireMock(
450388
'@backstage/catalog-model',

workspaces/scorecard/plugins/scorecard-backend-module-filecheck/src/metricProviders/FilecheckMetricProvider.ts

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

1717
import type { Entity } from '@backstage/catalog-model';
1818
import { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client';
19-
import {
20-
Metric,
21-
ThresholdConfig,
22-
} from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
19+
import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
2320
import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecard-node';
2421
import { FilecheckClient } from '../clients/FilecheckClient';
2522
import {
@@ -44,32 +41,17 @@ export class FilecheckMetricProvider implements MetricProvider<'boolean'> {
4441
return 'filecheck';
4542
}
4643

47-
getMetricIds(): string[] {
48-
return this.filesConfig.files.map(f => `filecheck.${f.id}`);
49-
}
50-
51-
getMetricType(): 'boolean' {
52-
return 'boolean';
53-
}
54-
55-
getMetric(): Metric<'boolean'> {
56-
return this.getMetrics()[0];
57-
}
58-
5944
getMetrics(): Metric<'boolean'>[] {
6045
return this.filesConfig.files.map(f => ({
6146
id: `filecheck.${f.id}`,
6247
title: `File: ${f.path}`,
6348
description: `Checks if ${f.path} exists in the repository.`,
6449
type: 'boolean' as const,
50+
threshold: DEFAULT_FILECHECK_THRESHOLDS,
6551
history: true,
6652
}));
6753
}
6854

69-
getMetricThresholds(): ThresholdConfig {
70-
return DEFAULT_FILECHECK_THRESHOLDS;
71-
}
72-
7355
getCatalogFilter(): Record<string, string | symbol | (string | symbol)[]> {
7456
return {
7557
kind: 'component',
@@ -78,12 +60,6 @@ export class FilecheckMetricProvider implements MetricProvider<'boolean'> {
7860
};
7961
}
8062

81-
async calculateMetric(entity: Entity): Promise<boolean> {
82-
const results = await this.calculateMetrics(entity);
83-
const firstId = this.getMetricIds()[0];
84-
return results.get(firstId) ?? false;
85-
}
86-
8763
async calculateMetrics(entity: Entity): Promise<Map<string, boolean>> {
8864
const filePaths = this.filesConfig.files.map(f => f.path);
8965
const existsMap = await this.client.checkFiles(entity, filePaths);

workspaces/scorecard/plugins/scorecard-backend-module-github/src/metricProviders/GithubOpenPRsProvider.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ jest.mock('../github/GithubClient');
3131

3232
describe('GithubOpenPRsProvider', () => {
3333
describe('fromConfig', () => {
34-
it('should create provider with default thresholds', () => {
34+
it('should create provider with default thresholds on metric', () => {
3535
const provider = GithubOpenPRsProvider.fromConfig(new ConfigReader({}));
36-
37-
expect(provider.getMetricThresholds()).toEqual(DEFAULT_NUMBER_THRESHOLDS);
36+
const metrics = provider.getMetrics();
37+
expect(metrics).toHaveLength(1);
38+
expect(metrics[0].threshold).toEqual(DEFAULT_NUMBER_THRESHOLDS);
3839
});
3940
});
4041

41-
describe('calculateMetric', () => {
42+
describe('calculateMetrics', () => {
4243
let provider: GithubOpenPRsProvider;
4344
const mockedGithubClient = GithubClient as jest.MockedClass<
4445
typeof GithubClient
@@ -66,9 +67,9 @@ describe('GithubOpenPRsProvider', () => {
6667
},
6768
};
6869

69-
const result = await provider.calculateMetric(mockEntity);
70+
const results = await provider.calculateMetrics(mockEntity);
7071

71-
expect(result).toBe(42);
72+
expect(results.get('github.open_prs')).toBe(42);
7273
expect(
7374
mockedGithubClientInstance.getOpenPullRequestsCount,
7475
).toHaveBeenCalledWith('https://github.com/org/orgRepo/tree/main/', {

0 commit comments

Comments
 (0)