fix: should redirect when nfs adapter support url #522
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
test/port/controller/package/DownloadPackageVersionTarController.test.ts
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #522 +/- ##
==========================================
- Coverage 97.08% 97.00% -0.08%
==========================================
Files 174 174
Lines 16518 16538 +20
Branches 2160 2162 +2
==========================================
+ Hits 16036 16043 +7
- Misses 482 495 +13
|
| @@ -222,6 +242,7 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. | |||
| .expect(302) | |||
| .expect('location', 'https://registry.npmjs.org/foo/-/foo-1.0.404404.tgz?t=123'); | |||
|
|
|||
| mock(app.config.cnpmcore, 'redirectNotFound', false); | |||
| // not redirect when package exists | |||
| await app.httpRequest() | |||
| .get(`/${name}/-/${name}-1.0.404404.tgz`) | |||
There was a problem hiding this comment.
这段代码是一个测试用例，主要负责测试当包版本不存在时的行为。建议在代码审查中注重以下几点：
-
确认该测试涵盖了所有可能情况
-
检查是否需要提供更多的上下文或者注释来帮助理解该测试
-
评估该测试的可读性和可维护性，包括变量和方法命名的清晰度以及代码格式的一致性
-
确保在修改代码时更新相应的测试用例，并考虑是否需要新增测试用例
在这个具体的代码片段中，测试增加了针对未发布版本的测试，还模拟了 redirectNotFound 和 url 方法。有两个测试用例被添加到测试套件中，一个测试了包版本不存在时的 404 响应，另一个则确保了当指定源注册表时，CNPM 应该将其重定向到该源。
目前代码看起来没有风险和改进的地方。
fengmk2
reviewed
Jun 17, 2023
| @@ -176,6 +177,25 @@ describe('test/port/controller/package/DownloadPackageVersionTarController.test. | |||
| }); | |||
| }); | |||
|
|
|||
| it('should redirect to source registry when package version not exists', async () => { | |||
fengmk2
pushed a commit
that referenced
this pull request
Jun 17, 2023
[skip ci] ## [3.34.2](v3.34.1...v3.34.2) (2023-06-17) ### Bug Fixes * should redirect when nfs adapter support url ([#522](#522)) ([3d6864c](3d6864c))
fengmk2
reviewed
Jun 20, 2023
| @@ -33,15 +33,17 @@ export class DownloadPackageVersionTarController extends AbstractController { | |||
| const version = this.getAndCheckVersionFromFilename(ctx, fullname, filenameWithVersion); | |||
| const storeKey = `/packages/${fullname}/${version}/${filenameWithVersion}.tgz`; | |||
| const downloadUrl = await this.nfsAdapter.getDownloadUrl(storeKey); | |||
| // check package version in database | |||
There was a problem hiding this comment.
这个改动会导致 registry.npmmirror.com 出现大量 db 查询，我先加一个开关
fengmk2
pushed a commit
that referenced
this pull request
Jun 20, 2023
[skip ci] ## [3.34.4](v3.34.3...v3.34.4) (2023-06-20) ### Bug Fixes * avoid db query on sync mode all ([#527](#527)) ([49855d9](49855d9)), closes [/github.com//pull/522/files#r1234655574](https://github.com/cnpm//github.com/cnpm/cnpmcore/pull/522/files/issues/r1234655574)
killagu
reviewed
Jun 20, 2023
| @@ -33,15 +33,17 @@ export class DownloadPackageVersionTarController extends AbstractController { | |||
| const version = this.getAndCheckVersionFromFilename(ctx, fullname, filenameWithVersion); | |||
| const storeKey = `/packages/${fullname}/${version}/${filenameWithVersion}.tgz`; | |||
There was a problem hiding this comment.
原来这里已经有跳过 db 直接访问 oss 的逻辑，计算 semver 逻辑里一样可以跳过了。
本来还单性可能 oss path 发生变化的情况。
hezhengxu2018
pushed a commit
that referenced
this pull request
Jun 20, 2023
[skip ci] ## [3.34.4](v3.34.3...v3.34.4) (2023-06-20) ### Bug Fixes * avoid db query on sync mode all ([#527](#527)) ([49855d9](49855d9)), closes [/github.com//pull/522/files#r1234655574](https://github.com/cnpm//github.com/cnpm/cnpmcore/pull/522/files/issues/r1234655574)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
will check package version on database before redirect to nfs store url
closes #521