feat: bundle assets with content (#2958)

* fix(media_folder_relative): use collection name in unpublished entry

* refactor: pass arguments as object to AssetProxy ctor

* feat: support media folders per collection

* feat: resolve media files path based on entry path

* fix: asset public path resolving

* refactor: introduce typescript for AssetProxy

* refactor: code cleanup

* refactor(asset-proxy): add tests,switch to typescript,extract arguments

* refactor: typescript for editorialWorkflow

* refactor: add typescript for media library actions

* refactor: fix type error on map set

* refactor: move locale selector into reducer

* refactor: add typescript for entries actions

* refactor: remove duplication between asset store and media lib

* feat: load assets from backend using API

* refactor(github): add typescript, cache media files

* fix: don't load media URL if already loaded

* feat: add media folder config to collection

* fix: load assets from API when not in UI state

* feat: load entry media files when opening media library

* fix: editorial workflow draft media files bug fixes

* test(unit): fix unit tests

* fix: editor control losing focus

* style: add eslint object-shorthand rule

* test(cypress): re-record mock data

* fix: fix non github backends, large media

* test: uncomment only in tests

* fix(backend-test): add missing displayURL property

* test(e2e): add media library tests

* test(e2e): enable visual testing

* test(e2e): add github backend media library tests

* test(e2e): add git-gateway large media tests

* chore: post rebase fixes

* test: fix tests

* test: fix tests

* test(cypress): fix tests

* docs: add media_folder docs

* test(e2e): add media library delete test

* test(e2e): try and fix image comparison on CI

* ci: reduce test machines from 9 to 8

* test: add reducers and selectors unit tests

* test(e2e): disable visual regression testing for now

* test: add getAsset unit tests

* refactor: use Asset class component instead of hooks

* build: don't inline source maps

* test: add more media path tests

packages/netlify-cms-backend-github/package.json

@@ -17,7 +17,7 @@
   "scripts": {
     "develop": "yarn build:esm --watch",
     "build": "cross-env NODE_ENV=production webpack",
-    "build:esm": "cross-env NODE_ENV=esm babel src --out-dir dist/esm --ignore \"**/__tests__\" --root-mode upward",
+    "build:esm": "cross-env NODE_ENV=esm babel src --out-dir dist/esm --ignore \"**/__tests__\" --root-mode upward --extensions \".js,.jsx,.ts,.tsx\"",
     "createFragmentTypes": "node scripts/createFragmentTypes.js"
   },
   "dependencies": {

packages/netlify-cms-backend-github/src/GraphQLAPI.js

@@ -43,6 +43,7 @@ export default class GraphQLAPI extends API {
     const authLink = setContext((_, { headers }) => {
       return {
         headers: {
+          'Content-Type': 'application/json; charset=utf-8',
           ...headers,
           authorization: this.token ? `token ${this.token}` : '',
         },
@@ -140,7 +141,7 @@ export default class GraphQLAPI extends API {
     return { owner, name };
   }
 
-  async retrieveContent(path, branch, repoURL) {
+  async retrieveContent({ path, branch, repoURL, parseText }) {
     const { owner, name } = this.getOwnerAndNameFromRepoUrl(repoURL);
     const { is_null, is_binary, text } = await this.retrieveBlobObject(
       owner,
@@ -152,11 +153,14 @@ export default class GraphQLAPI extends API {
     } else if (!is_binary) {
       return text;
     } else {
-      return super.retrieveContent(path, branch, repoURL);
+      return super.retrieveContent({ path, branch, repoURL, parseText });
     }
   }
 
-  async fetchBlobContent(sha, repoURL) {
+  async fetchBlobContent(sha, repoURL, parseText) {
+    if (!parseText) {
+      return super.fetchBlobContent(sha, repoURL);
+    }
     const { owner, name } = this.getOwnerAndNameFromRepoUrl(repoURL);
     const { is_null, is_binary, text } = await this.retrieveBlobObject(
       owner,

packages/netlify-cms-backend-github/src/__tests__/API.spec.js

@@ -106,7 +106,10 @@ describe('github API', () => {
       expect(result).toEqual('some response');
       expect(fetch).toHaveBeenCalledTimes(1);
       expect(fetch).toHaveBeenCalledWith('https://api.github.com/some-path?ts=1000', {
-        headers: { Authorization: 'token token', 'Content-Type': 'application/json' },
+        headers: {
+          Authorization: 'token token',
+          'Content-Type': 'application/json; charset=utf-8',
+        },
       });
     });
 
@@ -133,9 +136,10 @@ describe('github API', () => {
     it('should allow overriding requestHeaders to return a promise ', async () => {
       const api = new API({ branch: 'gh-pages', repo: 'my-repo', token: 'token' });
 
-      api.requestHeaders = jest
-        .fn()
-        .mockResolvedValue({ Authorization: 'promise-token', 'Content-Type': 'application/json' });
+      api.requestHeaders = jest.fn().mockResolvedValue({
+        Authorization: 'promise-token',
+        'Content-Type': 'application/json; charset=utf-8',
+      });
 
       fetch.mockResolvedValue({
         text: jest.fn().mockResolvedValue('some response'),
@@ -147,7 +151,10 @@ describe('github API', () => {
       expect(result).toEqual('some response');
       expect(fetch).toHaveBeenCalledTimes(1);
       expect(fetch).toHaveBeenCalledWith('https://api.github.com/some-path?ts=1000', {
-        headers: { Authorization: 'promise-token', 'Content-Type': 'application/json' },
+        headers: {
+          Authorization: 'promise-token',
+          'Content-Type': 'application/json; charset=utf-8',
+        },
       });
     });
   });
@@ -157,31 +164,30 @@ describe('github API', () => {
       const api = new API({ branch: 'master', repo: 'owner/repo' });
 
       const blob = {};
-      const response = { blob: jest.fn().mockResolvedValue(blob) };
-      api.fetchBlob = jest.fn().mockResolvedValue(response);
+      api.readFile = jest.fn().mockResolvedValue(blob);
 
       await expect(api.getMediaAsBlob('sha', 'static/media/image.png')).resolves.toBe(blob);
 
-      expect(api.fetchBlob).toHaveBeenCalledTimes(1);
-      expect(api.fetchBlob).toHaveBeenCalledWith('sha', '/repos/owner/repo');
-
-      expect(response.blob).toHaveBeenCalledTimes(1);
+      expect(api.readFile).toHaveBeenCalledTimes(1);
+      expect(api.readFile).toHaveBeenCalledWith('static/media/image.png', 'sha', {
+        parseText: false,
+      });
     });
 
-    it('should return test blob on non file', async () => {
+    it('should return text blob on svg file', async () => {
       const api = new API({ branch: 'master', repo: 'owner/repo' });
 
-      const response = { text: jest.fn().mockResolvedValue('svg') };
-      api.fetchBlob = jest.fn().mockResolvedValue(response);
+      const text = 'svg';
+      api.readFile = jest.fn().mockResolvedValue(text);
 
       await expect(api.getMediaAsBlob('sha', 'static/media/logo.svg')).resolves.toEqual(
-        new Blob(['svg'], { type: 'image/svg+xml' }),
+        new Blob([text], { type: 'image/svg+xml' }),
       );
 
-      expect(api.fetchBlob).toHaveBeenCalledTimes(1);
-      expect(api.fetchBlob).toHaveBeenCalledWith('sha', '/repos/owner/repo');
-
-      expect(response.text).toHaveBeenCalledTimes(1);
+      expect(api.readFile).toHaveBeenCalledTimes(1);
+      expect(api.readFile).toHaveBeenCalledWith('static/media/logo.svg', 'sha', {
+        parseText: true,
+      });
     });
   });
 
@@ -310,7 +316,6 @@ describe('github API', () => {
       const mediaFiles = [
         {
           path: '/static/media/image-1.png',
-          uploaded: true,
           sha: 'image-1.png',
         },
         {
@@ -321,8 +326,9 @@ describe('github API', () => {
 
       await api.persistFiles(entry, mediaFiles, { useWorkflow: true });
 
-      expect(api.uploadBlob).toHaveBeenCalledTimes(2);
+      expect(api.uploadBlob).toHaveBeenCalledTimes(3);
       expect(api.uploadBlob).toHaveBeenCalledWith(entry);
+      expect(api.uploadBlob).toHaveBeenCalledWith(mediaFiles[0]);
       expect(api.uploadBlob).toHaveBeenCalledWith(mediaFiles[1]);
 
       expect(api.editorialWorkflowGit).toHaveBeenCalledTimes(1);

packages/netlify-cms-backend-github/src/__tests__/implementation.spec.js

@@ -92,13 +92,12 @@ describe('github backend implementation', () => {
       });
     });
 
-    it('should persist media file when not draft', async () => {
+    it('should persist media file', async () => {
       const gitHubImplementation = new GitHubImplementation(config);
       gitHubImplementation.api = mockAPI;
 
       const mediaFile = {
-        value: 'image.png',
-        fileObj: { size: 100 },
+        fileObj: { size: 100, name: 'image.png' },
         path: '/media/image.png',
       };
 
@@ -109,7 +108,6 @@ describe('github backend implementation', () => {
         size: 100,
         displayURL: 'displayURL',
         path: 'media/image.png',
-        draft: undefined,
       });
 
       expect(persistFiles).toHaveBeenCalledTimes(1);
@@ -118,33 +116,6 @@ describe('github backend implementation', () => {
       expect(createObjectURL).toHaveBeenCalledWith(mediaFile.fileObj);
     });
 
-    it('should not persist media file when draft', async () => {
-      const gitHubImplementation = new GitHubImplementation(config);
-      gitHubImplementation.api = mockAPI;
-
-      createObjectURL.mockReturnValue('displayURL');
-
-      const mediaFile = {
-        value: 'image.png',
-        fileObj: { size: 100 },
-        path: '/media/image.png',
-      };
-
-      expect.assertions(4);
-      await expect(gitHubImplementation.persistMedia(mediaFile, { draft: true })).resolves.toEqual({
-        id: undefined,
-        name: 'image.png',
-        size: 100,
-        displayURL: 'displayURL',
-        path: 'media/image.png',
-        draft: true,
-      });
-
-      expect(persistFiles).toHaveBeenCalledTimes(0);
-      expect(createObjectURL).toHaveBeenCalledTimes(1);
-      expect(createObjectURL).toHaveBeenCalledWith(mediaFile.fileObj);
-    });
-
     it('should log and throw error on "persistFiles" error', async () => {
       const gitHubImplementation = new GitHubImplementation(config);
       gitHubImplementation.api = mockAPI;
@@ -168,7 +139,7 @@ describe('github backend implementation', () => {
     });
   });
 
-  describe('getMediaFiles', () => {
+  describe('loadEntryMediaFiles', () => {
     const getMediaAsBlob = jest.fn();
     const mockAPI = {
       getMediaAsBlob,
@@ -183,15 +154,11 @@ describe('github backend implementation', () => {
 
       const file = new File([blob], name);
 
-      const data = {
-        metaData: {
-          objects: {
-            files: [{ path: 'static/media/image.png', sha: 'image.png' }],
-          },
-        },
-      };
-
-      await expect(gitHubImplementation.getMediaFiles(data)).resolves.toEqual([
+      await expect(
+        gitHubImplementation.loadEntryMediaFiles([
+          { path: 'static/media/image.png', sha: 'image.png' },
+        ]),
+      ).resolves.toEqual([
         {
           id: 'image.png',
           sha: 'image.png',
@@ -217,14 +184,16 @@ describe('github backend implementation', () => {
     it('should return unpublished entry', async () => {
       const gitHubImplementation = new GitHubImplementation(config);
       gitHubImplementation.api = mockAPI;
-      gitHubImplementation.getMediaFiles = jest.fn().mockResolvedValue([{ path: 'image.png' }]);
+      gitHubImplementation.loadEntryMediaFiles = jest
+        .fn()
+        .mockResolvedValue([{ path: 'image.png', sha: 'sha' }]);
 
       generateContentKey.mockReturnValue('contentKey');
 
       const data = {
         fileData: 'fileData',
         isModification: true,
-        metaData: { objects: { entry: { path: 'entry-path' } } },
+        metaData: { objects: { entry: { path: 'entry-path' }, files: [{ path: 'image.png' }] } },
       };
       readUnpublishedBranchFile.mockResolvedValue(data);
 
@@ -233,8 +202,8 @@ describe('github backend implementation', () => {
         slug: 'slug',
         file: { path: 'entry-path' },
         data: 'fileData',
-        metaData: { objects: { entry: { path: 'entry-path' } } },
-        mediaFiles: [{ path: 'image.png' }],
+        metaData: { objects: { entry: { path: 'entry-path' }, files: [{ path: 'image.png' }] } },
+        mediaFiles: [{ path: 'image.png', sha: 'sha' }],
         isModification: true,
       });
 
@@ -244,8 +213,10 @@ describe('github backend implementation', () => {
       expect(readUnpublishedBranchFile).toHaveBeenCalledTimes(1);
       expect(readUnpublishedBranchFile).toHaveBeenCalledWith('contentKey');
 
-      expect(gitHubImplementation.getMediaFiles).toHaveBeenCalledTimes(1);
-      expect(gitHubImplementation.getMediaFiles).toHaveBeenCalledWith(data);
+      expect(gitHubImplementation.loadEntryMediaFiles).toHaveBeenCalledTimes(1);
+      expect(gitHubImplementation.loadEntryMediaFiles).toHaveBeenCalledWith(
+        data.metaData.objects.files,
+      );
     });
   });
 });

packages/netlify-cms-backend-github/src/implementation.js

@@ -2,7 +2,7 @@ import React from 'react';
 import trimStart from 'lodash/trimStart';
 import semaphore from 'semaphore';
 import { stripIndent } from 'common-tags';
-import { asyncLock } from 'netlify-cms-lib-util';
+import { asyncLock, basename } from 'netlify-cms-lib-util';
 import AuthenticationPage from './AuthenticationPage';
 import { get } from 'lodash';
 import API from './API';
@@ -306,19 +306,36 @@ export default class GitHub {
     }));
   }
 
-  getMedia() {
-    return this.api.listFiles(this.config.get('media_folder')).then(files =>
+  getMedia(mediaFolder = this.config.get('media_folder')) {
+    return this.api.listFiles(mediaFolder).then(files =>
       files.map(({ sha, name, size, path }) => {
         // load media using getMediaDisplayURL to avoid token expiration with GitHub raw content urls
         // for private repositories
-        return { id: sha, name, size, displayURL: { sha, path }, path };
+        return { id: sha, name, size, displayURL: { id: sha, path }, path };
       }),
     );
   }
 
+  async getMediaFile(path) {
+    const blob = await this.api.getMediaAsBlob(null, path);
+
+    const name = basename(path);
+    const fileObj = new File([blob], name);
+    const url = URL.createObjectURL(fileObj);
+
+    return {
+      displayURL: url,
+      path,
+      name,
+      size: fileObj.size,
+      file: fileObj,
+      url,
+    };
+  }
+
   async getMediaDisplayURL(displayURL) {
-    const { sha, path } = displayURL;
-    const mediaURL = await this.api.getMediaDisplayURL(sha, path);
+    const { id, path } = displayURL;
+    const mediaURL = await this.api.getMediaDisplayURL(id, path);
     return mediaURL;
   }
 
@@ -332,19 +349,15 @@ export default class GitHub {
 
   async persistMedia(mediaFile, options = {}) {
     try {
-      if (!options.draft) {
-        await this.api.persistFiles(null, [mediaFile], options);
-      }
-
-      const { sha, value, path, fileObj } = mediaFile;
+      await this.api.persistFiles(null, [mediaFile], options);
+      const { sha, path, fileObj } = mediaFile;
       const displayURL = URL.createObjectURL(fileObj);
       return {
         id: sha,
-        name: value,
+        name: fileObj.name,
         size: fileObj.size,
         displayURL,
         path: trimStart(path, '/'),
-        draft: options.draft,
       };
     } catch (error) {
       console.error(error);
@@ -356,25 +369,24 @@ export default class GitHub {
     return this.api.deleteFile(path, commitMessage, options);
   }
 
-  async getMediaFiles(data) {
-    const files = get(data, 'metaData.objects.files', []);
-    const mediaFiles = await Promise.all(
-      files.map(file =>
-        this.api.getMediaAsBlob(file.sha, file.path).then(blob => {
-          const name = file.path.substring(file.path.lastIndexOf('/') + 1);
-          const fileObj = new File([blob], name);
-          return {
-            id: file.sha,
-            sha: file.sha,
-            displayURL: URL.createObjectURL(fileObj),
-            path: file.path,
-            name: name,
-            size: fileObj.size,
-            file: fileObj,
-          };
-        }),
-      ),
-    );
+  async loadMediaFile(file) {
+    return this.api.getMediaAsBlob(file.sha, file.path).then(blob => {
+      const name = basename(file.path);
+      const fileObj = new File([blob], name);
+      return {
+        id: file.sha,
+        sha: file.sha,
+        displayURL: URL.createObjectURL(fileObj),
+        path: file.path,
+        name,
+        size: fileObj.size,
+        file: fileObj,
+      };
+    });
+  }
+
+  async loadEntryMediaFiles(files) {
+    const mediaFiles = await Promise.all(files.map(file => this.loadMediaFile(file)));
 
     return mediaFiles;
   }
@@ -425,13 +437,18 @@ export default class GitHub {
       });
   }
 
-  async unpublishedEntry(collection, slug) {
+  async unpublishedEntry(
+    collection,
+    slug,
+    { loadEntryMediaFiles = files => this.loadEntryMediaFiles(files) } = {},
+  ) {
     const contentKey = this.api.generateContentKey(collection.get('name'), slug);
     const data = await this.api.readUnpublishedBranchFile(contentKey);
     if (!data) {
       return null;
     }
-    const mediaFiles = await this.getMediaFiles(data);
+    const files = get(data, 'metaData.objects.files', []);
+    const mediaFiles = await loadEntryMediaFiles(files);
     return {
       slug,
       file: { path: data.metaData.objects.entry.path },
@@ -484,10 +501,9 @@ export default class GitHub {
 
   publishUnpublishedEntry(collection, slug) {
     // publishUnpublishedEntry is a transactional operation
-    return this.runWithLock(async () => {
-      const metaData = await this.api.publishUnpublishedEntry(collection, slug);
-      const mediaFiles = await this.getMediaFiles({ metaData });
-      return { mediaFiles };
-    }, 'Failed to acquire publish entry lock');
+    return this.runWithLock(
+      () => this.api.publishUnpublishedEntry(collection, slug),
+      'Failed to acquire publish entry lock',
+    );
   }
 }

packages/netlify-cms-backend-github/src/types/semaphore.d.ts

@@ -0,0 +1,5 @@
+declare module 'semaphore' {
+  export type Semaphore = { take: (f: Function) => void; leave: () => void };
+  const semaphore: (count: number) => Semaphore;
+  export default semaphore;
+}