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

feat: enhance data store #4081

Merged
merged 2 commits into from
Oct 11, 2024
Merged

feat: enhance data store #4081

merged 2 commits into from
Oct 11, 2024

Conversation

bytemain
Copy link
Member

@bytemain bytemain commented Oct 11, 2024

Types

  • 🎉 New Features

Background or solution

Changelog

Summary by CodeRabbit

  • 新功能

    • 更新了 InMemoryDataStore 类,增强了类型安全性和灵活性,支持更复杂的数据管理。
    • 新增 removeItemremoveAll 方法,简化了项目移除操作。
  • 文档

    • 更新了相关文档,反映了新的类型参数和方法签名的变化。
  • 修复

    • 修改了方法调用,确保使用一致的计数方法,提升了代码的可读性和一致性。

@opensumi opensumi bot added the 🎨 feature feature required label Oct 11, 2024
Copy link
Contributor

coderabbitai bot commented Oct 11, 2024

Walkthrough

此拉取请求引入了对多个文件的重大更改,主要集中在InMemoryDataStore类及其类型参数的增强。测试文件更新了InMemoryDataStore的实例化,允许第二个类型参数以指定存储项的键。此外,size方法被重命名为count,并在测试中进行了相应的更新。其他文件中也进行了类型定义和方法签名的更新,以提高灵活性和类型安全性。

Changes

文件路径 更改摘要
packages/core-common/__tests__/remote-service/data-store/index.test.ts 更新了InMemoryDataStore的类型参数,重命名sizecount,并更新了相应的测试断言。
packages/core-common/src/remote-service/data-store/store.ts 移除了DataStore接口,更新了InMemoryDataStore类,添加了新的方法并增强了类型安全性。
packages/core-common/src/remote-service/README.md 描述了后端服务架构的重大变更,包括禁止@Autowired引用和引入新的Remote Service概念。
packages/core-common/src/remote-service/data-store/decorators.ts 定义了新的类型DataStoreItem,更新了saveTokengenerateToken,并修改了相关装饰器。
packages/core-common/src/remote-service/data-store/select.ts 更新了QueryStore类型定义,修改了makeMatcher函数的返回类型以接受任意类型的项。
packages/utils/src/disposable.ts RefCountedDisposable类中添加了disposed方法以检查实例的处置状态。
packages/utils/src/event.ts 简化了Dispatcher类中的dispose方法,移除了对_emitter的条件检查。

Possibly related PRs

Suggested labels

🎨 feature

Suggested reviewers

  • Ricbet
  • winjo

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (9)
packages/core-common/src/remote-service/README.md (5)

Line range hint 23-108: 防止@Autowired引用back service的机制设计合理

使用密码机制和依赖注入来限制back service的实例化是一个巧妙的解决方案。这种方法有效地防止了未经授权的实例化,增强了系统的安全性和可控性。

建议:考虑将SECRET_TOKENRealSecret的生成逻辑封装到一个单独的工具类中,以提高代码的可维护性和复用性。

是否需要我为这个建议提供一个代码示例?


Line range hint 110-207: Remote Service概念的引入是一个重要改进

引入Remote Service概念很好地解决了之前controller和service层概念模糊的问题。使用装饰器来定义Remote Service简化了代码,提高了可读性和可维护性。这种方法使得服务的定义和使用更加直观和规范化。

建议:

  1. 考虑在文档中添加一个完整的Remote Service使用示例,包括前端如何调用这个服务。
  2. 可以考虑为Remote Service添加一些内置的日志或监控机制,以便于调试和性能分析。

需要我为这些建议提供更详细的实现方案吗?


Line range hint 209-344: 存储逻辑优化大大提升了代码质量和可维护性

引入InMemoryDataStore和GDataStore概念极大地改善了数据管理方式,使代码更加清晰和易于维护。从Flask和Feathers汲取灵感的做法很好地解决了会话数据和全局数据的存储问题。TerminalService的示例清楚地展示了这种方法的优势。

