diff --git a/app/common/adapter/CacheAdapter.ts b/app/common/adapter/CacheAdapter.ts index 5c48404d..b70fecaa 100644 --- a/app/common/adapter/CacheAdapter.ts +++ b/app/common/adapter/CacheAdapter.ts @@ -64,12 +64,13 @@ export class CacheAdapter { async usingLock(key: string, seconds: number, func: () => Promise) { const lockTimestamp = await this.lock(key, seconds); - if (!lockTimestamp) return; + if (!lockTimestamp) return false; try { await func(); } finally { await this.unlock(key, lockTimestamp); } + return true; } private getLockName(key: string) { diff --git a/app/port/controller/package/SavePackageVersionController.ts b/app/port/controller/package/SavePackageVersionController.ts index 3715e92d..369a4381 100644 --- a/app/port/controller/package/SavePackageVersionController.ts +++ b/app/port/controller/package/SavePackageVersionController.ts @@ -3,6 +3,7 @@ import { isEqual } from 'lodash'; import { UnprocessableEntityError, ForbiddenError, + ConflictError, } from 'egg-errors'; import { HTTPController, @@ -28,6 +29,7 @@ import { } from '../../typebox'; import { RegistryManagerService } from '../../../core/service/RegistryManagerService'; import { PackageJSONType } from '../../../repository/PackageRepository'; +import { CacheAdapter } from '../../../common/adapter/CacheAdapter'; const STRICT_CHECK_TARBALL_FIELDS: (keyof PackageJson)[] = [ 'name', 'version', 'scripts', 'dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies', 'license', 'licenses', 'bin' ]; @@ -74,6 +76,9 @@ export class SavePackageVersionController extends AbstractController { @Inject() private readonly registryManagerService: RegistryManagerService; + @Inject() + private readonly cacheAdapter: CacheAdapter; + // https://github.com/cnpm/cnpmjs.org/blob/master/docs/registry-api.md#publish-a-new-package // https://github.com/npm/libnpmpublish/blob/main/publish.js#L43 @HTTPMethod({ @@ -199,20 +204,30 @@ export class SavePackageVersionController extends AbstractController { } const registry = await this.registryManagerService.ensureSelfRegistry(); - const packageVersionEntity = await this.packageManagerService.publish({ - scope, - name, - version: packageVersion.version, - description: packageVersion.description, - packageJson: packageVersion as PackageJSONType, - readme, - dist: { - content: tarballBytes, - }, - tag: tagWithVersion.tag, - registryId: registry.registryId, - isPrivate: true, - }, user); + + let packageVersionEntity; + const lockRes = await this.cacheAdapter.usingLock(`${pkg.name}:publish`, 60, async () => { + packageVersionEntity = await this.packageManagerService.publish({ + scope, + name, + version: packageVersion.version, + description: packageVersion.description as string, + packageJson: packageVersion as PackageJSONType, + readme, + dist: { + content: tarballBytes, + }, + tag: tagWithVersion.tag, + registryId: registry.registryId, + isPrivate: true, + }, user); + }); + + // lock fail + if (!lockRes) { + this.logger.warn('[package:version:add] check lock fail'); + throw new ConflictError('Unable to create the publication lock, please try again later.'); + } this.logger.info('[package:version:add] %s@%s, packageVersionId: %s, tag: %s, userId: %s', packageVersion.name, packageVersion.version, packageVersionEntity.packageVersionId, diff --git a/test/port/controller/package/SavePackageVersionController.test.ts b/test/port/controller/package/SavePackageVersionController.test.ts index 9e660601..cf7525cd 100644 --- a/test/port/controller/package/SavePackageVersionController.test.ts +++ b/test/port/controller/package/SavePackageVersionController.test.ts @@ -1,4 +1,5 @@ import assert from 'assert'; +import { setTimeout } from 'node:timers/promises'; import { app, mock } from 'egg-mock/bootstrap'; import { TestUtil } from '../../../../test/TestUtil'; import { UserRepository } from '../../../../app/repository/UserRepository'; @@ -11,6 +12,7 @@ import { Token as TokenModel } from '../../../../app/repository/model/Token'; import { User } from '../../../../app/core/entity/User'; import dayjs from 'dayjs'; import { PackageManagerService } from '../../../../app/core/service/PackageManagerService'; +import { ForbiddenError } from 'egg-errors'; describe('test/port/controller/package/SavePackageVersionController.test.ts', () => { let userRepository: UserRepository; @@ -86,6 +88,47 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () assert(pkgEntity); assert.equal(pkgEntity.registryId, selfRegistry.registryId); }); + + it('should 409 when lock failed', async () => { + const { pkg, user } = await TestUtil.createPackage({ name: '@cnpm/banana', version: '1.0.0' }); + + const packageManagerService = await app.getEggObject(PackageManagerService); + + mock(packageManagerService, 'publish', async () => { + await setTimeout(50); + throw new ForbiddenError('mock error'); + }); + + const [ errorRes, conflictRes ] = await Promise.all([ + app.httpRequest() + .put(`/${pkg.name}`) + .set('authorization', user.authorization) + .set('user-agent', user.ua) + .send(pkg), + (async () => { + await setTimeout(10); + return app.httpRequest() + .put(`/${pkg.name}`) + .set('authorization', user.authorization) + .set('user-agent', user.ua) + .send(pkg); + })(), + ]); + assert(errorRes.error, '[FORBIDDEN] mock error'); + assert.equal(conflictRes.status, 409); + assert(conflictRes.error, '[CONFLICT] Unable to create the publication lock, please try again later.'); + + // release lock + await setTimeout(50); + const nextErrorRes = await app.httpRequest() + .put(`/${pkg.name}`) + .set('authorization', user.authorization) + .set('user-agent', user.ua) + .send(pkg); + assert(nextErrorRes.error, '[FORBIDDEN] mock error'); + + }); + it('should verify tgz and manifest', async () => { const { pkg, user } = await TestUtil.createPackage({ name: '@cnpm/banana', version: '1.0.0' }); const pkg2 = await TestUtil.getFullPackage({ name: pkg.name, version: '0.0.1' });