Skip to content

Commit 3362956

Browse files
LiGaCuJiatong Li
andauthored
fix(amazonq): avoid workspace context server missing historical dependency events (aws#1946)
Co-authored-by: Jiatong Li <[email protected]>
1 parent d428ddd commit 3362956

File tree

7 files changed

+135
-56
lines changed

7 files changed

+135
-56
lines changed

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/dependency/dependencyDiscoverer.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ import {
1010
} from './dependencyHandler/LanguageDependencyHandler'
1111
import { ArtifactManager } from '../artifactManager'
1212
import { supportedWorkspaceContextLanguages } from '../../../shared/languageDetection'
13+
import { DependencyEventBundler } from './dependencyEventBundler'
1314

1415
export class DependencyDiscoverer {
1516
private logging: Logging
1617
private workspaceFolders: WorkspaceFolder[]
1718
public dependencyHandlerRegistry: LanguageDependencyHandler<BaseDependencyInfo>[] = []
1819
private initializedWorkspaceFolder = new Map<WorkspaceFolder, boolean>()
1920
private sharedState: DependencyHandlerSharedState = { isDisposed: false, dependencyUploadedSizeSum: 0 }
21+
private dependencyEventsIngestedFolderUris = new Set<string>()
2022

2123
constructor(
2224
workspace: Workspace,
@@ -70,6 +72,9 @@ export class DependencyDiscoverer {
7072
async searchDependencies(folders: WorkspaceFolder[]): Promise<void> {
7173
this.logging.log('Starting dependency search across workspace folders')
7274

75+
// ingest recorded dependency events to corresponding dependency maps first
76+
this.ingestRecordedDependencyEvents(folders)
77+
7378
for (const workspaceFolder of folders) {
7479
if (
7580
this.initializedWorkspaceFolder.has(workspaceFolder) &&
@@ -144,12 +149,45 @@ export class DependencyDiscoverer {
144149
}
145150
}
146151

147-
async handleDependencyUpdateFromLSP(language: string, paths: string[], workspaceRoot?: WorkspaceFolder) {
152+
public isDependencyEventsIngested(workspaceFolderUri: string): boolean {
153+
return this.dependencyEventsIngestedFolderUris.has(workspaceFolderUri)
154+
}
155+
156+
private ingestRecordedDependencyEvents(workspaceFolders: WorkspaceFolder[]): void {
157+
let ingestedDependencyCount = 0
158+
for (const workspaceFolder of workspaceFolders) {
159+
for (const dependencyHandler of this.dependencyHandlerRegistry) {
160+
try {
161+
const recordedPaths = DependencyEventBundler.getRecordedDependencyPaths(
162+
dependencyHandler.language,
163+
workspaceFolder.uri
164+
)
165+
if (!recordedPaths) {
166+
continue
167+
}
168+
dependencyHandler.updateDependencyMapBasedOnLSP(recordedPaths, workspaceFolder)
169+
ingestedDependencyCount += recordedPaths.length
170+
} catch (error) {
171+
this.logging.debug(`Error ingesting dependency events for ${workspaceFolder.uri}: ${error}`)
172+
}
173+
}
174+
this.dependencyEventsIngestedFolderUris.add(workspaceFolder.uri)
175+
}
176+
if (ingestedDependencyCount > 0) {
177+
this.logging.log(`Ingested ${ingestedDependencyCount} dependencies from didChangeDependencyPaths events`)
178+
}
179+
}
180+
181+
async handleDependencyUpdateFromLSP(language: string, paths: string[], folder?: WorkspaceFolder) {
182+
if (folder === undefined) {
183+
return
184+
}
148185
for (const dependencyHandler of this.dependencyHandlerRegistry) {
149186
if (dependencyHandler.language != language) {
150187
continue
151188
}
152-
await dependencyHandler.updateDependencyMapBasedOnLSP(paths, workspaceRoot)
189+
const changedDependencyList = dependencyHandler.updateDependencyMapBasedOnLSP(paths, folder)
190+
await dependencyHandler.zipAndUploadDependenciesByChunk(changedDependencyList, folder)
153191
}
154192
}
155193

@@ -160,6 +198,7 @@ export class DependencyDiscoverer {
160198

161199
public dispose(): void {
162200
this.initializedWorkspaceFolder.clear()
201+
this.dependencyEventsIngestedFolderUris.clear()
163202
this.dependencyHandlerRegistry.forEach(dependencyHandler => {
164203
dependencyHandler.dispose()
165204
})
Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,34 @@
1-
import { Logging, WorkspaceFolder } from '@aws/language-server-runtimes/server-interface'
1+
import { Logging } from '@aws/language-server-runtimes/server-interface'
22
import { DependencyDiscoverer } from './dependencyDiscoverer'
3+
import { WorkspaceFolderManager } from '../workspaceFolderManager'
34

45
export interface DependencyEvent {
56
language: string
67
paths: string[]
7-
workspaceFolder: WorkspaceFolder | undefined
8+
workspaceFolderUri: string
89
}
910

1011
export class DependencyEventBundler {
12+
// Map storing historically received dependency events from extension
13+
// Key is <language>-<workspaceFolderUri> and value is a set of paths
14+
private static readonly recordedDependencies = new Map<string, Set<string>>()
15+
1116
private readonly logging: Logging
1217
private readonly dependencyDiscoverer: DependencyDiscoverer
18+
private readonly workspaceFolderManager: WorkspaceFolderManager
1319
private readonly BUNDLER_PROCESS_INTERVAL: number = 500 // 500 milliseconds
14-
public eventQueue: DependencyEvent[] = []
20+
private eventSendingQueue: DependencyEvent[] = []
1521
private eventBundlerInterval: NodeJS.Timeout | undefined
1622
private isBundlerWorking: boolean = false
1723

18-
constructor(logging: Logging, dependencyDiscoverer: DependencyDiscoverer) {
24+
constructor(
25+
logging: Logging,
26+
dependencyDiscoverer: DependencyDiscoverer,
27+
workspaceFolderManager: WorkspaceFolderManager
28+
) {
1929
this.logging = logging
2030
this.dependencyDiscoverer = dependencyDiscoverer
31+
this.workspaceFolderManager = workspaceFolderManager
2132
}
2233

2334
/**
@@ -32,11 +43,11 @@ export class DependencyEventBundler {
3243
}
3344
this.isBundlerWorking = true
3445
try {
35-
const allEvents = this.eventQueue.splice(0)
46+
const allEvents = this.eventSendingQueue.splice(0)
3647

3748
// Form bundles based on unique combination of language and workspaceFolder
3849
const dependencyEventBundles = allEvents.reduce((accumulator, event) => {
39-
const key = this.getBundleKey(event)
50+
const key = DependencyEventBundler.getBundleKey(event.language, event.workspaceFolderUri)
4051
if (!accumulator.has(key)) {
4152
accumulator.set(key, [])
4253
}
@@ -46,10 +57,12 @@ export class DependencyEventBundler {
4657

4758
// Process bundles one by one, concatenating all the paths within the bundle
4859
for (const [bundleKey, bundledEvents] of dependencyEventBundles) {
60+
const { language, workspaceFolderUri } = bundledEvents[0]
61+
const workspaceFolder = this.workspaceFolderManager.getWorkspaceFolder(workspaceFolderUri)
4962
await this.dependencyDiscoverer.handleDependencyUpdateFromLSP(
50-
bundledEvents[0].language,
63+
language,
5164
bundledEvents.flatMap(event => event.paths),
52-
bundledEvents[0].workspaceFolder
65+
workspaceFolder
5366
)
5467
}
5568
} catch (err) {
@@ -60,16 +73,39 @@ export class DependencyEventBundler {
6073
}, this.BUNDLER_PROCESS_INTERVAL)
6174
}
6275

63-
private getBundleKey(event: DependencyEvent) {
64-
if (event.workspaceFolder === undefined) {
65-
return `${event.language}-undefined`
66-
}
67-
return `${event.language}-${event.workspaceFolder.uri}`
76+
public sendDependencyEvent(event: DependencyEvent) {
77+
this.eventSendingQueue.push(event)
6878
}
6979

7080
public dispose(): void {
7181
if (this.eventBundlerInterval) {
7282
clearInterval(this.eventBundlerInterval)
7383
}
84+
this.eventSendingQueue = []
85+
}
86+
87+
private static getBundleKey(language: string, workspaceFolderUri: string) {
88+
return `${language}-${workspaceFolderUri}`
89+
}
90+
91+
public static recordDependencyEvent(event: DependencyEvent): void {
92+
const key = this.getBundleKey(event.language, event.workspaceFolderUri)
93+
if (!this.recordedDependencies.has(key)) {
94+
this.recordedDependencies.set(key, new Set())
95+
}
96+
const receivedPaths = this.recordedDependencies.get(key)
97+
if (receivedPaths) {
98+
event.paths.forEach(path => {
99+
receivedPaths.add(path)
100+
})
101+
}
102+
}
103+
104+
public static getRecordedDependencyPaths(language: string, workspaceFolderUri: string): string[] | undefined {
105+
const key = this.getBundleKey(language, workspaceFolderUri)
106+
const receivedPaths = this.recordedDependencies.get(key)
107+
if (receivedPaths) {
108+
return Array.from(receivedPaths)
109+
}
74110
}
75111
}

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/dependency/dependencyHandler/JSTSDependencyHandler.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,13 @@ export class JSTSDependencyHandler extends LanguageDependencyHandler<JSTSDepende
186186
const callBackDependencyUpdate = async (events: string[]) => {
187187
this.logging.log(`Change detected in ${packageJsonPath}`)
188188
const updatedDependencyMap = this.generateDependencyMap(jstsDependencyInfo)
189-
await this.compareAndUpdateDependencyMap(
189+
const changedDependencyList = this.compareAndUpdateDependencyMap(
190190
jstsDependencyInfo.workspaceFolder,
191-
updatedDependencyMap,
192-
true
191+
updatedDependencyMap
192+
)
193+
await this.zipAndUploadDependenciesByChunk(
194+
changedDependencyList,
195+
jstsDependencyInfo.workspaceFolder
193196
)
194197
}
195198
const watcher = new DependencyWatcher(
@@ -206,7 +209,9 @@ export class JSTSDependencyHandler extends LanguageDependencyHandler<JSTSDepende
206209
}
207210

208211
// JS and TS are not using LSP to sync dependencies
209-
override async updateDependencyMapBasedOnLSP(paths: string[], workspaceFolder?: WorkspaceFolder): Promise<void> {}
212+
override updateDependencyMapBasedOnLSP(paths: string[], workspaceFolder?: WorkspaceFolder): Dependency[] {
213+
return []
214+
}
210215
override transformPathToDependency(
211216
dependencyName: string,
212217
dependencyPath: string,

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/dependency/dependencyHandler/JavaDependencyHandler.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,7 @@ export class JavaDependencyHandler extends LanguageDependencyHandler<JavaDepende
5757
// TODO, check if try catch is necessary here
5858
try {
5959
let generatedDependencyMap: Map<string, Dependency> = this.generateDependencyMap(javaDependencyInfo)
60-
this.compareAndUpdateDependencyMap(javaDependencyInfo.workspaceFolder, generatedDependencyMap).catch(
61-
error => {
62-
this.logging.warn(`Error processing Java dependencies: ${error}`)
63-
}
64-
)
60+
this.compareAndUpdateDependencyMap(javaDependencyInfo.workspaceFolder, generatedDependencyMap)
6561
// Log found dependencies
6662
this.logging.log(
6763
`Total Java dependencies found: ${generatedDependencyMap.size} under ${javaDependencyInfo.pkgDir}`
@@ -92,10 +88,13 @@ export class JavaDependencyHandler extends LanguageDependencyHandler<JavaDepende
9288
const callBackDependencyUpdate = async (events: string[]) => {
9389
this.logging.log(`Change detected in ${dotClasspathPath}`)
9490
const updatedDependencyMap = this.generateDependencyMap(javaDependencyInfo)
95-
await this.compareAndUpdateDependencyMap(
91+
const changedDependencyList = this.compareAndUpdateDependencyMap(
9692
javaDependencyInfo.workspaceFolder,
97-
updatedDependencyMap,
98-
true
93+
updatedDependencyMap
94+
)
95+
await this.zipAndUploadDependenciesByChunk(
96+
changedDependencyList,
97+
javaDependencyInfo.workspaceFolder
9998
)
10099
}
101100
const watcher = new DependencyWatcher(

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/dependency/dependencyHandler/LanguageDependencyHandler.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -117,18 +117,17 @@ export abstract class LanguageDependencyHandler<T extends BaseDependencyInfo> {
117117
* @param paths
118118
* @param workspaceRoot
119119
*/
120-
async updateDependencyMapBasedOnLSP(paths: string[], workspaceFolder?: WorkspaceFolder): Promise<void> {
120+
updateDependencyMapBasedOnLSP(paths: string[], workspaceFolder: WorkspaceFolder): Dependency[] {
121121
const dependencyMap = new Map<string, Dependency>()
122122
paths.forEach((dependencyPath: string) => {
123123
// basename of the path should be the dependency name
124124
const dependencyName = path.basename(dependencyPath)
125125
this.transformPathToDependency(dependencyName, dependencyPath, dependencyMap)
126126
})
127127

128-
if (workspaceFolder) {
129-
await this.compareAndUpdateDependencyMap(workspaceFolder, dependencyMap, true)
130-
}
128+
return this.compareAndUpdateDependencyMap(workspaceFolder, dependencyMap)
131129
}
130+
132131
async zipDependencyMap(folders: WorkspaceFolder[]): Promise<void> {
133132
// Process each workspace folder sequentially
134133
for (const [workspaceFolder, correspondingDependencyMap] of this.dependencyMap) {
@@ -143,7 +142,7 @@ export abstract class LanguageDependencyHandler<T extends BaseDependencyInfo> {
143142
}
144143
}
145144

146-
private async zipAndUploadDependenciesByChunk(
145+
async zipAndUploadDependenciesByChunk(
147146
dependencyList: Dependency[],
148147
workspaceFolder: WorkspaceFolder
149148
): Promise<void> {
@@ -251,11 +250,10 @@ export abstract class LanguageDependencyHandler<T extends BaseDependencyInfo> {
251250
*/
252251
protected abstract generateDependencyMap(dependencyInfo: T, dependencyMap: Map<string, Dependency>): void
253252

254-
protected async compareAndUpdateDependencyMap(
253+
protected compareAndUpdateDependencyMap(
255254
workspaceFolder: WorkspaceFolder,
256-
updatedDependencyMap: Map<string, Dependency>,
257-
zipChanges: boolean = false
258-
): Promise<void> {
255+
updatedDependencyMap: Map<string, Dependency>
256+
): Dependency[] {
259257
const changes = {
260258
added: [] as Dependency[],
261259
updated: [] as Dependency[],
@@ -290,9 +288,7 @@ export abstract class LanguageDependencyHandler<T extends BaseDependencyInfo> {
290288
this.dependencyMap.get(workspaceFolder)?.set(name, newDep)
291289
})
292290

293-
if (zipChanges) {
294-
await this.zipAndUploadDependenciesByChunk([...changes.added, ...changes.updated], workspaceFolder)
295-
}
291+
return [...changes.added, ...changes.updated]
296292
}
297293

298294
private validateSingleDependencySize(workspaceFolder: WorkspaceFolder, dependency: Dependency): boolean {

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/dependency/dependencyHandler/PythonDependencyHandler.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,7 @@ export class PythonDependencyHandler extends LanguageDependencyHandler<PythonDep
7878
// TODO, check if the try catch is necessary here
7979
try {
8080
let generatedDependencyMap: Map<string, Dependency> = this.generateDependencyMap(pythonDependencyInfo)
81-
this.compareAndUpdateDependencyMap(pythonDependencyInfo.workspaceFolder, generatedDependencyMap).catch(
82-
error => {
83-
this.logging.warn(`Error processing Python dependencies: ${error}`)
84-
}
85-
)
81+
this.compareAndUpdateDependencyMap(pythonDependencyInfo.workspaceFolder, generatedDependencyMap)
8682
// Log found dependencies
8783
this.logging.log(
8884
`Total Python dependencies found: ${generatedDependencyMap.size} under ${pythonDependencyInfo.pkgDir}`
@@ -122,10 +118,13 @@ export class PythonDependencyHandler extends LanguageDependencyHandler<PythonDep
122118
this.handlePackageChange(sitePackagesPath, fileName, updatedDependencyMap)
123119
}
124120
}
125-
await this.compareAndUpdateDependencyMap(
121+
const changedDependencyList = this.compareAndUpdateDependencyMap(
126122
pythonDependencyInfo.workspaceFolder,
127-
updatedDependencyMap,
128-
true
123+
updatedDependencyMap
124+
)
125+
await this.zipAndUploadDependenciesByChunk(
126+
changedDependencyList,
127+
pythonDependencyInfo.workspaceFolder
129128
)
130129
} // end of callback function
131130

server/aws-lsp-codewhisperer/src/language-server/workspaceContext/workspaceContextServer.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { makeUserContextObject } from '../../shared/telemetryUtils'
2121
import { safeGet } from '../../shared/utils'
2222
import { AmazonQTokenServiceManager } from '../../shared/amazonQServiceManager/AmazonQTokenServiceManager'
2323
import { FileUploadJobManager, FileUploadJobType } from './fileUploadJobManager'
24-
import { DependencyEventBundler } from './dependency/dependencyEventBundler'
24+
import { DependencyEvent, DependencyEventBundler } from './dependency/dependencyEventBundler'
2525
import ignore = require('ignore')
2626
import { BUILDER_ID_START_URL, INTERNAL_USER_START_URL } from '../../shared/constants'
2727

@@ -244,7 +244,7 @@ export const WorkspaceContextServer = (): Server => features => {
244244
workspaceIdentifier
245245
)
246246
fileUploadJobManager = new FileUploadJobManager(logging, workspaceFolderManager)
247-
dependencyEventBundler = new DependencyEventBundler(logging, dependencyDiscoverer)
247+
dependencyEventBundler = new DependencyEventBundler(logging, dependencyDiscoverer, workspaceFolderManager)
248248
await updateConfiguration()
249249

250250
lsp.workspace.onDidChangeWorkspaceFolders(async params => {
@@ -511,17 +511,22 @@ export const WorkspaceContextServer = (): Server => features => {
511511

512512
lsp.extensions.onDidChangeDependencyPaths(async params => {
513513
try {
514+
const dependencyEvent: DependencyEvent = {
515+
language: params.runtimeLanguage,
516+
paths: params.paths,
517+
workspaceFolderUri: params.moduleName,
518+
}
519+
DependencyEventBundler.recordDependencyEvent(dependencyEvent)
520+
514521
if (!isUserEligibleForWorkspaceContext()) {
515522
return
516523
}
517-
logging.log(`Received onDidChangeDependencyPaths event for ${params.moduleName}`)
518524

519-
const workspaceFolder = workspaceFolderManager.getWorkspaceFolder(params.moduleName)
520-
dependencyEventBundler.eventQueue.push({
521-
language: params.runtimeLanguage,
522-
paths: params.paths,
523-
workspaceFolder: workspaceFolder,
524-
})
525+
// Only send events separately when dependency discovery has finished ingesting previous recorded events
526+
if (dependencyDiscoverer.isDependencyEventsIngested(params.moduleName)) {
527+
dependencyEventBundler.sendDependencyEvent(dependencyEvent)
528+
logging.log(`Processed onDidChangeDependencyPaths event for ${params.moduleName}`)
529+
}
525530
} catch (error) {
526531
logging.error(`Error handling didChangeDependencyPaths event: ${error}`)
527532
}

0 commit comments

Comments
 (0)