Merge pull request #3148 from github/koesie10/model-editor-capitalization

Fix incorrect model files on case-insensitive file systems

extensions/ql-vscode/src/model-editor/yaml.ts

@@ -181,14 +181,23 @@ function createDataExtensionYamlsByGrouping(
   >,
   createFilename: (method: Method) => string,
 ): Record<string, string> {
-  const methodsByFilename: Record<string, Record<string, ModeledMethod[]>> = {};
+  const actualFilenameByCanonicalFilename: Record<string, string> = {};
+
+  const methodsByCanonicalFilename: Record<
+    string,
+    Record<string, ModeledMethod[]>
+  > = {};
 
   // We only want to generate a yaml file when it's a known external API usage
   // and there are new modeled methods for it. This avoids us overwriting other
   // files that may contain data we don't know about.
   for (const method of methods) {
     if (method.signature in newModeledMethods) {
-      methodsByFilename[createFilename(method)] = {};
+      const filename = createFilename(method);
+      const canonicalFilename = canonicalizeFilename(filename);
+
+      methodsByCanonicalFilename[canonicalFilename] = {};
+      actualFilenameByCanonicalFilename[canonicalFilename] = filename;
     }
   }
 
@@ -196,10 +205,16 @@ function createDataExtensionYamlsByGrouping(
   for (const [filename, methodsBySignature] of Object.entries(
     existingModeledMethods,
   )) {
-    if (filename in methodsByFilename) {
+    const canonicalFilename = canonicalizeFilename(filename);
+
+    if (canonicalFilename in methodsByCanonicalFilename) {
       for (const [signature, methods] of Object.entries(methodsBySignature)) {
-        methodsByFilename[filename][signature] = [...methods];
+        methodsByCanonicalFilename[canonicalFilename][signature] = [...methods];
       }
+
+      // Ensure that if a file exists on disk, we use the same capitalization
+      // as the original file.
+      actualFilenameByCanonicalFilename[canonicalFilename] = filename;
     }
   }
 
@@ -209,19 +224,25 @@ function createDataExtensionYamlsByGrouping(
     const newMethods = newModeledMethods[method.signature];
     if (newMethods) {
       const filename = createFilename(method);
+      const canonicalFilename = canonicalizeFilename(filename);
 
       // Override any existing modeled methods with the new ones.
-      methodsByFilename[filename][method.signature] = [...newMethods];
+      methodsByCanonicalFilename[canonicalFilename][method.signature] = [
+        ...newMethods,
+      ];
     }
   }
 
   const result: Record<string, string> = {};
 
-  for (const [filename, methods] of Object.entries(methodsByFilename)) {
-    result[filename] = createDataExtensionYaml(
-      language,
-      Object.values(methods).flatMap((methods) => methods),
-    );
+  for (const [canonicalFilename, methods] of Object.entries(
+    methodsByCanonicalFilename,
+  )) {
+    result[actualFilenameByCanonicalFilename[canonicalFilename]] =
+      createDataExtensionYaml(
+        language,
+        Object.values(methods).flatMap((methods) => methods),
+      );
   }
 
   return result;
@@ -299,6 +320,13 @@ export function createFilenameForPackage(
   return `${prefix}${packageName}${suffix}.yml`;
 }
 
+function canonicalizeFilename(filename: string) {
+  // We want to canonicalize filenames so that they are always in the same format
+  // for comparison purposes. This is important because we want to avoid overwriting
+  // data extension YAML files on case-insensitive file systems.
+  return filename.toLowerCase();
+}
+
 function validateModelExtensionFile(data: unknown): data is ModelExtensionFile {
   modelExtensionFileSchemaValidate(data);
 

extensions/ql-vscode/test/unit-tests/model-editor/yaml.test.ts

@@ -6,8 +6,9 @@ import {
   createFilenameForPackage,
   loadDataExtensionYaml,
 } from "../../../src/model-editor/yaml";
-import { CallClassification } from "../../../src/model-editor/method";
+import { CallClassification, Method } from "../../../src/model-editor/method";
 import { QueryLanguage } from "../../../src/common/query-language";
+import { ModeledMethod } from "../../../src/model-editor/modeled-method";
 
 describe("createDataExtensionYaml", () => {
   it("creates the correct YAML file", () => {
@@ -980,6 +981,132 @@ describe("createDataExtensionYamlsForFrameworkMode", () => {
 `,
     });
   });
+
+  describe("with same package names but different capitalizations", () => {
+    const methods: Method[] = [
+      {
+        library: "HostTestAppDbContext",
+        signature:
+          "Volo.Abp.TestApp.MongoDb.HostTestAppDbContext#get_FifthDbContextDummyEntity()",
+        packageName: "Volo.Abp.TestApp.MongoDb",
+        typeName: "HostTestAppDbContext",
+        methodName: "get_FifthDbContextDummyEntity",
+        methodParameters: "()",
+        supported: false,
+        supportedType: "none",
+        usages: [],
+      },
+      {
+        library: "CityRepository",
+        signature:
+          "Volo.Abp.TestApp.MongoDB.CityRepository#FindByNameAsync(System.String)",
+        packageName: "Volo.Abp.TestApp.MongoDB",
+        typeName: "CityRepository",
+        methodName: "FindByNameAsync",
+        methodParameters: "(System.String)",
+        supported: false,
+        supportedType: "none",
+        usages: [],
+      },
+    ];
+    const newModeledMethods: Record<string, ModeledMethod[]> = {
+      "Volo.Abp.TestApp.MongoDb.HostTestAppDbContext#get_FifthDbContextDummyEntity()":
+        [
+          {
+            type: "sink",
+            input: "Argument[0]",
+            kind: "sql",
+            provenance: "df-generated",
+            signature:
+              "Volo.Abp.TestApp.MongoDb.HostTestAppDbContext#get_FifthDbContextDummyEntity()",
+            packageName: "Volo.Abp.TestApp.MongoDb",
+            typeName: "HostTestAppDbContext",
+            methodName: "get_FifthDbContextDummyEntity",
+            methodParameters: "()",
+          },
+        ],
+      "Volo.Abp.TestApp.MongoDB.CityRepository#FindByNameAsync(System.String)":
+        [
+          {
+            type: "neutral",
+            kind: "summary",
+            provenance: "df-generated",
+            signature:
+              "Volo.Abp.TestApp.MongoDB.CityRepository#FindByNameAsync(System.String)",
+            packageName: "Volo.Abp.TestApp.MongoDB",
+            typeName: "CityRepository",
+            methodName: "FindByNameAsync",
+            methodParameters: "(System.String)",
+          },
+        ],
+    };
+    const modelYaml = `extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sourceModel
+    data: []
+
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sinkModel
+    data:
+      - ["Volo.Abp.TestApp.MongoDb","HostTestAppDbContext",true,"get_FifthDbContextDummyEntity","()","","Argument[0]","sql","df-generated"]
+
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data: []
+
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: neutralModel
+    data:
+      - ["Volo.Abp.TestApp.MongoDB","CityRepository","FindByNameAsync","(System.String)","summary","df-generated"]
+`;
+
+    it("creates the correct YAML files when there are existing modeled methods", () => {
+      const yaml = createDataExtensionYamlsForFrameworkMode(
+        QueryLanguage.CSharp,
+        methods,
+        newModeledMethods,
+        {},
+      );
+
+      expect(yaml).toEqual({
+        "models/Volo.Abp.TestApp.MongoDB.model.yml": modelYaml,
+      });
+    });
+
+    it("creates the correct YAML files when there are existing modeled methods", () => {
+      const yaml = createDataExtensionYamlsForFrameworkMode(
+        QueryLanguage.CSharp,
+        methods,
+        newModeledMethods,
+        {
+          "models/Volo.Abp.TestApp.mongodb.model.yml": {
+            "Volo.Abp.TestApp.MongoDB.CityRepository#FindByNameAsync(System.String)":
+              [
+                {
+                  type: "neutral",
+                  kind: "summary",
+                  provenance: "manual",
+                  signature:
+                    "Volo.Abp.TestApp.MongoDB.CityRepository#FindByNameAsync(System.String)",
+                  packageName: "Volo.Abp.TestApp.MongoDB",
+                  typeName: "CityRepository",
+                  methodName: "FindByNameAsync",
+                  methodParameters: "(System.String)",
+                },
+              ],
+          },
+        },
+      );
+
+      expect(yaml).toEqual({
+        "models/Volo.Abp.TestApp.mongodb.model.yml": modelYaml,
+      });
+    });
+  });
 });
 
 describe("loadDataExtensionYaml", () => {