Skip to content

Commit 0e01836

Browse files
committed
sv
1 parent ff80ff1 commit 0e01836

File tree

12 files changed

+128
-44
lines changed

12 files changed

+128
-44
lines changed

spx-gui/AGENTS.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,28 @@ Keep import statements in order:
3535
- Enum names and enum members
3636
- Vue component names
3737

38+
### Identifier Resolution
39+
40+
When working with backend unique string identifiers such as `username`, project owner, and project name, distinguish unresolved identifiers from canonical identifiers.
41+
42+
* Treat values from route params, query params, and manual user input as unresolved identifiers.
43+
44+
* Treat backend-issued values as canonical identifiers, including:
45+
- HTTP API responses
46+
- JWT token fields such as `username`
47+
48+
* Prefer naming unresolved values explicitly, such as `ownerInput`, `projectNameInput`, or similar names that make the unresolved nature obvious.
49+
50+
* In models and resolved state, names like `owner`, `name`, and `username` should refer to canonical values.
51+
52+
* Keep normal strict and case-sensitive equality (`===` / `!==`) for consuming identifiers. Do not spread ad hoc case-normalization logic across comparison sites.
53+
54+
* Resolve unresolved identifiers at clear boundaries before consuming them. Typical resolution boundaries include project loading, user loading, and other backend-backed fetches.
55+
56+
* Avoid storing unresolved identifiers on long-lived models as if they were already canonical. Prefer passing unresolved identifiers as load or resolve parameters, then writing canonical values onto the model after resolution.
57+
58+
* Downstream logic should consume canonical values only. This includes ownership checks, permission checks, cache keys, project reuse checks, local-cache decisions, and similar logic.
59+
3860
## TypeScript Testing
3961

4062
* Use `describe` to group related tests.