建议:

  1. 考虑为GDataStore添加一个简单的缓存机制,以优化频繁访问的数据的性能。
  2. 可以考虑提供一个配置选项,允许用户选择将某些GDataStore的数据持久化到磁盘,以支持需要长期保存的数据。

是否需要我为这些优化建议提供更详细的实现方案?


Line range hint 346-371: Data Store的自动创建机制非常优雅

使用装饰器自动创建和使用Data Store的方法大大简化了代码,提高了开发效率。这种实现巧妙地利用了依赖注入系统,使得Data Store的管理更加灵活和高效。

建议:

  1. 考虑使用泛型来增强GDataStore装饰器的类型安全性。例如:
    export function GDataStore<T>(token: string, options?: DataStoreOptions): PropertyDecorator {
      // ... 实现逻辑 ...
    }
  2. 可以考虑添加一个选项,允许用户指定自定义的Data Store实现,以支持更复杂的场景。

需要我为这些建议提供更详细的代码示例吗?


Line range hint 1-371: 总体评价:架构改进显著提升了系统质量

这份文档详细描述了OpenSumi Back Service的重大架构改进,包括:

  1. 引入了防止误用@Autowired的机制
  2. 新增了Remote Service概念,明确了服务层次
  3. 优化了数据存储逻辑,引入了InMemoryDataStore和GDataStore
  4. 实现了Data Store的自动创建机制

这些变更有效解决了之前存在的问题,如数据持久化、内存泄漏、架构混乱等。新的架构设计更加清晰、灵活,有利于提高代码质量和可维护性。特别是Remote Service和GDataStore的引入,大大简化了服务的定义和数据管理。

建议:

  1. 考虑编写详细的迁移指南,帮助开发者将现有代码迁移到新架构。
  2. 可以考虑添加一些性能基准测试,以量化这些改进带来的性能提升。

需要我为这些建议提供更具体的实施方案吗?

packages/utils/src/disposable.ts (1)

372-374: 新增的 disposed getter 实现正确且有用。

这个新增的 disposed getter 提供了一种检查 RefCountedDisposable 对象是否已被处置的便捷方法。实现逻辑正确,与类的现有行为一致。

建议考虑添加一个简单的注释来解释这个 getter 的作用,以提高代码的可读性。

您可以考虑在 getter 上方添加如下注释:

/**
 * 获取当前对象是否已被处置。
 * @returns {boolean} 如果对象已被处置则返回 true,否则返回 false。
 */
get disposed() {
  return this._counter <= 0;
}
packages/core-common/src/remote-service/data-store/store.ts (3)

8-13: 考虑为 Query 类型提供更精确的类型定义

目前,Query 类型被定义为 Record<string, any>,这可能过于宽泛。为了增强类型安全性,建议对 any 进行限制,或者使用更具体的类型来描述查询条件。


27-31: 考虑对 Item 类型应用更严格的约束

在泛型中将 Item 定义为 Record<any, any> 使用了 any,这可能降低类型安全性。建议将 Item 定义为 Record<string, unknown> 或指定更具体的字段类型,以增强类型检查。


98-105: 新增 removeItem 方法,注意主键类型的一致性

removeItem 方法通过项的主键来删除项,但在将 item[this.id] 转换为 PrimaryKeyType 时,需确保该值确实符合 PrimaryKeyType 类型,防止类型不匹配。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7377d41 and 5bdf688.

📒 Files selected for processing (7)
  • packages/core-common/tests/remote-service/data-store/index.test.ts (4 hunks)
  • packages/core-common/src/remote-service/README.md (2 hunks)
  • packages/core-common/src/remote-service/data-store/decorators.ts (1 hunks)
  • packages/core-common/src/remote-service/data-store/select.ts (2 hunks)
  • packages/core-common/src/remote-service/data-store/store.ts (4 hunks)
  • packages/utils/src/disposable.ts (1 hunks)
  • packages/utils/src/event.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (26)
packages/core-common/src/remote-service/data-store/select.ts (1)

