Skip to content

refactor(native): one implementation of the resumable-download decisions - #688

Merged
sunnylqm merged 1 commit into
masterfrom
claude/resume-plan-core
Oct 16, 2026
Merged

sunnylqm merged 1 commit into
masterfrom
claude/resume-plan-core

Conversation

@sunnylqm

@sunnylqm sunnylqm commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

架构审查剩下的结构项:把断点续传的决策下沉到 C++,三端改为调用同一份实现。

为什么

三端的下载器各自实现了同一套续传决策,并且已经出现了漂移:

  • 完成后刷新 sidecar: 只有鸿蒙会在下载完成后用最终大小重写 sidecar。另外两端遇到 chunked 等事先不知道总长的归档,如果进程死在下载完成之后、解压之前,下次会整包重下。
  • sidecar 读取上限: 只有 Android 限制为 64KB,iOS 和鸿蒙没有上限。
  • Content-Range 解析: 三份解析器口径不一。鸿蒙的 parseInt 会接受 200abc 这种末尾带杂字符的写法,Android 的 Long.parseLong 会接受 +100 这种带符号的写法。

改了什么

cpp/patch_core/resume_plan.h 负责全部决策:

  • 读取 sidecar: 校验 URL 并限制大小。
  • 规划请求: 判定为已完整、续传还是从零开始。续传时给出 Range、Accept-Encoding: identity 和 If-Range 请求头。
  • 判定响应: 结果分为四类:已完整、丢弃后重来一次、HTTP 错误、开始写入。写入时同时给出追加还是覆盖、起点、总长、是否编码、是否写 sidecar。
  • 生成 sidecar: 生成要写入的 sidecar 文本。验证器取本次响应的值,缺失时沿用上一份。

三端只保留 IO、单次重试、进度和限额:Android 经 JNI 的新类 ResumePlan,鸿蒙经 NAPI,iOS 直接调用。三端统一的行为:下载完成且未编码时用最终大小刷新 sidecar;sidecar 读取上限为 64KB;Content-Range 只接受纯数字。

cpp/patch_core/tests/resume-plan-vectors.json 共 66 个用例,锁定上述行为,其中包含被删除的 JVM 与鸿蒙测试里的全部 Content-Range 用例,由 test-patch-core.sh 回放。JNI 和 NAPI 的绑定表由 scripts/contract.ts 生成。android/lib 下的 .so 已用 NDK 28.2 重新编译。

共 18 个文件,新增 457 行、删除 576 行。

验证

  • C++: SANITIZE=1 ./scripts/test-patch-core.sh 通过,66 个续传向量全部通过。新文件在三端 release 参数下都能干净编译:Android 关 RTTI、iOS C++20、鸿蒙 C++17。
  • Android: .so 重编后 verify-android-so.js 和 check-binary-strings.js 通过。testReleaseUnitTest 编译了改动后的 Java 代码,JVM 单测全部通过。
  • iOS: 用 Pods 头文件对 RCTPushyDownloader.mm 做语法编译,没有错误也没有警告。
  • 鸿蒙: 用 DevEco SDK 的 clang 编译 pushy.cpp 通过,鸿蒙类型检查也通过。
  • JS: bun lint 通过,包括生成器的一致性检查;bun test 456 个用例全部通过。
  • 端到端: 实际的下载与续传行为交给 CI 的 e2e-android 和 e2e-ios(含 native 用例组)验证。

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by CodeRabbit

  • Bug Fixes
    • Downloads can now recognize when a partial file is already complete, resume valid partial downloads, and restart from the beginning when saved progress is stale or invalid.
    • Resume information is refreshed after successful downloads, including transfers whose total size was initially unknown. Encoded downloads are not resumed.

Android, iOS and Harmony each implemented the same resume decisions around
a partial archive and its `.resume` sidecar, and they had drifted: only
Harmony refreshed the sidecar with the final size after a complete transfer
(so the other two re-downloaded a chunked archive whose process died between
download end and unzip), only Android capped the sidecar read at 64KB, and
the three Content-Range parsers disagreed on trailing text and signs.

cpp/patch_core/resume_plan.h now makes every decision: read and validate the
sidecar, plan the request (complete / resume with Range, Accept-Encoding:
identity and If-Range / fresh), classify the response (complete / restart /
httpError / write with append, base, total, encoded, persistSidecar) and
build the sidecar text. The downloaders keep only the IO: Android through JNI
(ResumePlan.java), Harmony through NAPI, iOS directly. All three refresh the
sidecar after a complete unencoded transfer, cap it at 64KB and accept digits
only in Content-Range.

cpp/patch_core/tests/resume-plan-vectors.json (66 cases, including every
Content-Range case the removed JVM and Harmony tests covered) pins the
behaviour; test-patch-core.sh replays it. JNI and NAPI tables are generated
from scripts/contract.ts; android/lib is rebuilt with NDK 28.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cbfa07b1-a518-4be5-ba90-4be5ddc9248f
📥 Commits

Reviewing files that changed from the base of the PR and between 22ee8e0 and 7fe1137.

