From 6c6f9a3ed9a0b717a39530589a7f546aa6adb98d Mon Sep 17 00:00:00 2001 From: newtonjjang Date: Tue, 9 Sep 2025 12:15:33 +0900 Subject: [PATCH 1/3] add validation filtering in getStaticPaths for OpenProcessing sketches - Add basic validation (visualID, type check) in sketch getStaticPaths - Remove unnecessary isNaN check from SketchLayout - Prevent invalid sketch pages from being generated during build --- src/layouts/SketchLayout.astro | 4 ---- src/pages/[locale]/sketches/[...slug].astro | 2 +- src/pages/sketches/[...slug].astro | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/layouts/SketchLayout.astro b/src/layouts/SketchLayout.astro index f4e1bce44a..a622d095df 100644 --- a/src/layouts/SketchLayout.astro +++ b/src/layouts/SketchLayout.astro @@ -24,10 +24,6 @@ const { sketchId, authorName } = Astro.props; const sketchIdNumber = Number(sketchId); -if (isNaN(sketchIdNumber)) { - console.error(`Invalid sketch ID: ${sketchId}`); -} - const { title, createdOn, instructions } = await getSketch(sketchIdNumber); const currentLocale = getCurrentLocale(Astro.url.pathname); diff --git a/src/pages/[locale]/sketches/[...slug].astro b/src/pages/[locale]/sketches/[...slug].astro index 5543ff3e2d..a202c72c3e 100644 --- a/src/pages/[locale]/sketches/[...slug].astro +++ b/src/pages/[locale]/sketches/[...slug].astro @@ -7,7 +7,7 @@ export async function getStaticPaths() { const sketches = await getCurationSketches(); const entries = await Promise.all( nonDefaultSupportedLocales.map(async (locale) => { - return sketches.map((sketch) => ({ + return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({ // Even though slug gets converted to string at runtime, // TypeScript infers it as number from sketch.visualID, so we explicitly convert to string. params: { locale, slug: String(sketch.visualID) }, diff --git a/src/pages/sketches/[...slug].astro b/src/pages/sketches/[...slug].astro index 33ccfba111..065dbe5a75 100644 --- a/src/pages/sketches/[...slug].astro +++ b/src/pages/sketches/[...slug].astro @@ -4,7 +4,7 @@ import { getCurationSketches } from "@src/api/OpenProcessing"; export async function getStaticPaths() { const sketches = await getCurationSketches(); - return sketches.map((sketch) => ({ + return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({ // Even though slug gets converted to string at runtime, // TypeScript infers it as number from sketch.visualID, so we explicitly convert to string. params: { slug: String(sketch.visualID) }, From a21097eee3b4d966751678a1c6cba745ffdafde5 Mon Sep 17 00:00:00 2001 From: newtonjjang Date: Thu, 11 Sep 2025 21:19:58 +0900 Subject: [PATCH 2/3] add OpenProcessing API caching tests - Add comprehensive tests for getCurationSketches memoization - Test getSketch cache usage and fallback to individual API calls - Test getSketchSize API calls during build simulation - Verify API call count regression to prevent performance issues - Include test for individual sketch fetching when not in cache --- src/api/OpenProcessing.ts | 6 +- test/api/OpenProcessing.test.ts | 148 ++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 test/api/OpenProcessing.test.ts diff --git a/src/api/OpenProcessing.ts b/src/api/OpenProcessing.ts index 1e1d29f1d4..ececa70f39 100644 --- a/src/api/OpenProcessing.ts +++ b/src/api/OpenProcessing.ts @@ -33,6 +33,9 @@ export type OpenProcessingCurationResponse = Array<{ fullname: string; }>; +// Selected Sketches from the 2025 curation +export const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712'] + /** * Get basic info for the sketches contained in a Curation * from the OpenProcessing API @@ -60,8 +63,7 @@ export const getCurationSketches = memoize(async ( } const payload2 = await response2.json(); - // Selected Sketches from the 2025 curation - const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712'] + const prioritySketches = payload2.filter( (sketch: OpenProcessingCurationResponse[number]) => priorityIds.includes(String(sketch.visualID))) diff --git a/test/api/OpenProcessing.test.ts b/test/api/OpenProcessing.test.ts new file mode 100644 index 0000000000..e38c544853 --- /dev/null +++ b/test/api/OpenProcessing.test.ts @@ -0,0 +1,148 @@ +// src/api/OpenProcessing.test.ts + +import { getCurationSketches, getSketch, getSketchSize, priorityIds, type OpenProcessingCurationResponse } from '@/src/api/OpenProcessing'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const mockFetch = vi.fn(); + +vi.stubGlobal('fetch', mockFetch); + +// Test data: first item is mock data, second uses actual priority ID from current curation +const getCurationSketchesData : OpenProcessingCurationResponse = [{ + visualID: 101, + title: 'Sketch One', + description: 'Description One', + instructions: 'Instructions One', + mode: 'p5js', + createdOn: '2025-01-01', + userID: 'User One', + submittedOn: '2025-01-01', + fullname: 'Fullname One' +}, +{ + visualID: Number(priorityIds[0]), // Real ID from current curation priority list + title: 'Sketch Two', + description: 'Description Two', + instructions: 'Instructions Two', + mode: 'p5js', + createdOn: '2025-01-01', + userID: 'User Two', + submittedOn: '2025-01-01', + fullname: 'Fullname Two' +}] + +describe('OpenProcessing API Caching', () => { + + beforeEach(() => { + vi.clearAllMocks(); + + getCurationSketches.cache.clear?.(); + getSketch.cache.clear?.(); + getSketchSize.cache.clear?.(); + }); + + // Case 1: Verify caching for getCurationSketches + it('should only call the API once even if getCurationSketches is called multiple times', async () => { + + mockFetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve(getCurationSketchesData), + }); + + await getCurationSketches(); + await getCurationSketches(); + + // Check if fetch was called exactly 2 times (for the two curation IDs). + // If this number becomes 4, it means the caching is broken. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + // Case 2: Verify getSketch uses cached data from getCurationSketches + it('should use cached data from getCurationSketches for getSketch calls', async () => { + + mockFetch.mockResolvedValueOnce({ // for curationId + ok: true, + json: () => Promise.resolve([getCurationSketchesData[0]]), + }).mockResolvedValueOnce({ // for newCurationId + ok: true, + json: () => Promise.resolve([getCurationSketchesData[1]]), + }); + + // Call the main function to populate the cache. + await getCurationSketches(); + // At this point, fetch has been called twice. + expect(mockFetch).toHaveBeenCalledTimes(2); + + // Now, call getSketch with an ID that should be in the cache. + const sketch = await getSketch(getCurationSketchesData[0].visualID); + expect(sketch.title).toBe('Sketch One'); + const sketch2 = await getSketch(getCurationSketchesData[1].visualID); + expect(sketch2.title).toBe('Sketch Two'); + + // Verify that no additional fetch calls were made. + // The call count should still be 2 because the data came from the cache. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + // Case 3: Verify getSketch fetches individual sketch when not in cache + it('should fetch individual sketch when not in cache', async () => { + + // for curationId + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve([]) + }).mockResolvedValueOnce({ // for newCurationId + ok: true, + json: () => Promise.resolve([]) + }); + + await getCurationSketches(); // Create empty cache + + // Individual sketch API call + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ visualID: 999, title: 'Individual Sketch' }) + }); + + const sketch = await getSketch(999); + expect(sketch.title).toBe('Individual Sketch'); + expect(mockFetch).toHaveBeenCalledTimes(3); // 2 for empty curations in getCurationSketches + 1 for individual call in getSketch + }); + + // Case 4: Overall regression test for total sketch page generation + it('should not exceed the expected number of API calls during a build simulation', async () => { + + // Mock the responses for getCurationSketches. + mockFetch.mockResolvedValueOnce({ // for curationId + ok: true, + json: () => Promise.resolve([getCurationSketchesData[0]]), + }).mockResolvedValueOnce({ // for newCurationId + ok: true, + json: () => Promise.resolve([getCurationSketchesData[1]]), + }); + + // 2. Mock the response for getSketchSize calls. + mockFetch.mockResolvedValue({ // for all subsequent calls + ok: true, + json: () => Promise.resolve([{ code: 'createCanvas(400, 400);' }]), + }); + + // --- sketch page build simulation --- + // This simulates what happens during `getStaticPaths`. + const sketches = await getCurationSketches(); // Makes 2 API calls. + + // This simulates what happens as each page is generated. + for (const sketch of sketches) { + // Inside the page component, getSketch and getSketchSize would be called. + await getSketch(sketch.visualID); // Uses cache (0 new calls). + await getSketchSize(sketch.visualID); // Makes 1 new API call. + } + // --- simulation end --- + + // Calculate the total expected calls. + // 2 for getCurationSketches + 1 for each sketch's getSketchSize call. + const expectedCalls = 2 + sketches.length; + expect(mockFetch).toHaveBeenCalledTimes(expectedCalls); + + }); +}); \ No newline at end of file From 393d3ba1b3b7445c37a358d1c103088102fb2b42 Mon Sep 17 00:00:00 2001 From: newtonjjang Date: Thu, 11 Sep 2025 21:45:41 +0900 Subject: [PATCH 3/3] improve error handling and add comprehensive error tests - Replace console.error with throw Error in all OpenProcessing API functions - Ensure build fails immediately when API calls fail instead of continuing with invalid data - Add comprehensive error handling tests for all API functions - Test various error scenarios: 500 Internal Server Error, 429 Rate Limit, 404 Not Found - Improve build reliability by failing fast on API issues - Export priorityIds for better test maintainability --- src/api/OpenProcessing.ts | 15 +++--- test/api/OpenProcessing.test.ts | 85 ++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/src/api/OpenProcessing.ts b/src/api/OpenProcessing.ts index ececa70f39..4398c65a18 100644 --- a/src/api/OpenProcessing.ts +++ b/src/api/OpenProcessing.ts @@ -50,16 +50,16 @@ export const getCurationSketches = memoize(async ( const response1 = await fetch( `${openProcessingEndpoint}curation/${curationId}/sketches?${limitParam}`, ); - if(!response1.ok){ //log error instead of throwing error to not cache result in memoize - console.error('getCurationSketches', response1.status, response1.statusText) + if(!response1.ok){ + throw new Error(`getCurationSketches: ${response1.status} ${response1.statusText}`) } const payload1 = await response1.json(); const response2 = await fetch( `${openProcessingEndpoint}curation/${newCurationId}/sketches?${limitParam}`, ); - if(!response2.ok){ //log error instead of throwing error to not cache result in memoize - console.error('getCurationSketches', response2.status, response2.statusText) + if(!response2.ok){ + throw new Error(`getCurationSketches: ${response2.status} ${response2.statusText}`) } const payload2 = await response2.json(); @@ -124,8 +124,7 @@ export const getSketch = memoize( // check for sketch data in Open Processing API const response = await fetch(`${openProcessingEndpoint}sketch/${id}`); if (!response.ok) { - //log error instead of throwing error to not cache result in memoize - console.error("getSketch", id, response.status, response.statusText); + throw new Error(`getSketch: ${id} ${response.status} ${response.statusText}`) } const payload = await response.json(); return payload as OpenProcessingSketchResponse; @@ -143,8 +142,8 @@ export const getSketchSize = memoize(async (id: number) => { } const response = await fetch(`${openProcessingEndpoint}sketch/${id}/code`); - if(!response.ok){ //log error instead of throwing error to not cache result in memoize - console.error('getSketchSize', id, response.status, response.statusText) + if(!response.ok){ + throw new Error(`getSketchSize: ${id} ${response.status} ${response.statusText}`) } const payload = await response.json(); diff --git a/test/api/OpenProcessing.test.ts b/test/api/OpenProcessing.test.ts index e38c544853..c8d7ccc7aa 100644 --- a/test/api/OpenProcessing.test.ts +++ b/test/api/OpenProcessing.test.ts @@ -145,4 +145,87 @@ describe('OpenProcessing API Caching', () => { expect(mockFetch).toHaveBeenCalledTimes(expectedCalls); }); -}); \ No newline at end of file +}); + +describe('Error Handling', () => { + + beforeEach(() => { + vi.clearAllMocks(); + + getCurationSketches.cache.clear?.(); + getSketch.cache.clear?.(); + getSketchSize.cache.clear?.(); + }); + + it('should throw an error when getCurationSketches API call fails', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + }); + + await expect(getCurationSketches()).rejects.toThrow( + 'getCurationSketches: 500 Internal Server Error' + ); + }); + + it('should throw an error when rate limit is exceeded (429)', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 429, + statusText: 'Too Many Requests', + }); + + await expect(getCurationSketches()).rejects.toThrow( + 'getCurationSketches: 429 Too Many Requests' + ); + }); + + it('should throw an error when getSketch API call fails for individual sketch', async () => { + // Setup empty curations first + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve([]) + }).mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve([]) + }); + + await getCurationSketches(); // Create empty cache + + // Individual sketch API call fails with 429 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 429, + statusText: 'Too Many Requests' + }); + + await expect(getSketch(999)).rejects.toThrow( + 'getSketch: 999 429 Too Many Requests' + ); + }); + + it('should throw an error when getSketchSize API call fails', async () => { + // Setup sketch data first + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve([getCurationSketchesData[0]]) + }).mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve([]) + }); + + await getCurationSketches(); + + // getSketchSize API call fails with rate limit + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 429, + statusText: 'Too Many Requests' + }); + + await expect(getSketchSize(getCurationSketchesData[0].visualID)).rejects.toThrow( + `getSketchSize: ${getCurationSketchesData[0].visualID} 429 Too Many Requests` + ); + }); + }); \ No newline at end of file