3-4: 类型定义的更改增加了灵活性,但需要谨慎使用

这些更改使 QueryStore<T> 类型更加灵活,允许使用任何类型的键。这种灵活性可能有利于某些用例,但也可能导致类型安全问题。

请确保这种更改不会在其他地方引起类型不兼容的问题。运行以下脚本来检查可能受影响的地方:

packages/core-common/__tests__/remote-service/data-store/index.test.ts (4)

11-12: 改进了类型安全性和明确性

这个变更通过添加第二个类型参数 'user' 和传递 id 选项,提高了 InMemoryDataStore 的类型安全性和使用明确性。这有助于在编译时捕获潜在的类型错误,并明确指定了用作唯一标识符的属性。


31-31: count() 方法支持查询过滤

count() 方法现在支持查询参数,这是一个很有用的特性。它允许我们轻松地计算符合特定条件的项目数量,提高了 API 的灵活性和功能性。


49-52: 类型参数更新一致,但缺少 id 选项

这里的 InMemoryDataStore 类型更新与之前的修改一致,通过添加第二个类型参数 'id' 提高了类型安全性,这很好。然而,在初始化存储时没有传递 id 选项。这可能表示有默认行为,但也可能是一个疏忽。

建议:

  1. 如果 'id' 是默认行为,请考虑添加注释说明这一点。
  2. 如果应该指定 id,请添加相应的选项,例如:{ id: 'id' }

请运行以下脚本来验证 InMemoryDataStore 的初始化方式:

#!/bin/bash
# 描述:验证 InMemoryDataStore 的初始化方式

# 测试:搜索 InMemoryDataStore 的初始化。预期:应该看到是否consistently使用了 id 选项。
echo "Searching for InMemoryDataStore initializations:"
rg --type typescript 'new InMemoryDataStore<.*>'

26-26: 方法名称更改:size() 到 count()

size() 方法重命名为 count() 可能是为了提高 API 的一致性。这是一个好的改进,但请确保这个变更在整个代码库中都得到了一致的应用。

请运行以下脚本来验证 count() 方法的使用情况:

✅ Verification successful

请运行以下脚本以重新验证 count() 方法的使用情况:


方法名称更改验证通过

size() 方法已成功重命名为 count(),并且在代码库中没有残留的 size() 调用,count() 方法的使用情况正确且一致。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 InMemoryDataStore 中 count() 方法的使用情况

# 测试:搜索 size() 方法的使用。预期:应该没有结果。
echo "Searching for size() method usage:"
rg --type typescript 'store\.size\(\)'

# 测试:搜索 count() 方法的使用。预期:应该有多个结果。
echo "Searching for count() method usage:"
rg --type typescript 'store\.count\(\)'

Length of output: 667


Script:

#!/bin/bash
# 描述:重新验证 InMemoryDataStore 中 count() 方法的使用情况

# 测试:搜索 size() 方法的使用。预期:应该没有结果。
echo "正在搜索 size() 方法的使用情况:"
rg --glob "*.ts" --glob "*.tsx" 'store\.size\(\)'

# 测试:搜索 count() 方法的使用。预期:应该有多个结果。
echo "正在搜索 count() 方法的使用情况:"
rg --glob "*.ts" --glob "*.tsx" 'store\.count\(\)'

Length of output: 420

packages/core-common/src/remote-service/README.md (1)

Line range hint 1-21: 引言和问题陈述清晰明确

这个引言很好地概述了OpenSumi中Back Service的现有问题,为后续的架构改进奠定了基础。问题陈述清晰,涵盖了数据持久化、内存泄漏、实例化机制等关键问题,有助于读者理解改进的必要性。

packages/utils/src/event.ts (1)

1048-1050: 验证 _emitter 的初始化

dispose 方法的简化可能引入潜在的空指针异常风险。请确保在调用 dispose 方法时,_emitter 总是被正确初始化。

