Skip to content

Commit

Permalink
refactor: api-server's watch build process (#11110)
Browse files Browse the repository at this point in the history
Follow-up to #11109.

While working on #11109, I started making changes that turned into a
larger refactor, that's why this separate PR:
- **Fix:** Centralized debounce logic with precedence of flags (e.g., if
a `rebuild: false` call happens while debounced, it takes precedence,
even if subsequent debounced calls were with `true`).
- Decoupled the build and HTTP server logic in `watch.ts`. The main
motivation was to test parts of the build process I modified. This also
improves readability with clearer responsibilities.
- Added tests for the debounce with precedence logic.
  • Loading branch information
callingmedic911 authored Aug 9, 2024
1 parent 3bde179 commit abdb184
Show file tree
Hide file tree
Showing 4 changed files with 291 additions and 141 deletions.
103 changes: 103 additions & 0 deletions packages/api-server/src/__tests__/buildManager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'

import { BuildManager } from '../buildManager'
import type { BuildAndRestartOptions } from '../buildManager'

const buildApi = vi.fn()
const cleanApiBuild = vi.fn()
const rebuildApi = vi.fn()

async function build(options: BuildAndRestartOptions) {
if (options.clean) {
await cleanApiBuild()
}

if (options.rebuild) {
await rebuildApi()
} else {
await buildApi()
}
}

describe('BuildManager', () => {
let buildManager: BuildManager

beforeEach(() => {
vi.clearAllMocks()
vi.useFakeTimers()
buildManager = new BuildManager(build)
})

afterEach(() => {
vi.runOnlyPendingTimers()
vi.useRealTimers()
})

it('should handle rebuild: false correctly', async () => {
buildManager.run({ rebuild: false })

await vi.runAllTimersAsync()

expect(rebuildApi).not.toHaveBeenCalled()
expect(buildApi).toHaveBeenCalled()
})

it('should handle clean: true correctly', async () => {
buildManager.run({ rebuild: true, clean: true })

await vi.runAllTimersAsync()

expect(cleanApiBuild).toHaveBeenCalled()
expect(rebuildApi).toHaveBeenCalled()
})

it('should prioritize rebuild:false', async () => {
buildManager.run({ rebuild: true, clean: true })
buildManager.run({ rebuild: false, clean: false })

await vi.runAllTimersAsync()

expect(cleanApiBuild).toHaveBeenCalled()
expect(rebuildApi).not.toHaveBeenCalled()
expect(buildApi).toHaveBeenCalled()
})

it('should prioritize clean: true', async () => {
buildManager.run({ rebuild: true, clean: true })
buildManager.run({ rebuild: false, clean: false })

await vi.runAllTimersAsync()

expect(cleanApiBuild).toHaveBeenCalled()
expect(rebuildApi).not.toHaveBeenCalled()
expect(buildApi).toHaveBeenCalled()
})

it('should reset flags after execution', async () => {
buildManager.run({ rebuild: true, clean: true })

await vi.runAllTimersAsync()

expect(buildApi).not.toHaveBeenCalled()
expect(rebuildApi).toHaveBeenCalled()
expect(cleanApiBuild).toHaveBeenCalled()

buildManager.run({ rebuild: false, clean: false })

await vi.runAllTimersAsync()

expect(buildApi).toHaveBeenCalled()
})

it('should debounce multiple calls', async () => {
buildManager.run({ rebuild: true, clean: true })
buildManager.run({ rebuild: true, clean: false })
buildManager.run({ rebuild: true, clean: true })

await vi.runAllTimersAsync()

expect(buildApi).not.toHaveBeenCalledOnce()
expect(rebuildApi).toHaveBeenCalledOnce()
expect(cleanApiBuild).toHaveBeenCalledOnce()
})
})
62 changes: 62 additions & 0 deletions packages/api-server/src/buildManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { debounce } from 'lodash'

export type BuildAndRestartOptions = {
rebuild?: boolean
clean?: boolean
}

