diff --git a/ghost/core/core/frontend/meta/asset-url.js b/ghost/core/core/frontend/meta/asset-url.js index 0547429a17..abb4a09112 100644 --- a/ghost/core/core/frontend/meta/asset-url.js +++ b/ghost/core/core/frontend/meta/asset-url.js @@ -44,13 +44,15 @@ function getAssetUrl(path, hasMinFile) { // Add the path for the requested asset output = urlUtils.urlJoin(output, path); - // Ensure we have an assetHash - // @TODO rework this! - if (!config.get('assetHash')) { - config.set('assetHash', (crypto.createHash('md5').update(Date.now().toString()).digest('hex')).substring(0, 10)); + // Ensure we have an asset_hash + // This is backcompat, generating a hash if no config value is provided. + // Theme config can also provide either `false`(to disable) or a specific string to use as the hash. + // eslint-disable-next-line eqeqeq + if (config.get('asset_hash') == null) { + config.set('asset_hash', (crypto.createHash('md5').update(Date.now().toString()).digest('hex')).substring(0, 10)); } - // if url has # make sure the hash is at th right place + // if url has # make sure the hash is at the right place let anchor; if (path.match('#')) { const index = output.indexOf('#'); @@ -58,8 +60,10 @@ function getAssetUrl(path, hasMinFile) { output = output.slice(0, index); } - // Finally add the asset hash to the output URL - output += '?v=' + config.get('assetHash'); + // Finally add the asset hash to the output URL unless it is explicitly disabled by config + if (config.get('asset_hash') !== false) { + output += '?v=' + config.get('asset_hash'); + } if (anchor) { output += anchor; diff --git a/ghost/core/core/frontend/services/theme-engine/active.js b/ghost/core/core/frontend/services/theme-engine/active.js index 34fa33bc86..ed446d46c8 100644 --- a/ghost/core/core/frontend/services/theme-engine/active.js +++ b/ghost/core/core/frontend/services/theme-engine/active.js @@ -106,8 +106,9 @@ class ActiveTheme { mount(siteApp) { // reset the asset hash - // @TODO: set this on the theme instead of globally, or use proper file-based hash - config.set('assetHash', null); + // @TODO: use proper file-based hash + config.set('asset_hash', this.config('asset_hash')); + // clear the view cache siteApp.cache = {}; // Set the views and engine diff --git a/ghost/core/core/frontend/services/theme-engine/config/defaults.json b/ghost/core/core/frontend/services/theme-engine/config/defaults.json index 1680d56147..dc230faa91 100644 --- a/ghost/core/core/frontend/services/theme-engine/config/defaults.json +++ b/ghost/core/core/frontend/services/theme-engine/config/defaults.json @@ -1,4 +1,5 @@ { "posts_per_page": 5, - "card_assets": true + "card_assets": true, + "asset_hash": null } diff --git a/ghost/core/core/frontend/services/theme-engine/config/index.js b/ghost/core/core/frontend/services/theme-engine/config/index.js index af2ed27428..014f33e123 100644 --- a/ghost/core/core/frontend/services/theme-engine/config/index.js +++ b/ghost/core/core/frontend/services/theme-engine/config/index.js @@ -1,6 +1,10 @@ const _ = require('lodash'); const defaultConfig = require('./defaults'); -const allowedKeys = ['posts_per_page', 'image_sizes', 'card_assets']; +const allowedKeys = ['posts_per_page', 'image_sizes', 'card_assets', 'asset_hash']; + +module.exports.getDefaults = function getDefaults() { + return _.cloneDeep(defaultConfig); +}; module.exports.create = function configLoader(packageJson) { let config = _.cloneDeep(defaultConfig); diff --git a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap index 2fc30edbb0..f0084a04d5 100644 --- a/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap +++ b/ghost/core/test/unit/frontend/helpers/__snapshots__/ghost_head.test.js.snap @@ -134,7 +134,7 @@ Object { - ", + ", } `; @@ -550,7 +550,7 @@ Object { opacity: 0.92; } - ", + ", } `; @@ -829,7 +829,7 @@ Object { - ", + ", } `; @@ -1056,7 +1056,7 @@ Object { - ", + ", } `; @@ -1170,7 +1170,7 @@ Object { - ", + ", } `; @@ -1334,7 +1334,7 @@ Object { - ", + ", } `; @@ -1722,7 +1722,7 @@ Object { - ", + ", } `; @@ -1773,7 +1773,7 @@ Object { - ", + ", } `; diff --git a/ghost/core/test/unit/frontend/helpers/asset.test.js b/ghost/core/test/unit/frontend/helpers/asset.test.js index 507b773e42..c4bb721817 100644 --- a/ghost/core/test/unit/frontend/helpers/asset.test.js +++ b/ghost/core/test/unit/frontend/helpers/asset.test.js @@ -13,7 +13,7 @@ describe('{{asset}} helper', function () { const localSettingsCache = {}; before(function () { - configUtils.set({assetHash: 'abc'}); + configUtils.set({asset_hash: false}); configUtils.set({useMinFiles: true}); sinon.stub(settingsCache, 'get').callsFake(function (key) { @@ -36,7 +36,7 @@ describe('{{asset}} helper', function () { it('handles ghost.css for default templates correctly', function () { rendered = asset('public/ghost.css'); should.exist(rendered); - String(rendered).should.equal('/public/ghost.css?v=abc'); + String(rendered).should.equal('/public/ghost.css'); }); it('handles custom favicon correctly', function () { @@ -70,19 +70,19 @@ describe('{{asset}} helper', function () { rendered = asset('public/asset.js'); should.exist(rendered); - String(rendered).should.equal('/public/asset.js?v=abc'); + String(rendered).should.equal('/public/asset.js'); }); it('handles theme assets correctly', function () { rendered = asset('js/asset.js'); should.exist(rendered); - String(rendered).should.equal('/assets/js/asset.js?v=abc'); + String(rendered).should.equal('/assets/js/asset.js'); }); it('handles hasMinFile assets correctly', function () { rendered = asset('js/asset.js', {hash: {hasMinFile: true}}); should.exist(rendered); - String(rendered).should.equal('/assets/js/asset.min.js?v=abc'); + String(rendered).should.equal('/assets/js/asset.min.js'); }); }); @@ -105,7 +105,37 @@ describe('{{asset}} helper', function () { it('handles ghost.css for default templates correctly', function () { rendered = asset('public/ghost.css'); should.exist(rendered); - String(rendered).should.equal('http://127.0.0.1/public/ghost.css?v=abc'); + String(rendered).should.equal('http://127.0.0.1/public/ghost.css'); + }); + }); + + describe('with asset_hash setting', function () { + after(async function () { + await configUtils.restore(); + }); + + it('should have a hash parameter when null', function () { + configUtils.set('asset_hash', null); + rendered = asset('public/ghost.css'); + String(rendered).should.equal(`/public/ghost.css?v=${configUtils.config.get('asset_hash')}`); + }); + + it('should have a random hash parameter when undefined', function () { + configUtils.set('asset_hash', undefined); + rendered = asset('public/ghost.css'); + String(rendered).should.equal(`/public/ghost.css?v=${configUtils.config.get('asset_hash')}`); + }); + + it('should have a hash parameter when a string', function () { + configUtils.set('asset_hash', 'abcd1234'); + rendered = asset('public/ghost.css'); + String(rendered).should.equal('/public/ghost.css?v=abcd1234'); + }); + + it('should have no hash parameter when false', function () { + configUtils.set('asset_hash', false); + rendered = asset('public/ghost.css'); + String(rendered).should.equal('/public/ghost.css'); }); }); }); diff --git a/ghost/core/test/unit/frontend/helpers/ghost_head.test.js b/ghost/core/test/unit/frontend/helpers/ghost_head.test.js index a8305e5636..18423465b7 100644 --- a/ghost/core/test/unit/frontend/helpers/ghost_head.test.js +++ b/ghost/core/test/unit/frontend/helpers/ghost_head.test.js @@ -368,8 +368,8 @@ describe('{{ghost_head}} helper', function () { settingsCache.get.withArgs('comments_enabled').returns('off'); settingsCache.get.withArgs('members_track_sources').returns(true); - // Force the usage of a fixed asset hash so we have reliable snapshots - configUtils.set('assetHash', 'asset-hash'); + // disable the random asset hash so we have reliable snapshots + configUtils.set('asset_hash', false); makeFixtures(); }); diff --git a/ghost/core/test/unit/frontend/meta/asset-url.test.js b/ghost/core/test/unit/frontend/meta/asset-url.test.js index 26b1ea2548..54272d2956 100644 --- a/ghost/core/test/unit/frontend/meta/asset-url.test.js +++ b/ghost/core/test/unit/frontend/meta/asset-url.test.js @@ -9,6 +9,10 @@ const config = configUtils.config; const getAssetUrl = require('../../../../core/frontend/meta/asset-url'); describe('getAssetUrl', function () { + beforeEach(function () { + config.set('asset_hash', false); + }); + afterEach(async function () { await configUtils.restore(); sinon.restore(); @@ -16,32 +20,32 @@ describe('getAssetUrl', function () { it('should return asset url with just context', function () { const testUrl = getAssetUrl('myfile.js'); - testUrl.should.equal('/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.js'); }); it('should return asset url with just context even with leading /', function () { const testUrl = getAssetUrl('/myfile.js'); - testUrl.should.equal('/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.js'); }); it('should not add asset to url if ghost.css for default templates', function () { const testUrl = getAssetUrl('public/ghost.css'); - testUrl.should.equal('/public/ghost.css?v=' + config.get('assetHash')); + testUrl.should.equal('/public/ghost.css'); }); it('should not add asset to url has public in it', function () { const testUrl = getAssetUrl('public/myfile.js'); - testUrl.should.equal('/public/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/public/myfile.js'); }); it('should return hash before #', function () { const testUrl = getAssetUrl('myfile.svg#arrow-up'); - testUrl.should.equal(`/assets/myfile.svg?v=${config.get('assetHash')}#arrow-up`); + testUrl.should.equal(`/assets/myfile.svg#arrow-up`); }); it('should handle Handlebars’ SafeString', function () { const testUrl = getAssetUrl(new SafeString('myfile.js')); - testUrl.should.equal('/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.js'); }); describe('favicon', function () { @@ -72,25 +76,25 @@ describe('getAssetUrl', function () { it('should return asset minified url when hasMinFile & useMinFiles are both set to true', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('myfile.js', true); - testUrl.should.equal('/assets/myfile.min.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.min.js'); }); it('should NOT return asset minified url when hasMinFile true but useMinFiles is false', function () { configUtils.set('useMinFiles', false); const testUrl = getAssetUrl('myfile.js', true); - testUrl.should.equal('/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.js'); }); it('should NOT return asset minified url when hasMinFile false but useMinFiles is true', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('myfile.js', false); - testUrl.should.equal('/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/myfile.js'); }); it('should not add min to anything besides the last .', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('test.page/myfile.js', true); - testUrl.should.equal('/assets/test.page/myfile.min.js?v=' + config.get('assetHash')); + testUrl.should.equal('/assets/test.page/myfile.min.js'); }); }); @@ -105,22 +109,22 @@ describe('getAssetUrl', function () { it('should return asset url with just context', function () { const testUrl = getAssetUrl('myfile.js'); - testUrl.should.equal('/blog/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/myfile.js'); }); it('should return asset url with just context even with leading /', function () { const testUrl = getAssetUrl('/myfile.js'); - testUrl.should.equal('/blog/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/myfile.js'); }); it('should not add asset to url if ghost.css for default templates', function () { const testUrl = getAssetUrl('public/ghost.css'); - testUrl.should.equal('/blog/public/ghost.css?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/public/ghost.css'); }); it('should not add asset to url has public in it', function () { const testUrl = getAssetUrl('public/myfile.js'); - testUrl.should.equal('/blog/public/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/public/myfile.js'); }); describe('favicon', function () { @@ -148,25 +152,25 @@ describe('getAssetUrl', function () { it('should return asset minified url when hasMinFile & useMinFiles are both set to true', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('myfile.js', true); - testUrl.should.equal('/blog/assets/myfile.min.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/myfile.min.js'); }); it('should NOT return asset minified url when hasMinFile true but useMinFiles is false', function () { configUtils.set('useMinFiles', false); const testUrl = getAssetUrl('myfile.js', true); - testUrl.should.equal('/blog/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/myfile.js'); }); it('should NOT return asset minified url when hasMinFile false but useMinFiles is true', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('myfile.js', false); - testUrl.should.equal('/blog/assets/myfile.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/myfile.js'); }); it('should not add min to anything besides the last .', function () { configUtils.set('useMinFiles', true); const testUrl = getAssetUrl('test.page/myfile.js', true); - testUrl.should.equal('/blog/assets/test.page/myfile.min.js?v=' + config.get('assetHash')); + testUrl.should.equal('/blog/assets/test.page/myfile.min.js'); }); }); }); diff --git a/ghost/core/test/unit/frontend/services/theme-engine/active.test.js b/ghost/core/test/unit/frontend/services/theme-engine/active.test.js index f7b9ad9a3b..203bb58897 100644 --- a/ghost/core/test/unit/frontend/services/theme-engine/active.test.js +++ b/ghost/core/test/unit/frontend/services/theme-engine/active.test.js @@ -36,7 +36,12 @@ describe('Themes', function () { fakeLoadedTheme = { name: 'casper', - path: 'my/fake/theme/path' + path: 'my/fake/theme/path', + 'package.json': { + config: { + asset_hash: 'abcd1234' + } + } }; fakeCheckedTheme = { templates: { @@ -58,10 +63,6 @@ describe('Themes', function () { // Call mount! theme.mount(fakeBlogApp); - // Check the asset hash gets reset - configStub.calledOnce.should.be.true(); - configStub.calledWith('assetHash', null).should.be.true(); - // Check te view cache was cleared fakeBlogApp.cache.should.eql({}); @@ -91,7 +92,7 @@ describe('Themes', function () { // Check the asset hash gets reset configStub.calledOnce.should.be.true(); - configStub.calledWith('assetHash', null).should.be.true(); + configStub.calledWith('asset_hash', fakeLoadedTheme['package.json'].config.asset_hash).should.be.true(); // Check te view cache was cleared fakeBlogApp.cache.should.eql({}); diff --git a/ghost/core/test/unit/frontend/services/theme-engine/config.test.js b/ghost/core/test/unit/frontend/services/theme-engine/config.test.js index 9e9641d0c5..a1b2865a2d 100644 --- a/ghost/core/test/unit/frontend/services/theme-engine/config.test.js +++ b/ghost/core/test/unit/frontend/services/theme-engine/config.test.js @@ -2,6 +2,8 @@ const should = require('should'); const sinon = require('sinon'); const themeConfig = require('../../../../../core/frontend/services/theme-engine/config'); +const defaultConfig = themeConfig.getDefaults(); + describe('Themes', function () { afterEach(function () { sinon.restore(); @@ -11,37 +13,26 @@ describe('Themes', function () { it('handles no package.json', function () { const config = themeConfig.create(); - config.should.eql({ - posts_per_page: 5, - card_assets: true - }); + config.should.eql(defaultConfig); }); it('handles package.json without config', function () { const config = themeConfig.create({name: 'casper'}); - config.should.eql({ - posts_per_page: 5, - card_assets: true - }); + config.should.eql(defaultConfig); }); it('handles allows package.json to override default', function () { - const config = themeConfig.create({name: 'casper', config: {posts_per_page: 3, card_assets: true}}); + const overrideConfig = {posts_per_page: 3, card_assets: true}; + const config = themeConfig.create({name: 'casper', config: overrideConfig}); - config.should.eql({ - posts_per_page: 3, - card_assets: true - }); + config.should.eql({...defaultConfig, ...overrideConfig}); }); it('handles ignores non-allowed config', function () { const config = themeConfig.create({name: 'casper', config: {magic: 'roundabout'}}); - config.should.eql({ - posts_per_page: 5, - card_assets: true - }); + config.should.eql(defaultConfig); }); }); }); diff --git a/ghost/core/test/unit/server/models/member.test.js b/ghost/core/test/unit/server/models/member.test.js index 72b6739e85..2ea12aeb4e 100644 --- a/ghost/core/test/unit/server/models/member.test.js +++ b/ghost/core/test/unit/server/models/member.test.js @@ -12,7 +12,7 @@ describe('Unit: models/member', function () { }); beforeEach(function () { - config.set('assetHash', '1'); + config.set('asset_hash', false); }); afterEach(async function () {