Skip to content

Commit 8982a6f

Browse files
mykalmaxmykalmax
authored
Fix: Handle windows path for models and files (#1707)
1 parent bc1d6bb commit 8982a6f

File tree

10 files changed

+191
-15
lines changed

10 files changed

+191
-15
lines changed

‎web/client/src/context/context.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export const useStoreContext = create<ContextStore>((set, get) => ({
9898
models: models.reduce((acc: Map<string, ModelSQLMeshModel>, model) => {
9999
let tempModel = s.models.get(model.path) ?? s.models.get(model.name)
100100

101-
if (tempModel == null) {
101+
if (isNil(tempModel)) {
102102
tempModel = new ModelSQLMeshModel(model)
103103
} else {
104104
tempModel.update(model)

‎web/client/src/library/components/documentation/Documentation.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import { Disclosure, Tab } from '@headlessui/react'
33
import { MinusCircleIcon, PlusCircleIcon } from '@heroicons/react/24/solid'
44
import { EnumFileExtensions } from '@models/file'
55
import {
6+
PATH_SEPARATOR,
67
isArrayNotEmpty,
78
isFalse,
8-
isNil,
99
isString,
1010
isTrue,
1111
toDateFormat,
@@ -52,7 +52,10 @@ const Documentation = function Documentation({
5252
<ul className="px-2 w-full">
5353
<DetailsItem
5454
name="Path"
55-
value={model.path.split('/').slice(0, -1).join('/')}
55+
value={model.path
56+
.split(PATH_SEPARATOR)
57+
.slice(0, -1)
58+
.join(PATH_SEPARATOR)}
5659
/>
5760
<DetailsItem
5861
name="Name"

‎web/client/src/models/artifact.posix.spec.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { describe, test, expect, vi } from 'vitest'
2+
import { ModelArtifact } from './artifact'
3+
4+
vi.mock('../utils/index', async () => {
5+
const actual: any = await vi.importActual('../utils/index')
6+
7+
return {
8+
...actual,
9+
PATH_SEPARATOR: '/',
10+
}
11+
})
12+
13+
describe('Model Artifact', () => {
14+
test('should create artifact with path', () => {
15+
const artifact1 = new ModelArtifact({
16+
name: 'test',
17+
path: '/Users/jsmith/Projects/sqlmesh/examples/sushi/',
18+
})
19+
20+
expect(artifact1.path).toBeTruthy()
21+
expect(artifact1.path).toBe('Users/jsmith/Projects/sqlmesh/examples/sushi')
22+
23+
const artifact2 = new ModelArtifact({
24+
name: 'test',
25+
path: 'Users/jsmith/Projects/sqlmesh/examples/sushi',
26+
})
27+
28+
expect(artifact2.path).toBeTruthy()
29+
expect(artifact2.path).toBe('Users/jsmith/Projects/sqlmesh/examples/sushi')
30+
})
31+
})

‎web/client/src/models/artifact.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isStringEmptyOrNil, toUniqueName } from '@utils/index'
1+
import { PATH_SEPARATOR, isStringEmptyOrNil, toUniqueName } from '@utils/index'
22
import { type ModelDirectory } from './directory'
33
import { ModelInitial } from './initial'
44

@@ -75,11 +75,15 @@ export class ModelArtifact<
7575

7676
private toPath(name: string, fallback: string = ''): string {
7777
return ModelArtifact.toPath(
78-
this.withParent ? `${this.parent?.path ?? ''}/${name}` : fallback,
78+
this.withParent
79+
? `${this.parent?.path ?? ''}${PATH_SEPARATOR}${name}`
80+
: fallback,
7981
)
8082
}
8183

8284
static toPath(...paths: string[]): string {
83-
return paths.flatMap(path => path.split('/').filter(Boolean)).join('/')
85+
return paths
86+
.flatMap(path => path.split(PATH_SEPARATOR).filter(Boolean))
87+
.join(PATH_SEPARATOR)
8488
}
8589
}

‎web/client/src/models/artifact.win.spec.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { describe, test, expect, vi } from 'vitest'
2+
import { ModelArtifact } from './artifact'
3+
4+
vi.mock('../utils/index', async () => {
5+
const actual: any = await vi.importActual('../utils/index')
6+
7+
return {
8+
...actual,
9+
PATH_SEPARATOR: '\\',
10+
}
11+
})
12+
13+
describe('Model Artifact', () => {
14+
test('should create artifact with path', () => {
15+
const artifact1 = new ModelArtifact({
16+
name: 'test',
17+
path: '\\Users\\jsmith\\Projects\\sqlmesh\\examples\\sushi\\',
18+
})
19+
20+
expect(artifact1.path).toBeTruthy()
21+
expect(artifact1.path).toBe(
22+
'Users\\jsmith\\Projects\\sqlmesh\\examples\\sushi',
23+
)
24+
25+
const artifact2 = new ModelArtifact({
26+
name: 'test',
27+
path: 'Users\\jsmith\\Projects\\sqlmesh\\examples\\sushi',
28+
})
29+
30+
expect(artifact2.path).toBeTruthy()
31+
expect(artifact2.path).toBe(
32+
'Users\\jsmith\\Projects\\sqlmesh\\examples\\sushi',
33+
)
34+
})
35+
})

web/client/src/models/directory.spec.ts renamed to web/client/src/models/directory.posix.spec.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
import { describe, test, expect } from 'vitest'
1+
import { describe, test, expect, vi } from 'vitest'
22
import { ModelDirectory } from './directory'
33

4+
vi.mock('../utils/index', async () => {
5+
const actual: any = await vi.importActual('../utils/index')
6+
7+
return {
8+
...actual,
9+
PATH_SEPARATOR: '/',
10+
}
11+
})
12+
413
describe('Model Directory', () => {
514
test('should create directory with nested atrifacts', () => {
615
const directory = new ModelDirectory(getFakePayloadDirectoryWithChildren())

‎web/client/src/models/directory.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { isFalse, isNotNil, isStringEmptyOrNil } from '@utils/index'
1+
import {
2+
PATH_SEPARATOR,
3+
isFalse,
4+
isNotNil,
5+
isStringEmptyOrNil,
6+
} from '@utils/index'
27
import type { Directory, File } from '../api/client'
38
import { type InitialArtifact, ModelArtifact } from './artifact'
49
import { ModelFile } from './file'
@@ -208,14 +213,14 @@ export class ModelDirectory extends ModelArtifact<InitialDirectory> {
208213
path: string,
209214
): ModelDirectory {
210215
const directories = directory.allDirectories
211-
const chain = path.split('/').reduce((acc: string[], path) => {
216+
const chain = path.split(PATH_SEPARATOR).reduce((acc: string[], path) => {
212217
if (acc.length === 0) return [path]
213218

214219
if (isStringEmptyOrNil(path)) return acc
215220

216221
const last = acc[acc.length - 1] ?? ''
217222

218-
acc.push(`${last}/${path}`)
223+
acc.push(`${last}${PATH_SEPARATOR}${path}`)
219224

220225
return acc
221226
}, [])

‎web/client/src/models/directory.win.spec.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, test, expect, vi } from 'vitest'
2+
import { ModelDirectory } from './directory'
3+
4+
vi.mock('../utils/index', async () => {
5+
const actual: any = await vi.importActual('../utils/index')
6+
7+
return {
8+
...actual,
9+
PATH_SEPARATOR: '\\',
10+
}
11+
})
12+
13+
describe('Model Directory', () => {
14+
test('should create directory with nested atrifacts', () => {
15+
const directory = new ModelDirectory(getFakePayloadDirectoryWithChildren())
16+
17+
expect(directory.id).toBeTruthy()
18+
expect(directory.level).toBe(0)
19+
expect(directory.name).toBe('project')
20+
expect(directory.path).toBe('')
21+
expect(directory.directories.length).toBe(1)
22+
expect(directory.allDirectories.length).toBe(4)
23+
expect(directory.allArtifacts.length).toBe(4)
24+
25+
const folder1 = ModelDirectory.findArtifactByPath(
26+
directory,
27+
'test\\folder_1',
28+
) as ModelDirectory
29+
30+
expect(folder1).toBeTruthy()
31+
expect(folder1?.level).toBe(2)
32+
33+
const folder3 = ModelDirectory.findArtifactByPath(
34+
directory,
35+
'test\\folder_1\\folder_2\\folder_3',
36+
) as ModelDirectory
37+
38+
expect(folder3).toBeTruthy()
39+
expect(folder3?.level).toBe(4)
40+
})
41+
42+
test('should find parent by path', () => {
43+
const directory = new ModelDirectory(getFakePayloadDirectoryWithChildren())
44+
45+
let found = ModelDirectory.findParentByPath(directory, '')
46+
47+
expect(found).toBe(directory)
48+
expect(found?.path).toBe('')
49+
expect(found?.level).toBe(0)
50+
51+
found = ModelDirectory.findParentByPath(directory, 'test\\folder_1')
52+
53+
expect(found).toBe(directory.directories[0]?.directories[0])
54+
expect(found?.path).toBe('test\\folder_1')
55+
expect(found?.level).toBe(2)
56+
})
57+
})
58+
59+
function getFakePayloadDirectoryWithChildren(): any {
60+
return {
61+
path: '',
62+
name: 'project',
63+
directories: [
64+
{
65+
name: 'test',
66+
path: 'test',
67+
directories: [
68+
{
69+
name: 'folder_1',
70+
path: 'test\\folder_1',
71+
directories: [
72+
{
73+
name: 'folder_2',
74+
path: 'test\\folder_1\\folder_2',
75+
directories: [
76+
{
77+
name: 'folder_3',
78+
path: 'test\\folder_1\\folder_2\\folder_3',
79+
},
80+
],
81+
},
82+
],
83+
},
84+
],
85+
},
86+
],
87+
}
88+
}

‎web/client/src/utils/index.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
export const PATH_SEPARATOR = navigator.userAgent.includes('Win') ? '\\' : '/'
2+
13
export function isTrue(value: unknown): boolean {
24
return value === true
35
}
@@ -50,7 +52,7 @@ export function isObjectNotEmpty(value: unknown): boolean {
5052

5153
export function isObject(value: unknown): boolean {
5254
return (
53-
typeof value === 'object' && value !== null && value.constructor === Object
55+
typeof value === 'object' && isNotNil(value) && value.constructor === Object
5456
)
5557
}
5658

@@ -141,7 +143,7 @@ export function debounceSync(
141143
let timeoutID: ReturnType<typeof setTimeout> | undefined
142144

143145
return function callback(...args: any) {
144-
const callNow = immediate && timeoutID == null
146+
const callNow = immediate && isNil(timeoutID)
145147

146148
clearTimeout(timeoutID)
147149

@@ -170,7 +172,7 @@ export function toUniqueName(prefix?: string, suffix?: string): string {
170172
// Should be enough for now
171173
const hex = (Date.now() % 100000).toString(16)
172174

173-
return `${prefix == null ? '' : `${prefix}_`}${hex}${
175+
return `${isNil(prefix) ? '' : `${prefix}_`}${hex}${
174176
suffix ?? ''
175177
}`.toLowerCase()
176178
}

‎web/server/api/endpoints/files.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import json
44
import os
5-
import pathlib
65
import typing as t
76
from pathlib import Path
87

@@ -74,7 +73,7 @@ async def write_file(
7473
replace_file(settings.project_path / path, settings.project_path / path_or_new_path)
7574
else:
7675
full_path = settings.project_path / path
77-
if content and pathlib.Path(path_or_new_path).suffix == ".sql":
76+
if content and Path(path_or_new_path).suffix == ".sql":
7877
path_to_model_mapping = await get_path_to_model_mapping(settings=settings)
7978
model = path_to_model_mapping.get(Path(full_path))
8079
default_dialect = context.config_for_path(Path(path_or_new_path)).dialect

0 commit comments

Comments
 (0)