From 7697b907d7bae750f4ec041a184188aa46995320 Mon Sep 17 00:00:00 2001 From: Erez Rokah Date: Sun, 6 Dec 2020 08:50:56 -0800 Subject: [PATCH] fix(large-media): mark pointer files as binary (#4678) --- .../src/GitHubAPI.ts | 14 ++++- .../src/implementation.ts | 61 +++++++++++-------- .../src/netlify-lfs-client.ts | 60 ++++++++++++------ .../netlify-cms-backend-github/src/API.ts | 29 ++++----- 4 files changed, 105 insertions(+), 59 deletions(-) diff --git a/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.ts b/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.ts index e99e0e24..f07e9912 100644 --- a/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.ts +++ b/packages/netlify-cms-backend-git-gateway/src/GitHubAPI.ts @@ -1,22 +1,26 @@ import { API as GithubAPI } from 'netlify-cms-backend-github'; -import { Config as GitHubConfig } from 'netlify-cms-backend-github/src/API'; +import { Config as GitHubConfig, Diff } from 'netlify-cms-backend-github/src/API'; import { APIError, FetchError } from 'netlify-cms-lib-util'; +import { Octokit } from '@octokit/rest'; type Config = GitHubConfig & { apiRoot: string; tokenPromise: () => Promise; commitAuthor: { name: string }; + isLargeMedia: (filename: string) => Promise; }; export default class API extends GithubAPI { tokenPromise: () => Promise; commitAuthor: { name: string }; + isLargeMedia: (filename: string) => Promise; constructor(config: Config) { super(config); this.apiRoot = config.apiRoot; this.tokenPromise = config.tokenPromise; this.commitAuthor = config.commitAuthor; + this.isLargeMedia = config.isLargeMedia; this.repoURL = ''; this.originRepoURL = ''; } @@ -113,4 +117,12 @@ export default class API extends GithubAPI { nextUrlProcessor() { return (url: string) => url.replace(/^(?:[a-z]+:\/\/.+?\/.+?\/.+?\/)/, `${this.apiRoot}/`); } + + async diffFromFile(file: Octokit.ReposCompareCommitsResponseFilesItem): Promise { + const diff = await super.diffFromFile(file); + return { + ...diff, + binary: diff.binary || (await this.isLargeMedia(file.filename)), + }; + } } diff --git a/packages/netlify-cms-backend-git-gateway/src/implementation.ts b/packages/netlify-cms-backend-git-gateway/src/implementation.ts index 1ec9c892..f7b026e3 100644 --- a/packages/netlify-cms-backend-git-gateway/src/implementation.ts +++ b/packages/netlify-cms-backend-git-gateway/src/implementation.ts @@ -162,7 +162,8 @@ export default class GitGateway implements Implementation { this.squashMerges = config.backend.squash_merges || false; this.cmsLabelPrefix = config.backend.cms_label_prefix || ''; this.mediaFolder = config.media_folder; - this.transformImages = config.backend.use_large_media_transforms_in_media_library || true; + const { use_large_media_transforms_in_media_library: transformImages = true } = config.backend; + this.transformImages = transformImages; const netlifySiteURL = localStorage.getItem('netlifySiteURL'); this.apiUrl = getEndpoint(config.backend.identity_url || defaults.identity, netlifySiteURL); @@ -333,6 +334,7 @@ export default class GitGateway implements Implementation { branch: this.branch, tokenPromise: this.tokenPromise!, commitAuthor: pick(userData, ['name', 'email']), + isLargeMedia: (filename: string) => this.isLargeMediaFile(filename), squashMerges: this.squashMerges, cmsLabelPrefix: this.cmsLabelPrefix, initialWorkflowStatus: this.options.initialWorkflowStatus, @@ -400,19 +402,24 @@ export default class GitGateway implements Implementation { return this.backend!.unpublishedEntryDataFile(collection, slug, path, id); } - async unpublishedEntryMediaFile(collection: string, slug: string, path: string, id: string) { + async isLargeMediaFile(path: string) { const client = await this.getLargeMediaClient(); - if (client.enabled && client.matchPath(path)) { + return client.enabled && client.matchPath(path); + } + + async unpublishedEntryMediaFile(collection: string, slug: string, path: string, id: string) { + const isLargeMedia = await this.isLargeMediaFile(path); + if (isLargeMedia) { const branch = this.backend!.getBranch(collection, slug); - const url = await this.getLargeMediaDisplayURL({ path, id }, branch); + const { url, blob } = await this.getLargeMediaDisplayURL({ path, id }, branch); return { id, name: basename(path), path, url, displayURL: url, - file: new File([], name), - size: 0, + file: new File([blob], name), + size: blob.size, }; } else { return this.backend!.unpublishedEntryMediaFile(collection, slug, path, id); @@ -496,20 +503,20 @@ export default class GitGateway implements Implementation { const pointerFile = parsePointerFile(entry.data); if (!pointerFile.sha) { console.warn(`Failed parsing pointer file ${path}`); - return path; + return { url: path, blob: new Blob() }; } const client = await this.getLargeMediaClient(); - const url = await client.getDownloadURL(pointerFile); - return url; + const { url, blob } = await client.getDownloadURL(pointerFile); + return { url, blob }; } async getMediaDisplayURL(displayURL: DisplayURL) { const { path, id } = displayURL as DisplayURLObject; - const client = await this.getLargeMediaClient(); - if (client.enabled && client.matchPath(path)) { - const largeMediaUrl = await this.getLargeMediaDisplayURL({ path, id }); - return largeMediaUrl; + const isLargeMedia = await this.isLargeMediaFile(path); + if (isLargeMedia) { + const { url } = await this.getLargeMediaDisplayURL({ path, id }); + return url; } if (typeof displayURL === 'string') { return displayURL; @@ -520,15 +527,17 @@ export default class GitGateway implements Implementation { } async getMediaFile(path: string) { - const client = await this.getLargeMediaClient(); - if (client.enabled && client.matchPath(path)) { - const url = await this.getLargeMediaDisplayURL({ path, id: null }); + const isLargeMedia = await this.isLargeMediaFile(path); + if (isLargeMedia) { + const { url, blob } = await this.getLargeMediaDisplayURL({ path, id: null }); return { id: url, name: basename(path), path, url, displayURL: url, + file: new File([blob], name), + size: blob.size, }; } return this.backend!.getMediaFile(path); @@ -549,15 +558,19 @@ export default class GitGateway implements Implementation { const displayURL = URL.createObjectURL(fileObj); const client = await this.getLargeMediaClient(); const fixedPath = path.startsWith('/') ? path.slice(1) : path; - if (!client.enabled || !client.matchPath(fixedPath)) { - return this.backend!.persistMedia(mediaFile, options); + const isLargeMedia = await this.isLargeMediaFile(fixedPath); + if (isLargeMedia) { + const persistMediaArgument = await getPointerFileForMediaFileObj( + client, + fileObj as File, + path, + ); + return { + ...(await this.backend!.persistMedia(persistMediaArgument, options)), + displayURL, + }; } - - const persistMediaArgument = await getPointerFileForMediaFileObj(client, fileObj as File, path); - return { - ...(await this.backend!.persistMedia(persistMediaArgument, options)), - displayURL, - }; + return await this.backend!.persistMedia(mediaFile, options); } deleteFiles(paths: string[], commitMessage: string) { return this.backend!.deleteFiles(paths, commitMessage); diff --git a/packages/netlify-cms-backend-git-gateway/src/netlify-lfs-client.ts b/packages/netlify-cms-backend-git-gateway/src/netlify-lfs-client.ts index 1528db71..d8f004db 100644 --- a/packages/netlify-cms-backend-git-gateway/src/netlify-lfs-client.ts +++ b/packages/netlify-cms-backend-git-gateway/src/netlify-lfs-client.ts @@ -1,4 +1,5 @@ import { flow, fromPairs, map } from 'lodash/fp'; +import { isPlainObject, isEmpty } from 'lodash'; import minimatch from 'minimatch'; import { ApiRequest, PointerFile, unsentRequest } from 'netlify-cms-lib-util'; @@ -46,25 +47,41 @@ const resourceExists = async ( // to fit }; -const getTransofrmationsParams = (t: ImageTransformations) => - `?nf_resize=${t.nf_resize}&w=${t.w}&h=${t.h}`; +const getTransofrmationsParams = (t: boolean | ImageTransformations) => { + if (isPlainObject(t) && !isEmpty(t)) { + const { nf_resize: resize, w, h } = t as ImageTransformations; + return `?nf_resize=${resize}&w=${w}&h=${h}`; + } + return ''; +}; -const getDownloadURL = ( +const getDownloadURL = async ( { rootURL, transformImages: t, makeAuthorizedRequest }: ClientConfig, { sha }: PointerFile, -) => - makeAuthorizedRequest( - `${rootURL}/origin/${sha}${ - t && Object.keys(t).length > 0 ? getTransofrmationsParams(t as ImageTransformations) : '' - }`, - ) - .then(res => (res.ok ? res : Promise.reject(res))) - .then(res => res.blob()) - .then(blob => URL.createObjectURL(blob)) - .catch((err: Error) => { - console.error(err); - return Promise.resolve(''); - }); +) => { + try { + const transformation = getTransofrmationsParams(t); + const transformedPromise = makeAuthorizedRequest(`${rootURL}/origin/${sha}${transformation}`); + const [transformed, original] = await Promise.all([ + transformedPromise, + // if transformation is defined, we need to load the original so we have the correct meta data + transformation ? makeAuthorizedRequest(`${rootURL}/origin/${sha}`) : transformedPromise, + ]); + if (!transformed.ok) { + const error = await transformed.json(); + throw new Error( + `Failed getting large media for sha '${sha}': '${error.code} - ${error.msg}'`, + ); + } + + const transformedBlob = await transformed.blob(); + const url = URL.createObjectURL(transformedBlob); + return { url, blob: transformation ? await original.blob() : transformedBlob }; + } catch (error) { + console.error(error); + return { url: '', blob: new Blob() }; + } +}; const uploadOperation = (objects: PointerFile[]) => ({ operation: 'upload', @@ -77,15 +94,17 @@ const getResourceUploadURLs = async ( rootURL, makeAuthorizedRequest, }: { rootURL: string; makeAuthorizedRequest: MakeAuthorizedRequest }, - objects: PointerFile[], + pointerFiles: PointerFile[], ) => { const response = await makeAuthorizedRequest({ url: `${rootURL}/objects/batch`, method: 'POST', headers: defaultContentHeaders, - body: JSON.stringify(uploadOperation(objects)), + body: JSON.stringify(uploadOperation(pointerFiles)), }); - return (await response.json()).objects.map( + + const { objects } = await response.json(); + const uploadUrls = objects.map( (object: { error?: { message: string }; actions: { upload: { href: string } } }) => { if (object.error) { throw new Error(object.error.message); @@ -93,6 +112,7 @@ const getResourceUploadURLs = async ( return object.actions.upload.href; }, ); + return uploadUrls; }; const uploadBlob = (uploadURL: string, blob: Blob) => @@ -132,7 +152,7 @@ const clientFns: Record = { export type Client = { resourceExists: (pointer: PointerFile) => Promise; getResourceUploadURLs: (objects: PointerFile[]) => Promise; - getDownloadURL: (pointer: PointerFile) => Promise; + getDownloadURL: (pointer: PointerFile) => Promise<{ url: string; blob: Blob }>; uploadResource: (pointer: PointerFile, blob: Blob) => Promise; matchPath: (path: string) => boolean; patterns: string[]; diff --git a/packages/netlify-cms-backend-github/src/API.ts b/packages/netlify-cms-backend-github/src/API.ts index d44b7615..30cba911 100644 --- a/packages/netlify-cms-backend-github/src/API.ts +++ b/packages/netlify-cms-backend-github/src/API.ts @@ -154,24 +154,13 @@ const getTreeFiles = (files: GitHubCompareFiles) => { return treeFiles; }; -type Diff = { +export type Diff = { path: string; newFile: boolean; sha: string; binary: boolean; }; -const diffFromFile = (diff: Octokit.ReposCompareCommitsResponseFilesItem): Diff => { - return { - path: diff.filename, - newFile: diff.status === 'added', - sha: diff.sha, - // media files diffs don't have a patch attribute, except svg files - // renamed files don't have a patch attribute too - binary: (diff.status !== 'renamed' && !diff.patch) || diff.filename.endsWith('.svg'), - }; -}; - let migrationNotified = false; export default class API { @@ -581,7 +570,7 @@ export default class API { const branch = branchFromContentKey(contentKey); const pullRequest = await this.getBranchPullRequest(branch); const { files } = await this.getDifferences(this.branch, pullRequest.head.sha); - const diffs = files.map(diffFromFile); + const diffs = await Promise.all(files.map(file => this.diffFromFile(file))); const label = pullRequest.labels.find(l => isCMSLabel(l.name, this.cmsLabelPrefix)) as { name: string; }; @@ -943,6 +932,18 @@ export default class API { }); } + // async since it is overridden in a child class + async diffFromFile(diff: Octokit.ReposCompareCommitsResponseFilesItem): Promise { + return { + path: diff.filename, + newFile: diff.status === 'added', + sha: diff.sha, + // media files diffs don't have a patch attribute, except svg files + // renamed files don't have a patch attribute too + binary: (diff.status !== 'renamed' && !diff.patch) || diff.filename.endsWith('.svg'), + }; + } + async editorialWorkflowGit( files: TreeFile[], slug: string, @@ -974,7 +975,7 @@ export default class API { await this.getHeadReference(branch), ); - const diffs = diffFiles.map(diffFromFile); + const diffs = await Promise.all(diffFiles.map(file => this.diffFromFile(file))); // mark media files to remove const mediaFilesToRemove: { path: string; sha: string | null }[] = []; for (const diff of diffs.filter(d => d.binary)) {