Add update package test

packages/catalog-server/package.json

@@ -17,6 +17,7 @@
     "start": "wireit",
     "start:dev": "wireit",
     "test": "wireit",
+    "test:manual": "wireit",
     "emulators:start": "firebase emulators:start --project wc-catalog",
     "firebase": "firebase"
   },
@@ -59,6 +60,16 @@
         "test.sh"
       ],
       "output": []
+    },
+    "test:manual": {
+      "command": "NODE_OPTIONS='--enable-source-maps' FIRESTORE_EMULATOR_HOST=localhost:8088 uvu test \"_test\\.js$\"",
+      "dependencies": [
+        "build"
+      ],
+      "files": [
+        "test.sh"
+      ],
+      "output": []
     }
   },
   "devDependencies": {

packages/catalog-server/src/lib/catalog.ts

@@ -154,6 +154,12 @@ export class Catalog {
         }
       }
 
+      // Remove the version to import from versionsToUpdate, because it
+      // doesn't exist yet
+      if (versionToImport !== undefined) {
+        versionsToUpdate.delete(versionToImport);
+      }
+
       // Write the tags
       console.log('Writing package dist tags...');
       await this.#repository.updateDistTags(

packages/catalog-server/src/lib/firestore/firestore-repository.ts

@@ -12,7 +12,7 @@ import {referenceString} from '@webcomponents/custom-elements-manifest-tools/lib
 import clean from 'semver/functions/clean.js';
 import semverValidRange from 'semver/ranges/valid.js';
 
-import {getDistTagsForVersion} from '../npm.js';
+import {distTagListToMap, getDistTagsForVersion} from '../npm.js';
 
 import {
   CustomElement,
@@ -83,11 +83,15 @@ export class FirestoreRepository implements Repository {
       ) {
         throw new Error(`Unexpected package status: ${packageData.status}`);
       }
-      await t.update(packageRef, {
+      // Note: update() does not use the Firestore data converters, so
+      // specific field conversion, like dist tags, must be done here.
+      // We remove the converter to fix the types:
+      // https://github.com/googleapis/nodejs-firestore/issues/1745
+      await t.update(packageRef.withConverter(null), {
         status: PackageStatus.READY,
         lastUpdate: FieldValue.serverTimestamp(),
         description: packageInfo.description ?? '',
-        distTags: packageInfo.distTags,
+        distTags: distTagListToMap(packageInfo.distTags),
       });
     });
   }
