From 6afe7bd513df6b5c200117d33ff81c575cfd0454 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 5 Sep 2023 13:32:08 +0200 Subject: [PATCH] feat: support `context.params` + `config.method` in functions + edge functions (#5970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: support context.params in functions * feat: support config.methods in functions * feat: support `method` matching in edge functions * Update src/lib/edge-functions/registry.mjs Co-authored-by: Eduardo Bouças * chore: use ts syntax of arrays * fix: oops, method isn't an array! * refactor: return matched route --------- Co-authored-by: Eduardo Bouças --- src/lib/edge-functions/proxy.mjs | 2 +- src/lib/edge-functions/registry.mjs | 7 ++++++- src/lib/functions/netlify-function.mjs | 16 ++++++++++++---- src/lib/functions/registry.mjs | 8 ++++---- src/lib/functions/server.mjs | 7 +++++-- src/utils/headers.mjs | 1 + src/utils/proxy.mjs | 13 ++++++++----- .../netlify/edge-functions/delete-product.js | 6 ++++++ .../functions/custom-path-expression.mjs | 2 +- .../functions/delete.mjs | 6 ++++++ .../commands/dev/edge-functions.test.ts | 18 ++++++++++++++++++ tests/integration/commands/dev/v2-api.test.ts | 9 ++++++++- 12 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 tests/integration/__fixtures__/dev-server-with-edge-functions/netlify/edge-functions/delete-product.js create mode 100644 tests/integration/__fixtures__/dev-server-with-v2-functions/functions/delete.mjs diff --git a/src/lib/edge-functions/proxy.mjs b/src/lib/edge-functions/proxy.mjs index f6c1b55f95a..f917d1b4161 100644 --- a/src/lib/edge-functions/proxy.mjs +++ b/src/lib/edge-functions/proxy.mjs @@ -146,7 +146,7 @@ export const initializeProxy = async ({ await registry.initialize() const url = new URL(req.url, `http://${LOCAL_HOST}:${mainPort}`) - const { functionNames, invocationMetadata, orphanedDeclarations } = registry.matchURLPath(url.pathname) + const { functionNames, invocationMetadata, orphanedDeclarations } = registry.matchURLPath(url.pathname, req.method) // If the request matches a config declaration for an Edge Function without // a matching function file, we warn the user. diff --git a/src/lib/edge-functions/registry.mjs b/src/lib/edge-functions/registry.mjs index cae41fc5100..f613ffb39ec 100644 --- a/src/lib/edge-functions/registry.mjs +++ b/src/lib/edge-functions/registry.mjs @@ -302,8 +302,9 @@ export class EdgeFunctionsRegistry { /** * @param {string} urlPath + * @param {string} method */ - matchURLPath(urlPath) { + matchURLPath(urlPath, method) { const declarations = this.#bundler.mergeDeclarations( this.#declarationsFromTOML, this.#userFunctionConfigs, @@ -330,6 +331,10 @@ export class EdgeFunctionsRegistry { const routeIndexes = [] routes.forEach((route, index) => { + if (route.methods && route.methods.length !== 0 && !route.methods.includes(method)) { + return + } + if (!route.pattern.test(urlPath)) { return } diff --git a/src/lib/functions/netlify-function.mjs b/src/lib/functions/netlify-function.mjs index baa7d1ce32c..db77ddd8eb9 100644 --- a/src/lib/functions/netlify-function.mjs +++ b/src/lib/functions/netlify-function.mjs @@ -158,12 +158,22 @@ export default class NetlifyFunction { } } - async matchURLPath(rawPath) { + /** + * Matches all routes agains the incoming request. If a match is found, then the matched route is returned. + * @param {string} rawPath + * @param {string} method + * @returns matched route + */ + async matchURLPath(rawPath, method) { await this.buildQueue const path = (rawPath.endsWith('/') ? rawPath.slice(0, -1) : rawPath).toLowerCase() const { routes = [] } = this.buildData - const isMatch = routes.some(({ expression, literal }) => { + return routes.find(({ expression, literal, methods }) => { + if (methods.length !== 0 && !methods.includes(method)) { + return false + } + if (literal !== undefined) { return path === literal } @@ -176,8 +186,6 @@ export default class NetlifyFunction { return false }) - - return isMatch } get url() { diff --git a/src/lib/functions/registry.mjs b/src/lib/functions/registry.mjs index 1c55deafaf1..d15c8a9420a 100644 --- a/src/lib/functions/registry.mjs +++ b/src/lib/functions/registry.mjs @@ -122,12 +122,12 @@ export class FunctionsRegistry { return this.functions.get(name) } - async getFunctionForURLPath(urlPath) { + async getFunctionForURLPath(urlPath, method) { for (const func of this.functions.values()) { - const isMatch = await func.matchURLPath(urlPath) + const route = await func.matchURLPath(urlPath, method) - if (isMatch) { - return func + if (route) { + return { func, route } } } } diff --git a/src/lib/functions/server.mjs b/src/lib/functions/server.mjs index 8be9717d570..5d7a8564ab8 100644 --- a/src/lib/functions/server.mjs +++ b/src/lib/functions/server.mjs @@ -7,7 +7,7 @@ import jwtDecode from 'jwt-decode' import { NETLIFYDEVERR, NETLIFYDEVLOG, error as errorExit, log } from '../../utils/command-helpers.mjs' import { CLOCKWORK_USERAGENT, getFunctionsDistPath, getInternalFunctionsDir } from '../../utils/functions/index.mjs' -import { NFFunctionName } from '../../utils/headers.mjs' +import { NFFunctionName, NFFunctionRoute } from '../../utils/headers.mjs' import { headers as efHeaders } from '../edge-functions/headers.mjs' import { getGeoLocation } from '../geo-location.mjs' @@ -56,11 +56,13 @@ export const createHandler = function (options) { const { functionsRegistry } = options return async function handler(request, response) { - // If this header is set, it means we've already matched a function and we + // If these headers are set, it means we've already matched a function and we // can just grab its name directly. We delete the header from the request // because we don't want to expose it to user code. let functionName = request.header(NFFunctionName) delete request.headers[NFFunctionName] + const functionRoute = request.header(NFFunctionRoute) + delete request.headers[NFFunctionRoute] // If we didn't match a function with a custom route, let's try to match // using the fixed URL format. @@ -148,6 +150,7 @@ export const createHandler = function (options) { isBase64Encoded, rawUrl, rawQuery, + route: functionRoute, } const clientContext = buildClientContext(request.headers) || {} diff --git a/src/utils/headers.mjs b/src/utils/headers.mjs index c570fe705aa..65165e42f1f 100644 --- a/src/utils/headers.mjs +++ b/src/utils/headers.mjs @@ -47,4 +47,5 @@ const getErrorMessage = function ({ message }) { } export const NFFunctionName = 'x-nf-function-name' +export const NFFunctionRoute = 'x-nf-function-route' export const NFRequestID = 'x-nf-request-id' diff --git a/src/utils/proxy.mjs b/src/utils/proxy.mjs index 54968fe48dc..bd8a5ba8677 100644 --- a/src/utils/proxy.mjs +++ b/src/utils/proxy.mjs @@ -31,7 +31,7 @@ import renderErrorTemplate from '../lib/render-error-template.mjs' import { NETLIFYDEVLOG, NETLIFYDEVWARN, log, chalk } from './command-helpers.mjs' import createStreamPromise from './create-stream-promise.mjs' -import { headersForPath, parseHeaders, NFFunctionName, NFRequestID } from './headers.mjs' +import { headersForPath, parseHeaders, NFFunctionName, NFRequestID, NFFunctionRoute } from './headers.mjs' import { generateRequestID } from './request-id.mjs' import { createRewriter, onChanges } from './rules-proxy.mjs' import { signRedirect } from './sign-redirect.mjs' @@ -328,7 +328,8 @@ const serveRedirect = async function ({ env, functionsRegistry, match, options, return proxy.web(req, res, { target: options.functionsServer }) } - const functionWithCustomRoute = functionsRegistry && (await functionsRegistry.getFunctionForURLPath(destURL)) + const functionWithCustomRoute = + functionsRegistry && (await functionsRegistry.getFunctionForURLPath(destURL, req.method)) const destStaticFile = await getStatic(dest.pathname, options.publicFolder) let statusValue if ( @@ -342,7 +343,9 @@ const serveRedirect = async function ({ env, functionsRegistry, match, options, } if (isFunction(options.functionsPort, req.url) || functionWithCustomRoute) { - const functionHeaders = functionWithCustomRoute ? { [NFFunctionName]: functionWithCustomRoute.name } : {} + const functionHeaders = functionWithCustomRoute + ? { [NFFunctionName]: functionWithCustomRoute.func.name, [NFFunctionRoute]: functionWithCustomRoute.route } + : {} const url = reqToURL(req, originalURL) req.headers['x-netlify-original-pathname'] = url.pathname req.headers['x-netlify-original-search'] = url.search @@ -600,12 +603,12 @@ const onRequest = async ( } // Does the request match a function on a custom URL path? - const functionMatch = functionsRegistry ? await functionsRegistry.getFunctionForURLPath(req.url) : null + const functionMatch = functionsRegistry ? await functionsRegistry.getFunctionForURLPath(req.url, req.method) : null if (functionMatch) { // Setting an internal header with the function name so that we don't // have to match the URL again in the functions server. - const headers = { [NFFunctionName]: functionMatch.name } + const headers = { [NFFunctionName]: functionMatch.func.name, [NFFunctionRoute]: functionMatch.route.pattern } return proxy.web(req, res, { headers, target: functionsServer }) } diff --git a/tests/integration/__fixtures__/dev-server-with-edge-functions/netlify/edge-functions/delete-product.js b/tests/integration/__fixtures__/dev-server-with-edge-functions/netlify/edge-functions/delete-product.js new file mode 100644 index 00000000000..621934013d9 --- /dev/null +++ b/tests/integration/__fixtures__/dev-server-with-edge-functions/netlify/edge-functions/delete-product.js @@ -0,0 +1,6 @@ +export default (_, context) => new Response(`Deleted item successfully: ${context.params.sku}`) + +export const config = { + path: '/products/:sku', + method: 'DELETE', +} diff --git a/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/custom-path-expression.mjs b/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/custom-path-expression.mjs index 16f01d3c7a7..3ee80eb14a0 100644 --- a/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/custom-path-expression.mjs +++ b/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/custom-path-expression.mjs @@ -1,4 +1,4 @@ -export default async (req) => new Response(`With expression path: ${req.url}`) +export default async (req, context) => new Response(`With expression path: ${JSON.stringify(context.params)}`) export const config = { path: '/products/:sku', diff --git a/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/delete.mjs b/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/delete.mjs new file mode 100644 index 00000000000..a6a1bb76457 --- /dev/null +++ b/tests/integration/__fixtures__/dev-server-with-v2-functions/functions/delete.mjs @@ -0,0 +1,6 @@ +export default async (req, context) => new Response(`Deleted item successfully: ${context.params.sku}`) + +export const config = { + path: '/products/:sku', + method: "DELETE" +} diff --git a/tests/integration/commands/dev/edge-functions.test.ts b/tests/integration/commands/dev/edge-functions.test.ts index 7dd769ba168..d394270e5c1 100644 --- a/tests/integration/commands/dev/edge-functions.test.ts +++ b/tests/integration/commands/dev/edge-functions.test.ts @@ -45,6 +45,24 @@ describe('edge functions', () => { product: 'bar', }) }) + + test('should respect config.methods field', async ({ devServer }) => { + const responseGet = await got(`http://localhost:${devServer.port}/products/really-bad-product`, { + method: "GET", + throwHttpErrors: false, + retry: { limit: 0 }, + }) + + expect(responseGet.statusCode).toBe(404) + + const responseDelete = await got(`http://localhost:${devServer.port}/products/really-bad-product`, { + method: "DELETE", + throwHttpErrors: false, + retry: { limit: 0 }, + }) + + expect(responseDelete.body).toEqual('Deleted item successfully: really-bad-product') + }) }) setupFixtureTests('dev-server-with-edge-functions', { devServer: true }, () => { diff --git a/tests/integration/commands/dev/v2-api.test.ts b/tests/integration/commands/dev/v2-api.test.ts index c40a01d57cf..2247ae38379 100644 --- a/tests/integration/commands/dev/v2-api.test.ts +++ b/tests/integration/commands/dev/v2-api.test.ts @@ -112,11 +112,18 @@ describe.runIf(gte(version, '18.13.0'))('v2 api', () => { expect(await response.text()).toBe(`With literal path: ${url}`) }) + test('supports custom URLs with method matching', async ({ devServer }) => { + const url = `http://localhost:${devServer.port}/products/really-bad-product` + const response = await fetch(url, { method: 'DELETE' }) + expect(response.status).toBe(200) + expect(await response.text()).toBe(`Deleted item successfully: really-bad-product`) + }) + test('supports custom URLs using an expression path', async ({ devServer }) => { const url = `http://localhost:${devServer.port}/products/netlify` const response = await fetch(url) expect(response.status).toBe(200) - expect(await response.text()).toBe(`With expression path: ${url}`) + expect(await response.text()).toBe(`With expression path: {"sku":"netlify"}`) }) describe('handles rewrites to a function', () => {