From 90b20bde822c8190fe760f86dd9fee8591a2e611 Mon Sep 17 00:00:00 2001 From: Gretchen Shelby-Dormer <35184207+gshel@users.noreply.github.com> Date: Mon, 4 Dec 2023 11:53:24 -0400 Subject: [PATCH] feat: Consolidate onBuild tests + verify imagesPath for multiple operating systems (#101) # Description Consolidated and added new tests to demonstrate bug where Windows imagesPath contains url-encoded `\`, caused by path.join() using os specific separator which is escaped `\` for windows ## Issue Ticket Number `#100` Fixes #100 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update # Checklist - [x] I have followed the contributing guidelines of this project as mentioned in [CONTRIBUTING.md](/CONTRIBUTING.md) - [x] I have created an [issue](https://github.com/colbyfayock/netlify-plugin-cloudinary/issues) ticket for this PR - [x] I have checked to ensure there aren't other open [Pull Requests](https://github.com/colbyfayock/netlify-plugin-cloudinary/pulls) for the same update/change? - [x] I have performed a self-review of my own code - [x] I have run tests locally to ensure they all pass - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes needed to the documentation --- netlify-plugin-cloudinary/src/index.ts | 3 +- .../tests/on-build.test.js | 271 ++++++++---------- 2 files changed, 121 insertions(+), 153 deletions(-) diff --git a/netlify-plugin-cloudinary/src/index.ts b/netlify-plugin-cloudinary/src/index.ts index fbf069c..ee9fcef 100644 --- a/netlify-plugin-cloudinary/src/index.ts +++ b/netlify-plugin-cloudinary/src/index.ts @@ -134,7 +134,7 @@ export async function onBuild({ cname, deliveryType, folder = process.env.SITE_NAME || '', - imagesPath = CLOUDINARY_ASSET_DIRECTORIES.find( + imagesPath = inputs.imagesPath || CLOUDINARY_ASSET_DIRECTORIES.find( ({ inputKey }) => inputKey === 'imagesPath', )?.path, maxSize, @@ -277,6 +277,7 @@ export async function onBuild({ } mediaPaths.forEach(async mediaPath => { + mediaPath = mediaPath.split(path.win32.sep).join(path.posix.sep); const cldAssetPath = `/${path.posix.join(PUBLIC_ASSET_PATH, mediaPath)}`; const cldAssetUrl = `${host}${cldAssetPath}`; try { diff --git a/netlify-plugin-cloudinary/tests/on-build.test.js b/netlify-plugin-cloudinary/tests/on-build.test.js index ab1f7d1..10e8b46 100644 --- a/netlify-plugin-cloudinary/tests/on-build.test.js +++ b/netlify-plugin-cloudinary/tests/on-build.test.js @@ -1,8 +1,20 @@ -import { vi, expect, describe, test, beforeEach,afterEach } from 'vitest'; import { promises as fs } from 'fs'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { onBuild } from '../src/'; import { ERROR_API_CREDENTIALS_REQUIRED } from '../src/data/errors'; +const contexts = [ + { + name: 'production', + url: 'https://netlify-plugin-cloudinary.netlify.app' + }, + { + name: 'deploy-preview', + url: 'https://deploy-preview-1234--netlify-plugin-cloudinary.netlify.app' + } +] + describe('onBuild', () => { const ENV_ORIGINAL = process.env; const readdir = fs.readdir; @@ -51,7 +63,7 @@ describe('onBuild', () => { }, utils: { build: { - failBuild: () => {} + failBuild: () => { } } } }); @@ -63,174 +75,129 @@ describe('onBuild', () => { describe('Redirects', () => { - test('should create redirects with defaut fetch-based configuration in production context', async () => { - const imagesFunctionName = 'cld_images'; + let deliveryType = 'fetch'; + let netlifyConfig; + let redirects; - fs.readdir.mockResolvedValue([imagesFunctionName]); - - process.env.CONTEXT = 'production'; - process.env.URL = 'https://netlify-plugin-cloudinary.netlify.app'; + const defaultRedirect = { + from: '/path', + to: '/other-path', + status: 200 + } + beforeEach(async () => { // Tests to ensure that delivery type of fetch works without API Key and Secret as it should delete process.env.CLOUDINARY_API_KEY; delete process.env.CLOUDINARY_API_SECRET; - const deliveryType = 'fetch'; - const imagesPath = '/images'; + // resets vars for each tests + redirects = [defaultRedirect]; - const defaultRedirect = { - from: '/path', - to: '/other-path', - status: 200 - } - - const redirects = [defaultRedirect]; - - const netlifyConfig = { + netlifyConfig = { redirects }; - await onBuild({ - netlifyConfig, - constants: { - PUBLISH_DIR: `.next/out${imagesPath}` - }, - inputs: { - deliveryType - } - }); - expect(redirects[0]).toEqual({ - from: `${imagesPath}/*`, - to: `https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/${deliveryType}/f_auto,q_auto/${process.env.URL}/cld-assets${imagesPath}/:splat`, - status: 302, - force: true - }); - - expect(redirects[1]).toEqual({ - from: `/cld-assets${imagesPath}/*`, - to: `${imagesPath}/:splat`, - status: 200, - force: true - }); - - expect(redirects[2]).toEqual(defaultRedirect); - }); - - test('should create redirects with defaut fetch-based configuration in deploy preview context', async () => { - const imagesFunctionName = 'cld_images'; - - fs.readdir.mockResolvedValue([imagesFunctionName]); - - process.env.CONTEXT = 'deploy-preview'; - process.env.DEPLOY_PRIME_URL = 'https://deploy-preview-1234--netlify-plugin-cloudinary.netlify.app'; - - const deliveryType = 'fetch'; - const imagesPath = '/images'; - - const defaultRedirect = { - from: '/path', - to: '/other-path', - status: 200 - } - - const redirects = [defaultRedirect]; - - const netlifyConfig = { - redirects - }; - - await onBuild({ - netlifyConfig, - constants: { - PUBLISH_DIR: `.next/out${imagesPath}` - }, - inputs: { - deliveryType - } - }); - - expect(redirects[0]).toEqual({ - from: `${imagesPath}/*`, - to: `https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/${deliveryType}/f_auto,q_auto/${process.env.DEPLOY_PRIME_URL}/cld-assets${imagesPath}/:splat`, - status: 302, - force: true - }); - - expect(redirects[1]).toEqual({ - from: `/cld-assets${imagesPath}/*`, - to: `${imagesPath}/:splat`, - status: 200, - force: true - }); - - expect(redirects[2]).toEqual(defaultRedirect); - }); - - test('should create redirects with multiple image paths', async () => { const imagesFunctionName = 'cld_images'; fs.readdir.mockResolvedValue([imagesFunctionName]); + }) - process.env.CONTEXT = 'deploy-preview'; - process.env.DEPLOY_PRIME_URL = 'https://deploy-preview-1234--netlify-plugin-cloudinary.netlify.app'; - - const deliveryType = 'fetch'; - const imagesPath = [ '/images', '/assets' ]; - - const defaultRedirect = { - from: '/path', - to: '/other-path', - status: 200 - } - - const redirects = [defaultRedirect]; - - const netlifyConfig = { - redirects + function validate(imagesPath, redirects, url) { + if (typeof (imagesPath) === 'string') { + imagesPath = [imagesPath] }; - await onBuild({ - netlifyConfig, - constants: { - PUBLISH_DIR: __dirname - }, - inputs: { - deliveryType, - imagesPath - } + let count = 0 + imagesPath?.reverse().forEach(element => { + let i = element.split(path.win32.sep).join(path.posix.sep).replace('/', ''); + + expect(redirects[count]).toEqual({ + from: `/${i}/*`, + to: `https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/${deliveryType}/f_auto,q_auto/${process.env.URL}/cld-assets/${i}/:splat`, + status: 302, + force: true + }); + + expect(redirects[count + 1]).toEqual({ + from: `/cld-assets/${i}/*`, + to: `/${i}/:splat`, + status: 200, + force: true + }); + count += 2; }); - - expect(redirects[0]).toEqual({ - from: `${imagesPath[1]}/*`, - to: `https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/${deliveryType}/f_auto,q_auto/${process.env.DEPLOY_PRIME_URL}/cld-assets${imagesPath[1]}/:splat`, - status: 302, - force: true + expect(redirects[redirects.length - 1]).toEqual(defaultRedirect); + } + + describe.each(['posix', 'win32'])('Operating system: %o', (os) => { + let separator = path[os].sep; + let imagesPathStrings = [ + '/images', + '/nest', + '/images/nesttest' + ]; + imagesPathStrings = imagesPathStrings.map(i => i.replace(/\//g, separator)); + + let imagesPathLists = [ + [['/images', '/assets']], + [['/images/nest1', '/assets/nest2']], + [['/example/hey', '/mixed', '/test']] + ]; + imagesPathLists = imagesPathLists.map(collection => + collection.map(imagesPath => + imagesPath.map(i => i.replace(/\//g, separator)) + ) + ); + + describe.each(contexts)(`should create redirects with default ${deliveryType}-based configuration in $name context`, async (context) => { + process.env.URL = context.url; + + test.each(imagesPathLists)('%o', async (imagesPath) => { + + await onBuild({ + netlifyConfig, + constants: { + PUBLISH_DIR: __dirname + }, + inputs: { + deliveryType, + imagesPath + } + }); + validate(imagesPath, redirects); + }); + + test.each(imagesPathStrings)('%o', async (imagesPath) => { + + await onBuild({ + netlifyConfig, + constants: { + PUBLISH_DIR: `.next/out${imagesPath}` + }, + inputs: { + deliveryType, + imagesPath + } + }); + validate(imagesPath, redirects); + }); + + test('imagesPath undefined', async () => { + + await onBuild({ + netlifyConfig, + constants: { + PUBLISH_DIR: '.next/out/' + }, + inputs: { + deliveryType + } + }); + let imagesPath = '/images'; + validate(imagesPath, redirects); + }); }); - - expect(redirects[1]).toEqual({ - from: `/cld-assets${imagesPath[1]}/*`, - to: `${imagesPath[1]}/:splat`, - status: 200, - force: true - }); - expect(redirects[2]).toEqual({ - from: `${imagesPath[0]}/*`, - to: `https://res.cloudinary.com/${process.env.CLOUDINARY_CLOUD_NAME}/image/${deliveryType}/f_auto,q_auto/${process.env.DEPLOY_PRIME_URL}/cld-assets${imagesPath[0]}/:splat`, - status: 302, - force: true - }); - - expect(redirects[3]).toEqual({ - from: `/cld-assets${imagesPath[0]}/*`, - to: `${imagesPath[0]}/:splat`, - status: 200, - force: true - }); - - expect(redirects[4]).toEqual(defaultRedirect); - }); - + }) }); - });