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

Support input url option for cli #203

Merged
merged 2 commits into from
Dec 11, 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
80 changes: 80 additions & 0 deletions frontend/packages/cli/src/cli/getInputContent.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import fs from 'node:fs'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { getInputContent } from './getInputContent.js'

vi.mock('node:fs')
vi.mock('node:https')

describe('getInputContent', () => {
afterEach(() => {
vi.restoreAllMocks()
})

it('should read local file content when given a valid file path', async () => {
const mockFilePath = '/path/to/local/file.txt'
const mockFileContent = 'Local file content'

vi.spyOn(fs, 'existsSync').mockReturnValue(true)
vi.spyOn(fs, 'readFileSync').mockReturnValue(mockFileContent)

const content = await getInputContent(mockFilePath)

expect(content).toBe(mockFileContent)
})

it('should throw an error if the local file path is invalid', async () => {
const mockFilePath = '/invalid/path/to/file.txt'

vi.spyOn(fs, 'existsSync').mockReturnValue(false)

await expect(getInputContent(mockFilePath)).rejects.toThrow(
'Invalid input path. Please provide a valid file.',
)
})

it('should download raw content from GitHub when given a GitHub blob URL', async () => {
const mockGitHubUrl = 'https://github.com/user/repo/blob/main/file.txt'
const mockRawUrl =
'https://raw.githubusercontent.com/user/repo/main/file.txt'
const mockGitHubContent = 'GitHub raw file content'

const mockFetch = vi
.spyOn(global, 'fetch')
.mockImplementation(
async () => new Response(mockGitHubContent, { status: 200 }),
)

const content = await getInputContent(mockGitHubUrl)

expect(content).toBe(mockGitHubContent)
expect(mockFetch).toHaveBeenCalledWith(mockRawUrl)
})

it('should download content from a regular URL', async () => {
const mockUrl = 'https://example.com/file.txt'
const mockUrlContent = 'Regular URL file content'

const mockFetch = vi
.spyOn(global, 'fetch')
.mockImplementation(
async () => new Response(mockUrlContent, { status: 200 }),
)

const content = await getInputContent(mockUrl)

expect(content).toBe(mockUrlContent)
expect(mockFetch).toHaveBeenCalledWith(mockUrl)
})

it('should throw an error when file download fails', async () => {
const mockUrl = 'https://example.com/file.txt'

vi.spyOn(global, 'fetch').mockImplementation(
async () => new Response('', { status: 404, statusText: 'Not Found' }),
)

await expect(getInputContent(mockUrl)).rejects.toThrow(
'Failed to download file: Not Found',
)
})
})
49 changes: 49 additions & 0 deletions frontend/packages/cli/src/cli/getInputContent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import fs from 'node:fs'
import { URL } from 'node:url'

function isValidUrl(url: string): boolean {
try {
new URL(url)
return true
} catch {
return false
}
}
Comment on lines +4 to +11
Copy link
Member

Choose a reason for hiding this comment

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

💭 I thought URL.canParse() could be used, but it looks like this was only supported in Node.js in v19.
We have not considered the minimum version of Node.js to be supported by the CLI, but since the LTS is v18, it would be better to support that.
So it looks good as it is 👍🏻

ref: https://developer.mozilla.org/en-US/docs/Web/API/URL/canParse_static


function isGitHubFileUrl(url: string): boolean {
const parsedUrl = new URL(url)
return parsedUrl.hostname === 'github.com' && url.includes('/blob/')
}

function readLocalFile(filePath: string): string {
if (!fs.existsSync(filePath)) {
throw new Error('Invalid input path. Please provide a valid file.')
}
return fs.readFileSync(filePath, 'utf8')
}

async function downloadGitHubRawContent(githubUrl: string): Promise<string> {
const rawFileUrl = githubUrl
.replace('github.com', 'raw.githubusercontent.com')
.replace('/blob', '')
return await downloadFile(rawFileUrl)
}

async function downloadFile(url: string): Promise<string> {
const response = await fetch(url)
if (!response.ok) {
throw new Error(`Failed to download file: ${response.statusText}`)
}
const data = await response.text()
return data
}

export async function getInputContent(inputPath: string): Promise<string> {
if (!isValidUrl(inputPath)) {
return readLocalFile(inputPath)
}

return isGitHubFileUrl(inputPath)
? await downloadGitHubRawContent(inputPath)
: await downloadFile(inputPath)
Comment on lines +46 to +48
Copy link
Member

Choose a reason for hiding this comment

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

Only GitHub gets special handling, I see.

}
Comment on lines +13 to +49
Copy link
Member

Choose a reason for hiding this comment

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

It seems that the CLI is handling quite a lot under the --input argument, which might be somewhat unexpected for users.

Perhaps generating the raw URL could be left as the user's responsibility. I’d like to hear others’ opinions on this as well.

Example:

@liam-hq/cli erd build --input https://raw.githubusercontent.com/mastodon/mastodon/refs/heads/main/db/schema.rb --format schemarb

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps generating the raw URL should be left to the user.

For private repositories, GitHub's raw URL appears to include a pre-signed token as a ?token= parameter.

Therefore, this URL conversion feature would only support public repositories. But it is ok, I think now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, I was wondering if i should convert url.
If it is not useful and understandable for user, let's change implementation👍

Yeah, only public repo is available for now.

Copy link
Member

Choose a reason for hiding this comment

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

IMO: I thought slightly extensive too, but it was a minor point of contention, so once merged, it looked good.

9 changes: 0 additions & 9 deletions frontend/packages/cli/src/cli/runPreprocess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,6 @@ describe('runPreprocess', () => {
},
)

it('should throw an error if the input file does not exist', async () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-distDir-'))
const nonExistentPath = path.join(tmpDir, 'non-existent.sql')

await expect(
runPreprocess(nonExistentPath, tmpDir, 'postgres'),
).rejects.toThrow('Invalid input path. Please provide a valid file.')
})

it('should throw an error if the format is invalid', async () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-distDir-'))
const inputPath = path.join(tmpDir, 'input.sql')
Expand Down
9 changes: 3 additions & 6 deletions frontend/packages/cli/src/cli/runPreprocess.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import fs, { readFileSync } from 'node:fs'
import fs from 'node:fs'
import path from 'node:path'
import {
type SupportedFormat,
parse,
supportedFormatSchema,
} from '@liam-hq/db-structure/parser'
import * as v from 'valibot'
import { getInputContent } from './getInputContent.js'

export async function runPreprocess(
inputPath: string,
outputDir: string,
format: SupportedFormat,
) {
if (!fs.existsSync(inputPath)) {
throw new Error('Invalid input path. Please provide a valid file.')
}

const input = readFileSync(inputPath, 'utf8')
const input = await getInputContent(inputPath)

if (!v.safeParse(supportedFormatSchema, format).success) {
throw new Error(
Expand Down
Loading