diff --git a/packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js b/packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js index eae06145d79fa..30dd315276b55 100644 --- a/packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js +++ b/packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js @@ -7,6 +7,7 @@ import { setupServer } from "msw/node" import { Writable } from "stream" import got from "got" import fs from "fs-extra" +import { slash } from "gatsby-core-utils/path" import { fetchRemoteFile } from "../fetch-remote-file" import * as storage from "../utils/get-storage" @@ -132,6 +133,7 @@ const server = setupServer( ctx.body(content) ) }), + rest.get(`http://external.com/dog`, async (req, res, ctx) => { const { content, contentLength } = await getFileContent( path.join(__dirname, `./fixtures/dog-thumbnail.jpg`), @@ -175,6 +177,19 @@ const server = setupServer( ctx.body(content) ) }), + rest.get(`http://external.com/dog-*.jpg`, async (req, res, ctx) => { + const { content, contentLength } = await getFileContent( + path.join(__dirname, `./fixtures/dog-thumbnail.jpg`), + req + ) + + return res( + ctx.set(`Content-Type`, `image/jpg`), + ctx.set(`Content-Length`, contentLength), + ctx.status(200), + ctx.body(content) + ) + }), rest.get(`http://external.com/404.jpg`, async (req, res, ctx) => { const content = `Page not found` @@ -456,96 +471,171 @@ Fetch details: `) }) - it(`should not re-download file if cache is set`, async () => { - const filePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - cache, - cacheKey: `1`, - }) - const cachedFilePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - cache, - cacheKey: `1`, - }) + let cacheVersion = 0 + describe.each([false, true])(`with excludeDigest %s`, excludeDigest => { + function getExternalUrl(cacheVersion) { + return `http://external.com/dog-${cacheVersion}.jpg?v=${cacheVersion}` + } - expect(filePath).toBe(cachedFilePath) - expect(gotStream).toBeCalledTimes(1) - expect(fs.pathExists).toBeCalledTimes(1) - expect(fs.copy).not.toBeCalled() - }) + it(`should not re-download file if cache is set`, async () => { + const filePath = await fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + cache, + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePath = await fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + cache, + cacheKey: `${cacheVersion}`, + excludeDigest, + }) - it(`should not re-download and use same path if ouputDir is not inside public folder`, async () => { - const filePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - directory: cache.directory, - cacheKey: `2`, - }) - const cachedFilePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - directory: path.join(cache.directory, `diff`), - cacheKey: `2`, + expect(filePath).toBe(cachedFilePath) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(1) + expect(fs.copy).not.toBeCalled() }) - expect(filePath).toBe(cachedFilePath) - expect(gotStream).toBeCalledTimes(1) - expect(fs.pathExists).toBeCalledTimes(1) - expect(fs.copy).not.toBeCalled() - }) + it(`should not re-download and use same path if ouputDir is not inside public folder`, async () => { + const filePath = await fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + directory: cache.directory, + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePath = await fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + directory: path.join(cache.directory, `diff`), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) - it(`should not re-download but copy file to public folder`, async () => { - const currentGlobal = global.__GATSBY - global.__GATSBY = { - root: cache.directory, - } - await fs.ensureDir(path.join(cache.directory, `public`)) - const filePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - directory: cache.directory, - cacheKey: `3`, + expect(filePath).toBe(cachedFilePath) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(1) + expect(fs.copy).not.toBeCalled() }) - const cachedFilePath = await fetchRemoteFile({ - url: `http://external.com/dog.jpg`, - directory: path.join(cache.directory, `public`), - cacheKey: `3`, + + it(`should not re-download but copy file to public folder`, async () => { + const currentGlobal = global.__GATSBY + global.__GATSBY = { + root: cache.directory, + } + await fs.ensureDir(path.join(cache.directory, `public`)) + const filePath = await fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + directory: cache.directory, + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePath = await fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + directory: path.join(cache.directory, `public`), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + + expect(filePath).not.toBe(cachedFilePath) + expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(1) + expect(fs.copy).toBeCalledTimes(1) + expect(await fs.pathExists(cachedFilePath)).toBe(true) + global.__GATSBY = currentGlobal }) - expect(filePath).not.toBe(cachedFilePath) - expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) - expect(gotStream).toBeCalledTimes(1) - expect(fs.pathExists).toBeCalledTimes(1) - expect(fs.copy).toBeCalledTimes(1) - expect(await fs.pathExists(cachedFilePath)).toBe(true) - global.__GATSBY = currentGlobal - }) + it(`should not re-download but copy file to public folder (with slashes)`, async () => { + const currentGlobal = global.__GATSBY + global.__GATSBY = { + root: cache.directory, + } + await fs.ensureDir(path.join(cache.directory, `public`)) + const filePath = await fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + directory: slash(cache.directory), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePath = await fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + directory: slash(path.join(cache.directory, `public`)), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) - it(`should not re-download but copy file to public folder when the same url is requested`, async () => { - const currentGlobal = global.__GATSBY - global.__GATSBY = { - root: cache.directory, - } - await fs.ensureDir(path.join(cache.directory, `public`)) - const filePathPromise = fetchRemoteFile({ - url: `http://external.com/dog.jpg?v=4`, - directory: cache.directory, - cacheKey: `4`, + expect(filePath).not.toBe(cachedFilePath) + expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(1) + expect(fs.copy).toBeCalledTimes(1) + expect(await fs.pathExists(cachedFilePath)).toBe(true) + global.__GATSBY = currentGlobal }) - const cachedFilePathPromise = fetchRemoteFile({ - url: `http://external.com/dog.jpg?v=4`, - directory: path.join(cache.directory, `public`), - cacheKey: `4`, + + it(`should not re-download but copy file to public folder when the same url is requested`, async () => { + const currentGlobal = global.__GATSBY + global.__GATSBY = { + root: cache.directory, + } + await fs.ensureDir(path.join(cache.directory, `public`)) + const filePathPromise = fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + directory: cache.directory, + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePathPromise = fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + directory: path.join(cache.directory, `public`), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + + const [filePath, cachedFilePath] = await Promise.all([ + filePathPromise, + cachedFilePathPromise, + ]) + + expect(filePath).not.toBe(cachedFilePath) + expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(0) + expect(fs.copy).toBeCalledTimes(1) + global.__GATSBY = currentGlobal }) - const [filePath, cachedFilePath] = await Promise.all([ - filePathPromise, - cachedFilePathPromise, - ]) + it(`should not re-download but copy file to public folder when the same url is requested (with slashes)`, async () => { + const currentGlobal = global.__GATSBY + global.__GATSBY = { + root: cache.directory, + } + await fs.ensureDir(path.join(cache.directory, `public`)) + const filePathPromise = fetchRemoteFile({ + url: getExternalUrl(++cacheVersion), + directory: slash(cache.directory), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) + const cachedFilePathPromise = fetchRemoteFile({ + url: getExternalUrl(cacheVersion), + directory: slash(path.join(cache.directory, `public`)), + cacheKey: `${cacheVersion}`, + excludeDigest, + }) - expect(filePath).not.toBe(cachedFilePath) - expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) - expect(gotStream).toBeCalledTimes(1) - expect(fs.pathExists).toBeCalledTimes(0) - expect(fs.copy).toBeCalledTimes(1) - global.__GATSBY = currentGlobal + const [filePath, cachedFilePath] = await Promise.all([ + filePathPromise, + cachedFilePathPromise, + ]) + + expect(filePath).not.toBe(cachedFilePath) + expect(cachedFilePath).toStartWith(path.join(cache.directory, `public`)) + expect(gotStream).toBeCalledTimes(1) + expect(fs.pathExists).toBeCalledTimes(0) + expect(fs.copy).toBeCalledTimes(1) + global.__GATSBY = currentGlobal + }) }) describe(`retries the download`, () => { diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index 770677ffb7744..ad49e8d62588a 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -158,7 +158,7 @@ async function fetchFile({ try { const digest = createContentDigest(url) const finalDirectory = excludeDigest - ? fileDirectory + ? path.resolve(fileDirectory) : path.join(fileDirectory, digest) if (!name) { diff --git a/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js b/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js index ddb790fa5b049..11cf82ac17338 100644 --- a/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js +++ b/packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js @@ -1,49 +1,14 @@ -jest.mock(`sharp`, () => { - const sharp = path => { - const pipeline = { - rotate: () => pipeline, - resize: () => pipeline, - png: () => pipeline, - jpeg: () => pipeline, - toFile: (_, cb) => cb(), - on: () => pipeline, - once: () => pipeline, - write: () => pipeline, - end: () => pipeline, - emit: () => pipeline, - } - return pipeline - } - - sharp.simd = jest.fn() - sharp.concurrency = jest.fn() - - return sharp -}) - -jest.mock(`fs-extra`, () => { - return { - ...jest.requireActual(`fs-extra`), - createReadStream: () => { - const stream = { - pipe: () => stream, - } - return stream - }, - } -}) +jest.mock(`os`, () => { + const path = require(`path`) -jest.mock(`potrace`, () => { - const circleSvgString = `` return { - trace: (_, _2, cb) => cb(null, circleSvgString), - Potrace: { - TURNPOLICY_MAJORITY: `wat`, - }, + ...jest.requireActual(`os`), + tmpdir: () => path.join(__dirname, `.cache`), } }) const path = require(`path`) +const fs = require(`fs-extra`) const traceSVGHelpers = require(`../trace-svg`) @@ -71,19 +36,26 @@ function getFileObject(absolutePath, name = path.parse(absolutePath).name) { absolutePath, extension: `png`, internal: { - contentDigest: `1234`, + contentDigest: `2022-01-13T13:27:56.654Z`, }, } } describe(`traceSVG memoization`, () => { const file = getFileObject(path.join(__dirname, `images/test.png`)) - const copyOfFile = getFileObject(path.join(__dirname, `images/test-copy.png`)) const differentFile = getFileObject( - path.join(__dirname, `images/different.png`) + path.join(__dirname, `images/144-density.png`) ) differentFile.internal.contentDigest = `4321` + beforeAll(async () => { + await fs.ensureDir(path.join(__dirname, `.cache`)) + }) + + afterAll(async () => { + await fs.remove(path.join(__dirname, `.cache`)) + }) + beforeEach(() => { traceSVGHelpers.clearMemoizeCaches() memoizedTraceSVG.mockClear() @@ -103,7 +75,7 @@ describe(`traceSVG memoization`, () => { expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) }) - it(`Is memoizing results for same args`, async () => { + it(`should memoizing results for same args`, async () => { await traceSVG({ file, }) @@ -118,119 +90,181 @@ describe(`traceSVG memoization`, () => { expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) }) - it(`Is calling functions with same input file when params change`, async () => { - await traceSVG({ - file, - args: { - color: `red`, - }, - fileArgs: { - width: 400, - }, - }) - await traceSVG({ - file, - args: { - color: `blue`, - }, - fileArgs: { - width: 400, - }, - }) - await traceSVG({ - file, - args: { - color: `red`, - }, - fileArgs: { - width: 200, - }, - }) - await traceSVG({ - file, - args: { - color: `blue`, - }, - fileArgs: { - width: 200, - }, - }) - await traceSVG({ - file: differentFile, - args: { - color: `red`, - }, - fileArgs: { - width: 400, - }, - }) - - expect(memoizedTraceSVG).toBeCalledTimes(5) - expect(notMemoizedtraceSVG).toBeCalledTimes(5) - expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(5) - // trace svg should be actually created just 3 times - // because it's affected just by `fileArgs`, and not `args` - // this makes sure we don't try to write to same input file multiple times - expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(3) - expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ + it( + `should call functions with same input file when params change`, + async () => { + await traceSVG({ file, - options: expect.objectContaining({ + args: { + color: `red`, + }, + fileArgs: { width: 400, - }), + }, }) - ) - expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ + await traceSVG({ file, - options: expect.objectContaining({ + args: { + color: `blue`, + }, + fileArgs: { + width: 400, + }, + }) + await traceSVG({ + file, + args: { + color: `red`, + }, + fileArgs: { width: 200, - }), + }, }) - ) - expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ + await traceSVG({ + file, + args: { + color: `blue`, + }, + fileArgs: { + width: 200, + }, + }) + await traceSVG({ file: differentFile, - options: expect.objectContaining({ + args: { + color: `red`, + }, + fileArgs: { width: 400, - }), + }, }) - ) - const usedTmpFilePaths = notMemoizedPrepareTraceSVGInputFile.mock.calls.map( - args => args[0].tmpFilePath - ) + expect(memoizedTraceSVG).toBeCalledTimes(5) + expect(notMemoizedtraceSVG).toBeCalledTimes(5) + expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(5) + // trace svg should be actually created just 3 times + // because it's affected just by `fileArgs`, and not `args` + // this makes sure we don't try to write to same input file multiple times + expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(3) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + file, + options: expect.objectContaining({ + width: 400, + }), + }) + ) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + file, + options: expect.objectContaining({ + width: 200, + }), + }) + ) + expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + file: differentFile, + options: expect.objectContaining({ + width: 400, + }), + }) + ) - // tmpFilePath was always unique - expect(usedTmpFilePaths.length).toBe(new Set(usedTmpFilePaths).size) - }) + const usedTmpFilePaths = + notMemoizedPrepareTraceSVGInputFile.mock.calls.map( + args => args[0].tmpFilePath + ) + + // tmpFilePath was always unique + expect(usedTmpFilePaths.length).toBe(new Set(usedTmpFilePaths).size) + }, + 10 * 1000 + ) it(`Use memoized results for file copies`, async () => { - await traceSVG({ - file, - args: { - color: `red`, - }, - fileArgs: { - width: 400, - }, - }) - await traceSVG({ - file: copyOfFile, - args: { - color: `red`, - }, - fileArgs: { - width: 400, - }, - }) + const copyPath = path.join(__dirname, `images/test-copy.png`) + await fs.copy(path.join(__dirname, `images/test.png`), copyPath) + + try { + const copyOfFile = getFileObject(copyPath) + await traceSVG({ + file, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + await traceSVG({ + file: copyOfFile, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + } finally { + await fs.remove(copyPath) + } expect(memoizedTraceSVG).toBeCalledTimes(2) expect(notMemoizedtraceSVG).toBeCalledTimes(1) expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1) }) + + it(`should work with long filenames`, async () => { + const copyPath = path.join( + __dirname, + `images/${`a`.repeat(10)} (1) ${`a`.repeat(100)}.png` + ) + await fs.copy(path.join(__dirname, `images/test.png`), copyPath) + expect.assertions(1) + + try { + const copyOfFile = getFileObject(copyPath) + await traceSVG({ + file: copyOfFile, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + expect(true).toBe(true) + } finally { + await fs.remove(copyPath) + } + }) + + it(`should work with long filenames that end with a dot`, async () => { + const copyPath = path.join(__dirname, `images/test${`.`.repeat(100)}.png`) + await fs.copy(path.join(__dirname, `images/test.png`), copyPath) + expect.assertions(1) + + try { + const copyOfFile = getFileObject(copyPath) + await traceSVG({ + file: copyOfFile, + args: { + color: `red`, + }, + fileArgs: { + width: 400, + }, + }) + expect(true).toBe(true) + } catch (err) { + await fs.remove(copyPath) + } finally { + await fs.remove(copyPath) + } + }) }) diff --git a/packages/gatsby-plugin-sharp/src/gatsby-node.js b/packages/gatsby-plugin-sharp/src/gatsby-node.js index 1f474d9f54f2a..919d9bee0c4f0 100644 --- a/packages/gatsby-plugin-sharp/src/gatsby-node.js +++ b/packages/gatsby-plugin-sharp/src/gatsby-node.js @@ -6,7 +6,7 @@ const { _lazyJobsEnabled, } = require(`./index`) const { pathExists } = require(`fs-extra`) -const { slash } = require(`gatsby-core-utils`) +const { slash } = require(`gatsby-core-utils/path`) const { setPluginOptions } = require(`./plugin-options`) const path = require(`path`) diff --git a/packages/gatsby-plugin-sharp/src/gatsby-worker.js b/packages/gatsby-plugin-sharp/src/gatsby-worker.js index 0b34c1a6579cb..439466a444d66 100644 --- a/packages/gatsby-plugin-sharp/src/gatsby-worker.js +++ b/packages/gatsby-plugin-sharp/src/gatsby-worker.js @@ -1,6 +1,6 @@ const path = require(`path`) const queue = require(`async/queue`) -const { cpuCoreCount } = require(`gatsby-core-utils`) +const { cpuCoreCount } = require(`gatsby-core-utils/cpu-core-count`) const { processFile } = require(`./process-file`) exports.IMAGE_PROCESSING_JOB_NAME = `IMAGE_PROCESSING` diff --git a/packages/gatsby-plugin-sharp/src/index.js b/packages/gatsby-plugin-sharp/src/index.js index 5ceadcfd28e47..1e30b2445e0f5 100644 --- a/packages/gatsby-plugin-sharp/src/index.js +++ b/packages/gatsby-plugin-sharp/src/index.js @@ -1,7 +1,7 @@ const sharp = require(`./safe-sharp`) const { generateImageData } = require(`./image-data`) const imageSize = require(`probe-image-size`) -const { isCI } = require(`gatsby-core-utils`) +const { isCI } = require(`gatsby-core-utils/ci`) const _ = require(`lodash`) const fs = require(`fs-extra`) diff --git a/packages/gatsby-plugin-sharp/src/process-file.js b/packages/gatsby-plugin-sharp/src/process-file.js index f7fafc934bc8c..94587f907d262 100644 --- a/packages/gatsby-plugin-sharp/src/process-file.js +++ b/packages/gatsby-plugin-sharp/src/process-file.js @@ -5,7 +5,9 @@ const debug = require(`debug`)(`gatsby:gatsby-plugin-sharp`) const duotone = require(`./duotone`) const { healOptions } = require(`./plugin-options`) const { SharpError } = require(`./sharp-error`) -const { createContentDigest } = require(`gatsby-core-utils`) +const { + createContentDigest, +} = require(`gatsby-core-utils/create-content-digest`) // Try to enable the use of SIMD instructions. Seems to provide a smallish // speedup on resizing heavy loads (~10%). Sharp disables this feature by diff --git a/packages/gatsby-plugin-sharp/src/trace-svg.js b/packages/gatsby-plugin-sharp/src/trace-svg.js index b9a894d9f66d3..49da6674ac7bb 100644 --- a/packages/gatsby-plugin-sharp/src/trace-svg.js +++ b/packages/gatsby-plugin-sharp/src/trace-svg.js @@ -8,7 +8,9 @@ const filenamify = require(`filenamify`) const duotone = require(`./duotone`) const { getPluginOptions, healOptions } = require(`./plugin-options`) const { reportError } = require(`./report-error`) -const { createContentDigest } = require(`gatsby-core-utils`) +const { + createContentDigest, +} = require(`gatsby-core-utils/create-content-digest`) exports.notMemoizedPrepareTraceSVGInputFile = async ({ file, @@ -114,9 +116,8 @@ exports.notMemoizedtraceSVG = async ({ file, args, fileArgs, reporter }) => { const tmpFilePath = path.join( tmpDir, - filenamify( - `${file.internal.contentDigest}-${file.name}-${optionsHash}.${file.extension}` - ) + filenamify(`${file.internal.contentDigest}-${file.name}-${optionsHash}`) + + `.${file.extension}` ) await exports.memoizedPrepareTraceSVGInputFile({