fix: review comments and small improvment

packages/context/src/api.ts

@@ -70,6 +70,7 @@ export async function getContext(
     const controlIds = getControlIds({
       manifestPath,
       documentPath,
+      content,
     });
     return {
       manifestDetails,

packages/context/src/cache.ts

@@ -3,6 +3,7 @@ import type { UI5SemanticModel } from "@ui5-language-assistant/semantic-model-ty
 import { accept, type XMLDocument } from "@xml-tools/ast";
 import type { App, ControlIdLocation, Project, YamlDetails } from "./types";
 import { createDocumentAst, IdsCollectorVisitor } from "./utils";
+import { FileChangeType } from "vscode-languageserver/node";
 
 type AbsoluteAppRoot = string;
 type AbsoluteProjectRoot = string;
@@ -14,8 +15,8 @@ class Cache {
   private CAPServices: Map<AbsoluteProjectRoot, Map<string, string>>;
   private ui5YamlDetails: Map<string, YamlDetails>;
   private ui5Model: Map<string, UI5SemanticModel>;
-  private viewFiles: Record<string, Record<string, XMLDocument>>;
-  private controlIds: Record<
+  private viewFiles: Map<string, Record<string, XMLDocument>>;
+  private controlIds: Map<
     string,
     Record<string, Map<string, ControlIdLocation[]>>
   >;
@@ -26,8 +27,8 @@ class Cache {
     this.CAPServices = new Map();
     this.ui5YamlDetails = new Map();
     this.ui5Model = new Map();
-    this.viewFiles = {};
-    this.controlIds = {};
+    this.viewFiles = new Map();
+    this.controlIds = new Map();
   }
   reset() {
     this.project = new Map();
@@ -36,8 +37,8 @@ class Cache {
     this.CAPServices = new Map();
     this.ui5YamlDetails = new Map();
     this.ui5Model = new Map();
-    this.viewFiles = {};
-    this.controlIds = {};
+    this.viewFiles = new Map();
+    this.controlIds = new Map();
   }
   /**
    * Get entries of cached project
@@ -138,14 +139,14 @@ class Cache {
    * Get entries of view files
    */
   getViewFiles(manifestPath: string): Record<string, XMLDocument> {
-    return this.viewFiles[manifestPath] ?? {};
+    return this.viewFiles.get(manifestPath) ?? {};
   }
 
   setViewFiles(
     manifestPath: string,
     viewFiles: Record<string, XMLDocument>
   ): void {
-    this.viewFiles[manifestPath] = viewFiles;
+    this.viewFiles.set(manifestPath, viewFiles);
   }
 
   /**
@@ -161,19 +162,21 @@ class Cache {
   async setViewFile(param: {
     manifestPath: string;
     documentPath: string;
-    operation: "create" | "delete";
+    operation: Exclude<FileChangeType, 2>;
     content?: string;
   }): Promise<void> {
     const { manifestPath, documentPath, operation, content } = param;
-    if (operation === "create") {
+    if (operation === FileChangeType.Created) {
       const viewFiles = this.getViewFiles(manifestPath);
       viewFiles[documentPath] = await createDocumentAst(documentPath, content);
+      // assign new view files to cache
       this.setViewFiles(manifestPath, viewFiles);
       return;
     }
 
     const viewFiles = this.getViewFiles(manifestPath);
     delete viewFiles[documentPath];
+    // assign new view files to cache
     this.setViewFiles(manifestPath, viewFiles);
   }
   /**
@@ -182,13 +185,13 @@ class Cache {
   getControlIds(
     manifestPath: string
   ): Record<string, Map<string, ControlIdLocation[]>> {
-    return this.controlIds[manifestPath] ?? {};
+    return this.controlIds.get(manifestPath) ?? {};
   }
   setControlIds(
     manifestPath: string,
     controlIds: Record<string, Map<string, ControlIdLocation[]>>
   ) {
-    this.controlIds[manifestPath] = controlIds;
+    this.controlIds.set(manifestPath, controlIds);
   }
 
   /**
@@ -201,24 +204,27 @@ class Cache {
   setControlIdsForViewFile(param: {
     manifestPath: string;
     documentPath: string;
-    operation: "create" | "delete";
+    operation: Exclude<FileChangeType, 2>;
   }): void {
     const { manifestPath, documentPath, operation } = param;
 
-    if (operation === "create") {
+    if (operation === FileChangeType.Created) {
       const viewFiles = this.getViewFiles(manifestPath);
       // for current document, re-collect and re-assign it to avoid cache issue
       if (viewFiles[documentPath]) {
         const idCollector = new IdsCollectorVisitor(documentPath);
         accept(viewFiles[documentPath], idCollector);
-        this.controlIds[manifestPath][documentPath] =
-          idCollector.getControlIds();
+        const idControls = this.getControlIds(manifestPath);
+        idControls[documentPath] = idCollector.getControlIds();
+        // assign new control ids to cache
+        this.setControlIds(manifestPath, idControls);
       }
       return;
     }
 
     const idControls = this.getControlIds(manifestPath);
     delete idControls[documentPath];
+    // assign new control ids to cache
     this.setControlIds(manifestPath, idControls);
   }
 }

packages/context/src/utils/control-ids.ts

@@ -1,3 +1,4 @@
+import { FileChangeType } from "vscode-languageserver/node";
 import { cache } from "../api";
 import { ControlIdLocation } from "../types";
 import { IdsCollectorVisitor } from "./ids-collector";
@@ -13,16 +14,19 @@ import { accept } from "@xml-tools/ast";
 function processControlIds(param: {
   manifestPath: string;
   documentPath: string;
+  content?: string;
 }): void {
-  const { documentPath, manifestPath } = param;
+  const { documentPath, manifestPath, content } = param;
   // check cache
   if (Object.keys(cache.getControlIds(manifestPath)).length > 0) {
-    // for current document, re-collect and re-assign it to avoid cache issue
-    cache.setControlIdsForViewFile({
-      manifestPath,
-      documentPath,
-      operation: "create",
-    });
+    if (content) {
+      // for current document, re-collect and re-assign it to avoid cache issue
+      cache.setControlIdsForViewFile({
+        manifestPath,
+        documentPath,
+        operation: FileChangeType.Created,
+      });
+    }
     return;
   }
 
@@ -49,6 +53,7 @@ function processControlIds(param: {
 export function getControlIds(param: {
   manifestPath: string;
   documentPath: string;
+  content?: string;
 }): Map<string, ControlIdLocation[]> {
   const { manifestPath } = param;
 

packages/context/src/utils/view-files.ts

@@ -5,6 +5,7 @@ import { isXMLView } from "@ui5-language-assistant/logic-utils";
 import { parse, DocumentCstNode } from "@xml-tools/parser";
 import { buildAst, XMLDocument } from "@xml-tools/ast";
 import { cache } from "../cache";
+import { FileChangeType } from "vscode-languageserver";
 
 export async function createDocumentAst(
   documentPath: string,
@@ -60,7 +61,7 @@ export async function getViewFiles(param: {
       await cache.setViewFile({
         manifestPath,
         documentPath,
-        operation: "create",
+        operation: FileChangeType.Created,
         content,
       });
     }

packages/context/src/watcher.ts

@@ -286,38 +286,24 @@ export async function reactOnViewFileChange(
   if (!manifestPath) {
     return;
   }
-  if (changeType === FileChangeType.Deleted) {
-    // remove document from cache
-    await cache.setViewFile({
-      manifestPath,
-      documentPath,
-      operation: "delete",
-    });
-
-    // remove controls id
-    cache.setControlIdsForViewFile({
-      manifestPath,
-      documentPath,
-      operation: "delete",
-    });
-  }
 
-  if (changeType === FileChangeType.Created) {
-    // add document to cache
+  if (
+    changeType === FileChangeType.Created ||
+    changeType === FileChangeType.Deleted
+  ) {
     await cache.setViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: changeType,
     });
 
-    // add controls id
     cache.setControlIdsForViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: changeType,
     });
+    await validator();
   }
-  await validator();
 }
 /**
  * React on package.json file. In `package.json` user can define `cds`, `sapux` or similar configurations

packages/context/test/unit/cache.test.ts

@@ -9,6 +9,7 @@ import {
   TestFramework,
 } from "@ui5-language-assistant/test-framework";
 import { join } from "path";
+import { FileChangeType } from "vscode-languageserver/node";
 
 const getManifestPath = (projectRoot: string) =>
   join(projectRoot, "app", "manage_travels", "webapp", "manifest.json");
@@ -83,7 +84,7 @@ describe("cache", () => {
     await cache.setViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: FileChangeType.Created,
     });
     // assert
     const cachedData = cache.getViewFiles(manifestPath)[documentPath];
@@ -102,7 +103,7 @@ describe("cache", () => {
     await cache.setViewFile({
       manifestPath,
       documentPath,
-      operation: "delete",
+      operation: FileChangeType.Deleted,
     });
     // assert
     const cachedData = cache.getViewFiles(manifestPath)[documentPath];
@@ -123,7 +124,7 @@ describe("cache", () => {
     await cache.setViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: FileChangeType.Created,
     });
     // create controls id
     const data = {};
@@ -133,7 +134,7 @@ describe("cache", () => {
     cache.setControlIdsForViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: FileChangeType.Created,
     });
     // assert
     const cachedData = cache.getControlIds(manifestPath)[documentPath];
@@ -152,7 +153,7 @@ describe("cache", () => {
     cache.setControlIdsForViewFile({
       manifestPath,
       documentPath,
-      operation: "delete",
+      operation: FileChangeType.Deleted,
     });
     // assert
     const cachedData = cache.getControlIds(manifestPath)[documentPath];

packages/context/test/unit/utils/control-ids.test.ts

@@ -8,6 +8,7 @@ import { join } from "path";
 import { getControlIds } from "../../../src/utils/control-ids";
 import { getViewFiles } from "../../../src/utils";
 import { cache } from "../../../src/cache";
+import { FileChangeType } from "vscode-languageserver/node";
 const getManifestPath = (projectRoot: string) =>
   join(projectRoot, "app", "manage_travels", "webapp", "manifest.json");
 const mainViewSeg = [
@@ -71,12 +72,12 @@ describe("control-ids", () => {
     await cache.setViewFile({
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: FileChangeType.Created,
       content,
     });
     await testFramework.updateFileContent(mainViewSeg, content);
     // act
-    const resultCached = getControlIds({ manifestPath, documentPath });
+    const resultCached = getControlIds({ manifestPath, documentPath, content });
     // assert
     expect(resultCached).toBeDefined();
     expect(resultCached.get("Main")).toBeUndefined();

packages/context/test/unit/utils/view-files.test.ts

@@ -8,6 +8,7 @@ import { join } from "path";
 import { getViewFiles } from "../../../src/utils";
 import { cache } from "../../../src/cache";
 import type { XMLDocument } from "@xml-tools/ast";
+import { FileChangeType } from "vscode-languageserver/node";
 
 const getManifestPath = (projectRoot: string) =>
   join(projectRoot, "app", "manage_travels", "webapp", "manifest.json");
@@ -78,7 +79,7 @@ describe("view-files", () => {
     expect(setViewFileStub).toHaveBeenNthCalledWith(1, {
       manifestPath,
       documentPath,
-      operation: "create",
+      operation: FileChangeType.Created,
       content,
     });
     expect(Object.keys(viewFiles).length).toBeGreaterThan(0);

packages/context/test/unit/watcher.test.ts

@@ -590,12 +590,12 @@ describe("watcher", () => {
       expect(setViewFileSpy).toHaveBeenNthCalledWith(1, {
         documentPath,
         manifestPath,
-        operation: "delete",
+        operation: FileChangeType.Deleted,
       });
       expect(setControlIdsForViewFileSpy).toHaveBeenNthCalledWith(1, {
         documentPath,
         manifestPath,
-        operation: "delete",
+        operation: FileChangeType.Deleted,
       });
       expect(validatorSpy).toHaveBeenCalledOnce();
     });
@@ -620,12 +620,12 @@ describe("watcher", () => {
       expect(setViewFileSpy).toHaveBeenNthCalledWith(1, {
         documentPath,
         manifestPath,
-        operation: "create",
+        operation: FileChangeType.Created,
       });
       expect(setControlIdsForViewFileSpy).toHaveBeenNthCalledWith(1, {
         documentPath,
         manifestPath,
-        operation: "create",
+        operation: FileChangeType.Created,
       });
       expect(validatorSpy).toHaveBeenCalledOnce();
     });