⛔ Files ignored due to path filters: 4
  • android/lib/arm64-v8a/librnpushy.so is excluded by !**/*.so
  • android/lib/armeabi-v7a/librnpushy.so is excluded by !**/*.so
  • android/lib/x86/librnpushy.so is excluded by !**/*.so
  • android/lib/x86_64/librnpushy.so is excluded by !**/*.so
📒 Files selected for processing: 20
  • CODE_AUDIT.md
  • android/src/main/java/cn/reactnative/modules/update/DownloadTask.java
  • android/src/main/java/cn/reactnative/modules/update/HttpUtils.java
  • android/src/main/java/cn/reactnative/modules/update/ResumePlan.java
  • android/src/test/java/cn/reactnative/modules/update/HttpUtilsTest.java
  • cpp/android_jni/jni_natives.h
  • cpp/android_jni/jni_registration.cpp
  • cpp/android_jni/resume_plan_jni.cpp
  • cpp/patch_core/resume_plan.cpp
  • cpp/patch_core/resume_plan.h
  • cpp/patch_core/tests/resume-plan-vectors.json
  • cpp/patch_core/tests/resume_plan_test.cpp
  • docs/NATIVE_CHECKUPDATE_DESIGN.md
  • harmony/pushy/src/main/cpp/pushy.cpp
  • harmony/pushy/src/main/ets/DownloadTask.ts
  • harmony/pushy/src/main/ets/NativePatchCore.ts
  • ios/RCTPushy/RCTPushyDownloader.mm
  • scripts/contract.ts
  • scripts/test-patch-core.sh
  • src/__tests__/harmonyDownloadTask.test.ts
💤 Files with no reviewable changes: 2
  • android/src/test/java/cn/reactnative/modules/update/HttpUtilsTest.java
  • android/src/main/java/cn/reactnative/modules/update/HttpUtils.java

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The change moves resumable-download planning, response classification, and sidecar generation into a shared C++ core. Android, iOS, and Harmony downloaders use the shared decisions through platform bindings or direct calls. Core vectors test the shared behavior.

Changes: Resumable downloads

Layer / File(s) Summary
Shared resume-planning logic and vectors
cpp/patch_core/resume_plan.*, cpp/patch_core/tests/*, scripts/test-patch-core.sh, docs/NATIVE_CHECKUPDATE_DESIGN.md, CODE_AUDIT.md
The C++ core plans fresh, complete, or resumed requests; validates response ranges; classifies responses; and generates sidecars. Vector tests cover these decisions. The design and audit text describe the shared implementation.
Native bindings for platform callers
android/src/main/java/cn/reactnative/modules/update/ResumePlan.java, cpp/android_jni/*, harmony/pushy/src/main/cpp/pushy.cpp, harmony/pushy/src/main/ets/NativePatchCore.ts, scripts/contract.ts
JNI and NAPI bindings expose request planning, response classification, and sidecar generation to platform callers.
Platform downloader integration
android/src/main/java/cn/reactnative/modules/update/DownloadTask.java, android/src/main/java/cn/reactnative/modules/update/HttpUtils.java, android/src/test/java/cn/reactnative/modules/update/HttpUtilsTest.java, ios/RCTPushy/RCTPushyDownloader.mm, harmony/pushy/src/main/ets/DownloadTask.ts, src/__tests__/harmonyDownloadTask.test.ts
The Android, iOS, and Harmony downloaders use shared plans and response decisions for request setup, transfer handling, retries, and sidecar persistence. Successful non-encoded downloads refresh the sidecar with the final file size. Platform-local Content-Range parsing is removed.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant Task as Harmony DownloadTask.transferArchive
  participant Core as NativePatchCore
  participant Planner as pushy::resume
  Task->>Core: resumePlanRequest(sidecar, URL, partial size)
  Core->>Planner: PlanRequestJson
  Planner-->>Core: Complete, resume, or fresh plan
  Core-->>Task: Request plan
  Task->>Core: resumeClassifyResponse(status, headers, partial state)
  Core->>Planner: ClassifyResponseJson
  Planner-->>Core: Complete, restart, HTTP error, or write decision
  Core-->>Task: Response decision and sidecar instructions
Loading
Merge Risk: ⚪ Minimal · up to 7fe11

Resumable-download decisions now come from one shared native implementation on all three platforms. No concrete defect was identified, and the new sources are included in each platform's native build, so the change appears ready to merge after the normal CI end-to-end runs.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 14 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: consolidating resumable-download decisions into one native implementation across platforms.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 14 files. (4 skipped: 4 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@sunnylqm
sunnylqm merged commit e35f7df into master Oct 16, 2026
14 checks passed
@sunnylqm sunnylqm mentioned this pull request Oct 16, 2026
sunnylqm added a commit that referenced this pull request Oct 17, 2026
Bump package.json and harmony/pushy/oh-package.json5 together, with the
bilingual release notes. Ships everything since 10.60.0: the shared native
round engine (#673-#681), the cross-platform alignment and fixes (#664,
#666, #671, #687, #688), the JS client fixes and split (#667, #684, #685),
the bounded host prepareBundle (#669), the generated cross-platform contract
(#683, #686) and the trimmed npm package (#670).

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant