-
Notifications
You must be signed in to change notification settings - Fork 47
fix:修复webdav备份恢复问题 #35
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
Conversation
WalkthroughThe pull request enhances backup restoration logic and WebDAV service resilience. SettingsPanel now restores repositories, releases, and custom categories with smart configuration merging that preserves existing credentials when backup data is masked. WebDAVService gains compression utilities, exponential backoff retry logic, file size analysis, improved XML parsing with fallbacks, and configuration validation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsPanel as SettingsPanel
participant Store as useAppStore
participant WebDAV as WebDAVService
User->>SettingsPanel: Trigger restore from backup
rect rgb(220, 240, 255)
note over SettingsPanel: Data Restoration Phase
SettingsPanel->>Store: setRepositories(backupRepos)
Store-->>SettingsPanel: ✓
SettingsPanel->>Store: setReleases(backupReleases)
Store-->>SettingsPanel: ✓
SettingsPanel->>Store: deleteCustomCategory(each)
SettingsPanel->>Store: addCustomCategory(each)
Store-->>SettingsPanel: ✓
end
rect rgb(240, 220, 255)
note over SettingsPanel: Smart Config Merge Phase
SettingsPanel->>SettingsPanel: Merge AI configs<br/>(preserve existing keys if backup masked)
SettingsPanel->>SettingsPanel: Merge WebDAV configs<br/>(preserve existing passwords if backup masked)
SettingsPanel->>Store: updateAIConfigs(merged)
SettingsPanel->>Store: updateWebDAVConfigs(merged)
Store-->>SettingsPanel: ✓
end
SettingsPanel->>User: Display: "Restored N repos, M releases, K categories"
sequenceDiagram
participant Client as Client Code
participant WebDAV as WebDAVService
participant Compress as compressData()
participant Analyze as analyzeFileSize()
participant Retry as retryUpload()
participant Server as WebDAV Server
Client->>WebDAV: uploadFile(config, filename, content)
rect rgb(255, 240, 220)
note over WebDAV: Pre-processing
WebDAV->>Analyze: analyzeFileSize(content)
Analyze-->>WebDAV: {sizeKB, isLarge, suggestions}
WebDAV->>WebDAV: Log size insights & suggestions
WebDAV->>Compress: compressData(content)
Compress-->>WebDAV: compressed content
end
rect rgb(240, 255, 240)
note over WebDAV: Upload with Retry
WebDAV->>Retry: retryUpload(uploadOperation, maxRetries=3)
loop Exponential Backoff on Timeout/Network Error
Retry->>Server: PUT /path/file.json
alt Success
Server-->>Retry: 201/204
Retry-->>WebDAV: ✓ result
else Timeout/Network Error
Server--xRetry: error
Retry->>Retry: delay (exponential)
Retry->>Server: Retry...
end
end
end
WebDAV-->>Client: Upload result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/webdavService.ts (1)
136-158: Bug: PROPFIND intestConnectionhas no timeoutYou create an
AbortControllerand 10s timeout, clear it after the HEAD request, then reuse the same controller for PROPFIND. This means PROPFIND can hang indefinitely with no timeout.Consider keeping the timer active for the whole block or using a separate controller/timeout for PROPFIND, e.g.:
const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 10000); try { const headResponse = await fetch(dirUrl, { method: 'HEAD', headers: { Authorization: this.getAuthHeader() }, signal: controller.signal }); if (headResponse.ok) return true; const propfindResponse = await fetch(dirUrl, { method: 'PROPFIND', headers: { Authorization: this.getAuthHeader(), Depth: '0' }, signal: controller.signal, }); return propfindResponse.ok || propfindResponse.status === 207; } catch (fetchError) { if ((fetchError as any).name === 'AbortError') { throw new Error('连接超时。请检查WebDAV服务器是否可访问。'); } throw fetchError; } finally { clearTimeout(timeoutId); }This preserves a bounded wait time for both requests.
🧹 Nitpick comments (7)
src/components/SettingsPanel.tsx (3)
305-334: Custom category restore is correct but could be more atomicThe "clear then re-add" logic fully replaces custom categories as the confirmation text promises, and the defensive try/catch is good. If the list becomes large, consider exposing a
setCustomCategoriesin the store to replace the entire list in one update instead of manydeletecalls, which would be both simpler and more efficient.
335-367: AI config merge logic preserves secrets but may over‑write optional fieldsThe merge correctly:
- Preserves existing
apiKeywhen backup is masked with***.- Keeps the existing
isActiveflag.- Seeds new configs with
isActive: falseand empty key when masked.Two minor improvements you might consider:
- Pass the
idfield through toupdateAIConfigto keep usage consistent withhandleSaveAI(unless the store intentionally treats the second arg as a partial).- When
cfg.concurrencyisundefined(old backups), fall back toexisting.concurrencyor a default (e.g.1) instead of overwriting withundefined.
369-399: WebDAV config merge semantics look good, with a small UX opportunityThe WebDAV merge correctly:
- Preserves existing passwords when the backup is masked (
***).- Avoids activating newly imported configs and leaves passwords blank when restoring sanitized backups on a new device.
This is functionally sound; the only thing you might add elsewhere is a gentle UI hint after restore that some WebDAV entries were imported without passwords and require re-entry, to avoid silent connection failures later.
src/services/webdavService.ts (4)
10-35: JSON compression and size analysis are reasonable; consider more accurate size calculation
compressDatais a safe minifier for JSON payloads, andanalyzeFileSizegives useful heuristics for logging and tuning. If you ever need a closer approximation to actual bytes on the wire (especially with non‑ASCII content),new Blob([content]).sizewould be more accurate thancontent.length / 1024, though the current approach is fine for rough thresholds.
37-72: retryUpload works but assumes an Error‑like objectThe retry logic with exponential backoff looks good for PUT uploads. The only fragility is assuming
error.messageis always a string; ifoperationever throws a non‑Error,error.message.includes(...)could itself throw. You can harden this by normalizing the message first, e.g.:const message = (error as any)?.message ?? String(error); const shouldRetry = message.includes('超时') || message.toLowerCase().includes('timeout') || message.includes('NetworkError') || message.includes('fetch');and then log
messageinstead oferror.message.
381-460: listFiles XML parsing is robust; small fallback tweak possibleThe new PROPFIND handling with a DOMParser first and regex fallbacks for displayname/href covers a wide range of WebDAV server quirks and correctly filters down to
.jsonfiles. One small enhancement would be to invoke the regex fallback not only when DOMParser throws, but also when it parses successfully yet yields zero<DAV:response>matches, so that malformed but parseable XML still gets a second chance at extraction.
512-533: getServerInfo is useful; consider adding a timeoutExposing
ServerandDAVheaders via an OPTIONS request is handy for diagnostics. To align with the rest of the service and avoid hanging indefinitely on misbehaving servers, you may want to wrap this in anAbortControllerwith a modest timeout (e.g. 10–15s), similar tofileExistsandlistFiles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/SettingsPanel.tsx(2 hunks)src/services/webdavService.ts(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/SettingsPanel.tsx (1)
src/types/index.ts (2)
AIConfig(69-79)WebDAVConfig(81-89)
🔇 Additional comments (5)
src/components/SettingsPanel.tsx (2)
52-55: Store wiring for restore looks correctWiring
setRepositories,setReleases,addCustomCategory, anddeleteCustomCategoryinto the panel matches the new restore flow and keeps responsibilities in the store instead of local state. No issues here.
402-403: Restore success message is informative and safeThe new message that reports counts for repositories, releases, and custom categories is a nice UX improvement and uses optional chaining/
?? 0defensively, so it won’t explode on older backups missing those fields.src/services/webdavService.ts (3)
187-250: Upload pre‑processing and retry are solidThe upload pipeline—size analysis, JSON minification, directory creation, dynamic timeout based on compressed size, and a wrapped PUT with structured HTTP error messages plus retry—is well thought out and should significantly improve resilience for large backups. I don’t see functional issues here.
271-296: Progressive MKCOL for nested paths is a good improvementCreating each path segment with MKCOL and treating 201/405/409 as success makes directory provisioning much more robust across different servers. If you start seeing odd behaviors with redirects or other status codes, you might later expand the “already exists” set, but the current logic is a clear improvement over a single MKCOL.
485-510: validateConfig provides clear, actionable errorsThe static
validateConfigcovers URL scheme, presence of username/password, and the leading/onpath, which matches how the rest of the service builds URLs. This gives the UI concrete messages without coupling it to implementation details.
codex 编码修复
Summary by CodeRabbit
New Features
Bug Fixes