Skip to content

Commit fe6cdb9

Browse files
authored
Merge pull request #971 from junseok44/feat-improve-sketch-build-validation
Improve OpenProcessing API error handling and add caching tests
2 parents 6e4af3f + b5478bf commit fe6cdb9

File tree

5 files changed

+244
-16
lines changed

5 files changed

+244
-16
lines changed

src/api/OpenProcessing.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ export type OpenProcessingCurationResponse = Array<{
3333
fullname: string;
3434
}>;
3535

36+
// Selected Sketches from the 2025 curation
37+
export const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712']
38+
3639
/**
3740
* Get basic info for the sketches contained in a Curation
3841
* from the OpenProcessing API
@@ -47,21 +50,20 @@ export const getCurationSketches = memoize(async (
4750
const response1 = await fetch(
4851
`${openProcessingEndpoint}curation/${curationId}/sketches?${limitParam}`,
4952
);
50-
if(!response1.ok){ //log error instead of throwing error to not cache result in memoize
51-
console.error('getCurationSketches', response1.status, response1.statusText)
53+
if(!response1.ok){
54+
throw new Error(`getCurationSketches: ${response1.status} ${response1.statusText}`)
5255
}
5356
const payload1 = await response1.json();
5457

5558
const response2 = await fetch(
5659
`${openProcessingEndpoint}curation/${newCurationId}/sketches?${limitParam}`,
5760
);
58-
if(!response2.ok){ //log error instead of throwing error to not cache result in memoize
59-
console.error('getCurationSketches', response2.status, response2.statusText)
61+
if(!response2.ok){
62+
throw new Error(`getCurationSketches: ${response2.status} ${response2.statusText}`)
6063
}
6164
const payload2 = await response2.json();
6265

63-
// Selected Sketches from the 2025 curation
64-
const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712']
66+
6567

6668
const prioritySketches = payload2.filter(
6769
(sketch: OpenProcessingCurationResponse[number]) => priorityIds.includes(String(sketch.visualID)))
@@ -122,8 +124,7 @@ export const getSketch = memoize(
122124
// check for sketch data in Open Processing API
123125
const response = await fetch(`${openProcessingEndpoint}sketch/${id}`);
124126
if (!response.ok) {
125-
//log error instead of throwing error to not cache result in memoize
126-
console.error("getSketch", id, response.status, response.statusText);
127+
throw new Error(`getSketch: ${id} ${response.status} ${response.statusText}`)
127128
}
128129
const payload = await response.json();
129130
return payload as OpenProcessingSketchResponse;
@@ -141,8 +142,8 @@ export const getSketchSize = memoize(async (id: number) => {
141142
}
142143

143144
const response = await fetch(`${openProcessingEndpoint}sketch/${id}/code`);
144-
if(!response.ok){ //log error instead of throwing error to not cache result in memoize
145-
console.error('getSketchSize', id, response.status, response.statusText)
145+
if(!response.ok){
146+
throw new Error(`getSketchSize: ${id} ${response.status} ${response.statusText}`)
146147
}
147148
const payload = await response.json();
148149

src/layouts/SketchLayout.astro

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ const { sketchId, authorName } = Astro.props;
2424
2525
const sketchIdNumber = Number(sketchId);
2626
27-
if (isNaN(sketchIdNumber)) {
28-
console.error(`Invalid sketch ID: ${sketchId}`);
29-
}
30-
3127
const { title, createdOn, instructions } = await getSketch(sketchIdNumber);
3228
3329
const currentLocale = getCurrentLocale(Astro.url.pathname);

src/pages/[locale]/sketches/[...slug].astro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export async function getStaticPaths() {
77
const sketches = await getCurationSketches();
88
const entries = await Promise.all(
99
nonDefaultSupportedLocales.map(async (locale) => {
10-
return sketches.map((sketch) => ({
10+
return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({
1111
// Even though slug gets converted to string at runtime,
1212
// TypeScript infers it as number from sketch.visualID, so we explicitly convert to string.
1313
params: { locale, slug: String(sketch.visualID) },

src/pages/sketches/[...slug].astro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { getCurationSketches } from "@src/api/OpenProcessing";
44
55
export async function getStaticPaths() {
66
const sketches = await getCurationSketches();
7-
return sketches.map((sketch) => ({
7+
return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({
88
// Even though slug gets converted to string at runtime,
99
// TypeScript infers it as number from sketch.visualID, so we explicitly convert to string.
1010
params: { slug: String(sketch.visualID) },

test/api/OpenProcessing.test.ts

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
// src/api/OpenProcessing.test.ts
2+
3+
import { getCurationSketches, getSketch, getSketchSize, priorityIds, type OpenProcessingCurationResponse } from '@/src/api/OpenProcessing';
4+
import { describe, it, expect, vi, beforeEach } from 'vitest';
5+
6+
const mockFetch = vi.fn();
7+
8+
vi.stubGlobal('fetch', mockFetch);
9+
10+
// Test data: first item is mock data, second uses actual priority ID from current curation
11+
const getCurationSketchesData : OpenProcessingCurationResponse = [{
12+
visualID: 101,
13+
title: 'Sketch One',
14+
description: 'Description One',
15+
instructions: 'Instructions One',
16+
mode: 'p5js',
17+
createdOn: '2025-01-01',
18+
userID: 'User One',
19+
submittedOn: '2025-01-01',
20+
fullname: 'Fullname One'
21+
},
22+
{
23+
visualID: Number(priorityIds[0]), // Real ID from current curation priority list
24+
title: 'Sketch Two',
25+
description: 'Description Two',
26+
instructions: 'Instructions Two',
27+
mode: 'p5js',
28+
createdOn: '2025-01-01',
29+
userID: 'User Two',
30+
submittedOn: '2025-01-01',
31+
fullname: 'Fullname Two'
32+
}]
33+
34+
describe('OpenProcessing API Caching', () => {
35+
36+
beforeEach(() => {
37+
vi.clearAllMocks();
38+
39+
getCurationSketches.cache.clear?.();
40+
getSketch.cache.clear?.();
41+
getSketchSize.cache.clear?.();
42+
});
43+
44+
// Case 1: Verify caching for getCurationSketches
45+
it('should only call the API once even if getCurationSketches is called multiple times', async () => {
46+
47+
mockFetch.mockResolvedValue({
48+
ok: true,
49+
json: () => Promise.resolve(getCurationSketchesData),
50+
});
51+
52+
await getCurationSketches();
53+
await getCurationSketches();
54+
55+
// Check if fetch was called exactly 2 times (for the two curation IDs).
56+
// If this number becomes 4, it means the caching is broken.
57+
expect(mockFetch).toHaveBeenCalledTimes(2);
58+
});
59+
60+
// Case 2: Verify getSketch uses cached data from getCurationSketches
61+
it('should use cached data from getCurationSketches for getSketch calls', async () => {
62+
63+
mockFetch.mockResolvedValueOnce({ // for curationId
64+
ok: true,
65+
json: () => Promise.resolve([getCurationSketchesData[0]]),
66+
}).mockResolvedValueOnce({ // for newCurationId
67+
ok: true,
68+
json: () => Promise.resolve([getCurationSketchesData[1]]),
69+
});
70+
71+
// Call the main function to populate the cache.
72+
await getCurationSketches();
73+
// At this point, fetch has been called twice.
74+
expect(mockFetch).toHaveBeenCalledTimes(2);
75+
76+
// Now, call getSketch with an ID that should be in the cache.
77+
const sketch = await getSketch(getCurationSketchesData[0].visualID);
78+
expect(sketch.title).toBe('Sketch One');
79+
const sketch2 = await getSketch(getCurationSketchesData[1].visualID);
80+
expect(sketch2.title).toBe('Sketch Two');
81+
82+
// Verify that no additional fetch calls were made.
83+
// The call count should still be 2 because the data came from the cache.
84+
expect(mockFetch).toHaveBeenCalledTimes(2);
85+
});
86+
87+
// Case 3: Verify getSketch fetches individual sketch when not in cache
88+
it('should fetch individual sketch when not in cache', async () => {
89+
90+
// for curationId
91+
mockFetch.mockResolvedValueOnce({
92+
ok: true,
93+
json: () => Promise.resolve([])
94+
}).mockResolvedValueOnce({ // for newCurationId
95+
ok: true,
96+
json: () => Promise.resolve([])
97+
});
98+
99+
await getCurationSketches(); // Create empty cache
100+
101+
// Individual sketch API call
102+
mockFetch.mockResolvedValueOnce({
103+
ok: true,
104+
json: () => Promise.resolve({ visualID: 999, title: 'Individual Sketch' })
105+
});
106+
107+
const sketch = await getSketch(999);
108+
expect(sketch.title).toBe('Individual Sketch');
109+
expect(mockFetch).toHaveBeenCalledTimes(3); // 2 for empty curations in getCurationSketches + 1 for individual call in getSketch
110+
});
111+
112+
// Case 4: Overall regression test for total sketch page generation
113+
it('should not exceed the expected number of API calls during a build simulation', async () => {
114+
115+
// Mock the responses for getCurationSketches.
116+
mockFetch.mockResolvedValueOnce({ // for curationId
117+
ok: true,
118+
json: () => Promise.resolve([getCurationSketchesData[0]]),
119+
}).mockResolvedValueOnce({ // for newCurationId
120+
ok: true,
121+
json: () => Promise.resolve([getCurationSketchesData[1]]),
122+
});
123+
124+
// 2. Mock the response for getSketchSize calls.
125+
mockFetch.mockResolvedValue({ // for all subsequent calls
126+
ok: true,
127+
json: () => Promise.resolve([{ code: 'createCanvas(400, 400);' }]),
128+
});
129+
130+
// --- sketch page build simulation ---
131+
// This simulates what happens during `getStaticPaths`.
132+
const sketches = await getCurationSketches(); // Makes 2 API calls.
133+
134+
// This simulates what happens as each page is generated.
135+
for (const sketch of sketches) {
136+
// Inside the page component, getSketch and getSketchSize would be called.
137+
await getSketch(sketch.visualID); // Uses cache (0 new calls).
138+
await getSketchSize(sketch.visualID); // Makes 1 new API call.
139+
}
140+
// --- simulation end ---
141+
142+
// Calculate the total expected calls.
143+
// 2 for getCurationSketches + 1 for each sketch's getSketchSize call.
144+
const expectedCalls = 2 + sketches.length;
145+
expect(mockFetch).toHaveBeenCalledTimes(expectedCalls);
146+
147+
});
148+
});
149+
150+
describe('Error Handling', () => {
151+
152+
beforeEach(() => {
153+
vi.clearAllMocks();
154+
155+
getCurationSketches.cache.clear?.();
156+
getSketch.cache.clear?.();
157+
getSketchSize.cache.clear?.();
158+
});
159+
160+
it('should throw an error when getCurationSketches API call fails', async () => {
161+
mockFetch.mockResolvedValue({
162+
ok: false,
163+
status: 500,
164+
statusText: 'Internal Server Error',
165+
});
166+
167+
await expect(getCurationSketches()).rejects.toThrow(
168+
'getCurationSketches: 500 Internal Server Error'
169+
);
170+
});
171+
172+
it('should throw an error when rate limit is exceeded (429)', async () => {
173+
mockFetch.mockResolvedValue({
174+
ok: false,
175+
status: 429,
176+
statusText: 'Too Many Requests',
177+
});
178+
179+
await expect(getCurationSketches()).rejects.toThrow(
180+
'getCurationSketches: 429 Too Many Requests'
181+
);
182+
});
183+
184+
it('should throw an error when getSketch API call fails for individual sketch', async () => {
185+
// Setup empty curations first
186+
mockFetch.mockResolvedValueOnce({
187+
ok: true,
188+
json: () => Promise.resolve([])
189+
}).mockResolvedValueOnce({
190+
ok: true,
191+
json: () => Promise.resolve([])
192+
});
193+
194+
await getCurationSketches(); // Create empty cache
195+
196+
// Individual sketch API call fails with 429
197+
mockFetch.mockResolvedValueOnce({
198+
ok: false,
199+
status: 429,
200+
statusText: 'Too Many Requests'
201+
});
202+
203+
await expect(getSketch(999)).rejects.toThrow(
204+
'getSketch: 999 429 Too Many Requests'
205+
);
206+
});
207+
208+
it('should throw an error when getSketchSize API call fails', async () => {
209+
// Setup sketch data first
210+
mockFetch.mockResolvedValueOnce({
211+
ok: true,
212+
json: () => Promise.resolve([getCurationSketchesData[0]])
213+
}).mockResolvedValueOnce({
214+
ok: true,
215+
json: () => Promise.resolve([])
216+
});
217+
218+
await getCurationSketches();
219+
220+
// getSketchSize API call fails with rate limit
221+
mockFetch.mockResolvedValueOnce({
222+
ok: false,
223+
status: 429,
224+
statusText: 'Too Many Requests'
225+
});
226+
227+
await expect(getSketchSize(getCurationSketchesData[0].visualID)).rejects.toThrow(
228+
`getSketchSize: ${getCurationSketchesData[0].visualID} 429 Too Many Requests`
229+
);
230+
});
231+
});

0 commit comments

Comments
 (0)