diff --git a/.github/workflows/chatgpt-cr.yml b/.github/workflows/chatgpt-cr.yml new file mode 100644 index 00000000..0b605d08 --- /dev/null +++ b/.github/workflows/chatgpt-cr.yml @@ -0,0 +1,23 @@ +name: 🤖 ChatGPT Code Review + +permissions: + contents: read + pull-requests: write + +on: + pull_request: + types: [opened, reopened, synchronize] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: anc95/ChatGPT-CodeReview@main + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + # Optional + LANGUAGE: Chinese + MODEL: + top_p: 1 + temperature: 1 diff --git a/app/port/controller/package/RemovePackageVersionController.ts b/app/port/controller/package/RemovePackageVersionController.ts index cbd943a0..5b0b09ba 100644 --- a/app/port/controller/package/RemovePackageVersionController.ts +++ b/app/port/controller/package/RemovePackageVersionController.ts @@ -14,6 +14,8 @@ import { import { AbstractController } from '../AbstractController'; import { FULLNAME_REG_STRING } from '../../../common/PackageUtil'; import { PackageManagerService } from '../../../core/service/PackageManagerService'; +import { Package } from '../../../core/entity/Package'; +import { PackageVersion } from '../../../core/entity/PackageVersion'; @HTTPController() export class RemovePackageVersionController extends AbstractController { @@ -21,14 +23,19 @@ export class RemovePackageVersionController extends AbstractController { private packageManagerService: PackageManagerService; // https://github.com/npm/cli/blob/latest/lib/commands/unpublish.js#L101 - // https://github.com/npm/libnpmpublish/blob/main/unpublish.js#L43 + // https://github.com/npm/libnpmpublish/blob/main/unpublish.js#L84 + // await npmFetch(`${tarballUrl}/-rev/${_rev}`, { + // ...opts, + // method: 'DELETE', + // ignoreBody: true, + // }) @HTTPMethod({ // DELETE /@cnpm/foo/-/foo-4.0.0.tgz/-rev/61af62d6295fcbd9f8f1c08f // DELETE /:fullname/-/:filenameWithVersion.tgz/-rev/:rev path: `/:fullname(${FULLNAME_REG_STRING})/-/:filenameWithVersion.tgz/-rev/:rev`, method: HTTPMethodEnum.DELETE, }) - async remove(@Context() ctx: EggContext, @HTTPParam() fullname: string, @HTTPParam() filenameWithVersion: string) { + async removeByTarballUrl(@Context() ctx: EggContext, @HTTPParam() fullname: string, @HTTPParam() filenameWithVersion: string) { const npmCommand = ctx.get('npm-command'); if (npmCommand !== 'unpublish') { throw new BadRequestError('Only allow "unpublish" npm-command'); @@ -37,14 +44,54 @@ export class RemovePackageVersionController extends AbstractController { const pkg = ensureRes.pkg!; const version = this.getAndCheckVersionFromFilename(ctx, fullname, filenameWithVersion); const packageVersion = await this.getPackageVersionEntity(pkg, version); + await this.#removePackageVersion(pkg, packageVersion); + return { ok: true }; + } + + // https://github.com/npm/libnpmpublish/blob/main/unpublish.js#L43 + // npm http fetch DELETE 404 http://localhost:62649/@cnpm%2ffoo/-rev/1-642f6e8b52d7b8eb03aef23f + // await npmFetch(`${pkgUri}/-rev/${pkg._rev}`, { + // ...opts, + // method: 'DELETE', + // ignoreBody: true, + // }) + @HTTPMethod({ + // DELETE /@cnpm/foo/-rev/61af62d6295fcbd9f8f1c08f + // DELETE /:fullname/-rev/:rev + path: `/:fullname(${FULLNAME_REG_STRING})/-rev/:rev`, + method: HTTPMethodEnum.DELETE, + }) + async removeByPkgUri(@Context() ctx: EggContext, @HTTPParam() fullname: string) { + const npmCommand = ctx.get('npm-command'); + if (npmCommand !== 'unpublish') { + throw new BadRequestError('Only allow "unpublish" npm-command'); + } + const ensureRes = await this.ensurePublishAccess(ctx, fullname, true); + const pkg = ensureRes.pkg!; + // try to remove the latest version first + const packageTag = await this.packageRepository.findPackageTag(pkg.packageId, 'latest'); + let packageVersion: PackageVersion | null = null; + if (packageTag) { + packageVersion = await this.packageRepository.findPackageVersion(pkg.packageId, packageTag.version); + } + if (packageVersion) { + await this.#removePackageVersion(pkg, packageVersion); + } else { + this.logger.info('[PackageController:unpublishPackage] %s, packageId: %s', + pkg.fullname, pkg.packageId); + await this.packageManagerService.unpublishPackage(pkg); + } + return { ok: true }; + } + + async #removePackageVersion(pkg: Package, packageVersion: PackageVersion) { // https://docs.npmjs.com/policies/unpublish // can unpublish anytime within the first 72 hours after publishing if (pkg.isPrivate && Date.now() - packageVersion.publishTime.getTime() >= 3600000 * 72) { - throw new ForbiddenError(`${pkg.fullname}@${version} unpublish is not allowed after 72 hours of released`); + throw new ForbiddenError(`${pkg.fullname}@${packageVersion.version} unpublish is not allowed after 72 hours of released`); } - ctx.logger.info('[PackageController:removeVersion] %s@%s, packageVersionId: %s', - pkg.fullname, version, packageVersion.packageVersionId); + this.logger.info('[PackageController:removeVersion] %s@%s, packageVersionId: %s', + pkg.fullname, packageVersion.version, packageVersion.packageVersionId); await this.packageManagerService.removePackageVersion(pkg, packageVersion); - return { ok: true }; } } diff --git a/test/cli/npm/install.test.ts b/test/cli/npm/install.test.ts index 5ede5e04..e15956da 100644 --- a/test/cli/npm/install.test.ts +++ b/test/cli/npm/install.test.ts @@ -50,6 +50,18 @@ describe('test/cli/npm/install.test.ts', () => { .debug() .expect('code', 0) .end(); + await coffee + .spawn('npm', [ + 'publish', + `--registry=${registry}`, + `--userconfig=${userconfig}`, + `--cache=${cacheDir}`, + ], { + cwd: TestUtil.getFixtures('@cnpm/foo-2.0.0'), + }) + .debug() + .expect('code', 0) + .end(); }); describe('npm install', () => { @@ -82,7 +94,7 @@ describe('test/cli/npm/install.test.ts', () => { cwd: demoDir, }) .debug() - .expect('stdout', /\/@cnpm\/foo\/\-\/foo-1.0.0.tgz/) + .expect('stdout', /\/@cnpm\/foo\/\-\/foo-2.0.0.tgz/) .expect('code', 0) .end(); @@ -99,7 +111,7 @@ describe('test/cli/npm/install.test.ts', () => { cwd: demoDir, }) .debug() - .expect('stdout', /latest: 1\.0\.0/) + .expect('stdout', /latest: 2\.0\.0/) .expect('code', 0) .end(); @@ -124,11 +136,27 @@ describe('test/cli/npm/install.test.ts', () => { .spawn('npm', [ 'unpublish', '-f', - '@cnpm/foo', + '@cnpm/foo@1.0.0', `--registry=${registry}`, `--userconfig=${userconfig}`, `--cache=${cacheDir}`, - // '--json', + '--verbose', + ], { + cwd: demoDir, + }) + .debug() + .expect('stdout', /\- \@cnpm\/foo/) + .expect('code', 0) + .end(); + await coffee + .spawn('npm', [ + 'unpublish', + '-f', + '@cnpm/foo@2.0.0', + `--registry=${registry}`, + `--userconfig=${userconfig}`, + `--cache=${cacheDir}`, + '--verbose', ], { cwd: demoDir, }) @@ -136,6 +164,10 @@ describe('test/cli/npm/install.test.ts', () => { .expect('stdout', /\- \@cnpm\/foo/) .expect('code', 0) .end(); + const res = await app.httpclient.request(`${registry}/@cnpm%2ffoo`, { dataType: 'json' }); + assert.equal(res.status, 200); + assert(res.data.time.unpublished); + assert.equal(res.data.versions, undefined); }); }); }); diff --git a/test/fixtures/@cnpm/foo-2.0.0/package.json b/test/fixtures/@cnpm/foo-2.0.0/package.json new file mode 100644 index 00000000..e4a75c30 --- /dev/null +++ b/test/fixtures/@cnpm/foo-2.0.0/package.json @@ -0,0 +1,11 @@ +{ + "name": "@cnpm/foo", + "version": "2.0.0", + "description": "cnpmcore local test package", + "main": "index.js", + "scripts": { + "test": "echo \"hello\"" + }, + "author": "", + "license": "MIT" +} diff --git a/test/port/controller/package/ShowPackageController.test.ts b/test/port/controller/package/ShowPackageController.test.ts index 64a69269..d077b14e 100644 --- a/test/port/controller/package/ShowPackageController.test.ts +++ b/test/port/controller/package/ShowPackageController.test.ts @@ -820,7 +820,7 @@ describe('test/port/controller/package/ShowPackageController.test.ts', () => { .set('user-agent', publisher.ua + ' node/16.0.0') .set('Accept', 'application/vnd.npm.install-v1+json'); assert(res.status === 404); - app.expectLog('[middleware:ErrorHandler][syncPackage] create sync package'); + // app.expectLog('[middleware:ErrorHandler][syncPackage] create sync package'); }); it('should redirect public non-scope package to source registry if package not exists when redirectNotFound=true', async () => {