Improve handling of bad versions during ext:install (#5305)

* Improve handling of bad versions during ext:install * Add changelog * Better variable names
firebase · Dec 6, 2022 · 4c92149 · 4c92149
1 parent 5712978
commit 4c92149
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,3 +6,4 @@
 - Support the x-goog-api-key header in auth emulator. (#5249)
 - Fix bug in deploying web frameworks when a predeploy hook was configured in firebase.json (#5199)
 - Fix bug where function deployments using --only filter sometimes failed deployments. (#5280)
+- Fix bug where `ext:install` would sometimes fail if no version was specified. (#5305)
diff --git a/src/deploy/extensions/planner.ts b/src/deploy/extensions/planner.ts
@@ -215,6 +215,7 @@ export async function resolveVersion(ref: refs.Ref): Promise<string> {
   }
   if (!ref.version || ref.version === "latest") {
     return versions
+      .filter((ev) => ev.spec.version !== undefined)
       .map((ev) => ev.spec.version)
       .sort(semver.compare)
       .pop()!;

diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts
@@ -833,14 +833,18 @@ export async function diagnoseAndFixProject(options: any): Promise<void> {
  * 1. Infer firebase publisher if not provided
  * 2. Infer "latest" as the version if not provided
  */
-export async function canonicalizeRefInput(extensionName: string): Promise<string> {
-  // Infer firebase if publisher ID not provided.
-  if (extensionName.split("/").length < 2) {
-    const [extensionID, version] = extensionName.split("@");
-    extensionName = `firebase/${extensionID}@${version || "latest"}`;
+export async function canonicalizeRefInput(refInput: string): Promise<string> {
+  let inferredRef = refInput;
+  // Infer 'firebase' if publisher ID not provided.
+  if (refInput.split("/").length < 2) {
+    inferredRef = `firebase/${inferredRef}`;
+  }
+  // Infer 'latest' if no version provided.
+  if (refInput.split("@").length < 2) {
+    inferredRef = `${inferredRef}@latest`;
   }
   // Get the correct version for a given extension reference from the Registry API.
-  const ref = refs.parse(extensionName);
+  const ref = refs.parse(inferredRef);
   ref.version = await resolveVersion(ref);
   return refs.toExtensionVersionRef(ref);
 }
diff --git a/src/test/deploy/extensions/planner.spec.ts b/src/test/deploy/extensions/planner.spec.ts
@@ -3,9 +3,9 @@ import * as sinon from "sinon";
 
 import * as planner from "../../../deploy/extensions/planner";
 import * as extensionsApi from "../../../extensions/extensionsApi";
-import { ExtensionInstance, ExtensionVersion } from "../../../extensions/types";
+import { ExtensionInstance } from "../../../extensions/types";
 
-function extensionVersion(version: string): ExtensionVersion {
+function extensionVersion(version?: string): any {
   return {
     name: `publishers/test/extensions/test/versions/${version}`,
     ref: `test/test@${version}`,
@@ -26,13 +26,12 @@ describe("Extensions Deployment Planner", () => {
     let listExtensionVersionsStub: sinon.SinonStub;
 
     before(() => {
-      listExtensionVersionsStub = sinon
-        .stub(extensionsApi, "listExtensionVersions")
-        .resolves([
-          extensionVersion("0.1.0"),
-          extensionVersion("0.1.1"),
-          extensionVersion("0.2.0"),
-        ]);
+      listExtensionVersionsStub = sinon.stub(extensionsApi, "listExtensionVersions").resolves([
+        extensionVersion("0.1.0"),
+        extensionVersion("0.1.1"),
+        extensionVersion("0.2.0"),
+        extensionVersion(), // Explicitly test that this doesn't break on bad data
+      ]);
     });
 
     after(() => {