🐛 FIX: Should sync package when registry id is null (#324)

app/common/adapter/NPMRegistry.ts

@@ -9,7 +9,7 @@ import {
   EggContextHttpClient,
   EggAppConfig,
 } from 'egg';
-import { HttpMethod } from 'urllib';
+import { HttpMethod } from 'urllib/src/Request';
 
 const INSTANCE_NAME = 'npmRegistry';
 

app/core/service/PackageSyncerService.ts

@@ -70,7 +70,7 @@ export class PackageSyncerService extends AbstractService {
     const [ scope, name ] = getScopeAndName(fullname);
     const pkg = await this.packageRepository.findPackage(scope, name);
     // sync task request registry is not same as package registry
-    if (pkg && options?.registryId) {
+    if (pkg && pkg.registryId && options?.registryId) {
       if (pkg.registryId !== options.registryId) {
         throw new RegistryNotMatchError(`package ${fullname} is not in registry ${options.registryId}`);
       }

package.json

@@ -41,8 +41,8 @@
     "lint": "eslint . --ext .ts",
     "lint:fix": "eslint . --ext .ts --fix",
     "test": "npm run lint:fix && npm run test-local",
-    "prepare-database": "ts-node test/prepare.ts",
-    "test-local": "npm run prepare-database && egg-bin test -r tsconfig-paths/register --full-trace --reporter dot",
+    "prepare-database": "rm -rf dist && ts-node test/prepare.ts",
+    "test-local": "npm run prepare-database && egg-bin test -r tsconfig-paths/register --full-trace",
     "t": "npm run lint:fix && npm run prepare-database && egg-bin test --changed -r tsconfig-paths/register --full-trace --reporter dot",
     "cov": "npm run prepare-database && egg-bin cov -r tsconfig-paths/register --full-trace --reporter dot",
     "ci": "npm run lint && npm run cov",
@@ -72,8 +72,8 @@
     "@eggjs/tegg-eventbus-plugin": "^1.3.2",
     "@eggjs/tegg-orm-decorator": "^1.4.0",
     "@eggjs/tegg-orm-plugin": "^2.1.0",
-    "@eggjs/tegg-schedule-plugin": "^2.2.0",
     "@eggjs/tegg-plugin": "^1.3.2",
+    "@eggjs/tegg-schedule-plugin": "^2.2.0",
     "@eggjs/tsconfig": "^1.0.0",
     "@node-rs/crc32": "^1.2.2",
     "@sinclair/typebox": "^0.23.0",
@@ -98,6 +98,7 @@
     "semver": "^7.3.5",
     "ssri": "^8.0.1",
     "type-fest": "^2.5.3",
+    "urllib": "^3.3.0",
     "validate-npm-package-name": "^3.0.0"
   },
   "devDependencies": {
@@ -106,7 +107,7 @@
     "@types/semver": "^7.3.12",
     "coffee": "^5.4.0",
     "egg-bin": "^4.16.4",
-    "egg-mock": "^4.2.0",
+    "egg-mock": "^5.0.1",
     "eslint": "^7.32.0",
     "eslint-config-egg": "^9.0.0",
     "git-contributor": "^1.0.10",

test/.setup.ts

@@ -8,6 +8,6 @@ beforeEach(async () => {
 });
 
 afterEach(async () => {
-  mock.restore();
   await TestUtil.truncateDatabase();
+  await mock.restore();
 });

test/common/UserUtil.test.ts

@@ -4,7 +4,7 @@ import { randomToken, checkToken } from '../../app/common/UserUtil';
 describe('test/common/UserUtil.test.ts', () => {
   describe('randomToken()', () => {
     it('should work', () => {
-      for (let i = 0; i < 200000; i++) {
+      for (let i = 0; i < 2000; i++) {
         const token = randomToken('cnpm');
         assert.match(token, /cnpm_\w{31,33}_\w{4,6}/);
         assert(checkToken(token, 'cnpm'));

test/common/adapter/CacheAdapter.test.ts

@@ -20,7 +20,7 @@ describe('test/common/adapter/CacheAdapter.test.ts', () => {
       assert(!lockId2);
       const lockId3 = await cache.lock('unittest', 1);
       assert(!lockId3);
-      await setTimeout(1500);
+      await setTimeout(1100);
       // lock timeout
       const lockId4 = await cache.lock('unittest', 1);
       assert(lockId4);

test/core/service/ChangesStreamService.test.ts

@@ -1,4 +1,4 @@
-import assert = require('assert');
+import assert = require('assert/strict');
 import { Readable } from 'node:stream';
 import { app, mock } from 'egg-mock/bootstrap';
 import { Context } from 'egg';
@@ -194,5 +194,4 @@ describe('test/core/service/ChangesStreamService.test.ts', () => {
       assert(task.data.since === '3');
     });
   });
-
 });

test/core/service/HookTriggerService.test.ts

@@ -14,7 +14,7 @@ import { CreateHookTriggerService } from '../../../app/core/service/CreateHookTr
 import { TaskRepository } from '../../../app/repository/TaskRepository';
 import { Hook } from '../../../app/core/entity/Hook';
 import { HookTriggerService } from '../../../app/core/service/HookTriggerService';
-import { RequestOptions } from 'urllib';
+import { RequestOptions } from 'urllib/src/Request';
 
 describe('test/core/service/HookTriggerService.test.ts', () => {
   let ctx: Context;

test/core/service/PackageSyncerService/createTask.test.ts

@@ -1,5 +1,5 @@
 import assert = require('assert');
-import { app } from 'egg-mock/bootstrap';
+import { app, mock } from 'egg-mock/bootstrap';
 import { Context } from 'egg';
 import { PackageSyncerService } from '../../../../app/core/service/PackageSyncerService';
 import { TestUtil } from '../../../TestUtil';
@@ -17,6 +17,7 @@ describe('test/core/service/PackageSyncerService/createTask.test.ts', () => {
     await TestUtil.createPackage({
       name: pkgName,
       registryId: 'mock_registry_id',
+      isPrivate: false,
     }, {
       name: username,
     });
@@ -33,4 +34,26 @@ describe('test/core/service/PackageSyncerService/createTask.test.ts', () => {
       });
     }, /package @cnpmcore\/foo is not in registry sync_registry_id/);
   });
+
+  it('should work when registryId is null', async () => {
+    mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
+    await TestUtil.createPackage({
+      name: 'binary-mirror-config',
+      isPrivate: false,
+    }, {
+      name: username,
+    });
+
+    const task = await packageSyncerService.createTask('binary-mirror-config', {
+      registryId: 'sync_registry_id',
+    });
+    assert(task);
+  });
+
+  it('should work when pkg not exists', async () => {
+    const task = await packageSyncerService.createTask('binary-mirror-config-not-exists', {
+      registryId: 'sync_registry_id',
+    });
+    assert(task);
+  });
 });

test/core/service/PackageSyncerService/executeTask.test.ts

@@ -1354,11 +1354,13 @@ describe('test/core/service/PackageSyncerService/executeTask.test.ts', () => {
       });
 
       it('should sync from target registry & default registry', async () => {
-        app.mockHttpclient(/https:\/\/custom\.npmjs\.com/, 'GET', () => {
-          throw new Error('mock error');
+        app.mockHttpclient(/https:\/\/custom\.npmjs\.com/, 'GET', {
+          status: 500,
+          data: 'mock error',
         });
-        app.mockHttpclient(/https:\/\/default\.npmjs\.com/, 'GET', () => {
-          throw new Error('mock error');
+        app.mockHttpclient(/https:\/\/default\.npmjs\.com/, 'GET', {
+          status: 500,
+          data: 'mock error',
         });
 
         await packageSyncerService.createTask('cnpm-pkg', { registryId: registry.registryId });

test/schedule/ChangesStreamWorker.test.ts

@@ -50,16 +50,6 @@ describe('test/schedule/ChangesStreamWorker.test.ts', () => {
     const result = await taskService.retryExecuteTimeoutTasks();
     assert(result.processing === 1);
     assert(result.waiting === 0);
-    // mock request https://replicate.npmjs.com/_changes error
-    app.mockHttpclient(/https:\/\/replicate.npmjs.com\/_changes/, () => {
-      throw new Error('mock request replicate _changes error');
-    });
-    app.mockHttpclient(/https:\/\/r.cnpmjs.org\/_changes/, () => {
-      throw new Error('mock request replicate _changes error');
-    });
-    await app.runSchedule(ChangesStreamWorkerPath);
-    app.expectLog('[ChangesStreamService.executeTask:error]');
-    app.expectLog('mock request replicate _changes error');
   });
 
   it('should work on replicate: r.cnpmjs.org', async () => {
@@ -94,28 +84,27 @@ describe('test/schedule/ChangesStreamWorker.test.ts', () => {
     assert(result.processing === 1);
     assert(result.waiting === 0);
     // mock request https://r.cnpmjs.org/_changes error
-    app.mockHttpclient(/https:\/\/r\.cnpmjs\.org\/_changes/, () => {
-      throw new Error('mock request replicate r.cnpmjs.org/_changes error');
+    app.mockHttpclient(/\/_changes/, {
+      status: 500,
+      data: 'mock request replicate /_changes error',
     });
     await app.runSchedule(ChangesStreamWorkerPath);
     app.expectLog('[ChangesStreamService.executeTask:error]');
-    app.expectLog('mock request replicate r.cnpmjs.org/_changes error');
+    app.expectLog('mock request replicate /_changes error');
   });
 
   it('should mock get update_seq error', async () => {
     app.mockLog();
     mock(app.config.cnpmcore, 'syncMode', 'all');
     mock(app.config.cnpmcore, 'enableChangesStream', true);
     mock(app.config.cnpmcore, 'changesStreamRegistry', 'https://r.cnpmjs.org');
-    app.mockHttpclient(/https:\/\/replicate\.npmjs\.com\//, () => {
-      throw new Error('mock request replicate.npmjs.com error');
-    });
-    app.mockHttpclient(/https:\/\/r\.cnpmjs\.org\//, () => {
-      throw new Error('mock request replicate.npmjs.com error');
+    app.mockHttpclient(/https:\/\/r\.cnpmjs\.org\//, 'GET', {
+      status: 500,
+      data: 'mock request changes stream error',
     });
     await app.runSchedule(ChangesStreamWorkerPath);
     app.expectLog('[ChangesStreamWorker:start]');
     app.expectLog('[ChangesStreamService.executeTask:error]');
-    app.expectLog('mock request replicate.npmjs.com error');
+    app.expectLog('mock request changes stream error');
   });
 });