// We want to delay execution when multiple files are modified on the filesystem,
// this usually happens when running RedwoodJS generator commands.
// Local writes are very fast, but writes in e2e environments are not,
// so allow the default to be adjusted with an env-var.
//
class BuildManager {
private shouldRebuild: boolean
private shouldClean: boolean
private debouncedBuild: ReturnType<typeof debounce>
private buildFn: (options: BuildAndRestartOptions) => Promise<void>

constructor(buildFn: (options: BuildAndRestartOptions) => Promise<void>) {
this.shouldRebuild = true
this.shouldClean = false
this.buildFn = buildFn
this.debouncedBuild = debounce(
async (options: BuildAndRestartOptions) => {
// Use flags with higher precedence to determine if we should rebuild or clean
try {
await this.buildFn({
...options,
rebuild: this.shouldRebuild,
clean: this.shouldClean,
})
} finally {
this.shouldRebuild = true
this.shouldClean = false
}
},
process.env.RWJS_DELAY_RESTART
? parseInt(process.env.RWJS_DELAY_RESTART, 10)
: 500,
)
}

// Wrapper method to handle options and set precedence flags.
// If we ever see a `rebuild: false` option while debouncing, we never want to rebuild.
// If we ever see a `clean: true` option, we always want to clean.
async run(options: BuildAndRestartOptions) {
if (options.rebuild === false) {
this.shouldRebuild = false
}
if (options.clean) {
this.shouldClean = true
}

await this.debouncedBuild(options)
}

cancelScheduledBuild() {
this.debouncedBuild.cancel()
}
}

export { BuildManager }
95 changes: 95 additions & 0 deletions packages/api-server/src/serverManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import type { ChildProcess } from 'child_process'
import { fork } from 'child_process'
import fs from 'fs'
import path from 'path'

import yargs from 'yargs'
import { hideBin } from 'yargs/helpers'

import { getConfig, getPaths, resolveFile } from '@redwoodjs/project-config'

const argv = yargs(hideBin(process.argv))
.option('debugPort', {
description: 'Port on which to expose API server debugger',
type: 'number',
alias: ['debug-port', 'dp'],
})
.option('port', {
description: 'The port to listen at',
type: 'number',
alias: 'p',
})
.parseSync()

const rwjsPaths = getPaths()

export class ServerManager {
private httpServerProcess: ChildProcess | null = null

private async startApiServer() {
const forkOpts = {
execArgv: process.execArgv,
}

// OpenTelemetry SDK Setup
if (getConfig().experimental.opentelemetry.enabled) {
// We expect the OpenTelemetry SDK setup file to be in a specific location
const opentelemetrySDKScriptPath = path.join(
rwjsPaths.api.dist,
'opentelemetry.js',
)
const opentelemetrySDKScriptPathRelative = path.relative(
rwjsPaths.base,
opentelemetrySDKScriptPath,
)
console.log(
`Setting up OpenTelemetry using the setup file: ${opentelemetrySDKScriptPathRelative}`,
)
if (fs.existsSync(opentelemetrySDKScriptPath)) {
forkOpts.execArgv = forkOpts.execArgv.concat([
`--require=${opentelemetrySDKScriptPath}`,
])
} else {
console.error(
`OpenTelemetry setup file does not exist at ${opentelemetrySDKScriptPathRelative}`,
)
}
}

const debugPort = argv['debug-port']
if (debugPort) {
forkOpts.execArgv = forkOpts.execArgv.concat([`--inspect=${debugPort}`])
}

const port = argv.port ?? getConfig().api.port

// Start API server

const serverFile = resolveFile(`${rwjsPaths.api.dist}/server`)
if (serverFile) {
this.httpServerProcess = fork(
serverFile,
['--apiPort', port.toString()],
forkOpts,
)
} else {
this.httpServerProcess = fork(
path.join(__dirname, 'bin.js'),
['api', '--port', port.toString()],
forkOpts,
)
}
}

async restartApiServer() {
this.killApiServer()
await this.startApiServer()
}

killApiServer() {
this.httpServerProcess?.emit('exit')
this.httpServerProcess?.kill()
}
}

export const serverManager = new ServerManager()
Loading

0 comments on commit abdb184

Please sign in to comment.