fix: allow to remove the package entity (#437)

closes #435
cnpm · Apr 7, 2023 · 613e0a1 · 613e0a1
1 parent 570d346
commit 613e0a1
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 11 deletions.
diff --git a/.github/workflows/chatgpt-cr.yml 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
@@ -14,21 +14,28 @@ 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 {
   @Inject()
   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
@@ -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,18 +136,38 @@ 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,
         })
         .debug()
         .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
@@ -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
@@ -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 () => {