@@ -145,11 +149,6 @@ export class FirestoreRepository implements Repository {
           const versionDistTags = getDistTagsForVersion(newDistTags, version);
           const versionRef = this.getPackageVersionRef(packageName, version);
 
-          // Updat the PackageVersion doc
-          await t.update(versionRef, {
-            distTags: versionDistTags,
-          });
-
           // Update all custom elements of the PackageVersion
           const customElementsRef = versionRef
             .collection('customElements')
@@ -160,6 +159,13 @@ export class FirestoreRepository implements Repository {
           // we can construct custom element refs without a query on the
           // collection first.
           const elements = await t.get(customElementsRef);
+
+          // Update the PackageVersion doc
+          await t.update(versionRef, {
+            distTags: versionDistTags,
+          });
+
+          // Update all elements
           await Promise.all(
             elements.docs.map(async (element) => {
               await t.update(element.ref, {

packages/catalog-server/src/lib/firestore/package-info-converter.ts

@@ -20,27 +20,26 @@ import {
   ReadablePackageStatus,
   UnreadablePackageInfo,
 } from '@webcomponents/catalog-api/lib/schema.js';
-import { distTagListToMap } from '../npm.js';
+import {distTagListToMap, distTagMapToList} from '../npm.js';
 
 export const packageInfoConverter: FirestoreDataConverter<PackageInfo> = {
   fromFirestore(snapshot: QueryDocumentSnapshot<DocumentData>): PackageInfo {
     const name = idToPackageName(snapshot.id);
     const status = snapshot.get('status');
     const lastUpdate = (snapshot.get('lastUpdate') as Timestamp).toDate();
 
-    if (status === ReadablePackageStatus.READY || status === ReadablePackageStatus.UPDATING) {
-      const distTagsMap = snapshot.get('distTags');
-      const distTags = distTagsMap && Object.entries(distTagsMap).map(
-        ([tag, version]) => ({tag, version} as DistTag)
-      );
+    if (
+      status === ReadablePackageStatus.READY ||
+      status === ReadablePackageStatus.UPDATING
+    ) {
       return {
         name,
         status,
         lastUpdate,
         description: snapshot.get('description'),
-        distTags,
+        distTags: distTagMapToList(snapshot.get('distTags')),
         // `version` is left to a sub-collection query
-      } as ReadablePackageInfo;  
+      } as ReadablePackageInfo;
     } else {
       return {
         name,

packages/catalog-server/src/lib/npm.ts

@@ -26,7 +26,7 @@ export const distTagMapToList = (distTags: {[tag: string]: string}) =>
   }));
 
 export const distTagListToMap = (
-  distTags: Array<{tag: string; version: string}>
+  distTags: ReadonlyArray<{readonly tag: string; readonly version: string}>
 ) => Object.fromEntries(distTags.map(({tag, version}) => [tag, version]));
 
 export const isValidSemver = (versionOrTag: string) =>

packages/catalog-server/src/test/lib/catalog_test.ts

@@ -9,12 +9,15 @@ import * as assert from 'uvu/assert';
 import {Catalog} from '../../lib/catalog.js';
 import {LocalFsPackageFiles} from '@webcomponents/custom-elements-manifest-tools/test/local-fs-package-files.js';
 import {FirestoreRepository} from '../../lib/firestore/firestore-repository.js';
-import * as path from 'path';
 import {
+  isReadablePackage,
   isReadablePackageVersion,
+  ReadablePackageInfo,
+  ReadablePackageVersion,
   VersionStatus,
 } from '@webcomponents/catalog-api/lib/schema.js';
 import {fileURLToPath} from 'url';
+import {Temporal} from '@js-temporal/polyfill';
 
 const test = suite('Catalog tests');
 
@@ -35,7 +38,16 @@ test('Imports a package with no problems', async () => {
   });
   const repository = new FirestoreRepository('catalog-test-1');
   const catalog = new Catalog({files, repository});
-  await catalog.importPackage(packageName);
+  const importResult = await catalog.importPackage(packageName);
+
+  assert.equal(isReadablePackage(importResult.packageInfo), true);
+  const packageInfo = importResult.packageInfo as ReadablePackageInfo;
+
+  // Check for dist-tags
+  assert.equal(
+    packageInfo.distTags.find((t) => t.tag === 'latest')?.version,
+    '0.0.0'
+  );
 
   // If there were validation problems, they will be on the package version,
   // so read that and check:
@@ -50,6 +62,28 @@ test('Imports a package with no problems', async () => {
   // assert.equal(problems?.length, 0);
 });
 
+test('A second import does nothing', async () => {
+  const packageName = 'test-1';
+  const files = new LocalFsPackageFiles({
+    path: testPackage1Path,
+    packageName,
+    publishedVersions: ['0.0.0'],
+    distTags: {
+      latest: '0.0.0',
+    },
+  });
+  // This must use the same namespace as in the previous test
+  const repository = new FirestoreRepository('catalog-test-1');
+  const catalog = new Catalog({files, repository});
+  const importResult = await catalog.importPackage(packageName);
+
+  // importPackage() returns an empty object if there was no import to
+  // be performed.
+  assert.equal(importResult.packageInfo, undefined);
+  assert.equal(importResult.packageVersion, undefined);
+  assert.equal(importResult.problems, undefined);
+});
+
 test('Gets package version data from imported package', async () => {
   const packageName = 'test-1';
   const version = '0.0.0';
@@ -96,7 +130,45 @@ test('Gets package version data from imported package', async () => {
   // assert.equal(problems?.length, 0);
 });
 
-// TODO: add a test the same as the first to make sure we handle a
-// import request for an existing package.
+test('Updates a package', async () => {
+  const packageName = 'test-1';
+  const files = new LocalFsPackageFiles({
+    path: testPackage1Path,
+    packageName,
+    publishedVersions: ['0.0.0', '1.0.0'],
+    distTags: {
+      latest: '1.0.0',
+    },
+  });
+
+  // This must use the same namespace as in the first (import) test
+  const repository = new FirestoreRepository('catalog-test-1');
+  const catalog = new Catalog({files, repository});
+  const importResult = await catalog.importPackage(
+    packageName,
+    Temporal.Duration.from({minutes: 0})
+  );
+
+  assert.ok(importResult.packageInfo);
+  assert.ok(importResult.packageVersion);
+  assert.equal(isReadablePackageVersion(importResult.packageVersion), true);
+
+  const packageInfo = importResult.packageInfo as ReadablePackageInfo;
+  const packageVersion = importResult.packageVersion as ReadablePackageVersion;
+
+  // Check for dist-tags
+  assert.equal(
+    packageInfo.distTags.find((t) => t.tag === 'latest')?.version,
+    '1.0.0'
+  );
+  assert.equal(
+    packageVersion.distTags.find((t) => t === 'latest'),
+    'latest'
+  );
+
+  // Make sure we can get the new version with 'latest'
+  const result = await catalog.getPackageVersion(packageName, 'latest');
+  assert.equal(isReadablePackageVersion(result), true);
+});
 
 test.run();

packages/catalog-server/src/test/lib/npm_test.ts

@@ -0,0 +1,32 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {suite} from 'uvu';
+import * as assert from 'uvu/assert';
+import {distTagListToMap, distTagMapToList} from '../../lib/npm.js';
+
+const test = suite('npm tests');
+
+test('distTagMapToList', () => {
+  const list = distTagMapToList({latest: '1.0.0', beta: '2.0.0'});
+  assert.equal(list.length, 2);
+  assert.equal(list[0]!.tag, 'latest');
+  assert.equal(list[0]!.version, '1.0.0');
+  assert.equal(list[1]!.tag, 'beta');
+  assert.equal(list[1]!.version, '2.0.0');
+});
+
+test('distTagListToMap', () => {
+  const map = distTagListToMap([
+    {tag: 'latest', version: '1.0.0'},
+    {tag: 'beta', version: '2.0.0'},
+  ]);
+  assert.equal(Object.entries(map).length, 2);
+  assert.equal(map['latest'], '1.0.0');
+  assert.equal(map['beta'], '2.0.0');
+});
+
+test.run();

packages/catalog-server/test/test-packages/test-1/1.0.0/custom-elements.json

@@ -0,0 +1,36 @@
+{
+  "schemaVersion": "1.0.0",
+  "modules": [
+    {
+      "kind": "javascript-module",
+      "path": "foo.js",
+      "exports": [
+        {
+          "kind": "js",
+          "name": "FooElement",
+          "declaration": {
+            "name": "FooElement"
+          }
+        },
+        {
+          "kind": "custom-element-definition",
+          "name": "foo-element",
+          "declaration": {
+            "name": "FooElement"
+          }
+        }
+      ],
+      "declarations": [
+        {
+          "kind": "class",
+          "customElement": true,
+          "tagName": "my-element",
+          "name": "FooElement",
+          "superclass": {
+            "name": "HTMLElement"
+          }
+        }
+      ]
+    }
+  ]
+}

packages/catalog-server/test/test-packages/test-1/1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "test-1",
+  "version": "1.0.0",
+  "customElements": "custom-elements.json"
+}