建议:

  1. 检查 Dispatcher 类的构造函数,确保 _emitter 在对象创建时就被初始化。
  2. 考虑添加一个空值检查,以增强代码的健壮性。
dispose(): void {
  if (this._emitter) {
    this._emitter.dispose();
  }
}
✅ Verification successful

_emitter 已正确初始化

经过验证,_emitter 在类实例化时通过类成员初始化方式被正确赋值。因此,dispose 方法中直接调用 this._emitter.dispose() 是安全的,不存在空指针异常的风险。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查 Dispatcher 类的构造函数
ast-grep --lang typescript --pattern $'class Dispatcher {
  $_
  constructor() {
    $$$
  }
  $$$
}'

# 搜索 _emitter 的初始化
rg "_emitter\s*=\s*new\s+Emitter" --type ts

Length of output: 355

packages/core-common/src/remote-service/data-store/store.ts (10)

4-4: 导入 isUndefined 函数

导入 isUndefined 函数以检查值是否为 undefined,提高了代码的可读性和一致性。


32-32: 更新 store 的类型以匹配主键类型

store 属性现在使用 Map<PrimaryKeyType, Item>(),确保主键类型与泛型定义一致,提高了类型安全性。


54-57: 在 find 方法中增加了可选的 query 参数

当未提供 query 时,find 方法将返回所有存储的项,增强了方法的灵活性。


61-62: 重命名 size 方法为 count,并支持可选的 query 参数

count 方法现在可以根据可选的 query 参数返回特定项的数量,增强了功能性和灵活性。


69-69: 更新 get 方法的 id 参数类型

get 方法的 id 参数类型由 string 更新为 PrimaryKeyType,增强了类型安全性和一致性。


73-73: 更新 has 方法的 id 参数类型

has 方法的 id 参数类型由 string 更新为 PrimaryKeyType,提高了类型准确性。


77-77: 更新 update 方法的 id 参数类型

update 方法的 id 参数类型由 string 更新为 PrimaryKeyType,增强了类型安全性。


79-79: 使用 isUndefined 检查当前项是否存在

update 方法中,使用 isUndefined(current) 来判断 current 是否为 undefined,确保只有在项存在时才执行更新操作。


89-89: 更新 remove 方法的 id 参数类型

remove 方法的 id 参数类型由 string 更新为 PrimaryKeyType,提高了类型一致性和安全性。


107-125: 新增 removeAll 方法,支持删除所有或特定项

removeAll 方法提供了删除所有项或根据查询条件删除特定项的功能,提升了数据管理的灵活性。

packages/core-common/src/remote-service/data-store/decorators.ts (9)

5-11: 良好的类型定义

DataStoreItem 类型的定义清晰,准确地描述了数据存储项的结构,便于后续使用和扩展。


14-15: 正确初始化 dataStore 对象

dataStore 对象包含 GDataStoreSessionDataStore,并初始化为空的 DataStoreItem,有利于统一管理数据存储。


33-37: 增强 GDataStore 类型的泛型定义

GDataStore 类型已更新为接受泛型参数,使其在处理不同的数据结构时更具灵活性和类型安全性。


44-48: 增强 SessionDataStore 类型的泛型定义

SessionDataStore 类型同样更新为接受泛型参数,增强了处理会话数据时的灵活性和类型安全性。


55-61: 正确实现 _injectDataStores 函数

_injectDataStores 函数正确地遍历了 stores,并使用 InMemoryDataStore 实例添加提供者,确保数据存储能够被正确地依赖注入。


68-68: 注入全局数据存储

injectGDataStores 函数成功调用 _injectDataStores,正确地注入了全局数据存储。


72-72: 注入会话数据存储

injectSessionDataStores 函数成功调用 _injectDataStores,正确地注入了会话数据存储。


39-41: 确保 GDataStore 和 SessionDataStore 的使用已更新

由于 GDataStoreSessionDataStore 的类型定义已更改,请验证代码库中所有使用这些类型的地方都已更新,以匹配新的泛型参数,防止类型不匹配问题。

