From e416f211112e5a8a90b6a6136209f4557c5a2765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Wed, 13 Oct 2021 17:14:21 +0200 Subject: [PATCH] chore(gatsby-source-contentful): download assets via gatsby-core-utils (#33482) Co-authored-by: Ward Peeters --- .../src/__tests__/download-with-retry.js | 87 ---------- .../src/__tests__/extend-node-type.js | 20 ++- .../src/cache-image.js | 85 ---------- .../src/download-with-retry.js | 51 ------ .../src/extend-node-type.js | 151 ++++++++---------- packages/gatsby-transformer-sqip/.babelrc | 2 +- .../src/extend-node-type.js | 52 +++--- 7 files changed, 119 insertions(+), 329 deletions(-) delete mode 100644 packages/gatsby-source-contentful/src/__tests__/download-with-retry.js delete mode 100644 packages/gatsby-source-contentful/src/cache-image.js delete mode 100644 packages/gatsby-source-contentful/src/download-with-retry.js diff --git a/packages/gatsby-source-contentful/src/__tests__/download-with-retry.js b/packages/gatsby-source-contentful/src/__tests__/download-with-retry.js deleted file mode 100644 index ef416c6b8c974..0000000000000 --- a/packages/gatsby-source-contentful/src/__tests__/download-with-retry.js +++ /dev/null @@ -1,87 +0,0 @@ -/** - * @jest-environment node - */ - -import nock from "nock" - -import downloadAndRetry from "../download-with-retry" - -nock.disableNetConnect() - -const host = `https://images.ctfassets.net` -const path = `/foo/bar/baz/image.jpg` -const url = [host, path].join(``) - -const reporter = { - verbose: jest.fn(), -} - -describe(`download-with-retry`, () => { - afterEach(() => { - nock.cleanAll() - reporter.verbose.mockClear() - }) - - test(`resolves regular response`, async () => { - const scope = nock(host).get(path).reply(200) - - await downloadAndRetry({ method: `get`, url }, reporter) - - expect(reporter.verbose).not.toHaveBeenCalled() - expect(scope.isDone()).toBeTruthy() - }) - - test(`does not retry for no reason`, async () => { - const scope = nock(host).get(path).twice().reply(200) - - await downloadAndRetry({ method: `get`, url }, reporter) - - expect(reporter.verbose).not.toHaveBeenCalled() - expect(scope.isDone()).toBeFalsy() - }) - - test(`does not retry on 404`, async () => { - const scope = nock(host).get(path).twice().reply(404) - - await expect( - downloadAndRetry({ method: `get`, url }, reporter) - ).rejects.toThrowError( - `Unable to download asset from https://images.ctfassets.net/foo/bar/baz/image.jpg. Request failed with status code 404` - ) - - expect(reporter.verbose).not.toHaveBeenCalled() - expect(scope.isDone()).toBeFalsy() - scope.persist(false) - }) - - test(`does retry on 503`, async () => { - const scope = nock(host).get(path).twice().reply(503).get(path).reply(200) - - await downloadAndRetry({ method: `get`, url }, reporter) - - expect(reporter.verbose).toHaveBeenCalledTimes(2) - expect(reporter.verbose).toHaveBeenCalledWith( - `Retry attempt #1 for https://images.ctfassets.net/foo/bar/baz/image.jpg` - ) - expect(reporter.verbose).toHaveBeenCalledWith( - `Retry attempt #2 for https://images.ctfassets.net/foo/bar/baz/image.jpg` - ) - expect(scope.isDone()).toBeTruthy() - }) - - test(`stops retry after 3 attempts`, async () => { - const scope = nock(host).get(path).times(4).reply(503) - - await expect( - downloadAndRetry({ method: `get`, url }, reporter) - ).rejects.toThrowError( - `Unable to download asset from https://images.ctfassets.net/foo/bar/baz/image.jpg. Request failed with status code 503` - ) - - expect(reporter.verbose).toHaveBeenCalledTimes(3) - expect(reporter.verbose).toHaveBeenCalledWith( - `Retry attempt #3 for https://images.ctfassets.net/foo/bar/baz/image.jpg` - ) - expect(scope.isDone()).toBeTruthy() - }) -}) diff --git a/packages/gatsby-source-contentful/src/__tests__/extend-node-type.js b/packages/gatsby-source-contentful/src/__tests__/extend-node-type.js index 2a5a7c2f79cda..30051260c6160 100644 --- a/packages/gatsby-source-contentful/src/__tests__/extend-node-type.js +++ b/packages/gatsby-source-contentful/src/__tests__/extend-node-type.js @@ -1,3 +1,5 @@ +jest.mock(`gatsby-core-utils`) +jest.mock(`fs-extra`) const { createUrl, resolveFixed, @@ -7,6 +9,9 @@ const { getBase64Image, } = require(`../extend-node-type`) +const { fetchRemoteFile } = require(`gatsby-core-utils`) +const fs = require(`fs-extra`) + describe(`contentful extend node type`, () => { describe(`createUrl`, () => { it(`allows you to create URls`, () => { @@ -102,6 +107,11 @@ describe(`contentful extend node type`, () => { }) describe(`getBase64Image`, () => { + beforeEach(() => { + fetchRemoteFile.mockClear() + fs.readFile.mockResolvedValue(Buffer.from(`test`)) + }) + const imageProps = { aspectRatio: 4.8698224852071, baseUrl: `//images.ctfassets.net/k8iqpp6u0ior/3ljGfnpegOnBTFGhV07iC1/94257340bda15ad4ca8462da3a8afa07/347966-contentful-logo-wordmark-dark__1_-4cd185-original-1582664935__1_.png`, @@ -128,16 +138,18 @@ describe(`contentful extend node type`, () => { } test(`keeps image format`, async () => { const result = await getBase64Image(imageProps) - const regex = /data:image\/png;base64,[a-zA-Z0-9/+]+=*/g - expect(result.match(regex)).not.toBe(null) + + expect(fetchRemoteFile).toHaveBeenCalled() + expect(result).toMatchInlineSnapshot(`"data:image/png;base64,dGVzdA=="`) }) test(`uses given image format`, async () => { const result = await getBase64Image({ ...imageProps, options: { ...imageProps.options, toFormat: `jpg` }, }) - const regex = /data:image\/jpg;base64,[a-zA-Z0-9/+]+=*/g - expect(result.match(regex)).not.toBe(null) + + expect(fetchRemoteFile).toHaveBeenCalled() + expect(result).toMatchInlineSnapshot(`"data:image/jpg;base64,dGVzdA=="`) }) }) diff --git a/packages/gatsby-source-contentful/src/cache-image.js b/packages/gatsby-source-contentful/src/cache-image.js deleted file mode 100644 index 3386eedb65be6..0000000000000 --- a/packages/gatsby-source-contentful/src/cache-image.js +++ /dev/null @@ -1,85 +0,0 @@ -const crypto = require(`crypto`) -const { resolve, parse } = require(`path`) - -const { pathExists, createWriteStream } = require(`fs-extra`) - -const downloadWithRetry = require(`./download-with-retry`).default - -const inFlightImageCache = new Map() - -module.exports = async function cacheImage(store, image, options, reporter) { - const program = store.getState().program - const CACHE_DIR = resolve(`${program.directory}/.cache/contentful/assets/`) - const { - file: { url, fileName, details }, - } = image - const { - width, - height, - maxWidth, - maxHeight, - resizingBehavior, - cropFocus, - background, - } = options - const userWidth = maxWidth || width - const userHeight = maxHeight || height - - const aspectRatio = details.image.height / details.image.width - const resultingWidth = Math.round(userWidth || 800) - const resultingHeight = Math.round(userHeight || resultingWidth * aspectRatio) - - const params = [`w=${resultingWidth}`, `h=${resultingHeight}`] - if (resizingBehavior) { - params.push(`fit=${resizingBehavior}`) - } - if (cropFocus) { - params.push(`f=${cropFocus}`) - } - if (background) { - params.push(`bg=${background}`) - } - - const optionsHash = crypto - .createHash(`md5`) - .update(JSON.stringify([url, ...params])) - .digest(`hex`) - - const { name, ext } = parse(fileName) - const absolutePath = resolve(CACHE_DIR, `${name}-${optionsHash}${ext}`) - - // Query the filesystem for file existence - const alreadyExists = await pathExists(absolutePath) - // Whether the file exists or not, if we are downloading it then await - const inFlight = inFlightImageCache.get(absolutePath) - if (inFlight) { - await inFlight - } else if (!alreadyExists) { - // File doesn't exist and is not being download yet - const downloadPromise = new Promise((resolve, reject) => { - const previewUrl = `http:${url}?${params.join(`&`)}` - - downloadWithRetry( - { - url: previewUrl, - responseType: `stream`, - }, - reporter - ) - .then(response => { - const file = createWriteStream(absolutePath) - response.data.pipe(file) - file.on(`finish`, resolve) - file.on(`error`, reject) - }) - .catch(reject) - }) - inFlightImageCache.set(absolutePath, downloadPromise) - await downloadPromise - // When the file is downloaded, remove the promise from the cache - inFlightImageCache.delete(absolutePath) - } - - // Now the file should be completely downloaded - return absolutePath -} diff --git a/packages/gatsby-source-contentful/src/download-with-retry.js b/packages/gatsby-source-contentful/src/download-with-retry.js deleted file mode 100644 index a8d575f231068..0000000000000 --- a/packages/gatsby-source-contentful/src/download-with-retry.js +++ /dev/null @@ -1,51 +0,0 @@ -const axios = require(`axios`) -const rax = require(`retry-axios`) -const { default: PQueue } = require(`p-queue`) - -/** - * Contentfuls APIs have a general rate limit of 79 uncached requests per second. - * A concurrency of 100 was recommended by Contentful backend and will ensure - * that we won't run into rate-limit errors. - */ -const queue = new PQueue({ - concurrency: 100, -}) - -let RetryAxios - -function getAxios(reporter) { - if (!RetryAxios) { - RetryAxios = axios.create() - - RetryAxios.defaults.raxConfig = { - instance: RetryAxios, - onRetryAttempt: err => { - const cfg = rax.getConfig(err) - reporter.verbose( - `Retry attempt #${cfg.currentRetryAttempt} for ${err.config.url}` - ) - }, - } - - rax.attach(RetryAxios) - } - return RetryAxios -} - -export default async function downloadWithRetry(requestConfig, reporter) { - if (!requestConfig.url) { - throw new Error(`requestConfig.url is missing`) - } - - const axiosInstance = getAxios(reporter) - - try { - const result = await queue.add(() => - axiosInstance.get(requestConfig.url, requestConfig) - ) - return result - } catch (err) { - err.message = `Unable to download asset from ${requestConfig.url}. ${err.message}` - throw err - } -} diff --git a/packages/gatsby-source-contentful/src/extend-node-type.js b/packages/gatsby-source-contentful/src/extend-node-type.js index 94273346fa83a..95cfc81dceee0 100644 --- a/packages/gatsby-source-contentful/src/extend-node-type.js +++ b/packages/gatsby-source-contentful/src/extend-node-type.js @@ -1,7 +1,6 @@ // @ts-check -const fs = require(`fs`) +const fs = require(`fs-extra`) const path = require(`path`) -const crypto = require(`crypto`) const { URLSearchParams } = require(`url`) const sortBy = require(`lodash/sortBy`) @@ -17,8 +16,8 @@ const { } = require(`gatsby/graphql`) const { stripIndent } = require(`common-tags`) -const cacheImage = require(`./cache-image`) -const downloadWithRetry = require(`./download-with-retry`).default +const { fetchRemoteFile } = require(`gatsby-core-utils`) + const { ImageFormatType, ImageResizingBehavior, @@ -35,21 +34,17 @@ const { // Supported Image Formats from https://www.contentful.com/developers/docs/references/images-api/#/reference/changing-formats/image-format const validImageFormats = new Set([`jpg`, `png`, `webp`, `gif`]) -if (process.env.GATSBY_REMOTE_CACHE) { - console.warn( - `Note: \`GATSBY_REMOTE_CACHE\` will be removed soon because it has been renamed to \`GATSBY_CONTENTFUL_EXPERIMENTAL_REMOTE_CACHE\`` - ) -} -if (process.env.GATSBY_CONTENTFUL_EXPERIMENTAL_REMOTE_CACHE) { - console.warn( - `Please be aware that the \`GATSBY_CONTENTFUL_EXPERIMENTAL_REMOTE_CACHE\` env flag is not officially supported and could be removed at any time` - ) -} -const REMOTE_CACHE_FOLDER = - process.env.GATSBY_CONTENTFUL_EXPERIMENTAL_REMOTE_CACHE ?? - process.env.GATSBY_REMOTE_CACHE ?? - path.join(process.cwd(), `.cache/remote_cache`) -const CACHE_IMG_FOLDER = path.join(REMOTE_CACHE_FOLDER, `images`) +const mimeTypeExtensions = new Map([ + [`image/jpeg`, `.jpg`], + [`image/jpg`, `.jpg`], + [`image/gif`, `.gif`], + [`image/png`, `.png`], + [`image/webp`, `.webp`], +]) + +exports.mimeTypeExtensions = mimeTypeExtensions + +const isImage = image => mimeTypeExtensions.has(image?.file?.contentType) // Promises that rejected should stay in this map. Otherwise remove promise and put their data in resolvedBase64Cache const inFlightBase64Cache = new Map() @@ -60,13 +55,8 @@ const resolvedBase64Cache = new Map() // @see https://www.contentful.com/developers/docs/references/images-api/#/reference/resizing-&-cropping/specify-width-&-height const CONTENTFUL_IMAGE_MAX_SIZE = 4000 -const isImage = image => - [`image/jpeg`, `image/jpg`, `image/png`, `image/webp`, `image/gif`].includes( - image?.file?.contentType - ) - // Note: this may return a Promise, body (sync), or null -const getBase64Image = (imageProps, reporter) => { +const getBase64Image = (imageProps, cache) => { if (!imageProps) { return null } @@ -101,54 +91,24 @@ const getBase64Image = (imageProps, reporter) => { return inFlight } - // Note: sha1 is unsafe for crypto but okay for this particular case - const shasum = crypto.createHash(`sha1`) - shasum.update(requestUrl) - const urlSha = shasum.digest(`hex`) - - // TODO: Find the best place for this step. This is definitely not it. - fs.mkdirSync(CACHE_IMG_FOLDER, { recursive: true }) - - const cacheFile = path.join(CACHE_IMG_FOLDER, urlSha + `.base64`) - - if (fs.existsSync(cacheFile)) { - // TODO: against dogma, confirm whether readFileSync is indeed slower - const promise = fs.promises.readFile(cacheFile, `utf8`) - inFlightBase64Cache.set(requestUrl, promise) - return promise.then(body => { - inFlightBase64Cache.delete(requestUrl) - resolvedBase64Cache.set(requestUrl, body) - return body - }) - } - const loadImage = async () => { - const imageResponse = await downloadWithRetry( - { - url: requestUrl, - responseType: `arraybuffer`, - }, - reporter - ) + const { + file: { contentType }, + } = imageProps.image - const base64 = Buffer.from(imageResponse.data, `binary`).toString(`base64`) + const extension = mimeTypeExtensions.get(contentType) - const body = `data:image/${toFormat || originalFormat};base64,${base64}` + const absolutePath = await fetchRemoteFile({ + url: requestUrl, + cache, + ext: extension, + }) - try { - // TODO: against dogma, confirm whether writeFileSync is indeed slower - await fs.promises.writeFile(cacheFile, body) - return body - } catch (e) { - console.error( - `Contentful:getBase64Image: failed to write ${body.length} bytes remotely fetched from \`${requestUrl}\` to: \`${cacheFile}\`\nError: ${e}` - ) - throw e - } + const base64 = (await fs.readFile(absolutePath)).toString(`base64`) + return `data:image/${toFormat || originalFormat};base64,${base64}` } const promise = loadImage() - inFlightBase64Cache.set(requestUrl, promise) return promise.then(body => { @@ -505,14 +465,14 @@ const resolveResize = (image, options) => { exports.resolveResize = resolveResize -const fixedNodeType = ({ name, getTracedSVG, reporter }) => { +const fixedNodeType = ({ name, getTracedSVG, cache }) => { return { type: new GraphQLObjectType({ name: name, fields: { base64: { type: GraphQLString, - resolve: imageProps => getBase64Image(imageProps, reporter), + resolve: imageProps => getBase64Image(imageProps, cache), }, tracedSVG: { type: GraphQLString, @@ -607,14 +567,14 @@ const fixedNodeType = ({ name, getTracedSVG, reporter }) => { } } -const fluidNodeType = ({ name, getTracedSVG, reporter }) => { +const fluidNodeType = ({ name, getTracedSVG, cache }) => { return { type: new GraphQLObjectType({ name: name, fields: { base64: { type: GraphQLString, - resolve: imageProps => getBase64Image(imageProps, reporter), + resolve: imageProps => getBase64Image(imageProps, cache), }, tracedSVG: { type: GraphQLString, @@ -711,7 +671,7 @@ const fluidNodeType = ({ name, getTracedSVG, reporter }) => { } } -exports.extendNodeType = ({ type, store, reporter }) => { +exports.extendNodeType = ({ type, cache }) => { if (type.name !== `ContentfulAsset`) { return {} } @@ -721,15 +681,23 @@ exports.extendNodeType = ({ type, store, reporter }) => { const { image, options } = args const { - file: { contentType }, + file: { contentType, url: imgUrl, fileName }, } = image if (contentType.indexOf(`image/`) !== 0) { return null } - const absolutePath = await cacheImage(store, image, options, reporter) - const extension = path.extname(absolutePath) + const extension = mimeTypeExtensions.get(contentType) + const url = createUrl(imgUrl, options) + const name = path.basename(fileName, extension) + + const absolutePath = await fetchRemoteFile({ + url, + name, + cache, + ext: extension, + }) return traceSVG({ file: { @@ -743,7 +711,7 @@ exports.extendNodeType = ({ type, store, reporter }) => { }) } - const getDominantColor = async ({ image, options, reporter }) => { + const getDominantColor = async ({ image, options }) => { let pluginSharp try { @@ -757,7 +725,29 @@ exports.extendNodeType = ({ type, store, reporter }) => { } try { - const absolutePath = await cacheImage(store, image, options, reporter) + const { + file: { contentType, url: imgUrl, fileName }, + } = image + + if (contentType.indexOf(`image/`) !== 0) { + return null + } + + // 256px should be enough to properly detect the dominant color + if (!options.width) { + options.width = 256 + } + + const extension = mimeTypeExtensions.get(contentType) + const url = createUrl(imgUrl, options) + const name = path.basename(fileName, extension) + + const absolutePath = await fetchRemoteFile({ + url, + name, + cache, + ext: extension, + }) if (!(`getDominantColor` in pluginSharp)) { console.error( @@ -805,7 +795,6 @@ exports.extendNodeType = ({ type, store, reporter }) => { imageProps.backgroundColor = await getDominantColor({ image, options, - reporter, }) } @@ -816,7 +805,7 @@ exports.extendNodeType = ({ type, store, reporter }) => { image, options, }, - reporter + cache ) } @@ -837,13 +826,13 @@ exports.extendNodeType = ({ type, store, reporter }) => { const fixedNode = fixedNodeType({ name: `ContentfulFixed`, getTracedSVG, - reporter, + cache, }) const fluidNode = fluidNodeType({ name: `ContentfulFluid`, getTracedSVG, - reporter, + cache, }) // gatsby-plugin-image @@ -923,7 +912,7 @@ exports.extendNodeType = ({ type, store, reporter }) => { fields: { base64: { type: GraphQLString, - resolve: imageProps => getBase64Image(imageProps, reporter), + resolve: imageProps => getBase64Image(imageProps, cache), }, tracedSVG: { type: GraphQLString, diff --git a/packages/gatsby-transformer-sqip/.babelrc b/packages/gatsby-transformer-sqip/.babelrc index 31043522b2321..ac0ad292bb087 100644 --- a/packages/gatsby-transformer-sqip/.babelrc +++ b/packages/gatsby-transformer-sqip/.babelrc @@ -1,3 +1,3 @@ { - "presets": [["babel-preset-gatsby-package", { "browser": true }]] + "presets": [["babel-preset-gatsby-package"]] } diff --git a/packages/gatsby-transformer-sqip/src/extend-node-type.js b/packages/gatsby-transformer-sqip/src/extend-node-type.js index 2c88f595d6b8b..a2ac87123e7d8 100644 --- a/packages/gatsby-transformer-sqip/src/extend-node-type.js +++ b/packages/gatsby-transformer-sqip/src/extend-node-type.js @@ -1,22 +1,22 @@ -const { resolve } = require(`path`) -const md5File = require(`md5-file`) - -const { - DuotoneGradientType, - ImageCropFocusType, -} = require(`gatsby-transformer-sharp/types`) -const { queueImageResizing } = require(`gatsby-plugin-sharp`) +const path = require(`path`) const Debug = require(`debug`) const fs = require(`fs-extra`) +const sharp = require(`sharp`) +const md5File = require(`md5-file`) + const { GraphQLObjectType, GraphQLString, GraphQLInt, GraphQLBoolean, } = require(`gatsby/graphql`) -const sharp = require(`sharp`) -const { ensureDir } = require(`fs-extra`) +const { queueImageResizing } = require(`gatsby-plugin-sharp`) +const { fetchRemoteFile } = require(`gatsby-core-utils`) +const { + DuotoneGradientType, + ImageCropFocusType, +} = require(`gatsby-transformer-sharp/types`) const generateSqip = require(`./generate-sqip`) @@ -42,13 +42,13 @@ module.exports = async args => { return {} } -async function sqipSharp({ cache, getNodeAndSavePathDependency, store }) { +async function sqipSharp({ type, cache, getNodeAndSavePathDependency, store }) { const program = store.getState().program - const cacheDir = resolve( + const cacheDir = path.resolve( `${program.directory}/node_modules/.cache/gatsby-transformer-sqip/` ) - await ensureDir(cacheDir) + await fs.ensureDir(cacheDir) return { sqip: { @@ -135,19 +135,17 @@ async function sqipSharp({ cache, getNodeAndSavePathDependency, store }) { } } -async function sqipContentful({ cache, store, reporter }) { +async function sqipContentful({ type, cache, store }) { const { schemes: { ImageResizingBehavior, ImageCropFocusType }, } = require(`gatsby-source-contentful`) - const cacheImage = require(`gatsby-source-contentful/cache-image`) - const program = store.getState().program - const cacheDir = resolve( + const cacheDir = path.resolve( `${program.directory}/node_modules/.cache/gatsby-transformer-sqip/` ) - await ensureDir(cacheDir) + await fs.ensureDir(cacheDir) return { sqip: { @@ -192,7 +190,12 @@ async function sqipContentful({ cache, store, reporter }) { }, async resolve(asset, fieldArgs) { const { - file: { contentType }, + createUrl, + mimeTypeExtensions, + } = require(`gatsby-source-contentful/extend-node-type`) + + const { + file: { contentType, url: imgUrl, fileName }, } = asset if (!contentType.includes(`image/`)) { @@ -223,7 +226,16 @@ async function sqipContentful({ cache, store, reporter }) { background, } - const absolutePath = await cacheImage(store, asset, options, reporter) + const extension = mimeTypeExtensions.get(contentType) + const url = createUrl(imgUrl, options) + const name = path.basename(fileName, extension) + + const absolutePath = await fetchRemoteFile({ + url, + name, + cache, + ext: extension, + }) const contentDigest = await md5File(absolutePath)