Skip to content

Commit 832fce8

Browse files
authored
🐛 Fix sha over deduplication (#22)
## What is this? We now use the name + sha to consider what needs to be deduplicated. --- This pull request introduces signature-based deduplication for screenshot uploads, improving how the system checks for existing screenshots by considering metadata such as browser and viewport dimensions. The main changes update the API service and uploader logic to support this new format, ensure backward compatibility, and add related test coverage. **Signature-based deduplication and API changes:** * Updated `ApiService.checkShas` to accept either an array of SHA strings (legacy) or an array of screenshot objects with metadata, and dynamically build the request body for the `/api/sdk/check-shas` endpoint. * Improved error handling in `ApiService.checkShas` to extract SHA values from either format for consistent fallback behavior. * Modified screenshot upload flow in `ApiService` to use the new signature-based format when checking for existing screenshots, including passing metadata such as browser and viewport. [[1]](diffhunk://#diff-35db277c4b1bd7577c6a6ae77fcb683696b154b2a93f8c7394f0cf1432a24e29L175-R220) [[2]](diffhunk://#diff-35db277c4b1bd7577c6a6ae77fcb683696b154b2a93f8c7394f0cf1432a24e29L195-R233) **Uploader logic enhancements:** * Refactored `checkExistingFiles` in `uploader.js` to batch and send screenshot objects with signature data (including browser and viewport) for deduplication, and added a helper to extract browser names from filenames. [[1]](diffhunk://#diff-8648aa90df287bc3a7eaf07f6f8d9a0243f8b46a9972bb94da8ee68258b6113dL301-R325) [[2]](diffhunk://#diff-8648aa90df287bc3a7eaf07f6f8d9a0243f8b46a9972bb94da8ee68258b6113dR345-R362) **Test updates for new format:** * Updated tests in `api-service.spec.js` to verify that requests use the new screenshot object format, including correct metadata extraction and fallback to defaults where necessary. [[1]](diffhunk://#diff-b42eca429b3d40549b6623640c048c1af75c9bd57e15acd0abfcceda1211fdb2L136-R144) [[2]](diffhunk://#diff-b42eca429b3d40549b6623640c048c1af75c9bd57e15acd0abfcceda1211fdb2L188-R204) [[3]](diffhunk://#diff-b42eca429b3d40549b6623640c048c1af75c9bd57e15acd0abfcceda1211fdb2L242-R266)
1 parent 784ca30 commit 832fce8

File tree

3 files changed

+107
-22
lines changed

3 files changed

+107
-22
lines changed

src/services/api-service.js

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,16 +126,38 @@ export class ApiService {
126126

127127
/**
128128
* Check if SHAs already exist on the server
129-
* @param {string[]} shas - Array of SHA256 hashes to check
129+
* @param {string[]|Object[]} shas - Array of SHA256 hashes to check, or array of screenshot objects with metadata
130130
* @param {string} buildId - Build ID for screenshot record creation
131131
* @returns {Promise<Object>} Response with existing SHAs and screenshot data
132132
*/
133133
async checkShas(shas, buildId) {
134134
try {
135+
let requestBody;
136+
137+
// Check if we're using the new signature-based format (array of objects) or legacy format (array of strings)
138+
if (
139+
Array.isArray(shas) &&
140+
shas.length > 0 &&
141+
typeof shas[0] === 'object' &&
142+
shas[0].sha256
143+
) {
144+
// New signature-based format
145+
requestBody = {
146+
buildId,
147+
screenshots: shas,
148+
};
149+
} else {
150+
// Legacy SHA-only format
151+
requestBody = {
152+
shas,
153+
buildId,
154+
};
155+
}
156+
135157
const response = await this.request('/api/sdk/check-shas', {
136158
method: 'POST',
137159
headers: { 'Content-Type': 'application/json' },
138-
body: JSON.stringify({ shas, buildId }),
160+
body: JSON.stringify(requestBody),
139161
});
140162
return response;
141163
} catch (error) {
@@ -144,7 +166,12 @@ export class ApiService {
144166
'SHA check failed, continuing without deduplication:',
145167
error.message
146168
);
147-
return { existing: [], missing: shas, screenshots: [] };
169+
// Extract SHAs for fallback response regardless of format
170+
const shaList =
171+
Array.isArray(shas) && shas.length > 0 && typeof shas[0] === 'object'
172+
? shas.map(s => s.sha256)
173+
: shas;
174+
return { existing: [], missing: shaList, screenshots: [] };
148175
}
149176
}
150177

@@ -172,14 +199,25 @@ export class ApiService {
172199
});
173200
}
174201

175-
// Normal flow with SHA deduplication
202+
// Normal flow with SHA deduplication using signature-based format
176203
const sha256 = crypto.createHash('sha256').update(buffer).digest('hex');
177204

178-
// Check if this SHA already exists
179-
const checkResult = await this.checkShas([sha256], buildId);
205+
// Create screenshot object with signature data for checking
206+
const screenshotCheck = [
207+
{
208+
sha256,
209+
name,
210+
browser: metadata?.browser || 'chrome',
211+
viewport_width: metadata?.viewport?.width || 1920,
212+
viewport_height: metadata?.viewport?.height || 1080,
213+
},
214+
];
215+
216+
// Check if this SHA with signature already exists
217+
const checkResult = await this.checkShas(screenshotCheck, buildId);
180218

181219
if (checkResult.existing && checkResult.existing.includes(sha256)) {
182-
// File already exists, screenshot record was automatically created
220+
// File already exists with same signature, screenshot record was automatically created
183221
const screenshot = checkResult.screenshots?.find(
184222
s => s.sha256 === sha256
185223
);
@@ -192,7 +230,7 @@ export class ApiService {
192230
};
193231
}
194232

