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

refactor: Generator parts from interfaces into zod objects #42

Merged
merged 9 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-router-dom": "^6.24.0",
"zod": "^3.23.8",
"zustand": "^4.5.3"
},
"lint-staged": {
Expand Down
3 changes: 2 additions & 1 deletion src/codegen/codegen.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
generateRequestSnippets,
generateVariableDeclarations,
} from './codegen'
import { TestRule, CorrelationStateMap } from '@/types/rules'
import { CorrelationStateMap } from '@/types/rules'
import { TestRule } from '@/schemas/rules'
import { generateSequentialInt } from '@/rules/utils'
import { ProxyData } from '@/types'
import { correlationRecording } from '@/test/fixtures/correlationRecording'
Expand Down
3 changes: 2 additions & 1 deletion src/codegen/codegen.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { GroupedProxyData, ProxyData, RequestSnippetSchema } from '@/types'
import { TestRule, CorrelationStateMap } from '@/types/rules'
import { CorrelationStateMap } from '@/types/rules'
import { TestRule } from '@/schemas/rules'
import { applyRule } from '@/rules/rules'
import { generateSequentialInt } from '@/rules/utils'

Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useGeneratorStore/fixtures.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TestRule } from '@/types/rules'
import { TestRule } from '@/schemas/rules'

export const rules: TestRule[] = [
{
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useGeneratorStore/slices/rules.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TestRule } from '@/types/rules'
import { TestRule } from '@/schemas/rules'
import { ImmerStateCreator } from '@/utils/typescript'
import { rules } from '../fixtures'

Expand Down
8 changes: 2 additions & 6 deletions src/hooks/useGeneratorStore/slices/testData.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { ImmerStateCreator } from '@/utils/typescript'
import { Variable } from '@/types'

interface State {
variables: Variable[]
}
import { Variable, TestData } from '@/schemas/testData'

interface Actions {
setVariables: (variables: Variable[]) => void
}

export type TestDataStore = State & Actions
export type TestDataStore = TestData & Actions

export const createTestDataSlice: ImmerStateCreator<TestDataStore> = (set) => ({
variables: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
RampingStage,
RampingVUsOptions,
SharedIterationsOptions,
} from '@/types/testOptions'
} from '@/schemas/testOptions'
import { ImmerStateCreator } from '@/utils/typescript'

interface SharedIterationsState
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ImmerStateCreator } from '@/utils/typescript'
import { SleepType, ThinkTime, Timing } from '@/types/testOptions'
import { SleepType, ThinkTime, Timing } from '@/schemas/testOptions'
import { createFixedTiming } from '@/utils/thinkTime'

