fix: create sync task by 'GET /:fullname/-/:filenameWithVersion.tgz' #526
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const packageVersion = await this.packageRepository.findPackageVersion(pkg.packageId, version); | ||
| if (!packageVersion) { | ||
| throw this.createPackageNotFoundErrorWithRedirect(pkg.fullname, version); | ||
| throw this.createPackageNotFoundErrorWithRedirect(pkg.fullname, version, allowSync); | ||
| } | ||
| return packageVersion; | ||
| } |
There was a problem hiding this comment.
这段代码通过增加一个新的可选参数 allowSync 来改进三个方法：getPackageEntityByFullname, getPackageEntity 和 getPackageVersionEntity. 这个参数允许控制数据库中检索实体时，是否可以跨过异步执行任务(例如基于 Promise 的查询)来同步获取数据. 在代码变更的详细实现方式看不到前面的某些代码。因此无法评价可能存在的任何其他风险或限制。
| const packageVersion = await this.getPackageVersionEntity(pkg, version); | ||
| const allowSync = this.getAllowSync(ctx); | ||
| const pkg = await this.getPackageEntityByFullname(fullname, allowSync); | ||
| const packageVersion = await this.getPackageVersionEntity(pkg, version, allowSync); | ||
|
|
||
| // read by nfs url | ||
| if (downloadUrl) { |
There was a problem hiding this comment.
这段代码可以进行如下改进：
1.注释修改：代码中的一些注释需要更改。特别是“// try nfs url first, avoid db query”和“// read by nfs url”的注释可能与实际情况不符。
2.异常处理：代码中没有异常处理机制，建议添加适当的异常处理机制。
3.代码可读性提高：代码中存在一些重复的方法调用，建议使用变量来存储结果。还建议在命名参数时使用有意义的名称。
4.潜在 bug 风险：下载 URL 可能会返回一个空字符串，这可能导致问题。建议添加一个检查以防止返回空字符串时出现问题。
Codecov Report
@@ Coverage Diff @@
## master #526 +/- ##
==========================================
- Coverage 97.00% 96.97% -0.03%
==========================================
Files 174 174
Lines 16538 16546 +8
Branches 2162 2167 +5
==========================================
+ Hits 16043 16046 +3
- Misses 495 500 +5
|
|
@hezhengxu2018 补充一个对应的单测 |
[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)
|
rebase 一下？带了已经合并的代码。 |
hezhengxu2018
force-pushed
the
fix-allow-sync
branch
from
95673d0
to
78be4f0
Compare
| const packageVersion = await this.getPackageVersionEntity(pkg, version); | ||
| const allowSync = this.getAllowSync(ctx); | ||
| const pkg = await this.getPackageEntityByFullname(fullname, allowSync); | ||
| const packageVersion = await this.getPackageVersionEntity(pkg, version, allowSync); | ||
|
|
||
| // read by nfs url | ||
| if (downloadUrl) { |
There was a problem hiding this comment.
这段代码需要下载软件包。以下是我对该代码的审查：
1.15行的'getPackageEntityByFullname'和28行的'getPackageVersionEntity'在哪里定义？是否存在将被调用的类型定义文件？如果没有，建议添加类型定义。
2.引入allowSync变量的设计和使用方式不清楚。建议提供有关其作用和目的的注释。
3.建议添加错误处理和恢复机制，以保证稳健性。
4.建议考虑支持其他协议（例如HTTP）中的URL下载。
5.42行建议更改为异步调用以优化性能
在上述东西得到满足之前不能确认缺陷和修复建议。
|
好了 |
[skip ci] ## [3.34.5](v3.34.4...v3.34.5) (2023-06-21) ### Bug Fixes * create sync task by 'GET /:fullname/-/:filenameWithVersion.tgz' ([#526](#526)) ([5ceaa6b](5ceaa6b))
used by pnpm project with lock
closes #525