mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-01-20 22:42:53 -05:00
Fixed inconsistent format of image sizes cache
refs https://github.com/TryGhost/Toolbox/issues/364
refs 147ec91162
- This looks like a subtle bug that has gone unnoticed for years. Have checked if we rely on the logic anywhere (mostly used in image-dimensions frontend helper) - we don't access the "url" directly.
- There is no reasoning attached behind why the cached size was stored as a url (see refed commit)
- WHY is this even being fixed? Caches can store anything... does not mean we should! Inconsistent data becomes a real PITA if the cache is persisted and is hard to repopulate (e.g. to migrate the cached data format).
This commit is contained in:
parent
e811a75972
commit
6cf49d8f89
2 changed files with 21 additions and 3 deletions
|
@ -2,11 +2,25 @@ const debug = require('@tryghost/debug')('utils:image-size-cache');
|
||||||
const errors = require('@tryghost/errors');
|
const errors = require('@tryghost/errors');
|
||||||
const logging = require('@tryghost/logging');
|
const logging = require('@tryghost/logging');
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @example
|
||||||
|
* {
|
||||||
|
* height: 50,
|
||||||
|
* url: 'https://mysite.com/images/cat.jpg',
|
||||||
|
* width: 50
|
||||||
|
* }
|
||||||
|
* @typedef ImageSizeCache
|
||||||
|
* @type {Object}
|
||||||
|
* @property {string} url image url
|
||||||
|
* @property {number} height image height
|
||||||
|
* @property {number} width image width
|
||||||
|
*/
|
||||||
|
|
||||||
class CachedImageSizeFromUrl {
|
class CachedImageSizeFromUrl {
|
||||||
/**
|
/**
|
||||||
*
|
*
|
||||||
* @param {Object} options
|
* @param {Object} options
|
||||||
* @param {Function} options.getImageSizeFromUrl - method that resolves images based on URL
|
* @param {(url: string) => Promise<ImageSizeCache>} options.getImageSizeFromUrl - method that resolves images based on URL
|
||||||
* @param {Object} options.cache - cache store instance
|
* @param {Object} options.cache - cache store instance
|
||||||
*/
|
*/
|
||||||
constructor({getImageSizeFromUrl, cache}) {
|
constructor({getImageSizeFromUrl, cache}) {
|
||||||
|
@ -18,7 +32,7 @@ class CachedImageSizeFromUrl {
|
||||||
* Get cached image size from URL
|
* Get cached image size from URL
|
||||||
* Always returns {object} imageSizeCache
|
* Always returns {object} imageSizeCache
|
||||||
* @param {string} url
|
* @param {string} url
|
||||||
* @returns {Promise<Object>} imageSizeCache
|
* @returns {Promise<ImageSizeCache>}
|
||||||
* @description Takes a url and returns image width and height from cache if available.
|
* @description Takes a url and returns image width and height from cache if available.
|
||||||
* If not in cache, `getImageSizeFromUrl` is called and returns the dimensions in a Promise.
|
* If not in cache, `getImageSizeFromUrl` is called and returns the dimensions in a Promise.
|
||||||
*/
|
*/
|
||||||
|
@ -50,7 +64,9 @@ class CachedImageSizeFromUrl {
|
||||||
}
|
}
|
||||||
|
|
||||||
// in case of error we just attach the url
|
// in case of error we just attach the url
|
||||||
this.cache.set(url, url);
|
this.cache.set(url, {
|
||||||
|
url
|
||||||
|
});
|
||||||
|
|
||||||
return this.cache.get(url);
|
return this.cache.get(url);
|
||||||
}
|
}
|
||||||
|
|
|
@ -74,6 +74,7 @@ describe('lib/image: image size cache', function () {
|
||||||
|
|
||||||
cacheStore.get(url).should.not.be.undefined;
|
cacheStore.get(url).should.not.be.undefined;
|
||||||
const image = cacheStore.get(url);
|
const image = cacheStore.get(url);
|
||||||
|
should.equal(image.url, 'http://mysite.com/content/image/mypostcoverimage.jpg');
|
||||||
should.not.exist(image.width);
|
should.not.exist(image.width);
|
||||||
should.not.exist(image.height);
|
should.not.exist(image.height);
|
||||||
});
|
});
|
||||||
|
@ -93,6 +94,7 @@ describe('lib/image: image size cache', function () {
|
||||||
|
|
||||||
cacheStore.get(url).should.not.be.undefined;
|
cacheStore.get(url).should.not.be.undefined;
|
||||||
const image = cacheStore.get(url);
|
const image = cacheStore.get(url);
|
||||||
|
should.equal(image.url, 'http://mysite.com/content/image/mypostcoverimage.jpg');
|
||||||
should.not.exist(image.width);
|
should.not.exist(image.width);
|
||||||
should.not.exist(image.height);
|
should.not.exist(image.height);
|
||||||
});
|
});
|
||||||
|
|
Loading…
Add table
Reference in a new issue