refactor(scorecard): scorecard metric loading logic

Signed-off-by: Ihor Mykhno <imykhno@redhat.com>

Author: imykhno

workspaces/scorecard/.changeset/free-rice-pick.md renamed to workspaces/scorecard/.changeset/olive-months-cross.md

@@ -6,4 +6,8 @@
 '@red-hat-developer-hub/backstage-plugin-scorecard': minor
 ---
 
-Adds database persistence and scheduled metric collection.
+Adds database persistence and scheduled metric collection
+
+- **BREAKING**: The supportsEntity function renamed to getCatalogFilter. The getCatalogFilter function not have parameters are not passed
+- Implement scheduler to load metrics by provider
+- Implement scheduler to cleanup saved metrics

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/scheduler.test.ts renamed to workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.test.ts

@@ -25,19 +25,9 @@ import type { Config } from '@backstage/config';
 import { CLEANUP_EXPIRED_METRICS_ID } from './constants';
 import { CleanupExpiredMetricsTask } from './tasks/CleanupExpiredMetricsTask';
 import { PullMetricsByProviderTask } from './tasks/PullMetricsByProviderTask';
-import { SchedulerTask } from './types';
+import { SchedulerOptions, SchedulerTask } from './types';
 import { DatabaseMetricValues } from '../database/DatabaseMetricValues';
 
