refactor: improve versions and dist-tag filters (#2650)

* refactor: improve versions and dist-tag filters

* chore: restore this later

* improve documentation of dis-tag normalizer

* chore: add changeset

.changeset/three-moles-drop.md

@@ -0,0 +1,7 @@
+---
+'@verdaccio/api': minor
+'@verdaccio/store': minor
+'@verdaccio/utils': minor
+---
+
+refactor: improve versions and dist-tag filters

package.json

@@ -112,7 +112,7 @@
     "docker": "docker build -t verdaccio/verdaccio:local . --no-cache",
     "format": "prettier --write \"**/*.{js,jsx,ts,tsx,json,yml,yaml,md}\"",
     "format:check": "prettier --check \"**/*.{js,jsx,ts,tsx,json,yml,yaml,md}\"",
-    "lint": "eslint  --max-warnings 46 \"**/*.{js,jsx,ts,tsx}\"",
+    "lint": "eslint  --max-warnings 47 \"**/*.{js,jsx,ts,tsx}\"",
     "test": "pnpm recursive test --filter ./packages",
     "test:e2e:cli": "pnpm test --filter ...@verdaccio/e2e-cli",
     "test:e2e:ui": "pnpm test --filter ...@verdaccio/e2e-ui",

packages/api/src/package.ts

@@ -44,6 +44,13 @@ export default function (route: Router, auth: IAuth, storage: Storage, config: C
     function (req: $RequestExtend, _res: $ResponseExtend, next: $NextFunctionVer): void {
       debug('init package by version');
       const name = req.params.package;
+      let queryVersion = req.params.version;
+      const requestOptions = {
+        protocol: req.protocol,
+        headers: req.headers as any,
+        // FIXME: if we migrate to req.hostname, the port is not longer included.
+        host: req.host,
+      };
       const getPackageMetaCallback = function (err, metadata: Package): void {
         if (err) {
           debug('error on fetch metadata for %o with error %o', name, err.message);
@@ -52,18 +59,17 @@ export default function (route: Router, auth: IAuth, storage: Storage, config: C
         debug('convert dist remote to local with prefix %o', config?.url_prefix);
         metadata = convertDistRemoteToLocalTarballUrls(
           metadata,
-          { protocol: req.protocol, headers: req.headers as any, host: req.host },
+          requestOptions,
           config?.url_prefix
         );
 
-        let queryVersion = req.params.version;
         debug('query by param version: %o', queryVersion);
         if (_.isNil(queryVersion)) {
           debug('param %o version found', queryVersion);
           return next(metadata);
         }
 
-        let version = getVersion(metadata, queryVersion);
+        let version = getVersion(metadata.versions, queryVersion);
         debug('query by latest version %o and result %o', queryVersion, version);
         if (_.isNil(version) === false) {
           debug('latest version found %o', version);
@@ -74,7 +80,7 @@ export default function (route: Router, auth: IAuth, storage: Storage, config: C
           if (_.isNil(metadata[DIST_TAGS][queryVersion]) === false) {
             queryVersion = metadata[DIST_TAGS][queryVersion];
             debug('dist-tag version found %o', queryVersion);
-            version = getVersion(metadata, queryVersion);
+            version = getVersion(metadata.versions, queryVersion);
             if (_.isNil(version) === false) {
               debug('dist-tag found %o', version);
               return next(version);

packages/store/src/storage-utils.ts

@@ -55,9 +55,7 @@ export function normalizePackage(pkg: Package): Package {
   }
 
   // normalize dist-tags
-  normalizeDistTags(pkg);
+  return normalizeDistTags(pkg);
-
-  return pkg;
 }
 
 export function generateRevision(rev: string): string {

packages/store/src/storage.ts

@@ -378,7 +378,7 @@ class Storage {
             return options.callback(err);
           }
 
-          normalizeDistTags(cleanUpLinksRef(result, options?.keepUpLinkData));
+          result = normalizeDistTags(cleanUpLinksRef(result, options?.keepUpLinkData));
 
           // npm can throw if this field doesn't exist
           result._attachments = {};

packages/utils/src/index.ts

@@ -3,3 +3,4 @@ export * from './utils';
 export * from './crypto-utils';
 export * from './replace-lodash';
 export * from './matcher';
+export * from './versions';

packages/utils/src/utils.ts

@@ -1,9 +1,8 @@
 import assert from 'assert';
 import _ from 'lodash';
-import semver from 'semver';
 
 import { DEFAULT_USER, DIST_TAGS } from '@verdaccio/core';
-import { Author, Package, Version } from '@verdaccio/types';
+import { Author, Package } from '@verdaccio/types';
 
 import { stringToMD5 } from './crypto-utils';
 
@@ -97,87 +96,6 @@ export function validateMetadata(object: Package, name: string): Package {
   return object;
 }
 
-/**
- * Gets version from a package object taking into account semver weirdness.
- * @return {String} return the semantic version of a package
- */
-export function getVersion(pkg: Package, version: any): Version | void {
-  // this condition must allow cast
-  if (_.isNil(pkg.versions[version]) === false) {
-    return pkg.versions[version];
-  }
-
-  try {
-    version = semver.parse(version, true);
-    for (const versionItem in pkg.versions) {
-      if (version.compare(semver.parse(versionItem, true)) === 0) {
-        return pkg.versions[versionItem];
-      }
-    }
-  } catch (err: any) {
-    return undefined;
-  }
-}
-
-/**
- * Function filters out bad semver versions and sorts the array.
- * @return {Array} sorted Array
- */
-export function semverSort(listVersions: string[] /* logger */): string[] {
-  return (
-    listVersions
-      .filter(function (x): boolean {
-        if (!semver.parse(x, true)) {
-          // FIXME: logger is always undefined
-          // logger.warn({ ver: x }, 'ignoring bad version @{ver}');
-          return false;
-        }
-        return true;
-      })
-      // FIXME: it seems the @types/semver do not handle a legitimate method named 'compareLoose'
-      // @ts-ignore
-      .sort(semver.compareLoose)
-      .map(String)
-  );
-}
-
-/**
- * Flatten arrays of tags.
- * @param {*} data
- */
-export function normalizeDistTags(pkg: Package): void {
-  let sorted;
-  if (!pkg[DIST_TAGS].latest) {
-    // overwrite latest with highest known version based on semver sort
-    sorted = semverSort(Object.keys(pkg.versions));
-    if (sorted?.length) {
-      pkg[DIST_TAGS].latest = sorted.pop();
-    }
-  }
-
-  for (const tag in pkg[DIST_TAGS]) {
-    if (_.isArray(pkg[DIST_TAGS][tag])) {
-      if (pkg[DIST_TAGS][tag].length) {
-        // sort array
-        // FIXME: this is clearly wrong, we need to research why this is like this.
-        // @ts-ignore
-        sorted = semverSort(pkg[DIST_TAGS][tag]);
-        if (sorted.length) {
-          // use highest version based on semver sort
-          pkg[DIST_TAGS][tag] = sorted.pop();
-        }
-      } else {
-        delete pkg[DIST_TAGS][tag];
-      }
-    } else if (_.isString(pkg[DIST_TAGS][tag])) {
-      if (!semver.parse(pkg[DIST_TAGS][tag], true)) {
-        // if the version is invalid, delete the dist-tag entry
-        delete pkg[DIST_TAGS][tag];
-      }
-    }
-  }
-}
-
 export function getLatestVersion(pkgInfo: Package): string {
   return pkgInfo[DIST_TAGS].latest;
 }

packages/utils/src/versions.ts

@@ -0,0 +1,108 @@
+import _ from 'lodash';
+import semver, { SemVer } from 'semver';
+
+import { DIST_TAGS } from '@verdaccio/core';
+import { Package, Version, Versions } from '@verdaccio/types';
+
+/**
+ * Gets version from a package object taking into account semver weirdness.
+ * @return {String} return the semantic version of a package
+ */
+export function getVersion(versions: Versions, version: any): Version | void {
+  if (!versions) {
+    return;
+  }
+
+  // this condition must allow cast
+  if (_.isNil(versions[version]) === false) {
+    return versions[version];
+  }
+
+  const versionSemver: SemVer | null = semver.parse(version, true);
+  if (versionSemver === null) {
+    return;
+  }
+
+  for (const versionItem in versions) {
+    if (Object.prototype.hasOwnProperty.call(versions, versionItem)) {
+      // @ts-ignore
+      if (versionSemver.compare(semver.parse(versionItem, true)) === 0) {
+        return versions[versionItem];
+      }
+    }
+  }
+}
+
+/**
+ * Function filters out bad semver versions and sorts the array.
+ * @return {Array} sorted Array
+ */
+export function sortVersionsAndFilterInvalid(listVersions: string[] /* logger */): string[] {
+  return (
+    listVersions
+      .filter(function (version): boolean {
+        if (!semver.parse(version, true)) {
+          return false;
+        }
+        return true;
+      })
+      // FIXME: it seems the @types/semver do not handle a legitimate method named 'compareLoose'
+      // @ts-ignore
+      .sort(semver.compareLoose)
+      .map(String)
+  );
+}
+
+/**
+ * Normalize dist-tags.
+ *
+ * There is a legacy behaviour where the dist-tags could be an array, in such
+ * case, the array is orderded and the highest version becames the
+ * latest.
+ *
+ * The dist-tag tags must be plain strigs, if the latest is empty (for whatever reason) is
+ * normalized to be the highest version available.
+ *
+ * This function cleans up every invalid version on dist-tags, but does not remove
+ * invalid versions from the manifest.
+ *
+ * @param {*} data
+ */
+export function normalizeDistTags(manifest: Package): Package {
+  let sorted;
+  // handle missing latest dist-tag
+  if (!manifest[DIST_TAGS].latest) {
+    // if there is no latest tag, set the highest known version based on semver sort
+    sorted = sortVersionsAndFilterInvalid(Object.keys(manifest.versions));
+    if (sorted?.length) {
+      // get the highest published version
+      manifest[DIST_TAGS].latest = sorted.pop();
+    }
+  }
+
+  for (const tag in manifest[DIST_TAGS]) {
+    // deprecated (will be removed un future majors)
+    // this should not happen, tags should be plain strings, legacy fallback
+    if (_.isArray(manifest[DIST_TAGS][tag])) {
+      if (manifest[DIST_TAGS][tag].length) {
+        // sort array
+        // FIXME: this is clearly wrong, we need to research why this is like this.
+        // @ts-ignore
+        sorted = sortVersionsAndFilterInvalid(manifest[DIST_TAGS][tag]);
+        if (sorted.length) {
+          // use highest version based on semver sort
+          manifest[DIST_TAGS][tag] = sorted.pop();
+        }
+      } else {
+        delete manifest[DIST_TAGS][tag];
+      }
+    } else if (_.isString(manifest[DIST_TAGS][tag])) {
+      if (!semver.parse(manifest[DIST_TAGS][tag], true)) {
+        // if the version is invalid, delete the dist-tag entry
+        delete manifest[DIST_TAGS][tag];
+      }
+    }
+  }
+
+  return manifest;
+}

packages/utils/test/utils.spec.ts

@@ -74,22 +74,6 @@ describe('Utilities', () => {
       });
     });
 
-    describe('getVersion', () => {
-      test('should get the right version', () => {
-        expect(getVersion(cloneMetadata(), '1.0.0')).toEqual(metadata.versions['1.0.0']);
-        expect(getVersion(cloneMetadata(), 'v1.0.0')).toEqual(metadata.versions['1.0.0']);
-      });
-
-      test('should return nothing on get non existing version', () => {
-        expect(getVersion(cloneMetadata(), '0')).toBeUndefined();
-        expect(getVersion(cloneMetadata(), '2.0.0')).toBeUndefined();
-        expect(getVersion(cloneMetadata(), 'v2.0.0')).toBeUndefined();
-        expect(getVersion(cloneMetadata(), undefined)).toBeUndefined();
-        expect(getVersion(cloneMetadata(), null)).toBeUndefined();
-        expect(getVersion(cloneMetadata(), 2)).toBeUndefined();
-      });
-    });
-
     describe('validateMetadata', () => {
       test('should fills an empty metadata object', () => {
         // intended to fail with flow, do not remove

packages/utils/test/versions.spec.ts

@@ -0,0 +1,125 @@
+import { DIST_TAGS } from '@verdaccio/core';
+import { Package } from '@verdaccio/types';
+
+import { getVersion, normalizeDistTags, sortVersionsAndFilterInvalid } from '../src/index';
+
+describe('Utilities', () => {
+  const dist = (version) => ({
+    tarball: `http://registry.org/npm_test/-/npm_test-${version}.tgz`,
+    shasum: `sha1-${version}`,
+  });
+
+  describe('getVersion', () => {
+    const metadata = {
+      '1.0.0': { dist: dist('1.0.0') },
+      '1.0.1': { dist: dist('1.0.1') },
+      '0.2.1-1': { dist: dist('0.2.1-1') },
+      '0.2.1-alpha': { dist: dist('0.2.1-alpha') },
+      '0.2.1-alpha.0': { dist: dist('0.2.1-alpha.0') },
+    };
+
+    test('should get the right version', () => {
+      expect(getVersion({ ...metadata } as any, '1.0.0')).toEqual({ dist: dist('1.0.0') });
+      expect(getVersion({ ...metadata } as any, 'v1.0.0')).toEqual({ dist: dist('1.0.0') });
+      expect(getVersion({ ...metadata } as any, 'v0.2.1-1')).toEqual({ dist: dist('0.2.1-1') });
+      expect(getVersion({ ...metadata } as any, '0.2.1-alpha')).toEqual({
+        dist: dist('0.2.1-alpha'),
+      });
+      expect(getVersion({ ...metadata } as any, '0.2.1-alpha.0')).toEqual({
+        dist: dist('0.2.1-alpha.0'),
+      });
+    });
+
+    test('should return nothing on get non existing version', () => {
+      expect(getVersion({ ...metadata } as any, '0')).toBeUndefined();
+      expect(getVersion({ ...metadata } as any, '2.0.0')).toBeUndefined();
+      expect(getVersion({ ...metadata } as any, 'v2.0.0')).toBeUndefined();
+    });
+
+    test('should return nothing on get invalid versions', () => {
+      expect(getVersion({ ...metadata } as any, undefined)).toBeUndefined();
+      expect(getVersion({ ...metadata } as any, null)).toBeUndefined();
+      expect(getVersion({ ...metadata } as any, 8)).toBeUndefined();
+    });
+
+    test('should handle no versions', () => {
+      expect(getVersion(undefined, undefined)).toBeUndefined();
+    });
+  });
+
+  describe('semverSort', () => {
+    test('should sort versions', () => {
+      expect(sortVersionsAndFilterInvalid(['1.0.0', '5.0.0', '2.0.0'])).toEqual([
+        '1.0.0',
+        '2.0.0',
+        '5.0.0',
+      ]);
+    });
+    test('should sort versions and filter out invalid', () => {
+      expect(sortVersionsAndFilterInvalid(['1.0.0', '5.0.0', '2.0.0', '', null])).toEqual([
+        '1.0.0',
+        '2.0.0',
+        '5.0.0',
+      ]);
+    });
+  });
+
+  describe('normalizeDistTags', () => {
+    const metadata = {
+      name: 'npm_test',
+      versions: {
+        '1.0.0': { dist: dist('1.0.0') },
+        '1.0.1': { dist: dist('1.0.1') },
+        '0.2.1-1': { dist: dist('0.2.1-1') },
+        '0.2.1-alpha': { dist: dist('0.2.1-alpha') },
+        '0.2.1-alpha.0': { dist: dist('0.2.1-alpha.0') },
+      },
+    };
+    const cloneMetadata: Package | any = (pkg = metadata) => Object.assign({}, pkg);
+
+    describe('tag as arrays [deprecated]', () => {
+      test('should convert any array of dist-tags to a plain string', () => {
+        const pkg = cloneMetadata();
+        pkg[DIST_TAGS] = {
+          latest: ['1.0.1'],
+        };
+
+        expect(normalizeDistTags(pkg)[DIST_TAGS]).toEqual({ latest: '1.0.1' });
+      });
+
+      test('should convert any empty array to empty list of dist-tags', () => {
+        const pkg = cloneMetadata();
+        pkg[DIST_TAGS] = {
+          latest: [],
+        };
+
+        expect(normalizeDistTags(pkg)[DIST_TAGS]).toEqual({});
+      });
+    });
+
+    test('should clean up a invalid latest version', () => {
+      const pkg = cloneMetadata();
+      pkg[DIST_TAGS] = {
+        latest: '20000',
+      };
+
+      expect(Object.keys(normalizeDistTags(pkg)[DIST_TAGS])).toHaveLength(0);
+    });
+
+    test('should handle empty dis-tags and define last published version as latest', () => {
+      const pkg = cloneMetadata();
+      pkg[DIST_TAGS] = {};
+
+      expect(normalizeDistTags(pkg)[DIST_TAGS]).toEqual({ latest: '1.0.1' });
+    });
+
+    test('should define last published version as latest with a custom dist-tag', () => {
+      const pkg = cloneMetadata();
+      pkg[DIST_TAGS] = {
+        beta: '1.0.1',
+      };
+
+      expect(normalizeDistTags(pkg)[DIST_TAGS]).toEqual({ beta: '1.0.1', latest: '1.0.1' });
+    });
+  });
+});