195-
// File doesn't exist, proceed with upload
233+
// File doesn't exist or has different signature, proceed with upload
196234
return this.request(`/api/sdk/builds/${buildId}/screenshots`, {
197235
method: 'POST',
198236
headers: { 'Content-Type': 'application/json' },

src/services/uploader.js

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -298,26 +298,31 @@ async function processFiles(files, signal, onProgress) {
298298
}
299299

300300
/**
301-
* Check which files already exist on the server
301+
* Check which files already exist on the server using signature-based deduplication
302302
*/
303303
async function checkExistingFiles(fileMetadata, api, signal, buildId) {
304-
const allShas = fileMetadata.map(f => f.sha256);
305304
const existingShas = new Set();
306305
const allScreenshots = [];
307306

308-
// Check in batches
309-
for (let i = 0; i < allShas.length; i += DEFAULT_SHA_CHECK_BATCH_SIZE) {
307+
// Check in batches using the new signature-based format
308+
for (let i = 0; i < fileMetadata.length; i += DEFAULT_SHA_CHECK_BATCH_SIZE) {
310309
if (signal.aborted) throw new UploadError('Operation cancelled');
311310

312-
const batch = allShas.slice(i, i + DEFAULT_SHA_CHECK_BATCH_SIZE);
311+
const batch = fileMetadata.slice(i, i + DEFAULT_SHA_CHECK_BATCH_SIZE);
312+
313+
// Convert file metadata to screenshot objects with signature data
314+
const screenshotBatch = batch.map(file => ({
315+
sha256: file.sha256,
316+
name: file.filename.replace(/\.png$/, ''), // Remove .png extension for name
317+
// Extract browser from filename if available (e.g., "homepage-chrome.png" -> "chrome")
318+
browser: extractBrowserFromFilename(file.filename) || 'chrome', // Default to chrome
319+
// Default viewport dimensions (these could be extracted from filename or metadata if available)
320+
viewport_width: 1920,
321+
viewport_height: 1080,
322+
}));
313323

314324
try {
315-
const res = await api.request('/api/sdk/check-shas', {
316-
method: 'POST',
317-
headers: { 'Content-Type': 'application/json' },
318-
body: JSON.stringify({ shas: batch, buildId }),
319-
signal,
320-
});
325+
const res = await api.checkShas(screenshotBatch, buildId);
321326
const { existing = [], screenshots = [] } = res || {};
322327
existing.forEach(sha => existingShas.add(sha));
323328
allScreenshots.push(...screenshots);
@@ -337,6 +342,24 @@ async function checkExistingFiles(fileMetadata, api, signal, buildId) {
337342
};
338343
}
339344

345+
/**
346+
* Extract browser name from filename
347+
* @param {string} filename - The screenshot filename
348+
* @returns {string|null} Browser name or null if not found
349+
*/
350+
function extractBrowserFromFilename(filename) {
351+
const browsers = ['chrome', 'firefox', 'safari', 'edge', 'webkit'];
352+
const lowerFilename = filename.toLowerCase();
353+
354+
for (const browser of browsers) {
355+
if (lowerFilename.includes(browser)) {
356+
return browser;
357+
}
358+
}
359+
360+
return null;
361+
}
362+
340363
/**
341364
* Upload files to Vizzly
342365
*/

tests/services/api-service.spec.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,15 @@ describe('ApiService', () => {
133133
expect(firstCall[1].method).toBe('POST');
134134
const firstRequestBody = JSON.parse(firstCall[1].body);
135135
expect(firstRequestBody.buildId).toBe(buildId);
136-
expect(firstRequestBody.shas).toEqual([sha256]);
136+
expect(firstRequestBody.screenshots).toEqual([
137+
{
138+
sha256,
139+
name,
140+
browser: metadata.browser || 'chrome',
141+
viewport_width: metadata.viewport?.width || 1920,
142+
viewport_height: metadata.viewport?.height || 1080,
143+
},
144+
]);
137145

138146
// Check that upload was called with correct data (second call)
139147
const secondCall = global.fetch.mock.calls[1];
@@ -185,7 +193,15 @@ describe('ApiService', () => {
185193
const firstCall = global.fetch.mock.calls[0];
186194
const firstRequestBody = JSON.parse(firstCall[1].body);
187195
expect(firstRequestBody.buildId).toBe(buildId);
188-
expect(firstRequestBody.shas).toEqual([sha256]);
196+
expect(firstRequestBody.screenshots).toEqual([
197+
{
198+
sha256,
199+
name,
200+
browser: 'chrome',
201+
viewport_width: 1920,
202+
viewport_height: 1080,
203+
},
204+
]);
189205

190206
// Check that upload was called with empty properties (second call)
191207
const secondCall = global.fetch.mock.calls[1];
@@ -239,7 +255,15 @@ describe('ApiService', () => {
239255
expect(firstCall[0]).toBe('https://vizzly.dev/api/sdk/check-shas');
240256
const requestBody = JSON.parse(firstCall[1].body);
241257
expect(requestBody.buildId).toBe(buildId);
242-
expect(requestBody.shas).toEqual([sha256]);
258+
expect(requestBody.screenshots).toEqual([
259+
{
260+
sha256,
261+
name,
262+
browser: 'chrome',
263+
viewport_width: 1920,
264+
viewport_height: 1080,
265+
},
266+
]);
243267

244268
expect(result).toEqual({
245269
message: 'Screenshot already exists, skipped upload',

0 commit comments

Comments
 (0)