From aa5f46d5c80c2830b11ac01e0fb8801a1390b922 Mon Sep 17 00:00:00 2001 From: Kai-W Date: Mon, 25 Mar 2024 16:48:08 +0100 Subject: [PATCH] changed order of normalize and transform in load sprites --- CHANGELOG.md | 2 +- src/style/load_sprite.test.ts | 54 ++++++++++++++++++++++++++--------- src/style/load_sprite.ts | 14 ++++++--- src/style/style.test.ts | 8 ++---- src/util/request_manager.ts | 3 +- 5 files changed, 55 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b020ac07606..bbd867c63e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - Added const enum for actor messages to improve readability and maintainability. In tsconfig.json, `isolatedModules` flag is set to false in favor of generated JS size. ([#3879](https://github.com/maplibre/maplibre-gl-js/issues/3879)) ### 🐞 Bug fixes -- Fix normalizeSpriteURL before transformRequest throwing an Error with reelative URLs ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897)) +- Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897)) - _...Add new stuff here..._ ## 4.1.2 diff --git a/src/style/load_sprite.test.ts b/src/style/load_sprite.test.ts index 4dc379a458e..dbb65ea0b10 100644 --- a/src/style/load_sprite.test.ts +++ b/src/style/load_sprite.test.ts @@ -66,9 +66,39 @@ describe('loadSprite', () => { const result = await promise; - expect(transform).toHaveBeenCalledTimes(2); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1.json', 'SpriteJSON'); - expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1.png', 'SpriteImage'); + expect(transform).toHaveBeenCalledTimes(1); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); + + expect(Object.keys(result)).toHaveLength(1); + expect(Object.keys(result)[0]).toBe('default'); + + Object.values(result['default']).forEach(styleImage => { + expect(styleImage.spriteData).toBeTruthy(); + expect(styleImage.spriteData.context).toBeInstanceOf(CanvasRenderingContext2D); + }); + + expect(server.requests[0].url).toBe('http://localhost:9966/test/unit/assets/sprite1.json'); + expect(server.requests[1].url).toBe('http://localhost:9966/test/unit/assets/sprite1.png'); + }); + + test('transform of invalid url', async () => { + const transform = jest.fn().mockImplementation((url, type) => { + return {url: `http://localhost:9966${url}`, type}; + }); + + const manager = new RequestManager(transform); + + server.respondWith('GET', 'http://localhost:9966/test/unit/assets/sprite1.json', fs.readFileSync(path.join(__dirname, '../../test/unit/assets/sprite1.json')).toString()); + server.respondWith('GET', 'http://localhost:9966/test/unit/assets/sprite1.png', bufferToArrayBuffer(fs.readFileSync(path.join(__dirname, '../../test/unit/assets/sprite1.png')))); + + const promise = loadSprite('/test/unit/assets/sprite1', manager, 1, new AbortController()); + + server.respond(); + + const result = await promise; + + expect(transform).toHaveBeenCalledTimes(1); + expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1', 'Sprite'); expect(Object.keys(result)).toHaveLength(1); expect(Object.keys(result)[0]).toBe('default'); @@ -98,9 +128,8 @@ describe('loadSprite', () => { const result = await promise; - expect(transform).toHaveBeenCalledTimes(2); - expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1.json', 'SpriteJSON'); - expect(transform).toHaveBeenNthCalledWith(2, '/test/unit/assets/sprite1.png', 'SpriteImage'); + expect(transform).toHaveBeenCalledTimes(1); + expect(transform).toHaveBeenNthCalledWith(1, '/test/unit/assets/sprite1', 'Sprite'); expect(Object.keys(result)).toHaveLength(1); expect(Object.keys(result)[0]).toBe('default'); @@ -131,11 +160,9 @@ describe('loadSprite', () => { server.respond(); const result = await promise; - expect(transform).toHaveBeenCalledTimes(4); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1.json', 'SpriteJSON'); - expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1.png', 'SpriteImage'); - expect(transform).toHaveBeenNthCalledWith(3, 'http://localhost:9966/test/unit/assets/sprite2.json', 'SpriteJSON'); - expect(transform).toHaveBeenNthCalledWith(4, 'http://localhost:9966/test/unit/assets/sprite2.png', 'SpriteImage'); + expect(transform).toHaveBeenCalledTimes(2); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); + expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite2', 'Sprite'); expect(Object.keys(result)).toHaveLength(2); expect(Object.keys(result)[0]).toBe('sprite1'); @@ -209,9 +236,8 @@ describe('loadSprite', () => { server.respond(); const result = await promise; - expect(transform).toHaveBeenCalledTimes(2); - expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1@2x.json', 'SpriteJSON'); - expect(transform).toHaveBeenNthCalledWith(2, 'http://localhost:9966/test/unit/assets/sprite1@2x.png', 'SpriteImage'); + expect(transform).toHaveBeenCalledTimes(1); + expect(transform).toHaveBeenNthCalledWith(1, 'http://localhost:9966/test/unit/assets/sprite1', 'Sprite'); expect(Object.keys(result)).toHaveLength(1); expect(Object.keys(result)[0]).toBe('default'); diff --git a/src/style/load_sprite.ts b/src/style/load_sprite.ts index a4c31bb25b8..b8c4788b7e7 100644 --- a/src/style/load_sprite.ts +++ b/src/style/load_sprite.ts @@ -34,11 +34,17 @@ export async function loadSprite( const imagesMap: {[id: string]: Promise>} = {}; for (const {id, url} of spriteArray) { - const jsonRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.json'), ResourceType.SpriteJSON); - jsonsMap[id] = getJSON(jsonRequestParameters, abortController); + const requestParameters = requestManager.transformRequest(url, ResourceType.Sprite); - const imageRequestParameters = requestManager.transformRequest(normalizeSpriteURL(url, format, '.png'), ResourceType.SpriteImage); - imagesMap[id] = ImageRequest.getImage(imageRequestParameters, abortController); + jsonsMap[id] = getJSON({ + ...requestParameters, + url: normalizeSpriteURL(requestParameters.url, format, '.json') + }, abortController); + + imagesMap[id] = ImageRequest.getImage({ + ...requestParameters, + url: normalizeSpriteURL(requestParameters.url, format, '.png') + }, abortController); } await Promise.all([...Object.values(jsonsMap), ...Object.values(imagesMap)]); diff --git a/src/style/style.test.ts b/src/style/style.test.ts index b4cea1f62f0..ae8b00b4e20 100644 --- a/src/style/style.test.ts +++ b/src/style/style.test.ts @@ -338,11 +338,9 @@ describe('Style#loadJSON', () => { await style.once('style.load'); - expect(transformSpy).toHaveBeenCalledTimes(2); - expect(transformSpy.mock.calls[0][0]).toBe('http://example.com/sprites/bright-v8.json'); - expect(transformSpy.mock.calls[0][1]).toBe('SpriteJSON'); - expect(transformSpy.mock.calls[1][0]).toBe('http://example.com/sprites/bright-v8.png'); - expect(transformSpy.mock.calls[1][1]).toBe('SpriteImage'); + expect(transformSpy).toHaveBeenCalledTimes(1); + expect(transformSpy.mock.calls[0][0]).toBe('http://example.com/sprites/bright-v8'); + expect(transformSpy.mock.calls[0][1]).toBe('Sprite'); }); test('emits an error on non-existant vector source layer', done => { diff --git a/src/util/request_manager.ts b/src/util/request_manager.ts index bb98b284eac..f679dd0f856 100644 --- a/src/util/request_manager.ts +++ b/src/util/request_manager.ts @@ -7,8 +7,7 @@ export const enum ResourceType { Glyphs = 'Glyphs', Image = 'Image', Source = 'Source', - SpriteImage = 'SpriteImage', - SpriteJSON = 'SpriteJSON', + Sprite = 'Sprite', Style = 'Style', Tile = 'Tile', Unknown = 'Unknown',