spx-gui/src/components/common/ListResultWrapper.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { UILoading, UIError, UIEmpty } from '@/components/ui'
1313
import type { ByPage } from '@/apis/common'
1414
1515
const props = defineProps<{
16-
queryRet: QueryRet<T>
16+
queryRet: QueryRet<T | null>
1717
height?: number
1818
/** Type of the list content */
1919
contentType?: 'project'

spx-gui/src/components/community/ProjectsSection.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ type Context = 'home' | 'user' | 'project'
4646
4747
defineProps<{
4848
linkTo?: string | null
49-
queryRet: QueryRet<unknown[]>
49+
queryRet: QueryRet<unknown[] | null>
5050
context: Context
5151
numInRow: number
5252
}>()

spx-gui/src/components/community/project/ReleaseHistory.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { UITimeline, UITimelineItem, UILoading, UIError } from '@/components/ui'
66
import TextView from '../TextView.vue'
77
88
defineProps<{
9-
queryRet: QueryRet<ProjectRelease[]>
9+
queryRet: QueryRet<ProjectRelease[] | null>
1010
}>()
1111
</script>
1212

spx-gui/src/components/copilot/CopilotRoot.vue

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,25 @@ class Retriever {
5858
private cloudHelpers: CloudHelpers
5959
) {}
6060
61+
private matchesIdentifierInput(canonical: string | null | undefined, input: string | null | undefined) {
62+
return canonical != null && input != null && canonical.toLowerCase() === input.toLowerCase()
63+
}
64+
6165
async getProject(project: string | undefined, signal?: AbortSignal): Promise<SpxProject> {
6266
const currentProject = this.editorCtxRef.value?.project
6367
if (project == null) {
6468
if (currentProject == null) throw new Error('No project specified and no current editing project available')
6569
return currentProject
6670
}
6771
const { owner, name } = parseProjectIdentifier(project)
68-
if (currentProject != null && currentProject.owner === owner && currentProject.name === name) {
72+
if (
73+
currentProject != null &&
74+
this.matchesIdentifierInput(currentProject.owner, owner) &&
75+
this.matchesIdentifierInput(currentProject.name, name)
76+
) {
6977
return currentProject
7078
}
71-
const p = new SpxProject(owner, name)
79+
const p = new SpxProject()
7280
const serialized = await this.cloudHelpers.load(owner, name, true, signal)
7381
await p.load(serialized)
7482
return p

spx-gui/src/components/editor/editing.test.ts

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { type Files } from '@/models/common/file'
88
import type { CloudHelpers } from '@/models/common/cloud'
99
import { mockFile, MockProject } from '@/models/common/test'
1010
import type { IProject, ProjectSerialized } from '@/models/project'
11-
import { Editing, SavingState, EditingMode, type ILocalCache, type UIHelpersForLoadingProject } from './editing'
11+
import { Editing, SavingState, EditingMode, type ILocalCache, type LoadProjectParams, type UIHelpersForLoadingProject } from './editing'
1212

1313
function makeFiles() {
1414
return {
@@ -284,6 +284,10 @@ describe('Editing.loadProject', () => {
284284
}
285285
}
286286

287+
function makeLoadProjectParams(ownerInput = 'alice', projectNameInput = 'my-project'): LoadProjectParams {
288+
return { ownerInput, projectNameInput }
289+
}
290+
287291
function makeSerialized(owner: string, name: string, version?: number): ProjectSerialized {
288292
return {
289293
metadata: { owner, name, version },
@@ -301,7 +305,7 @@ describe('Editing.loadProject', () => {
301305
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
302306

303307
const ac = new AbortController()
304-
await editing.loadProject(makeUIHelpers(), makeReporter(), ac.signal)
308+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), ac.signal)
305309

306310
expect(cloudHelper.load).toHaveBeenCalledWith('alice', 'my-project', false, ac.signal)
307311
expect(project.load).toHaveBeenCalledWith(cloudData, ac.signal)
@@ -318,7 +322,7 @@ describe('Editing.loadProject', () => {
318322
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
319323

320324
const ac = new AbortController()
321-
await editing.loadProject(makeUIHelpers(), makeReporter(), ac.signal)
325+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), ac.signal)
322326

323327
expect(cloudHelper.load).toHaveBeenCalledWith('alice', 'my-project', false, ac.signal)
324328
expect(project.load).toHaveBeenCalledWith(cloudData, ac.signal)
@@ -335,7 +339,7 @@ describe('Editing.loadProject', () => {
335339
const project = makeProject({ owner: 'alice', name: 'my-project' })
336340
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
337341

338-
await editing.loadProject(makeUIHelpers(), makeReporter(), new AbortController().signal)
342+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), new AbortController().signal)
339343

340344
expect(project.load).toHaveBeenCalledWith(localData, expect.anything())
341345
})
@@ -350,7 +354,7 @@ describe('Editing.loadProject', () => {
350354
const project = makeProject({ owner: 'alice', name: 'my-project' })
351355
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
352356

353-
await editing.loadProject(makeUIHelpers(), makeReporter(), new AbortController().signal)
357+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), new AbortController().signal)
354358

355359
expect(project.load).toHaveBeenCalledWith(cloudData, expect.anything())
356360
expect(localCacheHelper.clear).toHaveBeenCalled()
@@ -366,7 +370,7 @@ describe('Editing.loadProject', () => {
366370
const project = makeProject({ owner: 'alice', name: 'my-project' })
367371
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
368372

369-
await editing.loadProject(makeUIHelpers(), makeReporter(), new AbortController().signal)
373+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), new AbortController().signal)
370374

371375
expect(localCacheHelper.clear).toHaveBeenCalled()
372376
expect(project.load).toHaveBeenCalledWith(cloudData, expect.anything())
@@ -385,7 +389,7 @@ describe('Editing.loadProject', () => {
385389
const helpers = makeUIHelpers()
386390
vi.mocked(helpers.confirmOpenTargetWithAnotherInCache).mockResolvedValue(true)
387391

388-
await editing.loadProject(helpers, makeReporter(), new AbortController().signal)
392+
await editing.loadProject(makeLoadProjectParams(), helpers, makeReporter(), new AbortController().signal)
389393

390394
expect(helpers.confirmOpenTargetWithAnotherInCache).toHaveBeenCalledWith('my-project', 'other-project')
391395
expect(localCacheHelper.clear).toHaveBeenCalled()
@@ -404,7 +408,7 @@ describe('Editing.loadProject', () => {
404408

405409
const helpers = makeUIHelpers()
406410

407-
await editing.loadProject(helpers, makeReporter(), new AbortController().signal)
411+
await editing.loadProject(makeLoadProjectParams(), helpers, makeReporter(), new AbortController().signal)
408412

409413
expect(helpers.confirmOpenTargetWithAnotherInCache).not.toHaveBeenCalled()
410414
expect(localCacheHelper.clear).not.toHaveBeenCalled()
@@ -424,19 +428,23 @@ describe('Editing.loadProject', () => {
424428
const helpers = makeUIHelpers()
425429
vi.mocked(helpers.confirmOpenTargetWithAnotherInCache).mockResolvedValue(false)
426430

427-
await expect(editing.loadProject(helpers, makeReporter(), new AbortController().signal)).rejects.toThrow(Cancelled)
431+
await expect(editing.loadProject(makeLoadProjectParams(), helpers, makeReporter(), new AbortController().signal)).rejects.toThrow(Cancelled)
428432
expect(localCacheHelper.clear).not.toHaveBeenCalled()
429433
expect(helpers.openProject).toHaveBeenCalledWith('alice', 'cached-project')
430434
expect(project.load).not.toHaveBeenCalled()
431435
})
432436

433-
it('should throw when project has no owner or name', async () => {
437+
it('should load project when the model starts without owner or name', async () => {
438+
const cloudData = makeSerialized('alice', 'my-project', 1)
439+
const cloudHelper = makeCloudHelper()
440+
vi.mocked(cloudHelper.load).mockResolvedValue(cloudData)
434441
const project = new MockProject(undefined, undefined, makeFiles())
435-
const editing = makeEditing({ project })
442+
const editing = makeEditing({ project, cloudHelper, signedInUsername: 'alice' })
436443

437-
await expect(editing.loadProject(makeUIHelpers(), makeReporter(), new AbortController().signal)).rejects.toThrow(
438-
'Project owner and name expected'
439-
)
444+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), new AbortController().signal)
445+
446+
expect(project.load).toHaveBeenCalledWith(cloudData, expect.anything())
447+
expect(editing.mode).toBe(EditingMode.AutoSave)
440448
})
441449

442450
it('should set preferPublishedContent to false when signed-in user is the owner', async () => {
@@ -449,7 +457,7 @@ describe('Editing.loadProject', () => {
449457
const editing = new Editing(project, cloudHelper, localCacheHelper, () => true, 'alice')
450458

451459
const ac = new AbortController()
452-
await editing.loadProject(makeUIHelpers(), makeReporter(), ac.signal)
460+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), ac.signal)
453461

454462
expect(cloudHelper.load).toHaveBeenCalledWith('alice', 'my-project', false, ac.signal)
455463
})
@@ -464,11 +472,30 @@ describe('Editing.loadProject', () => {
464472
const editing = new Editing(project, cloudHelper, localCacheHelper, () => true, 'bob')
465473

466474
const ac = new AbortController()
467-
await editing.loadProject(makeUIHelpers(), makeReporter(), ac.signal)
475+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), ac.signal)
468476

469477
expect(cloudHelper.load).toHaveBeenCalledWith('alice', 'my-project', true, ac.signal)
470478
})
471479

480+
it('should treat owner input with different casing as the same owner', async () => {
481+
const cloudData = makeSerialized('alice', 'my-project', 1)
482+
const localData = makeSerialized('alice', 'my-project', 2)
483+
const cloudHelper = makeCloudHelper()
484+
vi.mocked(cloudHelper.load).mockResolvedValue(cloudData)
485+
const localCacheHelper = makeLocalCache(localData)
486+
487+
const project = new MockProject(undefined, undefined, makeFiles())
488+
const editing = makeEditing({ project, cloudHelper, localCacheHelper, signedInUsername: 'alice' })
489+
490+
const ac = new AbortController()
491+
await editing.loadProject(makeLoadProjectParams('Alice', 'my-project'), makeUIHelpers(), makeReporter(), ac.signal)
492+
493+
expect(cloudHelper.load).toHaveBeenCalledWith('Alice', 'my-project', false, ac.signal)
494+
expect(localCacheHelper.clear).not.toHaveBeenCalled()
495+
expect(project.load).toHaveBeenCalledWith(localData, expect.anything())
496+
expect(editing.mode).toBe(EditingMode.AutoSave)
497+
})
498+
472499
it('should prefer local cache when cloud version is undefined and local version is undefined', async () => {
473500
const cloudData = makeSerialized('alice', 'my-project')
474501
const localData = makeSerialized('alice', 'my-project')
@@ -479,7 +506,7 @@ describe('Editing.loadProject', () => {
479506
const project = makeProject({ owner: 'alice', name: 'my-project' })
480507
const editing = makeEditing({ project, cloudHelper, localCacheHelper })
481508

482-
await editing.loadProject(makeUIHelpers(), makeReporter(), new AbortController().signal)
509+
await editing.loadProject(makeLoadProjectParams(), makeUIHelpers(), makeReporter(), new AbortController().signal)
483510

484511
// Both versions are undefined (-1 == -1), so cloudVersion > localVersion is false, local data wins
485512
expect(project.load).toHaveBeenCalledWith(localData, expect.anything())

spx-gui/src/components/editor/editing.ts

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,15 @@ export type UIHelpersForLoadingProject = {
9595
openProject: (owner: string, name: string) => void
9696
}
9797

98+
export type LoadProjectParams = {
99+
ownerInput: string
100+
projectNameInput: string
101+
}
102+
103+
function matchesIdentifierInput(canonical: string | null | undefined, input: string | null | undefined) {
104+
return canonical != null && input != null && canonical.toLowerCase() === input.toLowerCase()
105+
}
106+
98107
/**
99108
* `Editing` manages the lifecycle of editing a project, including:
100109
* - Determining the editing mode (AutoSave vs EffectFree) based on user ownership
@@ -110,7 +119,7 @@ export type UIHelpersForLoadingProject = {
110119
* renaming it with a more accurate name.
111120
*/
112121
export class Editing extends Disposable {
113-
mode: EditingMode
122+
mode = EditingMode.AutoSave
114123
constructor(
115124
private project: IProject,
116125
private cloudHelpers: CloudHelpers,
@@ -119,8 +128,13 @@ export class Editing extends Disposable {
119128
private signedInUsername: string | null
120129
) {
121130
super()
131+
this.updateMode()
132+
}
133+
134+
private updateMode() {
135+
const owner = this.project.owner
122136
this.mode = EditingMode.AutoSave
123-
if (signedInUsername == null || signedInUsername !== this.project.owner) {
137+
if (this.signedInUsername == null || owner == null || this.signedInUsername !== owner) {
124138
this.mode = EditingMode.EffectFree
125139
}
126140
}
@@ -188,7 +202,12 @@ export class Editing extends Disposable {
188202
)
189203
}
190204

191-
async loadProject(helpers: UIHelpersForLoadingProject, reporter: ProgressReporter, signal: AbortSignal) {
205+
async loadProject(
206+
{ ownerInput, projectNameInput }: LoadProjectParams,
207+
helpers: UIHelpersForLoadingProject,
208+
reporter: ProgressReporter,
209+
signal: AbortSignal
210+
) {
192211
const collector = ProgressCollector.collectorFor(reporter)
193212
const loadFromLocalCacheReporter = collector.getSubReporter(
194213
{ en: 'Reading local cache...', zh: '读取本地缓存中...' },
@@ -200,9 +219,6 @@ export class Editing extends Disposable {
200219
)
201220
const projectLoadReporter = collector.getSubReporter({ en: 'Loading project...', zh: '加载项目中...' }, 5)
202221

203-
const { owner: ownerName, name: projectName } = this.project
204-
if (ownerName == null || projectName == null) throw new Error('Project owner and name expected')
205-
206222
// For details about local-cache saving & restoring
207223
// https://github.com/goplus/builder/issues/259
208224
// https://github.com/goplus/builder/issues/393
@@ -217,22 +233,23 @@ export class Editing extends Disposable {
217233
if (localData != null) {
218234
const localMetadata = localData.metadata
219235
// If owner mismatch, ignore & clear the cached data
220-
if (localMetadata.owner !== ownerName) {
236+
if (!matchesIdentifierInput(localMetadata.owner, ownerInput)) {
221237
localData = null
222238
this.localCacheHelper.clear().catch((e) => capture(e, 'Failed to clear local cache'))
223239
}
224240
if (
225-
localMetadata.owner === ownerName &&
241+
matchesIdentifierInput(localMetadata.owner, ownerInput) &&
226242
localMetadata.name != null &&
227-
localMetadata.name.toLowerCase() !== projectName.toLowerCase()
243+
!matchesIdentifierInput(localMetadata.name, projectNameInput)
228244
) {
229245
// If project name mismatch, ask user whether to open the cached project or not, to avoid potential data loss by clearing cache
230-
const stillOpenTarget = await helpers.confirmOpenTargetWithAnotherInCache(projectName, localMetadata.name)
246+
const stillOpenTarget = await helpers.confirmOpenTargetWithAnotherInCache(projectNameInput, localMetadata.name)
231247
signal.throwIfAborted()
232248
if (stillOpenTarget) {
233249
localData = null
234250
this.localCacheHelper.clear().catch((e) => capture(e, 'Failed to clear local cache'))
235251
} else {
252+
if (localMetadata.owner == null) throw new Error('Local cache owner expected')
236253
helpers.openProject(localMetadata.owner, localMetadata.name)
237254
throw new Cancelled('Open another project')
238255
}
@@ -242,8 +259,8 @@ export class Editing extends Disposable {
242259
loadFromLocalCacheReporter.report(1)
243260

244261
// For projects not owned by the signed-in user, we prefer to load the published version.
245-
const preferPublishedContent = this.signedInUsername !== ownerName
246-
const cloudData = await this.cloudHelpers.load(ownerName, projectName, preferPublishedContent, signal)
262+
const preferPublishedContent = !matchesIdentifierInput(this.signedInUsername, ownerInput)
263+
const cloudData = await this.cloudHelpers.load(ownerInput, projectNameInput, preferPublishedContent, signal)
247264
signal.throwIfAborted()
248265
loadFromCloudReporter.report(1)
249266

@@ -259,6 +276,7 @@ export class Editing extends Disposable {
259276
}
260277
}
261278
await this.project.load(finalData, signal)
279+
this.updateMode()
262280
signal.throwIfAborted()
263281
projectLoadReporter.report(1)
264282
}

spx-gui/src/pages/community/project.vue

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const {
5454
refetch: reloadProject
5555
} = useQuery(
5656
async (ctx) => {
57-
const p = new SpxProject(props.owner, props.name)
57+
const p = new SpxProject()
5858
;(window as any).project = p // for debug purpose, TODO: remove me
5959
const serialized = await cloudHelpers.load(props.owner, props.name, true, ctx.signal)
6060
await p.load(serialized)
@@ -96,7 +96,12 @@ watch(
9696
}
9797
)
9898
99-
const isOwner = computed(() => props.owner === getSignedInUsername())
99+
const isOwner = computed(() => {
100+
const signedInUsername = getSignedInUsername()
101+
const projectOwner = project.value?.owner
102+
if (signedInUsername == null || projectOwner == null) return false
103+
return projectOwner === signedInUsername
104+
})
100105
const { data: liking } = useIsLikingProject(() => ({ owner: props.owner, name: props.name }))
101106
102107
const projectRunnerRef = ref<InstanceType<typeof ProjectRunnerSurface> | null>(null)

spx-gui/src/pages/community/user/index.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const { data: user, error, refetch } = useUser(() => props.name)
2020
<template v-else-if="user != null">
2121
<UserHeader :user="user" />
2222
<div class="main">
23-
<UserSidebar class="sidebar" :username="name" />
23+
<UserSidebar class="sidebar" :username="user.username" />
2424
<div class="content">
2525
<router-view />
2626
</div>

0 commit comments

Comments
 (0)