fix: should redirect when nfs adapter support url (#522)

will check package version on database before redirect to nfs store url

closes #521

---------

Co-authored-by: fengmk2 <fengmk2@gmail.com>

app/port/controller/AbstractController.ts

@@ -164,7 +164,7 @@ export abstract class AbstractController extends MiddlewareController {
  protected async getPackageVersionEntity(pkg: PackageEntity, version: string): Promise<PackageVersionEntity> {
  const packageVersion = await this.packageRepository.findPackageVersion(pkg.packageId, version);
  if (!packageVersion) {
- throw new NotFoundError(`${pkg.fullname}@${version} not found`);
+ throw this.createPackageNotFoundErrorWithRedirect(pkg.fullname, version);
  }
  return packageVersion;
  }

app/port/controller/package/DownloadPackageVersionTar.ts

@@ -33,15 +33,17 @@ export class DownloadPackageVersionTarController extends AbstractController {
  const version = this.getAndCheckVersionFromFilename(ctx, fullname, filenameWithVersion);
  const storeKey = `/packages/${fullname}/${version}/${filenameWithVersion}.tgz`;
  const downloadUrl = await this.nfsAdapter.getDownloadUrl(storeKey);
+ // check package version in database
+ const pkg = await this.getPackageEntityByFullname(fullname);
+ const packageVersion = await this.getPackageVersionEntity(pkg, version);
+
+ // read by nfs url
  if (downloadUrl) {
  this.packageManagerService.plusPackageVersionCounter(fullname, version);
  ctx.redirect(downloadUrl);
  return;
  }
-
  // read from database
- const pkg = await this.getPackageEntityByFullname(fullname);
- const packageVersion = await this.getPackageVersionEntity(pkg, version);
  ctx.logger.info('[PackageController:downloadVersionTar] %s@%s, packageVersionId: %s',
  pkg.fullname, version, packageVersion.packageVersionId);
  const urlOrStream = await this.packageManagerService.downloadPackageVersionTar(packageVersion);

test/port/controller/package/DownloadPackageVersionTarController.test.ts

@@ -155,6 +155,7 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test.
  });
 
  it('should 404 when package version not exists', async () => {
+ mock(app.config.cnpmcore, 'redirectNotFound', false);
  if (process.env.CNPMCORE_NFS_TYPE === 'oss') {
  mock(nfsClientAdapter, 'url', async () => {
  return undefined;
@@ -176,6 +177,25 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test.
  });
  });
 
+ it('should redirect to source registry when package version not exists', async () => {
+ mock(nfsClientAdapter, 'url', async () => {
+ return 'http://foo.com/foo.tgz';
+ });
+
+ await app.httpRequest()
+ .get(`/${name}/-/${name}-1.0.404404.tgz`)
+ .expect(302)
+ .expect('location', `https://registry.npmjs.org/${name}/-/${name}-1.0.404404.tgz`);
+
+ // not redirect the private package
+ await app.httpRequest()
+ .get(`/${scopedName}/-/${name}-1.0.404404.tgz`)
+ .expect(404)
+ .expect({
+ error: `[NOT_FOUND] ${scopedName}@1.0.404404 not found`,
+ });
+ });
+
  it('should not redirect public package to source registry when syncMode=all', async () => {
  if (process.env.CNPMCORE_NFS_TYPE === 'oss') {
  mock(nfsClientAdapter, 'url', async () => {
@@ -222,6 +242,7 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test.
  .expect(302)
  .expect('location', 'https://registry.npmjs.org/foo/-/foo-1.0.404404.tgz?t=123');
 
+ mock(app.config.cnpmcore, 'redirectNotFound', false);
  // not redirect when package exists
  await app.httpRequest()
  .get(`/${name}/-/${name}-1.0.404404.tgz`)