Skip to content

Commit

Permalink
refactor: use client to stream instead of fetch
Browse files Browse the repository at this point in the history
  • Loading branch information
ricokahler authored and binoy14 committed Aug 1, 2024
1 parent 9fd927c commit c45a63e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const mockFsPromisesReaddir = mockFsPromises.readdir as unknown as jest.Mock<
const mockClient = {
request: jest.fn(),
config: jest.fn(),
getUrl: jest.fn(),
} as unknown as SanityClient
const mockClientRequest = mockClient.request as jest.Mock<SanityClient['request']>

Expand Down Expand Up @@ -67,7 +66,7 @@ describe('getOrCreateUserApplication', () => {
})

const result = await getOrCreateUserApplication({client: mockClient, context})
expect(mockClient.request).toHaveBeenCalledWith({
expect(mockClientRequest).toHaveBeenCalledWith({
uri: '/user-applications',
query: {default: 'true'},
})
Expand All @@ -86,7 +85,7 @@ describe('getOrCreateUserApplication', () => {
context,
})

expect(mockClient.request).toHaveBeenCalledWith({
expect(mockClientRequest).toHaveBeenCalledWith({
uri: '/user-applications',
query: {appHost: 'example.sanity.studio'},
})
Expand All @@ -104,7 +103,7 @@ describe('getOrCreateUserApplication', () => {
context,
})

expect(mockClient.request).toHaveBeenCalledWith({
expect(mockClientRequest).toHaveBeenCalledWith({
uri: '/user-applications',
method: 'POST',
body: {appHost: 'newhost', isDefault: true},
Expand Down Expand Up @@ -142,19 +141,13 @@ describe('getOrCreateUserApplication', () => {
describe('createDeployment', () => {
beforeEach(() => {
jest.clearAllMocks()
;(mockClient.config as jest.Mock<any>).mockReturnValue({token: 'fake-token'})
;(mockClient.getUrl as jest.Mock<SanityClient['getUrl']>).mockImplementation(
(uri) => `http://example.api.sanity.io${uri}`,
)
})

it('sends the correct request to create a deployment and includes authorization header if token is present', async () => {
const tarball = Readable.from([Buffer.from('example chunk', 'utf-8')]) as Gzip
const applicationId = 'test-app-id'
const version = '1.0.0'

mockFetch.mockResolvedValueOnce(new Response())

await createDeployment({
client: mockClient,
applicationId,
Expand All @@ -163,23 +156,18 @@ describe('createDeployment', () => {
tarball,
})

// Check URL and method
expect(mockClient.getUrl).toHaveBeenCalledWith(
`/user-applications/${applicationId}/deployments`,
)
expect(mockFetch).toHaveBeenCalledTimes(1)
const url = mockFetch.mock.calls[0][0] as URL
expect(url.toString()).toBe(
'http://example.api.sanity.io/user-applications/test-app-id/deployments',
)
expect(mockClientRequest).toHaveBeenCalledTimes(1)

// Extract and validate form data
const mockFetchCalls = mockFetch.mock.calls as Parameters<typeof fetch>[]
const formData = mockFetchCalls[0][1]?.body as unknown as Readable
const mockRequestCalls = mockClientRequest.mock.calls as Parameters<typeof mockClientRequest>[]
const {uri, method, body} = mockRequestCalls[0][0]

expect(uri).toBe(`/user-applications/${applicationId}/deployments`)
expect(method).toBe('POST')

// dump the raw content of the form data into a string
let content = ''
for await (const chunk of formData) {
for await (const chunk of body) {
content += chunk
}

Expand All @@ -188,10 +176,6 @@ describe('createDeployment', () => {
expect(content).toContain('version')
expect(content).toContain(version)
expect(content).toContain('example chunk')

// Check Authorization header
const headers = mockFetchCalls[0][1]?.headers as Headers
expect(headers.get('Authorization')).toBe('Bearer fake-token')
})

it('streams the tarball contents', async () => {
Expand All @@ -207,9 +191,6 @@ describe('createDeployment', () => {

const mockTarball = Readable.from(createMockStream()) as Gzip
const applicationId = 'test-app-id'

mockFetch.mockResolvedValueOnce(new Response())

const version = '1.0.0'

await createDeployment({
Expand All @@ -220,29 +201,15 @@ describe('createDeployment', () => {
tarball: mockTarball,
})

// Check URL and method
expect(mockClient.getUrl).toHaveBeenCalledWith(
`/user-applications/${applicationId}/deployments`,
)
expect(mockFetch).toHaveBeenCalledTimes(1)
const url = mockFetch.mock.calls[0][0] as URL
expect(url.toString()).toBe(
'http://example.api.sanity.io/user-applications/test-app-id/deployments',
)

// Extract and validate form data
const mockFetchCalls = mockFetch.mock.calls as Parameters<typeof fetch>[]
const requestInit = mockFetchCalls[0][1] as any

// this is required to enable streaming with the native fetch API
// https://github.com/nodejs/node/issues/46221
expect(requestInit.duplex).toBe('half')
expect(mockClientRequest).toHaveBeenCalledTimes(1)

const formData = requestInit.body as Readable
const mockRequestCalls = mockClientRequest.mock.calls as Parameters<typeof mockClientRequest>[]
const {body} = mockRequestCalls[0][0]

// Extract and validate form data
let emissions = 0
let content = ''
for await (const chunk of formData) {
for await (const chunk of body) {
content += chunk
emissions++
}
Expand Down
21 changes: 3 additions & 18 deletions packages/sanity/src/_internal/cli/actions/deploy/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,30 +140,15 @@ export async function createDeployment({
isAutoUpdating,
version,
}: CreateDeploymentOptions): Promise<void> {
const config = client.config()

const formData = new FormData()
formData.append('isAutoUpdating', isAutoUpdating.toString())
formData.append('version', version)
formData.append('tarball', tarball, {contentType: 'application/gzip'})

const url = new URL(client.getUrl(`/user-applications/${applicationId}/deployments`))
const headers = new Headers({
...(config.token && {Authorization: `Bearer ${config.token}`}),
})
formData.append('tarball', tarball, {contentType: 'application/gzip', filename: 'app.tar.gz'})

await fetch(url, {
await client.request({
uri: `/user-applications/${applicationId}/deployments`,
method: 'POST',
headers,
// NOTE:
// - the fetch API in node.js supports streams but it's not in the types
// - the PassThrough is required because `form-data` does not fully conform
// to the node.js stream API
// eslint-disable-next-line @typescript-eslint/no-explicit-any
body: formData.pipe(new PassThrough()) as any,
// @ts-expect-error the `duplex` param is required in order to send a stream
// https://github.com/nodejs/node/issues/46221#issuecomment-1383246036
duplex: 'half',
})
}

Expand Down

0 comments on commit c45a63e

Please sign in to comment.