fix: Handle {major}.json and {minor}.json symlinks when publishin…

…g older versions (#483) Rewrite symlinking to take previously existing symlinks into account. This way, we can more safely bump the respective symlinks, especially when publishing versions that should not be the latest version (e.g. patching an older major). Added tests to cover the most important + edge case paths. Co-authored-by: Anton Ovchinnikov <anton@tonyo.info> Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
getsentry · Oct 3, 2023 · e391121 · e391121
1 parent 34c4a64
commit e391121
Show file tree

Hide file tree

Showing 4 changed files with 332 additions and 19 deletions.
diff --git a/src/utils/__tests__/symlink.test.ts b/src/utils/__tests__/symlink.test.ts
@@ -0,0 +1,227 @@
+import { createSymlinks } from '../symlink';
+import { withTempDir } from '../files';
+import { promises as fsPromises } from 'fs';
+
+import * as path from 'path';
+import * as fs from 'fs';
+
+describe('createSymlinks', () => {
+  it.each([
+    {
+      _name: 'major version upgrade',
+      oldVersion: '0.9.0',
+      newVersion: '1.0.0',
+      expectedSymlinks: ['1.json', '1.0.json', 'latest.json'],
+    },
+    {
+      _name: 'minor version upgrade',
+      oldVersion: '0.2.3',
+      newVersion: '0.3.0',
+      expectedSymlinks: ['0.json', '0.3.json', 'latest.json'],
+    },
+    {
+      _name: 'patch version upgrade',
+      oldVersion: '0.2.3',
+      newVersion: '0.2.4',
+      expectedSymlinks: ['0.json', '0.2.json', 'latest.json'],
+    },
+    {
+      _name: 'old version is undefined',
+      oldVersion: undefined,
+      newVersion: '1.2.3',
+      expectedSymlinks: ['1.json', '1.2.json', 'latest.json'],
+    },
+    {
+      _name: 'updating older major (patch)',
+      oldVersion: '2.3.4',
+      newVersion: '1.2.3',
+      expectedSymlinks: ['1.json', '1.2.json'],
+    },
+    {
+      _name: 'updating older major (minor)',
+      oldVersion: '2.3.4',
+      newVersion: '1.3.0',
+      expectedSymlinks: ['1.json', '1.3.json'],
+    },
+  ])('$_name', async ({ oldVersion, newVersion, expectedSymlinks }) =>
+    withTempDir(async tmpDir => {
+      const versionFile = `${newVersion}.json`;
+      const versionFilePath = path.join(tmpDir, versionFile);
+
+      createSymlinks(versionFilePath, newVersion, oldVersion);
+
+      // Check that there are no other files in the directory
+      const filesInDir = await fsPromises.readdir(tmpDir);
+      expect(filesInDir.sort()).toStrictEqual(expectedSymlinks.sort());
+
+      for (const symlinkFile of filesInDir) {
+        const symlinkFilePath = path.join(tmpDir, symlinkFile);
+
+        // Make sure it's a symbolic link
+        const fileStat = await fsPromises.lstat(symlinkFilePath);
+        expect(fileStat.isSymbolicLink()).toBe(true);
+
+        // Make sure it points to the right file
+        const symlinkDestination = await fsPromises.readlink(symlinkFilePath);
+        expect(symlinkDestination).toBe(versionFile);
+      }
+    }, true)
+  );
+
+  it('handles updating an old major version', async () =>
+    withTempDir(async tmpDir => {
+      // fill the directory with some "previous" versions and symlinks
+      // (we also need to also the concrete version file to avoid broken symlinks)
+
+      fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0');
+
+      fs.writeFileSync(path.join(tmpDir, '2.0.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '2.0.0.json'), '2.0.0', '1.0.0');
+
+      // now update 1.x (minor)
+      fs.writeFileSync(path.join(tmpDir, '1.5.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.5.0.json'), '1.5.0', '2.0.0');
+
+      // now update a version in between 1.x and 1.5.x
+      fs.writeFileSync(path.join(tmpDir, '1.2.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.2.0.json'), '1.2.0', '2.0.0');
+
+      // now update 1.5.x (patch)
+      fs.writeFileSync(path.join(tmpDir, '1.5.1.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.5.1.json'), '1.5.1', '2.0.0');
+
+      const filesInDir = await fsPromises.readdir(tmpDir);
+      expect(filesInDir.sort()).toStrictEqual(
+        [
+          '1.0.0.json',
+          '1.2.0.json',
+          '1.5.0.json',
+          '1.5.1.json',
+          '2.0.0.json',
+
+          '1.json',
+          '2.json',
+
+          '1.0.json',
+          '1.2.json',
+          '1.5.json',
+          '2.0.json',
+
+          'latest.json',
+        ].sort()
+      );
+
+      const latestLink = await fsPromises.readlink(
+        path.join(tmpDir, 'latest.json')
+      );
+      const major1Link = await fsPromises.readlink(path.join(tmpDir, '1.json'));
+      const major2Link = await fsPromises.readlink(path.join(tmpDir, '2.json'));
+      const minor10Link = await fsPromises.readlink(
+        path.join(tmpDir, '1.0.json')
+      );
+      const minor12Link = await fsPromises.readlink(
+        path.join(tmpDir, '1.2.json')
+      );
+      const minor15Link = await fsPromises.readlink(
+        path.join(tmpDir, '1.5.json')
+      );
+      const minor20Link = await fsPromises.readlink(
+        path.join(tmpDir, '2.0.json')
+      );
+
+      expect(latestLink).toBe('2.0.0.json');
+
+      expect(major1Link).toBe('1.5.1.json');
+      expect(major2Link).toBe('2.0.0.json');
+
+      expect(minor10Link).toBe('1.0.0.json');
+      expect(minor12Link).toBe('1.2.0.json');
+      expect(minor15Link).toBe('1.5.1.json');
+      expect(minor20Link).toBe('2.0.0.json');
+    }, true));
+
+  it('handles updating a previous minor version on the same major', async () =>
+    withTempDir(async tmpDir => {
+      fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0');
+
+      fs.writeFileSync(path.join(tmpDir, '1.1.0.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.1.0.json'), '1.1.0', '1.0.0');
+
+      fs.writeFileSync(path.join(tmpDir, '1.0.1.json'), 'x');
+      createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.1.0');
+
+      const filesInDir = await fsPromises.readdir(tmpDir);
+      expect(filesInDir.sort()).toStrictEqual(
+        [
+          '1.0.0.json',
+          '1.0.1.json',
+          '1.1.0.json',
+
+          '1.json',
+
+          '1.0.json',
+          '1.1.json',
+
+          'latest.json',
+        ].sort()
+      );
+
+      const latestLink = await fsPromises.readlink(
+        path.join(tmpDir, 'latest.json')
+      );
+      const major1Link = await fsPromises.readlink(path.join(tmpDir, '1.json'));
+      const minor10Link = await fsPromises.readlink(
+        path.join(tmpDir, '1.0.json')
+      );
+      const minor11Link = await fsPromises.readlink(
+        path.join(tmpDir, '1.1.json')
+      );
+
+      expect(latestLink).toBe('1.1.0.json');
+      expect(major1Link).toBe('1.1.0.json');
+      expect(minor10Link).toBe('1.0.1.json');
+      expect(minor11Link).toBe('1.1.0.json');
+    }, true));
+});
+
+// This is quite an edge case but nevertheless good to know it's covered:
+it('handles updating a previous patch version on the same minor', async () =>
+  withTempDir(async tmpDir => {
+    fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x');
+    createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0');
+
+    fs.writeFileSync(path.join(tmpDir, '1.0.2.json'), 'x');
+    createSymlinks(path.join(tmpDir, '1.0.2.json'), '1.0.2', '1.0.0');
+
+    fs.writeFileSync(path.join(tmpDir, '1.0.1.json'), 'x');
+    createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.0.2');
+
+    const filesInDir = await fsPromises.readdir(tmpDir);
+    expect(filesInDir.sort()).toStrictEqual(
+      [
+        '1.0.0.json',
+        '1.0.1.json',
+        '1.0.2.json',
+
+        '1.json',
+
+        '1.0.json',
+
+        'latest.json',
+      ].sort()
+    );
+
+    const latestLink = await fsPromises.readlink(
+      path.join(tmpDir, 'latest.json')
+    );
+    const major1Link = await fsPromises.readlink(path.join(tmpDir, '1.json'));
+    const minor10Link = await fsPromises.readlink(
+      path.join(tmpDir, '1.0.json')
+    );
+
+    expect(latestLink).toBe('1.0.2.json');
+    expect(major1Link).toBe('1.0.2.json');
+    expect(minor10Link).toBe('1.0.2.json');
+  }));
diff --git a/src/utils/__tests__/version.test.ts b/src/utils/__tests__/version.test.ts
@@ -7,6 +7,7 @@ import {
   isValidVersion,
   parseVersion,
   SemVer,
+  semVerToString,
   versionGreaterOrEqualThan,
 } from '../version';
 
@@ -202,3 +203,29 @@ describe('getPackageVersion', () => {
     expect(isValidVersion(version)).toBe(true);
   });
 });
+
+describe('semVerToString', () => {
+  test.each([
+    ['basic', { major: 1, minor: 2, patch: 3 }, '1.2.3'],
+    [
+      'with pre-release',
+      { major: 1, minor: 2, patch: 3, pre: 'beta.1' },
+      '1.2.3-beta.1',
+    ],
+    [
+      'with build metadata',
+      { major: 1, minor: 2, patch: 3, build: 'linux' },
+      '1.2.3+linux',
+    ],
+    [
+      'with pre-release and build metadata',
+      { major: 1, minor: 2, patch: 3, pre: 'beta.1', build: 'linux' },
+      '1.2.3-beta.1+linux',
+    ],
+  ])(
+    'converts a SemVer object (%s) to a string',
+    (_, semver, expectedString) => {
+      expect(semVerToString(semver)).toBe(expectedString);
+    }
+  );
+});
diff --git a/src/utils/symlink.ts b/src/utils/symlink.ts
@@ -3,7 +3,12 @@ import * as path from 'path';
 
 import { logger } from '../logger';
 import { ConfigurationError } from './errors';
-import { parseVersion, versionGreaterOrEqualThan } from './version';
+import {
+  SemVer,
+  parseVersion,
+  semVerToString,
+  versionGreaterOrEqualThan,
+} from './version';
 
 /**
  * Creates a symlink, overwriting the existing one
@@ -21,8 +26,9 @@ function forceSymlink(target: string, newFile: string): void {
 /**
  * Create symbolic links to the new version file
  *
- * "latest.json" link is not updated if the new version is "older" (e.g., it's
- * a patch release for an older major version).
+ * "latest.json", "{major}.json" and "{minor}.json" links are respectively not
+ * updated if the new version is "older" (e.g., it's a patch release for an
+ * older major version) than the currently linked versions.
  *
  * @param versionFilePath Path to the new version file
  * @param newVersion The new version
@@ -39,29 +45,73 @@ export function createSymlinks(
   }
   const parsedOldVersion =
     (oldVersion ? parseVersion(oldVersion) : undefined) || undefined;
+
   const baseVersionName = path.basename(versionFilePath);
   const packageDir = path.dirname(versionFilePath);
 
-  // link latest, but only if the new version is "newer"
   if (
-    parsedOldVersion &&
-    !versionGreaterOrEqualThan(parsedNewVersion, parsedOldVersion)
+    !parsedOldVersion ||
+    versionGreaterOrEqualThan(parsedNewVersion, parsedOldVersion)
   ) {
-    logger.warn(
-      `Not updating the latest version file: current version is "${oldVersion}", new version is "${newVersion}"`
-    );
-  } else {
-    logger.debug(
-      `Changing symlink for "latest.json" from version "${oldVersion}" to "${newVersion}"`
-    );
+    logger.debug('Symlink "latest.json"', {
+      before: oldVersion,
+      after: newVersion,
+    });
     forceSymlink(baseVersionName, path.join(packageDir, 'latest.json'));
   }
 
-  // link major
-  const majorVersionLink = `${parsedNewVersion.major}.json`;
-  forceSymlink(baseVersionName, path.join(packageDir, majorVersionLink));
+  // Read possibly existing symlinks for major and minor versions of the new version
+  const existingLinkedMajorVersion = getExistingSymlinkedVersion(
+    path.join(packageDir, `${parsedNewVersion.major}.json`)
+  );
+  const existingLinkedMinorVersion = getExistingSymlinkedVersion(
+    path.join(
+      packageDir,
+      `${parsedNewVersion.major}.${parsedNewVersion.minor}.json`
+    )
+  );
+
+  // link {major}.json if there's no link yet for that major
+  // or if the new version is newer than the currently linked one
+  if (
+    !existingLinkedMajorVersion ||
+    versionGreaterOrEqualThan(parsedNewVersion, existingLinkedMajorVersion)
+  ) {
+    const majorVersionLink = `${parsedNewVersion.major}.json`;
+    logger.debug(`Symlink "${majorVersionLink}"`, {
+      before:
+        existingLinkedMajorVersion &&
+        semVerToString(existingLinkedMajorVersion),
+      after: newVersion,
+    });
+    forceSymlink(baseVersionName, path.join(packageDir, majorVersionLink));
+  }
 
-  // link minor
-  const minorVersionLink = `${parsedNewVersion.major}.${parsedNewVersion.minor}.json`;
-  forceSymlink(baseVersionName, path.join(packageDir, minorVersionLink));
+  // link {minor}.json if there's no link yet for that minor
+  // or if the new version is newer than the currently linked one
+  if (
+    !existingLinkedMinorVersion ||
+    versionGreaterOrEqualThan(parsedNewVersion, existingLinkedMinorVersion)
+  ) {
+    const minorVersionLink = `${parsedNewVersion.major}.${parsedNewVersion.minor}.json`;
+    logger.debug(`Symlink "${minorVersionLink}"`, {
+      before:
+        existingLinkedMinorVersion &&
+        semVerToString(existingLinkedMinorVersion),
+      after: newVersion,
+    });
+    forceSymlink(baseVersionName, path.join(packageDir, minorVersionLink));
+  }
+}
+
+function getExistingSymlinkedVersion(symlinkPath: string): SemVer | null {
+  try {
+    // using lstat instead of exists because broken symlinks return false for exists
+    fs.lstatSync(symlinkPath);
+  } catch {
+    // this means the symlink doesn't exist
+    return null;
+  }
+  const linkedFile = fs.readlinkSync(symlinkPath);
+  return parseVersion(path.basename(linkedFile));
 }
diff --git a/src/utils/version.ts b/src/utils/version.ts
@@ -155,3 +155,12 @@ export function getPackageVersion(): string {
 
   return buildInfo ? `${version} (${buildInfo})` : version;
 }
+
+/**
+ * Returns the stringified version of the passed SemVer object.
+ */
+export function semVerToString(s: SemVer) {
+  return `${s.major}.${s.minor}.${s.patch}${s.pre ? `-${s.pre}` : ''}${
+    s.build ? `+${s.build}` : ''
+  }`;
+}