-export interface SchedulerOptions {
-  auth: AuthService;
-  catalog: CatalogService;
-  config: Config;
-  logger: LoggerService;
-  scheduler: SchedulerService;
-  database: DatabaseMetricValues;
-  metricProvidersRegistry: MetricProvidersRegistry;
-}
-
 export class Scheduler {
   private readonly auth: AuthService;
   private readonly catalog: CatalogService;
@@ -91,12 +81,12 @@ export class Scheduler {
     this.tasks = [
       {
         name: CLEANUP_EXPIRED_METRICS_ID,
-        task: new CleanupExpiredMetricsTask(
-          this.scheduler,
-          this.logger,
-          this.database,
-          this.config,
-        ),
+        task: new CleanupExpiredMetricsTask({
+          scheduler: this.scheduler,
+          logger: this.logger,
+          database: this.database,
+          config: this.config,
+        }),
       },
     ];
   }
@@ -108,12 +98,14 @@ export class Scheduler {
       this.tasks.push({
         name: provider.getProviderId(),
         task: new PullMetricsByProviderTask(
-          this.scheduler,
-          this.logger,
-          this.database,
-          this.config,
-          this.catalog,
-          this.auth,
+          {
+            scheduler: this.scheduler,
+            logger: this.logger,
+            database: this.database,
+            config: this.config,
+            catalog: this.catalog,
+            auth: this.auth,
+          },
           provider,
         ),
       });

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/index.ts

@@ -53,12 +53,12 @@ describe('CleanupExpiredMetricsTask', () => {
       mockTaskRunner as any,
     );
 
-    task = new CleanupExpiredMetricsTask(
-      mockScheduler,
-      mockLogger,
-      mockDatabase,
-      mockConfig,
-    );
+    task = new CleanupExpiredMetricsTask({
+      scheduler: mockScheduler,
+      logger: mockLogger,
+      database: mockDatabase,
+      config: mockConfig,
+    });
   });
 
   afterEach(() => {
@@ -132,20 +132,6 @@ describe('CleanupExpiredMetricsTask', () => {
         'Deleted 12 expired metrics older than 2 days',
       );
     });
-
-    it('should log error message when cleanup expired metrics fails', async () => {
-      const error = new Error('DB is not responding');
-
-      mockDatabase.cleanupExpiredMetrics.mockRejectedValue(error);
-      await expect(
-        (task as any).cleanupExpiredMetrics(mockLogger),
-      ).rejects.toThrow(error.message);
-
-      expect(mockLogger.error).toHaveBeenCalledWith(
-        'Failed to cleanup expired metrics:',
-        error,
-      );
-    });
   });
 
   describe('getDataRetentionDays', () => {

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/CleanupExpiredMetricsTask.test.ts

@@ -25,22 +25,34 @@ import { daysToMilliseconds } from './utils';
 import type { Config } from '@backstage/config';
 import { SchedulerTask } from '../types';
 import { DatabaseMetricValues } from '../../database/DatabaseMetricValues';
+import { SchedulerOptions } from '../types';
+
+type Options = Pick<
+  SchedulerOptions,
+  'scheduler' | 'logger' | 'database' | 'config'
+>;
 
 export class CleanupExpiredMetricsTask implements SchedulerTask {
+  private readonly config: Config;
+  private readonly logger: LoggerService;
+  private readonly scheduler: SchedulerService;
+  private readonly database: DatabaseMetricValues;
+
   private static readonly CLEANUP_SCHEDULE: SchedulerServiceTaskScheduleDefinition =
     {
       frequency: { days: 1 },
       timeout: { minutes: 2 },
       initialDelay: { seconds: 3 },
     };
+
   private static readonly DEFAULT_DATA_RETENTION_DAYS = 365;
 
-  constructor(
-    private readonly scheduler: SchedulerService,
-    private readonly logger: LoggerService,
-    private readonly database: DatabaseMetricValues,
-    private readonly config: Config,
-  ) {}
+  constructor(options: Options) {
+    this.config = options.config;
+    this.logger = options.logger;
+    this.scheduler = options.scheduler;
+    this.database = options.database;
+  }
 
   async start(): Promise<void> {
     const taskRunner = this.scheduler.createScheduledTaskRunner(
@@ -71,15 +83,10 @@ export class CleanupExpiredMetricsTask implements SchedulerTask {
       Date.now() - daysToMilliseconds(dataRetentionDays),
     );
 
-    try {
-      const deletedCount = await this.database.cleanupExpiredMetrics(olderThan);
-      logger.info(
-        `Deleted ${deletedCount} expired metrics older than ${dataRetentionDays} days`,
-      );
-    } catch (error) {
-      logger.error(`Failed to cleanup expired metrics:`, error);
-      throw error;
-    }
+    const deletedCount = await this.database.cleanupExpiredMetrics(olderThan);
+    logger.info(
+      `Deleted ${deletedCount} expired metrics older than ${dataRetentionDays} days`,
+    );
   }
 
   private getDataRetentionDays(): number {

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/CleanupExpiredMetricsTask.ts

@@ -68,12 +68,14 @@ describe('PullMetricsByProviderTask', () => {
     );
 
     task = new PullMetricsByProviderTask(
-      mockScheduler,
-      mockLogger,
-      mockDatabase,
-      mockConfig,
-      mockCatalog,
-      mockAuth,
+      {
+        scheduler: mockScheduler,
+        logger: mockLogger,
+        database: mockDatabase,
+        config: mockConfig,
+        catalog: mockCatalog,
+        auth: mockAuth,
+      },
       mockProvider,
     );
   });

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.test.ts

@@ -28,29 +28,41 @@ import { MetricProvider } from '@red-hat-developer-hub/backstage-plugin-scorecar
 import { v4 as uuid } from 'uuid';
 import { stringifyEntityRef } from '@backstage/catalog-model';
 import { DbMetricValue } from '../../database/types';
-import { SchedulerTask } from '../types';
+import { SchedulerOptions, SchedulerTask } from '../types';
+
+type Options = Pick<
+  SchedulerOptions,
+  'scheduler' | 'logger' | 'database' | 'config' | 'catalog' | 'auth'
+>;
 
 export class PullMetricsByProviderTask implements SchedulerTask {
+  private readonly config: Config;
+  private readonly auth: AuthService;
   private readonly providerId: string;
+  private readonly logger: LoggerService;
+  private readonly catalog: CatalogService;
+  private readonly provider: MetricProvider;
+  private readonly scheduler: SchedulerService;
+  private readonly database: DatabaseMetricValues;
 
   private static readonly CATALOG_BATCH_SIZE = 50;
+
   private static readonly DEFAULT_SCHEDULE: SchedulerServiceTaskScheduleDefinition =
     {
       frequency: { hours: 1 },
       timeout: { minutes: 15 },
       initialDelay: { seconds: 3 },
     };
 
-  constructor(
-    private readonly scheduler: SchedulerService,
-    private readonly logger: LoggerService,
-    private readonly database: DatabaseMetricValues,
-    private readonly config: Config,
-    private readonly catalog: CatalogService,
-    private readonly auth: AuthService,
-    private readonly provider: MetricProvider,
-  ) {
-    this.providerId = this.provider.getProviderId();
+  constructor(options: Options, provider: MetricProvider) {
+    this.config = options.config;
+    this.auth = options.auth;
+    this.providerId = provider.getProviderId();
+    this.logger = options.logger;
+    this.catalog = options.catalog;
+    this.provider = provider;
+    this.scheduler = options.scheduler;
+    this.database = options.database;
   }
 
   async start(): Promise<void> {

workspaces/scorecard/plugins/scorecard-backend/src/scheduler/tasks/PullMetricsByProviderTask.ts

@@ -14,6 +14,26 @@
  * limitations under the License.
  */
 
+import type { Config } from '@backstage/config';
+import { CatalogService } from '@backstage/plugin-catalog-node';
+import { DatabaseMetricValues } from '../database/DatabaseMetricValues';
+import { MetricProvidersRegistry } from '../providers/MetricProvidersRegistry';
+import {
+  AuthService,
+  LoggerService,
+  SchedulerService,
+} from '@backstage/backend-plugin-api';
+
 export interface SchedulerTask {
   start(): Promise<void>;
 }
+
+export interface SchedulerOptions {
+  auth: AuthService;
+  catalog: CatalogService;
+  config: Config;
+  logger: LoggerService;
+  scheduler: SchedulerService;
+  database: DatabaseMetricValues;
+  metricProvidersRegistry: MetricProvidersRegistry;
+}