From 079f7023241ff8beebe9fe9e6b1b4d3014dd88a0 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Mon, 12 Nov 2018 12:50:40 +0000 Subject: [PATCH 1/5] Re-store cat SVGs from assets server These files have diverged from the versions stored on the assets server due to the way Git handles line endings. Re-save them and keep their line endings from changing again by treating SVGs as binary in this repo. --- .gitattributes | 5 ++ .../09dc888b0b7df19f70d81588ae73420e.svg | 82 +++++++++---------- .../3696356a03a8d938318876a593572843.svg | 72 ++++++++-------- 3 files changed, 82 insertions(+), 77 deletions(-) diff --git a/.gitattributes b/.gitattributes index 84e89375a72..e81c38bb515 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,6 +6,11 @@ # File types which we know are binary +# Treat SVG files as binary so that their contents don't change due to line +# endings. The contents of SVGs must not change from the way they're stored +# on assets.scratch.mit.edu so that MD5 calculations don't change. +*.svg binary + # Prefer LF for most file types *.frag text eol=lf *.htm text eol=lf diff --git a/src/lib/default-project/09dc888b0b7df19f70d81588ae73420e.svg b/src/lib/default-project/09dc888b0b7df19f70d81588ae73420e.svg index d449b3d15b9..cf8e01588d5 100755 --- a/src/lib/default-project/09dc888b0b7df19f70d81588ae73420e.svg +++ b/src/lib/default-project/09dc888b0b7df19f70d81588ae73420e.svg @@ -1,42 +1,42 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/lib/default-project/3696356a03a8d938318876a593572843.svg b/src/lib/default-project/3696356a03a8d938318876a593572843.svg index 0ecb2de81d9..657f5b598fc 100755 --- a/src/lib/default-project/3696356a03a8d938318876a593572843.svg +++ b/src/lib/default-project/3696356a03a8d938318876a593572843.svg @@ -1,37 +1,37 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From 44d948bf6595bac320c40f787650ccd1cd38fce1 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Mon, 5 Nov 2018 20:42:06 +0000 Subject: [PATCH 2/5] Save project JSON and assets separately Instead of saving .sb3 files to the projects server, save just the project JSON. Save assets to the assets server. This happens via the asset types being routed to different web sources by storage. --- src/lib/project-saver-hoc.jsx | 50 ++++++++++++++++++++++------------- src/lib/storage.js | 11 +++++++- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/lib/project-saver-hoc.jsx b/src/lib/project-saver-hoc.jsx index 4d44e143ae0..62180d3dcbd 100644 --- a/src/lib/project-saver-hoc.jsx +++ b/src/lib/project-saver-hoc.jsx @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import {connect} from 'react-redux'; import VM from 'scratch-vm'; +import log from '../lib/log'; import storage from '../lib/storage'; import {showStandardAlert} from '../reducers/alerts'; import { @@ -136,25 +137,36 @@ const ProjectSaverHOC = function (WrappedComponent) { * @param {?object} requestParams - object of params to add to request body */ storeProject (projectId, requestParams) { - return this.props.vm.saveProjectSb3() - .then(content => { - const assetType = storage.AssetType.Project; - const dataFormat = storage.DataFormat.SB3; - const body = new FormData(); - body.append('sb3_file', content, 'sb3_file'); - if (requestParams) { - for (const key in requestParams) { - body.append(key, requestParams[key]); - } - } - // when id is undefined or null, storage.store as we have - // configured it will create a new project with id - return storage.store( - assetType, - dataFormat, - body, - projectId - ); + requestParams = requestParams || {}; + return Promise.all(this.props.vm.assets + .filter(asset => !asset.clean) + .map( + asset => storage.store( + asset.assetType, + asset.dataFormat, + asset.data, + asset.assetId + ).then( + () => (asset.clean = true) + ) + ) + ).then(() => { + const body = new FormData(); + const sb3Json = new Blob([this.props.vm.toJSON()], {type: 'application/json'}); + body.append('sb3_file', sb3Json, 'sb3_file'); + for (const key in requestParams) { + if (requestParams.hasOwnProperty(key)) body.append(key, requestParams[key]); + } + return storage.store( + storage.AssetType.Project, + storage.DataFormat.JSON, + body, + projectId + ); + }) + .catch(err => { + // @todo do something here + log.error(err); }); } render () { diff --git a/src/lib/storage.js b/src/lib/storage.js index 59c0ecc6bd0..b300544b57c 100644 --- a/src/lib/storage.js +++ b/src/lib/storage.js @@ -20,7 +20,9 @@ class Storage extends ScratchStorage { ); this.addWebStore( [this.AssetType.ImageVector, this.AssetType.ImageBitmap, this.AssetType.Sound], - this.getAssetGetConfig.bind(this) + this.getAssetGetConfig.bind(this), + this.getAssetCreateConfig.bind(this), + this.getAssetCreateConfig.bind(this) ); this.addWebStore( [this.AssetType.Sound], @@ -51,6 +53,13 @@ class Storage extends ScratchStorage { getAssetGetConfig (asset) { return `${this.assetHost}/internalapi/asset/${asset.assetId}.${asset.dataFormat}/get/`; } + getAssetCreateConfig (asset) { + return { + method: 'post', + url: `${this.assetHost}/${asset.assetId}.${asset.dataFormat}`, + withCredentials: true + }; + } setTranslatorFunction (translator) { this.translator = translator; this.cacheDefaultProject(); From 8857652c93852943456fede76a1274aeb2b23822 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Mon, 12 Nov 2018 16:48:59 +0000 Subject: [PATCH 3/5] Add some explanatory comments Hopefully they are actually clarifying --- src/lib/storage.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/lib/storage.js b/src/lib/storage.js index b300544b57c..142d818780e 100644 --- a/src/lib/storage.js +++ b/src/lib/storage.js @@ -21,6 +21,9 @@ class Storage extends ScratchStorage { this.addWebStore( [this.AssetType.ImageVector, this.AssetType.ImageBitmap, this.AssetType.Sound], this.getAssetGetConfig.bind(this), + // We set both the create and update configs to the same method because + // storage assumes it should update if there is an assetId, but the + // asset store uses the assetId as part of the create URI. this.getAssetCreateConfig.bind(this), this.getAssetCreateConfig.bind(this) ); @@ -55,6 +58,10 @@ class Storage extends ScratchStorage { } getAssetCreateConfig (asset) { return { + // There is no such thing as updating assets, but storage assumes it + // should update if there is an assetId, and the asset store uses the + // assetId as part of the create URI. So, force the method to POST. + // Then when storage finds this config to use for the "update", still POSTs method: 'post', url: `${this.assetHost}/${asset.assetId}.${asset.dataFormat}`, withCredentials: true From 71baad2097a2dca74655c75ea3ac1f67ea035c4c Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 15 Nov 2018 10:45:22 +0000 Subject: [PATCH 4/5] Bump scratch-vm to get vm.assets API --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e5e37b79bec..842ac207840 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ "scratch-render": "0.1.0-prerelease.20181102130522", "scratch-storage": "1.2.0", "scratch-svg-renderer": "0.2.0-prerelease.20181101210634", - "scratch-vm": "0.2.0-prerelease.20181114192419", + "scratch-vm": "0.2.0-prerelease.20181115103725", "selenium-webdriver": "3.6.0", "startaudiocontext": "1.2.1", "style-loader": "^0.23.0", From 70a622280f554350ed64b0dd9025a17f1f1b423c Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 15 Nov 2018 11:22:40 +0000 Subject: [PATCH 5/5] Fix tests --- test/unit/util/project-saver-hoc.test.jsx | 30 +++++++++++++++-------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/test/unit/util/project-saver-hoc.test.jsx b/test/unit/util/project-saver-hoc.test.jsx index 9b0fc020095..40bf5efed92 100644 --- a/test/unit/util/project-saver-hoc.test.jsx +++ b/test/unit/util/project-saver-hoc.test.jsx @@ -142,9 +142,11 @@ describe('projectSaverHOC', () => { }); test('if we enter creating new state, vm project should be requested', () => { - vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); + const mockedStoreProject = jest.fn(() => Promise.resolve()); + // The first wrapper is redux's Connect HOC + WrappedComponent.WrappedComponent.prototype.storeProject = mockedStoreProject; const mounted = mount( { isCreatingNew: true, loadingState: LoadingState.CREATING_NEW }); - expect(vm.saveProjectSb3).toHaveBeenCalled(); + expect(mockedStoreProject).toHaveBeenCalled(); }); test('if we enter remixing state, vm project should be requested, and alert should show', () => { - vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); const mockedShowCreatingAlert = jest.fn(); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); + const mockedStoreProject = jest.fn(() => Promise.resolve()); + // The first wrapper is redux's Connect HOC + WrappedComponent.WrappedComponent.prototype.storeProject = mockedStoreProject; const mounted = mount( { isRemixing: true, loadingState: LoadingState.REMIXING }); - expect(vm.saveProjectSb3).toHaveBeenCalled(); + expect(mockedStoreProject).toHaveBeenCalled(); expect(mockedShowCreatingAlert).toHaveBeenCalled(); }); test('if we enter creating copy state, vm project should be requested, and alert should show', () => { - vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); const mockedShowCreatingAlert = jest.fn(); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); + const mockedStoreProject = jest.fn(() => Promise.resolve()); + // The first wrapper is redux's Connect HOC + WrappedComponent.WrappedComponent.prototype.storeProject = mockedStoreProject; const mounted = mount( { isCreatingCopy: true, loadingState: LoadingState.CREATING_COPY }); - expect(vm.saveProjectSb3).toHaveBeenCalled(); + expect(mockedStoreProject).toHaveBeenCalled(); expect(mockedShowCreatingAlert).toHaveBeenCalled(); }); test('if we enter updating/saving state, vm project should be requested', () => { - vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); + const mockedStoreProject = jest.fn(() => Promise.resolve()); + // The first wrapper is redux's Connect HOC + WrappedComponent.WrappedComponent.prototype.storeProject = mockedStoreProject; const mounted = mount( { isUpdating: true, loadingState: LoadingState.MANUAL_UPDATING }); - expect(vm.saveProjectSb3).toHaveBeenCalled(); + expect(mockedStoreProject).toHaveBeenCalled(); }); test('if we are already in updating/saving state, vm project ' + 'should NOT requested, alert should NOT show', () => { - vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); const mockedShowCreatingAlert = jest.fn(); const Component = () =>
; const WrappedComponent = projectSaverHOC(Component); + const mockedStoreProject = jest.fn(() => Promise.resolve()); + // The first wrapper is redux's Connect HOC + WrappedComponent.WrappedComponent.prototype.storeProject = mockedStoreProject; const mounted = mount( { loadingState: LoadingState.AUTO_UPDATING, reduxProjectId: '99' // random change to force a re-render and componentDidUpdate }); - expect(vm.saveProjectSb3).not.toHaveBeenCalled(); + expect(mockedStoreProject).not.toHaveBeenCalled(); expect(mockedShowCreatingAlert).not.toHaveBeenCalled(); });