请运行以下脚本以查找未更新的使用方式:

#!/bin/bash
# 描述:查找未使用泛型参数的 GDataStore 和 SessionDataStore

# 预期:没有匹配结果,表示所有使用已更新
rg -P '\bGDataStore\b(?!<)' --type typescript || echo "所有 GDataStore 使用都已更新为泛型形式"
rg -P '\bSessionDataStore\b(?!<)' --type typescript || echo "所有 SessionDataStore 使用都已更新为泛型形式"

Also applies to: 50-52


19-30: 重命名函数并更新参数类型

saveToken 函数已重命名为 generateToken,并将参数类型更新为 DataStoreType。请确保代码库中所有对 saveToken 的引用都已更新为 generateToken,以避免潜在的函数调用错误。

请运行以下脚本以验证是否已更新所有引用:

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

Attention: Patch coverage is 36.73469% with 31 lines in your changes missing coverage. Please review.

Project coverage is 54.38%. Comparing base (7377d41) to head (24228bf).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
...core-common/src/remote-service/data-store/store.ts 43.75% 15 Missing and 3 partials ⚠️
...common/src/remote-service/data-store/decorators.ts 15.38% 11 Missing ⚠️
packages/utils/src/disposable.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4081      +/-   ##
==========================================
- Coverage   54.40%   54.38%   -0.03%     
==========================================
  Files        1591     1591              
  Lines       97288    97335      +47     
  Branches    19911    19916       +5     
==========================================
- Hits        52934    52932       -2     
- Misses      36838    36876      +38     
- Partials     7516     7527      +11     
Flag Coverage Δ
jsdom 49.92% <36.73%> (-0.03%) ⬇️
node 15.63% <36.73%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5bdf688 and 24228bf.

📒 Files selected for processing (2)
  • packages/core-common/tests/remote-service/data-store/index.test.ts (5 hunks)
  • packages/core-common/src/remote-service/data-store/store.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/core-common/src/remote-service/data-store/store.ts (3)

29-33: 良好的泛型设计,提高了类的灵活性

引入泛型参数 ItemPrimaryKeyPrimaryKeyType,增强了 InMemoryDataStore 类的灵活性和类型安全性。这使得数据存储能够适应不同的数据结构和主键类型。


71-79: 方法参数类型更改,需更新调用方式

gethasupdateremove 方法的参数类型从 string 更改为 PrimaryKeyType。这可能会影响到现有的调用代码。请确保所有调用这些方法的地方都已更新为使用正确的参数类型。

请运行以下脚本检查受影响的方法调用:

#!/bin/bash
# 描述:查找调用了受影响方法的代码
rg --type typescript --fixed-strings '.get(' '.has(' '.update(' '.remove('

63-64: 注意方法名从 size 改为 count

size 方法已重命名为 count。请确保在代码库中所有对 size 方法的调用都已更新为 count,以避免出现未定义的方法错误。

请运行以下脚本查找仍在使用 size 方法的地方:

packages/core-common/__tests__/remote-service/data-store/index.test.ts (2)

11-13: 改进了 InMemoryDataStore 的类型参数

在实例化 InMemoryDataStore 时,添加了第二个类型参数 'user',增强了类型安全性。


44-46: ⚠️ Potential issue

TestItemid 属性设为必填可能导致类型问题

TestItem 接口中的 id 属性从可选 id?: string; 更改为必填 id: string;,但在创建 TestItem 实例时,未提供 id 属性(例如第 65 行的 const item = { name: 'test' };)。

这可能导致 TypeScript 类型错误。建议:

  • 如果 store.create(item) 方法会自动生成并分配 id,应将 id 属性保留为可选 id?: string;
  • 或者,在创建 item 时显式提供 id 属性。

运行以下脚本以查找未提供 id 属性的 TestItem 实例创建:

@bytemain bytemain merged commit 67c2f89 into main Oct 11, 2024
12 checks passed
@bytemain bytemain deleted the feat/enhance-data-store branch October 11, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 feature feature required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants