Skip to content

Conversation

@coffeedeveloper
Copy link
Member

@coffeedeveloper coffeedeveloper commented Jan 28, 2026

Types

  • 🎉 New Features
  • 🐛 Bug Fixes
  • 📚 Documentation Changes
  • 💄 Code Style Changes
  • 💄 Style Changes
  • 🪚 Refactors
  • 🚀 Performance Improvements
  • 🏗️ Build System
  • ⏱ Tests
  • 🧹 Chores
  • Other Changes

Background

$saveByChange in FileSchemeDocNodeServiceImpl was the only method still using raw fs-extra synchronous/asynchronous calls (existsSync, statSync, readFile, writeFile) and manual iconvEncode/iconvDecode for encoding conversion.
Meanwhile, the sibling method $saveByContent and $getMd5 already use this.fileService APIs (getFileStat,
resolveContent, setContent), which handle encoding internally and go through the unified file service layer. This
inconsistency meant $saveByChange bypassed any file service middleware (permissions, event hooks, virtual file systems) and pulled in unnecessary direct filesystem dependencies.

Solution

  • Replaced all raw fs-extra and iconv calls in $saveByChange with the equivalent this.fileService APIs
  • Add fallback save using $saveByContent when $saveByChange is failed

Changelog

  • Refactored $saveByChange to use fileService APIs (getFileStat, resolveContent, setContent) instead of raw fs-extra
    and iconv calls, aligning with $saveByContent's pattern.
  • Removed direct fs-extra and iconvEncode/iconvDecode dependencies from file-scheme-doc.service.ts.
  • Updated node test mocks to support the refactored file service–based implementation.

Summary by CodeRabbit

发布说明

  • 新功能

    • 增强了对非UTF-8编码文件的支持,现已支持编码感知的文件操作。
  • Bug修复

    • 改进文件保存机制,当增量保存失败时会自动回退至完整内容保存,提升保存可靠性。
  • 测试

    • 扩展测试覆盖范围,验证文件保存容错行为和编码处理的正确性。

✏️ Tip: You can customize this high-level summary in your review settings.

@opensumi opensumi bot added ⚙️ refactor Refactor code 🐞 bug Something isn't working labels Jan 28, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

功能概览

该PR添加了文件保存失败时的回退机制,当按变更保存失败时自动回退到按完整内容保存,并将文件系统操作重构为使用 IFileService 接口。

变更详情

内聚体 / 文件 变更摘要
测试扩展
packages/file-scheme/__tests__/browser/resource.test.ts
添加测试用例验证回退行为:saveByChange 返回 USE_BY_CONTENT 错误时,自动回退到 saveByContent;验证两条保存路径均被调用且最终状态为 SUCCESS
测试更新
packages/file-scheme/__tests__/node/file-doc-node.test.ts
更新 mock 函数签名支持文件编码;resolveContent 改为异步并支持编码选项;setContent 添加编码选项参数;lastModification 改用文件系统实际值
浏览器保存逻辑
packages/file-scheme/src/browser/file-doc.ts
在 saveDocumentModel 中添加回退逻辑:当 saveByChange 返回 USE_BY_CONTENT 错误时,自动调用 saveByContent 重试保存
Node 层服务重构
packages/file-scheme/src/node/file-scheme-doc.service.ts
将直接文件系统操作(fs-extra、URI、编码)重构为 IFileService 接口调用;更新 $saveByChange 和 $saveByContent 的实现;在文件不存在或变更对比失败时返回 USE_BY_CONTENT 错误触发回退

序列图

sequenceDiagram
    participant Client as 客户端
    participant BrowserDoc as FileSchemeDocumentProvider
    participant FileService as IFileService
    participant NodeService as FileSchemeDocNodeService

    Client->>BrowserDoc: saveDocumentModel(内容变更)
    alt 内容较小 (≤ 阈值)
        BrowserDoc->>FileService: setContent(完整内容)
        FileService-->>BrowserDoc: SUCCESS
    else 内容较大 (> 阈值)
        BrowserDoc->>NodeService: $saveByChange(变更)
        NodeService->>FileService: getFileStat(URI)
        FileService-->>NodeService: stat
        NodeService->>FileService: resolveContent(URI)
        FileService-->>NodeService: 当前内容
        NodeService->>NodeService: 计算MD5并对比
        alt MD5一致且无冲突
            NodeService->>FileService: setContent(应用变更后)
            FileService-->>NodeService: SUCCESS
        else 文件不存在或MD5不匹配
            NodeService-->>BrowserDoc: ERROR (USE_BY_CONTENT)
        end
        alt 收到 USE_BY_CONTENT 错误
            BrowserDoc->>NodeService: $saveByContent(完整内容)
            NodeService->>FileService: setContent(完整内容)
            FileService-->>NodeService: SUCCESS
            NodeService-->>BrowserDoc: SUCCESS
        else 其他错误
            BrowserDoc-->>Client: 返回原错误
        end
    end
    BrowserDoc-->>Client: 保存结果
Loading

代码审查工作量

🎯 3 (中等) | ⏱️ ~20 分钟

建议标签

🐞 bug

建议审查者

  • Ricbet
  • bytemain
  • ensorrow
  • hacke2
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 该PR标题准确概括了主要变更:在$saveByChange失败时添加使用$saveByContent的回退逻辑,与改动内容完全相关。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/save-by-change-fileservice

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.88%. Comparing base (b0d6ed3) to head (4172808).

Files with missing lines Patch % Lines
packages/file-scheme/src/browser/file-doc.ts 75.00% 1 Missing ⚠️
...es/file-scheme/src/node/file-scheme-doc.service.ts 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4697      +/-   ##
==========================================
+ Coverage   48.17%   52.88%   +4.70%     
==========================================
  Files        1686     1686              
  Lines      104888   104890       +2     
  Branches    22953    22942      -11     
==========================================
+ Hits        50535    55476    +4941     
+ Misses      45032    41055    -3977     
+ Partials     9321     8359     -962     
Flag Coverage Δ
jsdom 48.18% <15.00%> (+<0.01%) ⬆️
node 12.25% <75.00%> (?)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coffeedeveloper
Copy link
Member Author

/next

@opensumi
Copy link
Contributor

opensumi bot commented Jan 28, 2026

🎉 PR Next publish successful!

3.9.1-next-1769591692.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Something isn't working ⚙️ refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants