-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(cache): repair cache validation and add comprehensive tests
Fixes several critical caching issues while adding thorough test coverage: Cache validation fixes: - Fix broken cache invalidation logic to properly handle ETags - Correctly handle 304 Not Modified responses with ETags - Properly update cache metadata timestamps on revalidation - Fix incorrect caching headers during revalidation requests - Add proper handling of Last-Modified headers - Handle missing or corrupt cache metadata - Fix race conditions in concurrent requests - Add proper cleanup of partial cache entries - Fix error recovery during failed cache updates - Add proper source node cleanup in playback - Handle invalid data URLs Test coverage: - Add comprehensive test suite covering cache validation, network handling, storage, and concurrency - Add specific tests for ETag/304 handling - Test cache header management and revalidation flows - Implement robust mock utilities for testing cache scenarios - Set up proper Cache API mocking in test environment The changes make the caching system significantly more reliable by fixing core validation issues while ensuring correctness through thorough test coverage.
- Loading branch information
Showing
7 changed files
with
1,074 additions
and
178 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
import { vi, expect, describe, it, beforeEach } from "vitest"; | ||
import { AudioCache } from "../../cache"; | ||
import { | ||
createTestUrls, | ||
createTestAudioData, | ||
createMockAudioBuffer, | ||
createCacheState, | ||
mockResponses, | ||
simulateError | ||
} from "../mockUtils"; | ||
import { audioContextMock } from "../../setupTests"; | ||
|
||
describe("AudioCache - Basic Operations", () => { | ||
beforeEach(() => { | ||
AudioCache.clearMemoryCache(); | ||
vi.clearAllMocks(); | ||
}); | ||
|
||
describe("LRU Cache Implementation", () => { | ||
it("should store and retrieve decoded buffers", async () => { | ||
const url = createTestUrls.audio(1); | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
|
||
// Setup cache miss scenario | ||
const cache = createCacheState.empty(); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
// Mock successful fetch | ||
global.fetch = vi.fn().mockResolvedValueOnce( | ||
mockResponses.success(arrayBuffer) | ||
); | ||
|
||
// Mock successful decode | ||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockResolvedValueOnce(audioBuffer); | ||
|
||
// First request should fetch and decode | ||
const result1 = await AudioCache.getAudioBuffer(audioContextMock, url); | ||
expect(result1).toBe(audioBuffer); | ||
expect(fetch).toHaveBeenCalledTimes(1); | ||
expect(audioContextMock.decodeAudioData).toHaveBeenCalledTimes(1); | ||
expect(cache.put).toHaveBeenCalledTimes(2); // One for data, one for metadata | ||
|
||
// Second request should use cached buffer | ||
const result2 = await AudioCache.getAudioBuffer(audioContextMock, url); | ||
expect(result2).toBe(audioBuffer); | ||
expect(fetch).toHaveBeenCalledTimes(1); // No additional fetch | ||
expect(audioContextMock.decodeAudioData).toHaveBeenCalledTimes(1); // No additional decode | ||
}); | ||
|
||
it("should evict least recently used items when cache is full", async () => { | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
|
||
// Setup cache | ||
const cache = createCacheState.empty(); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
// Mock fetch and decode for all requests | ||
global.fetch = vi.fn().mockImplementation(() => | ||
Promise.resolve(mockResponses.success(arrayBuffer)) | ||
); | ||
|
||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockImplementation(() => Promise.resolve(audioBuffer)); | ||
|
||
// Fill cache to its limit (DEFAULT_CACHE_SIZE = 100) | ||
const urls = Array.from( | ||
{ length: 101 }, | ||
(_, i) => createTestUrls.audio(i) | ||
); | ||
|
||
// Load all URLs | ||
await Promise.all(urls.map(url => | ||
AudioCache.getAudioBuffer(audioContextMock, url) | ||
)); | ||
|
||
// Reset mocks to verify new fetch | ||
vi.clearAllMocks(); | ||
|
||
// Request the first URL again - should have been evicted | ||
await AudioCache.getAudioBuffer(audioContextMock, urls[0]); | ||
|
||
// Should need to fetch and decode again | ||
expect(fetch).toHaveBeenCalledTimes(1); | ||
expect(audioContextMock.decodeAudioData).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it("should handle cache initialization errors", async () => { | ||
const url = createTestUrls.audio(1); | ||
|
||
// Simulate Cache API not available | ||
global.caches = undefined; | ||
|
||
await expect( | ||
AudioCache.getAudioBuffer(audioContextMock, url) | ||
).rejects.toThrow("Cache API is not supported"); | ||
|
||
// Restore Cache API | ||
Object.defineProperty(global, 'caches', { | ||
value: { open: vi.fn().mockResolvedValue(createCacheState.empty()) } | ||
}); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
import { vi, expect, describe, it, beforeEach } from "vitest"; | ||
import { AudioCache } from "../../cache"; | ||
import { | ||
createTestUrls, | ||
createTestAudioData, | ||
createMockAudioBuffer, | ||
mockResponses, | ||
simulateError, | ||
createCacheState, | ||
mockNetworkConditions | ||
} from "../mockUtils"; | ||
import { audioContextMock } from "../../setupTests"; | ||
|
||
describe("AudioCache - Network Operations", () => { | ||
beforeEach(() => { | ||
AudioCache.clearMemoryCache(); | ||
vi.clearAllMocks(); | ||
}); | ||
|
||
describe("Fetch Operations", () => { | ||
it("makes initial fetch request with correct headers", async () => { | ||
const url = createTestUrls.audio(1); | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
|
||
const cache = createCacheState.empty(); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
const fetchSpy = vi.fn().mockResolvedValue( | ||
mockResponses.success(arrayBuffer) | ||
); | ||
global.fetch = fetchSpy; | ||
|
||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockResolvedValueOnce(audioBuffer); | ||
|
||
await AudioCache.getAudioBuffer(audioContextMock, url); | ||
|
||
expect(fetchSpy).toHaveBeenCalledWith(url, expect.any(Object)); | ||
const requestInit = fetchSpy.mock.calls[0][1]; | ||
expect(requestInit.headers).toBeDefined(); | ||
}); | ||
|
||
it("handles 304 responses correctly", async () => { | ||
const url = createTestUrls.audio(1); | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
|
||
// Setup cache with existing data | ||
const cache = createCacheState.withEntry( | ||
url, | ||
arrayBuffer, | ||
{ | ||
url, | ||
etag: 'W/"123"', | ||
lastModified: new Date().toUTCString(), | ||
timestamp: Date.now() | ||
} | ||
); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
// Mock 304 response | ||
global.fetch = vi.fn().mockResolvedValueOnce( | ||
mockResponses.notModified() | ||
); | ||
|
||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockResolvedValueOnce(audioBuffer); | ||
|
||
const result = await AudioCache.getAudioBuffer(audioContextMock, url); | ||
expect(result).toBe(audioBuffer); | ||
}); | ||
|
||
it("handles network errors gracefully", async () => { | ||
const url = createTestUrls.audio(1); | ||
|
||
const cache = createCacheState.empty(); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
simulateError.network(); | ||
|
||
await expect( | ||
AudioCache.getAudioBuffer(audioContextMock, url) | ||
).rejects.toThrow('Network error'); | ||
}); | ||
|
||
it("handles timeout errors", async () => { | ||
const url = createTestUrls.audio(1); | ||
|
||
const cache = createCacheState.empty(); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
global.fetch = vi.fn().mockImplementationOnce(mockNetworkConditions.timeout); | ||
|
||
await expect( | ||
AudioCache.getAudioBuffer(audioContextMock, url) | ||
).rejects.toThrow('Network timeout'); | ||
}); | ||
}); | ||
|
||
describe("Cache Headers", () => { | ||
it("sends correct cache headers when etag exists", async () => { | ||
const url = createTestUrls.audio(1); | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
const etag = 'W/"123"'; | ||
|
||
// Setup cache with existing etag | ||
const cache = createCacheState.withEntry( | ||
url, | ||
arrayBuffer, | ||
{ | ||
url, | ||
etag, | ||
lastModified: new Date().toUTCString(), | ||
timestamp: Date.now() - (25 * 60 * 60 * 1000) // Make it 25 hours old to force revalidation | ||
} | ||
); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
const fetchSpy = vi.fn().mockResolvedValueOnce( | ||
mockResponses.success(arrayBuffer) | ||
); | ||
global.fetch = fetchSpy; | ||
|
||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockResolvedValueOnce(audioBuffer); | ||
|
||
await AudioCache.getAudioBuffer(audioContextMock, url); | ||
|
||
expect(fetchSpy).toHaveBeenCalled(); | ||
const requestInit = fetchSpy.mock.calls[0][1]; | ||
expect(requestInit.headers.get('If-None-Match')).toBe(etag); | ||
}); | ||
|
||
it("updates stored headers after successful fetch", async () => { | ||
const url = createTestUrls.audio(1); | ||
const arrayBuffer = createTestAudioData.small(); | ||
const audioBuffer = createMockAudioBuffer(); | ||
const newEtag = 'W/"456"'; | ||
|
||
const cache = createCacheState.empty(); | ||
const cachePutSpy = vi.spyOn(cache, 'put'); | ||
vi.spyOn(caches, 'open').mockResolvedValue(cache); | ||
|
||
global.fetch = vi.fn().mockResolvedValueOnce( | ||
mockResponses.success(arrayBuffer, { 'ETag': newEtag }) | ||
); | ||
|
||
vi.spyOn(audioContextMock, "decodeAudioData") | ||
.mockResolvedValueOnce(audioBuffer); | ||
|
||
await AudioCache.getAudioBuffer(audioContextMock, url); | ||
|
||
// Verify metadata was stored with new ETag | ||
const metadataCall = cachePutSpy.mock.calls.find( | ||
call => call[0] === `${url}:meta` | ||
); | ||
expect(metadataCall).toBeDefined(); | ||
const storedMetadata = await metadataCall[1].json(); | ||
expect(storedMetadata.etag).toBe(newEtag); | ||
}); | ||
}); | ||
}); |
Oops, something went wrong.