fix: wrong version negotiation for OpenUI5 framework (#561)

* fix: wrong version negotiation for OpenUI5 fr-work

* fix: work on pr comment

Author: vadson71

package.json

@@ -64,6 +64,7 @@
     "lint-staged": "10.5.3",
     "make-dir": "3.1.0",
     "mocha": "10.0.0",
+    "mock-fs": "^5.2.0",
     "npm-run-all": "4.1.5",
     "nyc": "15.1.0",
     "prettier": "2.2.1",

packages/context/src/ui5-model.ts

@@ -292,11 +292,14 @@ async function getLibs(
 }
 
 // if library is not found, resolve next minor highest patch
-let versionMap: Record<
-  string,
-  { version: string; support: string; lts: boolean }
->; // just an in-memory cache!
-const resolvedVersions: Record<string, string> = Object.create(null);
+const versionMap: Record<
+  UI5Framework,
+  Record<string, { version: string; support: string; lts: boolean }>
+> = Object.create(null); // just an in-memory cache!
+const resolvedVersions: Record<
+  UI5Framework,
+  Record<string, string>
+> = Object.create(null);
 /*
  * VERSION RESOLUTION LOGIC:
  * =========================
@@ -353,17 +356,18 @@ export async function negotiateVersionWithFetcher(
   // try to negotiate version
   let isFallback = false;
   let isIncorrectVersion = false;
+  let versions = versionMap[framework];
   if (!version) {
     // no version defined, using default version
     getLogger().warn(
       "No version defined! Please check the minUI5Version in your manifest.json!"
     );
     version = DEFAULT_UI5_VERSION;
     isFallback = true;
-  } else if (resolvedVersions[version]) {
+  } else if (resolvedVersions[framework]?.[version]) {
     // version already resolved?
     const versionDefined = version;
-    version = resolvedVersions[version];
+    version = resolvedVersions[framework]?.[version];
     if (versionDefined !== version) {
       isIncorrectVersion = true;
     }
@@ -377,12 +381,12 @@ export async function negotiateVersionWithFetcher(
   ) {
     const requestedVersion = version;
     // no version information found, try to negotiate the version
-    if (!versionMap) {
+    if (!versions) {
       // retrieve the version mapping (only exists for SAPUI5 so far)
-      const url = getVersionJsonUrl();
+      const url = getVersionJsonUrl(framework);
       const response = await versionJsonFetcher(url);
       if (response.ok) {
-        versionMap = (await response.json()) as Record<
+        versionMap[framework] = (await response.json()) as Record<
           string,
           { version: string; support: string; lts: boolean }
         >;
@@ -395,22 +399,23 @@ export async function negotiateVersionWithFetcher(
             DEFAULT_UI5_VERSION,
           }
         );
-        versionMap = {
+        versionMap[framework] = {
           latest: {
             version: DEFAULT_UI5_VERSION,
             support: "Maintenance",
             lts: true,
           },
         };
       }
+      versions = versionMap[framework];
     }
     // coerce the version (check for invalid version, which indicates development scenario)
     const parsedVersion = semver.coerce(version);
     if (parsedVersion) {
-      if (versionMap[`${parsedVersion.major}.${parsedVersion.minor}`]) {
+      if (versions[`${parsedVersion.major}.${parsedVersion.minor}`]) {
         // lookup for a valid major.minor entry
         version =
-          versionMap[`${parsedVersion.major}.${parsedVersion.minor}`].version;
+          versions[`${parsedVersion.major}.${parsedVersion.minor}`].version;
       }
       if (
         !(await getVersionInfo(
@@ -423,25 +428,28 @@ export async function negotiateVersionWithFetcher(
         // find closest supported version
         version =
           semverMinSatisfying(
-            Object.values(versionMap).map((entry) => {
+            Object.values(versions).map((entry) => {
               return entry.version;
             }) as string[],
             `^${version}`
-          ) || versionMap["latest"].version;
+          ) || versions["latest"].version;
 
         isIncorrectVersion = true;
       }
     } else {
       // development scenario => use latest version
-      version = versionMap["latest"].version;
+      version = versions["latest"].version;
       isIncorrectVersion = true;
     }
     // store the resolved version
     if (requestedVersion) {
       if (requestedVersion !== version) {
         isIncorrectVersion = true;
       }
-      resolvedVersions[requestedVersion] = version;
+      if (!resolvedVersions[framework]) {
+        resolvedVersions[framework] = Object.create(null);
+      }
+      resolvedVersions[framework][requestedVersion] = version;
     }
   }
   return {

packages/context/src/ui5-yaml.ts

@@ -99,18 +99,19 @@ async function readUI5YamlFile(
 export async function findYamlPath(
   documentPath: string
 ): Promise<string | undefined> {
-  return findUp(FileName.Ui5Yaml, { cwd: documentPath });
+  return await findUp(FileName.Ui5Yaml, { cwd: documentPath });
 }
 
 /**
  * Get yaml of an app
  * @param ui5YamlRoot absolute root to a yaml file of an app e.g. /some/other/path/parts/app/manage_travels/webapp/ui5.yaml
  */
 export async function getUI5Yaml(
-  ui5YamlRoot: string
+  ui5YamlRoot: string,
+  ignoreCache?: boolean
 ): Promise<YamlDetails | undefined> {
   const cachedYaml = cache.getYamlDetails(ui5YamlRoot);
-  if (cachedYaml) {
+  if (cachedYaml && !ignoreCache) {
     return cachedYaml;
   }
   try {
@@ -130,13 +131,14 @@ export async function getUI5Yaml(
  * @param documentPath path to a file e.g. absolute/path/webapp/ext/main/Main.view.xml
  */
 export async function getYamlDetails(
-  documentPath: string
+  documentPath: string,
+  ignoreCache?: boolean
 ): Promise<YamlDetails> {
   const yamlPath = await findYamlPath(documentPath);
   if (!yamlPath) {
     return { framework: DEFAULT_UI5_FRAMEWORK, version: undefined };
   }
-  const yamlData = await getUI5Yaml(yamlPath);
+  const yamlData = await getUI5Yaml(yamlPath, ignoreCache);
   if (yamlData) {
     return yamlData;
   }

packages/context/src/utils/ui5.ts

@@ -12,9 +12,8 @@ export function getCDNBaseUrl(
   return url;
 }
 
-export function getVersionJsonUrl(/* framework: UI5Framework */): string {
-  // version.json is only supported for SAPUI5 right now
-  return `${UI5_FRAMEWORK_CDN_BASE_URL["SAPUI5"]}version.json`;
+export function getVersionJsonUrl(framework: UI5Framework): string {
+  return `${UI5_FRAMEWORK_CDN_BASE_URL[framework]}version.json`;
 }
 
 export function getVersionInfoUrl(

packages/context/src/watcher.ts

@@ -94,7 +94,7 @@ export const reactOnUI5YamlChange = async (
     case 1: //created
     case 2: {
       //changed
-      const response = await getYamlDetails(ui5YamlUri);
+      const response = await getYamlDetails(ui5YamlPath, true);
       // We want to keep last successfully read state - yaml file may be actively edited
       cache.setYamlDetails(ui5YamlPath, response);
       return;

packages/context/test/loader-spec.ts

@@ -16,6 +16,7 @@ import { Manifest } from "@sap-ux/project-access";
 import { ProjectKind, UI5_PROJECT_TYPE } from "../src/types";
 import { getProjectData } from "./utils";
 import { getManifestDetails, getUI5Manifest } from "../src/manifest";
+import { getApp } from "../src/loader";
 
 describe("loader", () => {
   let testFramework: TestFramework;
@@ -121,7 +122,7 @@ describe("loader", () => {
       expect(getAppSpy).to.have.been.called;
       expect(app).to.deep.equal(getAppSpy.returnValues[0]);
     });
-    it("does not fined mainService and returns undefined", async () => {
+    it("does not find mainService and returns empty services", async () => {
       const projectRoot = testFramework.getProjectRoot();
       const { appRoot, manifestDetails, projectInfo } = await getProjectData(
         projectRoot
@@ -136,9 +137,9 @@ describe("loader", () => {
         manifestDetails,
         projectInfo
       );
-      expect(app).to.be.undefined;
+      expect(app?.localServices.size).to.eq(0);
     });
-    it("does not parser service files and returns undefined", async () => {
+    it("does not parse service files and returns empty services", async () => {
       const parseServiceFilesStub = stub(parser, "parseServiceFiles").returns(
         undefined
       );
@@ -151,15 +152,15 @@ describe("loader", () => {
       } = await getProjectData(projectRoot);
       // for consistency remove cache
       cache.deleteApp(appRoot);
-      const app = await loader.getApp(
+      const app = await getApp(
         projectRoot,
         appRoot,
         manifest,
         manifestDetails,
         projectInfo
       );
       expect(parseServiceFilesStub).to.have.been.called;
-      expect(app).to.be.undefined;
+      expect(app?.localServices.size).to.eq(0);
     });
   });
   context("getCAPProject", () => {

packages/context/test/manifest-spec.ts

@@ -39,14 +39,16 @@ describe("manifest", () => {
     restore();
   });
   context("getUI5Manifest", () => {
-    it("get undefined", async () => {
+    it("get undefined", async function () {
+      this.timeout(20000);
       const manifestRoot = join("/wrong/path", FileName.Manifest);
       // for consistency remove cache
       cache.deleteManifest(manifestRoot);
       const result = await getUI5Manifest(manifestRoot);
       expect(result).to.be.undefined;
     });
-    it("get UI5 manifest", async () => {
+    it("get UI5 manifest", async function () {
+      this.timeout(20000);
       const projectRoot = testFramework.getProjectRoot();
       const appRoot = getAppRoot(projectRoot);
       const manifestRoot = join(appRoot, FileName.Manifest);

packages/context/test/services-spec.ts

@@ -27,6 +27,9 @@ describe("services", () => {
     };
     testFramework = new TestFramework(useConfig);
   });
+  beforeEach(() => {
+    cache.reset();
+  });
   afterEach(() => {
     restore();
   });