Skip to content

Commit

Permalink
fix: revert slash validation and change validation message (netlify/e…
Browse files Browse the repository at this point in the history
…dge-bundler#343)

* fix: change validation message

* Apply suggestions from code review

Co-authored-by: Kristen Lavavej <35638702+klavavej@users.noreply.github.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* chore: fix snapshots

* chore: do not validate pattern for beginning slash

---------

Co-authored-by: Kristen Lavavej <35638702+klavavej@users.noreply.github.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
  • Loading branch information
3 people committed Apr 5, 2023
1 parent cca2ee3 commit 04ab695
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 53 deletions.
1 change: 0 additions & 1 deletion packages/edge-bundler/node/feature_flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const defaultFlags = {
edge_functions_correct_order: false,
edge_functions_fail_unsupported_regex: false,
edge_functions_invalid_config_throw: false,
edge_functions_manifest_validate_slash: false,
}

type FeatureFlag = keyof typeof defaultFlags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,25 +122,12 @@ ADDTIONAL PROPERTY must NOT have additional properties
exports[`route > should throw on invalid pattern 1`] = `
"Validation of Edge Functions manifest failed
FORMAT pattern needs to be a regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards
FORMAT pattern must be a regex that starts with ^ and ends with $ (e.g. ^/blog/[d]{4}$)
10 | \\"name\\": \\"name\\",
11 | \\"function\\": \\"hello\\",
> 12 | \\"pattern\\": \\"/^/hello/?$/\\",
| ^^^^^^^^^^^^^^ 👈🏽 format pattern needs to be a regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards
13 | \\"generator\\": \\"@netlify/fake-plugin@1.0.0\\"
14 | }
15 | ],"
`;
exports[`route > should throw on missing beginning slash with FF 1`] = `
"Validation of Edge Functions manifest failed
FORMAT pattern needs to be a regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards
10 | \\"name\\": \\"name\\",
11 | \\"function\\": \\"hello\\",
> 12 | \\"pattern\\": \\"^hello/?$\\",
| ^^^^^^^^^^^ 👈🏽 format pattern needs to be a regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards
| ^^^^^^^^^^^^^^ 👈🏽 format pattern must be a regex that starts with ^ and ends with $ (e.g. ^/blog/[d]{4}$)
13 | \\"generator\\": \\"@netlify/fake-plugin@1.0.0\\"
14 | }
15 | ],"
Expand Down
35 changes: 5 additions & 30 deletions packages/edge-bundler/node/validation/manifest/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import chalk from 'chalk'
import { test, expect, describe, beforeEach, vi } from 'vitest'
import { test, expect, describe } from 'vitest'

import { validateManifest, ManifestValidationError } from './index.js'

Expand Down Expand Up @@ -112,57 +112,32 @@ describe('bundle', () => {
})

describe('route', () => {
let freshValidateManifest: typeof validateManifest

beforeEach(async () => {
// reset all modules, to get a fresh AJV validator for FF changes
vi.resetModules()
const indexImport = await import('./index.js')
freshValidateManifest = indexImport.validateManifest
})

test('should throw on additional property', () => {
const manifest = getBaseManifest()
manifest.routes[0].foo = 'bar'

expect(() => freshValidateManifest(manifest)).toThrowErrorMatchingSnapshot()
expect(() => validateManifest(manifest)).toThrowErrorMatchingSnapshot()
})

test('should throw on invalid pattern', () => {
const manifest = getBaseManifest()
manifest.routes[0].pattern = '/^/hello/?$/'

expect(() => freshValidateManifest(manifest)).toThrowErrorMatchingSnapshot()
})

test('should not throw on missing beginning slash without FF', () => {
const manifest = getBaseManifest()
manifest.routes[0].pattern = '^hello/?$'

expect(() => freshValidateManifest(manifest, { edge_functions_manifest_validate_slash: false })).not.toThrowError()
})

test('should throw on missing beginning slash with FF', () => {
const manifest = getBaseManifest()
manifest.routes[0].pattern = '^hello/?$'

expect(() =>
freshValidateManifest(manifest, { edge_functions_manifest_validate_slash: true }),
).toThrowErrorMatchingSnapshot()
expect(() => validateManifest(manifest)).toThrowErrorMatchingSnapshot()
})

test('should throw on missing function', () => {
const manifest = getBaseManifest()
delete manifest.routes[0].function

expect(() => freshValidateManifest(manifest)).toThrowErrorMatchingSnapshot()
expect(() => validateManifest(manifest)).toThrowErrorMatchingSnapshot()
})

test('should throw on missing pattern', () => {
const manifest = getBaseManifest()
delete manifest.routes[0].pattern

expect(() => freshValidateManifest(manifest)).toThrowErrorMatchingSnapshot()
expect(() => validateManifest(manifest)).toThrowErrorMatchingSnapshot()
})
})

Expand Down
9 changes: 5 additions & 4 deletions packages/edge-bundler/node/validation/manifest/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import edgeManifestSchema from './schema.js'

let manifestValidator: ValidateFunction<Manifest>

const initializeValidator = (featureFlags: FeatureFlags) => {
const initializeValidator = () => {
if (manifestValidator === undefined) {
const ajv = new Ajv({ allErrors: true })
ajvErrors(ajv)

// regex pattern for manifest route pattern
// checks if the pattern string starts with ^ and ends with $
const normalizedPatternRegex = featureFlags.edge_functions_manifest_validate_slash ? /^\^\/.*\$$/ : /^\^.*\$$/
const normalizedPatternRegex = /^\^.*\$$/
ajv.addFormat('regexPattern', {
validate: (data: string) => normalizedPatternRegex.test(data),
})
Expand All @@ -30,8 +30,9 @@ const initializeValidator = (featureFlags: FeatureFlags) => {
}

// throws on validation error
export const validateManifest = (manifestData: unknown, featureFlags: FeatureFlags = {}): void => {
const validate = initializeValidator(featureFlags)
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export const validateManifest = (manifestData: unknown, _featureFlags: FeatureFlags = {}): void => {
const validate = initializeValidator()

const valid = validate(manifestData)

Expand Down
5 changes: 2 additions & 3 deletions packages/edge-bundler/node/validation/manifest/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ const routesSchema = {
pattern: {
type: 'string',
format: 'regexPattern',
errorMessage:
'pattern needs to be a regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards',
errorMessage: 'pattern must be a regex that starts with ^ and ends with $ (e.g. ^/blog/[d]{4}$)',
},
generator: { type: 'string' },
},
Expand All @@ -35,7 +34,7 @@ const functionConfigSchema = {
type: 'string',
format: 'regexPattern',
errorMessage:
'excluded_patterns needs to be an array of regex that starts with ^ followed by / and ends with $ without any additional slashes before and afterwards',
'excluded_patterns must be an array of regex that starts with ^ and ends with $ (e.g. ^/blog/[d]{4}$)',
},
},
on_error: { type: 'string' },
Expand Down

0 comments on commit 04ab695

Please sign in to comment.