From aad22c00c457663af9d9854b0aa358f174f9cb49 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Wed, 14 Sep 2016 04:27:12 -0700 Subject: [PATCH] Calculate asset hash from file contents, cache hashes Summary: This makes the `hash` property of asset data depend on asset file contents rather than modification time of files. That means that bundles will be consistent across different checkouts. Reviewed By: martinbigio Differential Revision: D3856815 fbshipit-source-id: 8bfea4e0a714f48fc6a4ae5ed2a1426dc8d5868e --- .../AssetServer/__tests__/AssetServer-test.js | 110 ++++++++++++------ react-packager/src/AssetServer/index.js | 53 +++++++-- react-packager/src/Server/index.js | 3 +- .../src/node-haste/__mocks__/graceful-fs.js | 31 ++++- 4 files changed, 148 insertions(+), 49 deletions(-) diff --git a/react-packager/src/AssetServer/__tests__/AssetServer-test.js b/react-packager/src/AssetServer/__tests__/AssetServer-test.js index 9ee5a1e9..4bbbdcec 100644 --- a/react-packager/src/AssetServer/__tests__/AssetServer-test.js +++ b/react-packager/src/AssetServer/__tests__/AssetServer-test.js @@ -11,27 +11,31 @@ jest.disableAutomock(); -jest - .mock('crypto') - .mock('fs'); +jest.mock('fs'); const Promise = require('promise'); -var AssetServer = require('../'); -var crypto = require('crypto'); -var fs = require('fs'); +const AssetServer = require('../'); +const crypto = require('crypto'); +const {EventEmitter} = require('events'); +const fs = require('fs'); + +const {objectContaining} = jasmine; describe('AssetServer', () => { + let fileWatcher; beforeEach(() => { const NodeHaste = require('../../node-haste'); NodeHaste.getAssetDataFromName = require.requireActual('../../node-haste/lib/getAssetDataFromName'); + fileWatcher = new EventEmitter(); }); describe('assetServer.get', () => { - pit('should work for the simple case', () => { + it('should work for the simple case', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -53,10 +57,11 @@ describe('AssetServer', () => { ); }); - pit('should work for the simple case with platform ext', () => { + it('should work for the simple case with platform ext', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -90,10 +95,11 @@ describe('AssetServer', () => { }); - pit('should work for the simple case with jpg', () => { + it('should work for the simple case with jpg', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png', 'jpg'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -116,10 +122,11 @@ describe('AssetServer', () => { ); }); - pit('should pick the bigger one', () => { + it('should pick the bigger one', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -138,10 +145,11 @@ describe('AssetServer', () => { ); }); - pit('should pick the bigger one with platform ext', () => { + it('should pick the bigger one with platform ext', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -169,10 +177,11 @@ describe('AssetServer', () => { ]); }); - pit('should support multiple project roots', () => { + it('should support multiple project roots', () => { const server = new AssetServer({ projectRoots: ['/root', '/root2'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -197,18 +206,11 @@ describe('AssetServer', () => { }); describe('assetServer.getAssetData', () => { - pit('should get assetData', () => { - const hash = { - update: jest.fn(), - digest: jest.fn(), - }; - - hash.digest.mockImpl(() => 'wow such hash'); - crypto.createHash.mockImpl(() => hash); - + it('should get assetData', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -223,8 +225,7 @@ describe('AssetServer', () => { }); return server.getAssetData('imgs/b.png').then(data => { - expect(hash.update.mock.calls.length).toBe(4); - expect(data).toEqual({ + expect(data).toEqual(objectContaining({ type: 'png', name: 'b', scales: [1, 2, 4, 4.5], @@ -234,23 +235,15 @@ describe('AssetServer', () => { '/root/imgs/b@4x.png', '/root/imgs/b@4.5x.png', ], - hash: 'wow such hash', - }); + })); }); }); - pit('should get assetData for non-png images', () => { - const hash = { - update: jest.fn(), - digest: jest.fn(), - }; - - hash.digest.mockImpl(() => 'wow such hash'); - crypto.createHash.mockImpl(() => hash); - + it('should get assetData for non-png images', () => { const server = new AssetServer({ projectRoots: ['/root'], assetExts: ['png', 'jpeg'], + fileWatcher, }); fs.__setMockFilesystem({ @@ -265,8 +258,7 @@ describe('AssetServer', () => { }); return server.getAssetData('imgs/b.jpg').then(data => { - expect(hash.update.mock.calls.length).toBe(4); - expect(data).toEqual({ + expect(data).toEqual(objectContaining({ type: 'jpg', name: 'b', scales: [1, 2, 4, 4.5], @@ -276,7 +268,51 @@ describe('AssetServer', () => { '/root/imgs/b@4x.jpg', '/root/imgs/b@4.5x.jpg', ], - hash: 'wow such hash', + })); + }); + }); + + describe('hash:', () => { + let server, fileSystem; + beforeEach(() => { + server = new AssetServer({ + projectRoots: ['/root'], + assetExts: ['jpg'], + fileWatcher, + }); + + fileSystem = { + 'root': { + imgs: { + 'b@1x.jpg': 'b1 image', + 'b@2x.jpg': 'b2 image', + 'b@4x.jpg': 'b4 image', + 'b@4.5x.jpg': 'b4.5 image', + } + } + }; + + fs.__setMockFilesystem(fileSystem); + }); + + it('uses the file contents to build the hash', () => { + const hash = crypto.createHash('md5'); + for (const name in fileSystem.root.imgs) { + hash.update(fileSystem.root.imgs[name]); + } + + return server.getAssetData('imgs/b.jpg').then(data => + expect(data).toEqual(objectContaining({hash: hash.digest('hex')})) + ); + }); + + it('changes the hash when the passed-in file watcher emits an `all` event', () => { + return server.getAssetData('imgs/b.jpg').then(initialData => { + fileSystem.root.imgs['b@4x.jpg'] = 'updated data'; + fileWatcher.emit('all', 'arbitrary', '/root', 'imgs/b@4x.jpg'); + return server.getAssetData('imgs/b.jpg').then(data => + expect(data.hash).not.toEqual(initialData.hash) + ); }); }); }); diff --git a/react-packager/src/AssetServer/index.js b/react-packager/src/AssetServer/index.js index 15a13f2b..14ea7f28 100644 --- a/react-packager/src/AssetServer/index.js +++ b/react-packager/src/AssetServer/index.js @@ -43,6 +43,10 @@ const validateOpts = declareOpts({ type: 'array', required: true, }, + fileWatcher: { + type: 'object', + required: true, + } }); class AssetServer { @@ -50,6 +54,11 @@ class AssetServer { const opts = validateOpts(options); this._roots = opts.projectRoots; this._assetExts = opts.assetExts; + this._hashes = new Map(); + this._files = new Map(); + + opts.fileWatcher + .on('all', (type, root, file) => this._onFileChange(type, root, file)); } get(assetPath, platform = null) { @@ -76,21 +85,33 @@ class AssetServer { data.scales = record.scales; data.files = record.files; - return Promise.all( - record.files.map(file => stat(file)) - ); - }).then(stats => { - const hash = crypto.createHash('md5'); - stats.forEach(fstat => - hash.update(fstat.mtime.getTime().toString()) - ); + if (this._hashes.has(assetPath)) { + data.hash = this._hashes.get(assetPath); + return data; + } - data.hash = hash.digest('hex'); - return data; + return new Promise((resolve, reject) => { + const hash = crypto.createHash('md5'); + hashFiles(data.files.slice(), hash, error => { + if (error) { + reject(error); + } else { + data.hash = hash.digest('hex'); + this._hashes.set(assetPath, data.hash); + data.files.forEach(f => this._files.set(f, assetPath)); + resolve(data); + } + }); + }); }); } + _onFileChange(type, root, file) { + const asset = this._files.get(path.join(root, file)); + this._hashes.delete(asset); + } + /** * Given a request for an image by path. That could contain a resolution * postfix, we need to find that image (or the closest one to it's resolution) @@ -213,4 +234,16 @@ function getAssetKey(assetName, platform) { } } +function hashFiles(files, hash, callback) { + if (!files.length) { + callback(null); + return; + } + + fs.createReadStream(files.shift()) + .on('data', data => hash.update(data)) + .once('end', () => hashFiles(files, hash, callback)) + .once('error', error => callback(error)); +} + module.exports = AssetServer; diff --git a/react-packager/src/Server/index.js b/react-packager/src/Server/index.js index ecf670fb..54ef2b1b 100644 --- a/react-packager/src/Server/index.js +++ b/react-packager/src/Server/index.js @@ -219,8 +219,9 @@ class Server { : new FileWatcher(watchRootConfigs, {useWatchman: true}); this._assetServer = new AssetServer({ - projectRoots: opts.projectRoots, assetExts: opts.assetExts, + fileWatcher: this._fileWatcher, + projectRoots: opts.projectRoots, }); const bundlerOpts = Object.create(opts); diff --git a/react-packager/src/node-haste/__mocks__/graceful-fs.js b/react-packager/src/node-haste/__mocks__/graceful-fs.js index 24bd7712..1cfcb5cf 100644 --- a/react-packager/src/node-haste/__mocks__/graceful-fs.js +++ b/react-packager/src/node-haste/__mocks__/graceful-fs.js @@ -9,6 +9,7 @@ 'use strict'; const fs = jest.genMockFromModule('fs'); +const stream = require.requireActual('stream'); const noop = () => {}; function asyncCallback(cb) { @@ -185,7 +186,35 @@ fs.close.mockImpl((fd, callback = noop) => { let filesystem; -fs.__setMockFilesystem = (object) => filesystem = object; +fs.createReadStream.mockImpl(path => { + if (!path.startsWith('/')) { + throw Error('Cannot open file ' + path); + } + + const parts = path.split('/').slice(1); + let file = filesystem; + + for (const part of parts) { + file = file[part]; + if (!file) { + break; + } + } + + if (typeof file !== 'string') { + throw Error('Cannot open file ' + path); + } + + return new stream.Readable({ + read() { + this.push(file, 'utf8'); + this.push(null); + } + }); +}); + + +fs.__setMockFilesystem = (object) => (filesystem = object); function getToNode(filepath) { // Ignore the drive for Windows paths.