Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update source registry #537

Merged
merged 1 commit into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions app/core/service/PackageManagerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,9 @@ export class PackageManagerService extends AbstractService {
};

// add _registry_name field to cmd.packageJson
if (!cmd.packageJson._source_registry_name) {
const registry = await this.getSourceRegistry(pkg);
if (registry) {
cmd.packageJson._source_registry_name = registry.name;
}
const registry = await this.getSourceRegistry(pkg);
if (registry) {
cmd.packageJson._source_registry_name = registry.name;
}

// https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md#abbreviated-version-object
Expand Down Expand Up @@ -832,11 +830,10 @@ export class PackageManagerService extends AbstractService {
await this.bugVersionService.fixPackageBugVersions(bugVersion, fullname, data.versions);
}
// set _source_registry_name in full manifestDist
if (!data._source_registry_name && isFullManifests) {
if (registry) {
data._source_registry_name = registry?.name;
}
if (registry) {
data._source_registry_name = registry?.name;
}

const distBytes = Buffer.from(JSON.stringify(data));
const distIntegrity = await calculateIntegrity(distBytes);
etag = `"${distIntegrity.shasum}"`;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码主要是在 PackageManagerService 类中的一个方法中进行了修改。我将逐步解释每个修改并提出改进建议:

  1. 在第一处修改中,原先的代码是判断 cmd.packageJson._source_registry_name 是否存在,如果不存在则获取 registry 并将其赋值给 _source_registry_name。现在的修改将获取 registry 的部分移到了条件判断之外,并且不再进行存在性的检查,而是直接将获取到的 registry.name 赋值给 cmd.packageJson._source_registry_name

    改进建议:这样的修改使得代码更加简洁和一致。条件判断是否存在 _source_registry_name 可以被移除,这样无论 _source_registry_name 是否存在都会进行赋值操作。在调用 this.getSourceRegistry(pkg) 之前,可以添加对 pkg 是否存在的检查来避免潜在的错误。

  2. 第二处修改与第一处类似,在满足条件的情况下,将 registry?.name 赋值给 data._source_registry_name

    改进建议:与第一处修改相同,可以移除对 _source_registry_name 存在性的检查,直接进行赋值操作。

总体而言,这些修改使代码更简洁、易读,并且保持了一致性。没有明显的错误风险,但仍然可以根据实际需求进一步优化和简化代码。

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', ()
pkgEntity.registryId = '';
await packageRepository.savePackage(pkgEntity!);

res = await app.httpRequest()
.get(`/${pkg.name}`);
assert.equal(res.body._source_registry_name, 'default');

pkg = await TestUtil.getFullPackage({ name, version: '2.0.0' });
res = await app.httpRequest()
.put(`/${pkg.name}`)
Expand All @@ -189,6 +193,10 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', ()

pkgEntity = await packageRepository.findPackage('@cnpm', 'publish-package-test');
assert(pkgEntity?.registryId);

res = await app.httpRequest()
.get(`/${pkg.name}`);
assert.equal(res.body._source_registry_name, 'self');
});

it('should publish on user custom scopes', async () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对于给定的代码补丁,以下是一个简要的代码审查:

  1. 在修改之前:

    • 通过GET请求,在调用savePackage函数之后,断言res.body._source_registry_name等于'default'。
    • 删除了一些没有提供的相关代码,检查是否会对修改产生影响。
  2. 在修改之后:

    • 通过GET请求,在调用findPackage函数之后,断言res.body._source_registry_name等于'self'。
    • 删除了一些没有提供的相关代码,检查是否会对修改产生影响。

改进建议:

  • 添加更多详细的错误处理和异常情况处理逻辑。
  • 根据你的应用需求,可能需要进一步检查其他功能和边界情况的测试。

风险:

  • 没有提供完整的代码和上下文,因此无法全面评估潜在的风险。请确保修改不会破坏现有的功能,并且通过充分的测试覆盖率来验证修改的正确性。

请注意,这只是一个简要的代码审查意见,具体的问题和改进取决于代码的实际运行环境和需求。

Expand Down