Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enforce required body parameters for RunIt #883

Merged
merged 5 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion packages/run-it/src/RunIt.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('RunIt', () => {
})

test('the form submit handler invokes the request callback on submit', async () => {
renderRunIt()
renderRunIt(api, api.methods.me)
const defaultRequestCallback = jest
.spyOn(sdk.authSession.transport, 'rawRequest')
.mockResolvedValueOnce(testTextResponse)
Expand All @@ -108,6 +108,22 @@ describe('RunIt', () => {
).toBeInTheDocument()
})
})

test('run_inline_query has required body parameters', async () => {
renderRunIt()
const defaultRequestCallback = jest
.spyOn(sdk.authSession.transport, 'rawRequest')
.mockResolvedValueOnce(testTextResponse)
const button = screen.getByRole('button', { name: run })
expect(button).toBeInTheDocument()
userEvent.click(button)
await waitFor(() => {
expect(defaultRequestCallback).not.toHaveBeenCalled()
expect(screen.queryByRole('status')).toHaveTextContent(
'Error: Required properties "model, view" must be provided in the body'
)
})
})
})

describe('not configured or authenticated', () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/run-it/src/RunIt.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,12 @@ export const RunIt: FC<RunItProps> = ({
requestContent
)
if (body) {
const message = validateBody(body)
const [bodyParam] = method.bodyParams
const requiredKeys = Object.keys(bodyParam.type.requiredProperties)
const message = validateBody(body, requiredKeys)
setValidationMessage(message)
if (message) {
// syntax error, don't run
// body has an error, don't run
return
}
}
Expand Down
39 changes: 23 additions & 16 deletions packages/run-it/src/components/RequestForm/formUtils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,27 +270,34 @@ describe('Complex Item', () => {
})

describe('validateBody', () => {
const requiredKeys = ['model', 'view']
test.each`
value | error
value | expected | requiredKeys
${{
view: 'users',
fields: ['users.id', 'users.first_name'],
}} | ${'Error: Required properties "model" must be provided'} | ${requiredKeys}
${{
model: 'thelook',
view: 'users',
fields: ['users.id', 'users.first_name'],
}} | ${''}
${'na.-_me=Vapor&age=3&luckyNumbers[]=5&luckyNumbers[]=7'} | ${''}
${'name=Vapor&age=3&luckyNumbers[]=5&luckyNumbers[]7'} | ${'luckyNumbers[]7'}
${'{'} | ${'Unexpected end of JSON input'}
${'}'} | ${'Unexpected token } in JSON at position 0'}
${'['} | ${'Unexpected end of JSON input'}
${'"'} | ${'Unexpected end of JSON input'}
${'"foo"'} | ${''}
${''} | ${''}
${'{}'} | ${''}
`('it validates a body value of "$value"', ({ value, error }) => {
const actual = validateBody(value)
const expected = error ? `Syntax error in the body: ${error}` : error
expect(actual).toEqual(expected)
})
}} | ${''} | ${requiredKeys}
${'na.-_me=Vapor&age=3&luckyNumbers[]=5&luckyNumbers[]=7'} | ${''} | ${[]}
${'name=Vapor&age=3&luckyNumbers[]=5&luckyNumbers[]7'} | ${'Syntax error in the body: luckyNumbers[]7'} | ${[]}
${'{'} | ${'Syntax error in the body: Unexpected end of JSON input'} | ${[]}
${'}'} | ${'Syntax error in the body: Unexpected token } in JSON at position 0'} | ${[]}
${'['} | ${'Syntax error in the body: Unexpected end of JSON input'} | ${[]}
${'"'} | ${'Syntax error in the body: Unexpected end of JSON input'} | ${[]}
${'"foo"'} | ${''} | ${[]}
${''} | ${''} | ${[]}
${'{}'} | ${''} | ${[]}
`(
'it validates a body value of "$value"',
({ value, expected, requiredKeys }) => {
const actual = validateBody(value, requiredKeys)
expect(actual).toEqual(expected)
}
)
})
})

Expand Down
44 changes: 32 additions & 12 deletions packages/run-it/src/components/RequestForm/formUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import {
import { Info } from '@styled-icons/material'
import { DateFormat, InputDate } from '@looker/components-date'
import { CodeEditor } from '@looker/code-editor'

import type { RunItInput, RunItValues } from '../../RunIt'
import { FormItem } from './FormItem'

Expand Down Expand Up @@ -345,23 +344,44 @@ export const validateEncodedValues = (body: string) => {
* Returns an error message if the body is not JSON or valid form url encoding
*
* @param body string to validate
* @param requiredKeys keys that are required in the body parameter
*/
export const validateBody = (body: string | Record<string, any>) => {
export const validateBody = (
body: string | Record<string, any>,
requiredKeys: string[]
) => {
let parsed

let result = ''
if (body && typeof body === 'string') {
if (/^[[{}"]/.test(body)) {
// most likely JSON
try {
JSON.parse(body)
} catch (e: any) {
result = e.message
if (body) {
if (typeof body === 'string') {
if (/^[[{}"]/.test(body)) {
// most likely JSON
try {
parsed = JSON.parse(body)
} catch (e: any) {
result = e.message
}
} else {
result = validateEncodedValues(body)
}
if (result) {
result = `Syntax error in the body: ${result}`
}
} else {
result = validateEncodedValues(body)
parsed = body
}
}
if (result) {
result = `Syntax error in the body: ${result}`

if (parsed && requiredKeys && requiredKeys.length > 0) {
const required = new Set<string>(requiredKeys)
const keys = new Set<string>(Object.keys(parsed))
const missing = new Set<string>([...required].filter((k) => !keys.has(k)))
if (missing.size > 0) {
result = `Error: Required properties "${Array.from(missing).join(
', '
)}" must be provided`
}
}
return result
}