From 58ae7fb04a85f0633e2090319e807af395d254da Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 15:52:43 +0300 Subject: [PATCH 1/7] fix: S3. Change name generation from Date.now to UUID closes https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/462 --- src/static/aws/s3.service.ts | 3 ++- src/static/hdd/hdd.service.ts | 3 ++- src/static/utils.ts | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/static/aws/s3.service.ts b/src/static/aws/s3.service.ts index 4ff2a97..42a7a4c 100644 --- a/src/static/aws/s3.service.ts +++ b/src/static/aws/s3.service.ts @@ -4,6 +4,7 @@ import { Static } from '../static.interface'; import { DeleteObjectCommand, GetObjectCommand, PutObjectCommand, S3Client } from '@aws-sdk/client-s3'; import { Readable } from 'stream'; import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; +import { generateNewImageName } from '../utils'; export class AWSS3Service implements Static { private readonly logger: Logger = new Logger(AWSS3Service.name); @@ -26,7 +27,7 @@ export class AWSS3Service implements Static { } async saveImage(type: 'screenshot' | 'diff' | 'baseline', imageBuffer: Buffer): Promise { - const imageName = `${Date.now()}.${type}.png`; + const imageName = generateNewImageName(type); try { await this.s3Client.send( new PutObjectCommand({ diff --git a/src/static/hdd/hdd.service.ts b/src/static/hdd/hdd.service.ts index 09b7052..878f0af 100644 --- a/src/static/hdd/hdd.service.ts +++ b/src/static/hdd/hdd.service.ts @@ -5,12 +5,13 @@ import { PNG, PNGWithMetadata } from 'pngjs'; import uuidAPIKey from 'uuid-apikey'; import { Static } from '../static.interface'; import { HDD_IMAGE_PATH } from './constants'; +import { generateNewImageName } from '../utils'; export class HddService implements Static { private readonly logger: Logger = new Logger(HddService.name); generateNewImage(type: 'screenshot' | 'diff' | 'baseline'): { imageName: string; imagePath: string } { - const imageName = `${uuidAPIKey.create({ noDashes: true }).apiKey}.${type}.png`; + const imageName = generateNewImageName(type); return { imageName, imagePath: this.getImagePath(imageName), diff --git a/src/static/utils.ts b/src/static/utils.ts index a684b66..fea00bd 100644 --- a/src/static/utils.ts +++ b/src/static/utils.ts @@ -1,3 +1,5 @@ +import uuidAPIKey from 'uuid-apikey'; + export function isHddStaticServiceConfigured() { return !process.env.STATIC_SERVICE || process.env.STATIC_SERVICE === 'hdd'; } @@ -5,3 +7,7 @@ export function isHddStaticServiceConfigured() { export function isS3ServiceConfigured() { return !process.env.STATIC_SERVICE || process.env.STATIC_SERVICE === 's3'; } + +export function generateNewImageName(type: 'screenshot' | 'diff' | 'baseline'): string { + return`${uuidAPIKey.create({ noDashes: true }).apiKey}.${type}.png`; + } \ No newline at end of file From 109b94e98035101aff2004a698235746b5932080 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 15:55:30 +0300 Subject: [PATCH 2/7] Update hdd.service.ts --- src/static/hdd/hdd.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/static/hdd/hdd.service.ts b/src/static/hdd/hdd.service.ts index 878f0af..60ad363 100644 --- a/src/static/hdd/hdd.service.ts +++ b/src/static/hdd/hdd.service.ts @@ -2,7 +2,6 @@ import { Logger } from '@nestjs/common'; import path from 'path'; import { writeFileSync, readFileSync, unlink, mkdirSync, existsSync } from 'fs'; import { PNG, PNGWithMetadata } from 'pngjs'; -import uuidAPIKey from 'uuid-apikey'; import { Static } from '../static.interface'; import { HDD_IMAGE_PATH } from './constants'; import { generateNewImageName } from '../utils'; From 96a77ebc44db1b88410c0e5dd3ee108ce97c8fe0 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 15:57:33 +0300 Subject: [PATCH 3/7] Update utils.ts --- src/static/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/static/utils.ts b/src/static/utils.ts index fea00bd..310061d 100644 --- a/src/static/utils.ts +++ b/src/static/utils.ts @@ -9,5 +9,5 @@ export function isS3ServiceConfigured() { } export function generateNewImageName(type: 'screenshot' | 'diff' | 'baseline'): string { - return`${uuidAPIKey.create({ noDashes: true }).apiKey}.${type}.png`; - } \ No newline at end of file + return `${uuidAPIKey.create({ noDashes: true }).apiKey}.${type}.png`; +} From 0ca49ffc29480fb1205121758484a5508359df53 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 16:11:09 +0300 Subject: [PATCH 4/7] Update workflow.yml --- .github/workflows/workflow.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 38bb365..e9a9cc7 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -59,7 +59,7 @@ jobs: run: docker compose -f docker-compose.yml -f docker-compose.ldap.yml down - name: SonarCloud Scan - uses: SonarSource/sonarcloud-github-action@master + uses: SonarSource/sonarqube-scan-action@v5.2.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }} From 67a28c068bad7b2a975dbad6f476d3e9c75966e1 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 16:28:31 +0300 Subject: [PATCH 5/7] Update workflow.yml --- .github/workflows/workflow.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index e9a9cc7..806eb70 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -59,9 +59,8 @@ jobs: run: docker compose -f docker-compose.yml -f docker-compose.ldap.yml down - name: SonarCloud Scan - uses: SonarSource/sonarqube-scan-action@v5.2.0 + uses: SonarSource/sonarqube-scan-action@v5 env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }} test-migrations: From d2c3bfd83fe1c51abbaf70b97f536bf80e9b6727 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 21:02:45 +0300 Subject: [PATCH 6/7] fix: Add proper error message for Odiff with S3 closes https://github.com/Visual-Regression-Tracker/backend/issues/327 closes https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker/issues/458 --- docker-compose.yml | 8 +++- src/compare/compare.service.spec.ts | 38 +++++++++++++++++++ src/compare/compare.service.ts | 20 +++++++--- .../libs/looks-same/looks-same.service.ts | 2 +- src/compare/libs/odiff/odiff.service.ts | 8 +--- .../libs/pixelmatch/pixelmatch.service.ts | 2 +- 6 files changed, 62 insertions(+), 16 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 33b87eb..b99aa76 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,10 +6,16 @@ services: dockerfile: Dockerfile environment: DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB} + STATIC_SERVICE: ${STATIC_SERVICE} + AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID} + AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY} + AWS_REGION: ${AWS_REGION} + AWS_S3_BUCKET_NAME: ${AWS_S3_BUCKET_NAME} JWT_SECRET: ${JWT_SECRET} JWT_LIFE_TIME: ${JWT_LIFE_TIME} - BODY_PARSER_JSON_LIMIT: ${BODY_PARSER_JSON_LIMIT} APP_FRONTEND_URL: ${APP_FRONTEND_URL} + BODY_PARSER_JSON_LIMIT: ${BODY_PARSER_JSON_LIMIT} + ELASTIC_URL: ${ELASTIC_URL} ports: - "${APP_PORT}:3000" expose: diff --git a/src/compare/compare.service.spec.ts b/src/compare/compare.service.spec.ts index 3c04db9..dcb47f1 100644 --- a/src/compare/compare.service.spec.ts +++ b/src/compare/compare.service.spec.ts @@ -5,9 +5,14 @@ import { LookSameService } from './libs/looks-same/looks-same.service'; import { OdiffService } from './libs/odiff/odiff.service'; import { PixelmatchService } from './libs/pixelmatch/pixelmatch.service'; import { StaticModule } from '../static/static.module'; +import { ImageComparison } from '@prisma/client'; +import * as utils from '../static/utils'; describe('CompareService', () => { let service: CompareService; + let pixelmatchService: PixelmatchService; + let lookSameService: LookSameService; + let odiffService: OdiffService; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -16,9 +21,42 @@ describe('CompareService', () => { }).compile(); service = module.get(CompareService); + pixelmatchService = module.get(PixelmatchService); + lookSameService = module.get(LookSameService); + odiffService = module.get(OdiffService); }); it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('getComparator', () => { + it('should return pixelmatchService', () => { + const result = service.getComparator(ImageComparison.pixelmatch); + expect(result).toBe(pixelmatchService); + }); + + it('should return lookSameService', () => { + const result = service.getComparator(ImageComparison.lookSame); + expect(result).toBe(lookSameService); + }); + + it('should return odiffService', () => { + jest.spyOn(utils, 'isHddStaticServiceConfigured').mockReturnValue(true); + + expect(service.getComparator(ImageComparison.odiff)).toBe(odiffService); + }); + + it('should throw if not HDD for Odiff', () => { + jest.spyOn(utils, 'isHddStaticServiceConfigured').mockReturnValue(false); + + expect(() => service.getComparator(ImageComparison.odiff)).toThrow( + "Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD."); + }); + + it('should return pixelmatchService for unknown value', () => { + const result = service.getComparator('unknown' as ImageComparison); + expect(result).toBe(pixelmatchService); + }); + }); }); diff --git a/src/compare/compare.service.ts b/src/compare/compare.service.ts index 0af46c7..864023c 100644 --- a/src/compare/compare.service.ts +++ b/src/compare/compare.service.ts @@ -1,5 +1,5 @@ import { ImageComparison, Project } from '@prisma/client'; -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { PixelmatchService } from './libs/pixelmatch/pixelmatch.service'; import { ImageComparator } from './libs/image-comparator.interface'; import { ImageCompareInput } from './libs/ImageCompareInput'; @@ -7,15 +7,18 @@ import { PrismaService } from '../prisma/prisma.service'; import { DiffResult } from '../test-runs/diffResult'; import { LookSameService } from './libs/looks-same/looks-same.service'; import { OdiffService } from './libs/odiff/odiff.service'; +import { isHddStaticServiceConfigured } from '../static/utils'; @Injectable() export class CompareService { + private readonly logger: Logger = new Logger(CompareService.name); + constructor( - private pixelmatchService: PixelmatchService, - private lookSameService: LookSameService, - private odiffService: OdiffService, - private prismaService: PrismaService - ) {} + private readonly pixelmatchService: PixelmatchService, + private readonly lookSameService: LookSameService, + private readonly odiffService: OdiffService, + private readonly prismaService: PrismaService + ) { } async getDiff({ projectId, data }: { projectId: string; data: ImageCompareInput }): Promise { const project: Project = await this.prismaService.project.findUnique({ where: { id: projectId } }); @@ -33,9 +36,14 @@ export class CompareService { return this.lookSameService; } case ImageComparison.odiff: { + if (!isHddStaticServiceConfigured()) { + throw new Error('Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.'); + } + return this.odiffService; } default: { + this.logger.warn(`Unknown ImageComparison value: ${imageComparison}. Falling back to pixelmatch.`); return this.pixelmatchService; } } diff --git a/src/compare/libs/looks-same/looks-same.service.ts b/src/compare/libs/looks-same/looks-same.service.ts index 30beb3e..ef686a2 100644 --- a/src/compare/libs/looks-same/looks-same.service.ts +++ b/src/compare/libs/looks-same/looks-same.service.ts @@ -21,7 +21,7 @@ export const DEFAULT_CONFIG: LooksSameConfig = { export class LookSameService implements ImageComparator { private readonly logger: Logger = new Logger(LookSameService.name); - constructor(private staticService: StaticService) {} + constructor(private readonly staticService: StaticService) {} parseConfig(configJson: string): LooksSameConfig { return parseConfig(configJson, DEFAULT_CONFIG, this.logger); diff --git a/src/compare/libs/odiff/odiff.service.ts b/src/compare/libs/odiff/odiff.service.ts index 3394e0c..e25c656 100644 --- a/src/compare/libs/odiff/odiff.service.ts +++ b/src/compare/libs/odiff/odiff.service.ts @@ -10,7 +10,6 @@ import { compare } from 'odiff-bin'; import { IgnoreAreaDto } from 'src/test-runs/dto/ignore-area.dto'; import { OdiffConfig, OdiffIgnoreRegions, OdiffResult } from './odiff.types'; import { HddService } from 'src/static/hdd/hdd.service'; -import { isHddStaticServiceConfigured } from '../../../static/utils'; export const DEFAULT_CONFIG: OdiffConfig = { outputDiffMask: true, @@ -24,12 +23,7 @@ export class OdiffService implements ImageComparator { private readonly logger: Logger = new Logger(OdiffService.name); private readonly hddService: HddService; - constructor(private staticService: StaticService) { - if (!isHddStaticServiceConfigured()) { - return undefined; - // If we throw an exception, the application does not start. - // throw new Error('OdiffService can only be used with HddService'); - } + constructor(private readonly staticService: StaticService) { this.hddService = this.staticService as unknown as HddService; } diff --git a/src/compare/libs/pixelmatch/pixelmatch.service.ts b/src/compare/libs/pixelmatch/pixelmatch.service.ts index 0f39912..ff2bd77 100644 --- a/src/compare/libs/pixelmatch/pixelmatch.service.ts +++ b/src/compare/libs/pixelmatch/pixelmatch.service.ts @@ -16,7 +16,7 @@ export const DEFAULT_CONFIG: PixelmatchConfig = { threshold: 0.1, ignoreAntialia export class PixelmatchService implements ImageComparator { private readonly logger: Logger = new Logger(PixelmatchService.name); - constructor(private staticService: StaticService) {} + constructor(private readonly staticService: StaticService) {} parseConfig(configJson: string): PixelmatchConfig { return parseConfig(configJson, DEFAULT_CONFIG, this.logger); From 8d97962ff9397d7c8679050fd0511beb5c997600 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Sat, 28 Jun 2025 21:06:35 +0300 Subject: [PATCH 7/7] styling --- src/compare/compare.service.spec.ts | 3 ++- src/compare/compare.service.ts | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/compare/compare.service.spec.ts b/src/compare/compare.service.spec.ts index dcb47f1..9940b17 100644 --- a/src/compare/compare.service.spec.ts +++ b/src/compare/compare.service.spec.ts @@ -51,7 +51,8 @@ describe('CompareService', () => { jest.spyOn(utils, 'isHddStaticServiceConfigured').mockReturnValue(false); expect(() => service.getComparator(ImageComparison.odiff)).toThrow( - "Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD."); + 'Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.' + ); }); it('should return pixelmatchService for unknown value', () => { diff --git a/src/compare/compare.service.ts b/src/compare/compare.service.ts index 864023c..6bb144a 100644 --- a/src/compare/compare.service.ts +++ b/src/compare/compare.service.ts @@ -18,7 +18,7 @@ export class CompareService { private readonly lookSameService: LookSameService, private readonly odiffService: OdiffService, private readonly prismaService: PrismaService - ) { } + ) {} async getDiff({ projectId, data }: { projectId: string; data: ImageCompareInput }): Promise { const project: Project = await this.prismaService.project.findUnique({ where: { id: projectId } }); @@ -37,7 +37,9 @@ export class CompareService { } case ImageComparison.odiff: { if (!isHddStaticServiceConfigured()) { - throw new Error('Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.'); + throw new Error( + 'Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.' + ); } return this.odiffService;