interface Actions {
Expand Down
28 changes: 27 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,38 @@ ipcMain.on('generator:save', async (event, generatorFile: string) => {
await writeFile(dialogResult.filePath, generatorFile)
})

ipcMain.handle('generator:open', async (event) => {
console.info('generator:open event received')
const browserWindow = browserWindowFromEvent(event)

const dialogResult = await dialog.showOpenDialog(browserWindow, {
message: 'Open Generator file',
properties: ['openFile'],
filters: [{ name: 'JSON', extensions: ['json'] }],
})

if (!dialogResult.canceled && dialogResult.filePaths[0]) {
const fileHandle = await open(dialogResult.filePaths[0], 'r')
try {
const data = await fileHandle?.readFile({ encoding: 'utf-8' })
// TODO: we might want to send an error on wrong file, a system to send and show errors from the main process
// could be levereged for many things mmmm
Comment on lines +251 to +252
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either messaging between render and main OR do the parsing on render side and let it handle there 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I left the comment there! 😄

Regardless of leveraging it in here or not, that system would be useful for having a channel between main/renderer to just send errors on any operations, the item has already been added on the todo list.

In the future we might move more operations on the main process, so handling the parsing on the renderer side would just be probably a temporary solution 🤔

const generator = await JSON.parse(data)

return { path: dialogResult.filePaths[0], content: generator }
} finally {
await fileHandle?.close()
}
}

return
})

// Settings
ipcMain.on('settings:toggle-theme', () => {
nativeTheme.themeSource = nativeTheme.shouldUseDarkColors ? 'light' : 'dark'
})


const browserWindowFromEvent = (
event: Electron.IpcMainEvent | Electron.IpcMainInvokeEvent
) => {
Expand Down
4 changes: 4 additions & 0 deletions src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { ipcRenderer, contextBridge, IpcRendererEvent } from 'electron'
import { ProxyData, K6Log } from './types'
import { HarFile } from './types/har'
import { GeneratorFile } from './types/generator'

// Create listener and return clean up function to be used in useEffect
function createListener<T>(channel: string, callback: (data: T) => void) {
Expand Down Expand Up @@ -79,6 +80,9 @@ const generator = {
saveGenerator: (generatorFile: string) => {
ipcRenderer.send('generator:save', generatorFile)
},
loadGenerator: (): Promise<GeneratorFile | void> => {
return ipcRenderer.invoke('generator:open')
},
} as const

const studio = {
Expand Down
3 changes: 2 additions & 1 deletion src/rules/correlation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ProxyData, RequestSnippetSchema, Response, Request } from '@/types'
import { CorrelationRule, CorrelationStateMap } from '@/types/rules'
import { CorrelationStateMap } from '@/types/rules'
import { CorrelationRule } from '@/schemas/rules'
import { cloneDeep, get, isEqual } from 'lodash-es'
import { canonicalHeaderKey, matchFilter } from './utils'
import { getHeaderValues } from '@/utils/headers'
Expand Down
2 changes: 1 addition & 1 deletion src/rules/correlation.utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CorrelationRule } from '@/types/rules'
import { CorrelationRule } from '@/schemas/rules'
import { Header, Request } from '@/types'

export function safeJsonParse(value: string) {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/customCode.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { RequestSnippetSchema } from '@/types'
import { CustomCodeRule } from '@/types/rules'
import { CustomCodeRule } from '@/schemas/rules'
import { matchFilter } from './utils'

export function applyCustomCodeRule(
Expand Down
3 changes: 2 additions & 1 deletion src/rules/rules.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { RequestSnippetSchema } from '@/types'
import { TestRule, CorrelationStateMap } from '@/types/rules'
import { CorrelationStateMap } from '@/types/rules'
import { TestRule } from '@/schemas/rules'
import { exhaustive } from '../utils/typescript'
import { applyCustomCodeRule } from './customCode'
import { applyCorrelationRule } from './correlation'
Expand Down
2 changes: 1 addition & 1 deletion src/rules/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TestRule } from '@/types/rules'
import { TestRule } from '@/schemas/rules'
import { RequestSnippetSchema } from '@/types'
import { exhaustive } from '@/utils/typescript'

Expand Down
16 changes: 16 additions & 0 deletions src/schemas/generator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { z } from 'zod'
import { TestRule } from '@/schemas/rules'
import { TestData } from '@/schemas/testData'
import { TestOptions } from '@/schemas/testOptions'

export const GeneratorFileData = z.object({
name: z.string(),
version: z.string(),
recordingPath: z.string(),
options: TestOptions,
testData: TestData,
rules: TestRule.array(),
allowlist: z.string().array(),
})

export type GeneratorFileData = z.infer<typeof GeneratorFileData>
Comment on lines +6 to +16
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there are no conflicts between exported type and const of the same name, but I would still prefer zod objects to be appended by Schema to be more explicit:

export const GeneratorFileDataSchema = z.object({ .. })
export type GeneratorFileData = z.infer<typeof GeneratorFileData>

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I like that the names are shared so depending on where you use it you get the right "object" 🤔

In theory you know that it's a schema because it will be imported from src/schemas/... and inspecting the object also clearly gives it away.
I find it really nice and reduces repetition/verbosity.

It's also true that I come from Python where it's done the same way, you use the object both as the object to construct and as a type, so maybe for typescript let's wait for more feedback before taking a decision 💭

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit magical and I was surprised to learn TypeScript doesn't throw an error in such cases.
Another downside of using the same names is that it's harder to understand what's being imported when you look at the import statements (it's probably a nitpick though)

I think I agree with Edgar that we should append Schema to the schema consts names.
I also suggest we keep the types in src/types - for example, in src/types/rules.ts you'd just import the schema and export the inferred type like this:

import { TestRuleSchema } from '@schemas/rules.ts'

export type TestRuleSchema = z.infer<typeof TestRuleSchema>

This way we'll end up keeping schemas and types in separate folders and will be importing types from src/types (among other benefits, schema files will only schema files only have schema definitions in them, which is minor, but I think it'll improve readability)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense, once you "compile" the code types are stripped out so if you are importing and using something it's the actual object.

I have a preference for that approach probably because I'm coming from python and it's less verbose but if the majority of the team thinks that separating schemas & types aids with readability, I think it's worth the cost of a few more lines 🙌

133 changes: 133 additions & 0 deletions src/schemas/rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { z } from 'zod'

export const VariableValue = z.object({
type: z.literal('variable'),
variableName: z.string(),
})

export const ArrayValue = z.object({
type: z.literal('array'),
arrayName: z.string(),
})

export const CustomCodeValue = z.object({
type: z.literal('customCode'),
getValue: z
.function()
.returns(z.union([z.string(), z.number(), z.null(), z.void()])),
})

export const RecordedValue = z.object({
type: z.literal('recordedValue'),
})

export const Filter = z.object({
path: z.string(),
})

export const BeginEndSelector = z.object({
type: z.literal('begin-end'),
from: z.enum(['headers', 'body', 'url']),
begin: z.string(),
end: z.string(),
})

export const RegexSelector = z.object({
type: z.literal('regex'),
from: z.enum(['headers', 'body', 'url']),
regex: z.string(),
})

export const JsonSelector = z.object({
type: z.literal('json'),
from: z.literal('body'),
path: z.string(),
})

export const CustomCodeSelector = z.object({
type: z.literal('custom-code'),
snippet: z.string(),
})

export const Selector = z.discriminatedUnion('type', [
BeginEndSelector,
RegexSelector,
JsonSelector,
])

export const CorrelationExtractor = z.object({
filter: Filter,
selector: Selector,
variableName: z.string().optional(),
})

export const CorrelationReplacer = z.object({
filter: Filter,
selector: Selector,
})

export const RuleBase = z.object({
id: z.string(),
})

export const ParameterizationRule = RuleBase.extend({
type: z.literal('parameterization'),
filter: Filter,
selector: Selector,
value: z.discriminatedUnion('type', [
VariableValue,
ArrayValue,
CustomCodeValue,
]),
})

export const CorrelationRule = RuleBase.extend({
type: z.literal('correlation'),
extractor: CorrelationExtractor,
replacer: CorrelationReplacer.optional(),
})

export const VerificationRule = RuleBase.extend({
type: z.literal('verification'),
filter: Filter,
selector: Selector,
value: z.discriminatedUnion('type', [
VariableValue,
ArrayValue,
CustomCodeValue,
RecordedValue,
]),
})

export const CustomCodeRule = RuleBase.extend({
type: z.literal('customCode'),
filter: Filter,
placement: z.enum(['before', 'after']),
snippet: z.string(),
})

export const TestRule = z.discriminatedUnion('type', [
ParameterizationRule,
CorrelationRule,
VerificationRule,
CustomCodeRule,
])

export type VariableValue = z.infer<typeof VariableValue>
export type ArrayValue = z.infer<typeof ArrayValue>
export type CustomCodeValue = z.infer<typeof CustomCodeValue>
export type RecordedValue = z.infer<typeof RecordedValue>
export type Filter = z.infer<typeof Filter>
export type BeginEndSelector = z.infer<typeof BeginEndSelector>
export type RegexSelector = z.infer<typeof RegexSelector>
export type JsonSelector = z.infer<typeof JsonSelector>
export type CustomCodeSelector = z.infer<typeof CustomCodeSelector>
export type Selector = z.infer<typeof Selector>
export type CorrelationExtractor = z.infer<typeof CorrelationExtractor>
export type CorrelationReplacer = z.infer<typeof CorrelationReplacer>
export type RuleBase = z.infer<typeof RuleBase>
export type ParameterizationRule = z.infer<typeof ParameterizationRule>
export type CorrelationRule = z.infer<typeof CorrelationRule>
export type VerificationRule = z.infer<typeof VerificationRule>
export type CustomCodeRule = z.infer<typeof CustomCodeRule>
export type TestRule = z.infer<typeof TestRule>
13 changes: 13 additions & 0 deletions src/schemas/testData.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { z } from 'zod'

export const Variable = z.object({
name: z.string(),
value: z.string(),
})

export const TestData = z.object({
variables: Variable.array(),
})

export type Variable = z.infer<typeof Variable>
export type TestData = z.infer<typeof TestData>
Loading