Feat: Startup version check & fail, Bump immich openapi#676
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe change updates Immich asset-search calls and OpenAPI schemas, adds recently-added and HLS contracts, and replaces startup version logging with compatibility validation that terminates startup when configured servers are unsupported. ChangesImmich compatibility updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Program
participant ImmichServerVersionChecker
participant IServerSettings
participant ImmichAPI
Program->>ImmichServerVersionChecker: CheckServerVersions(services, logger)
ImmichServerVersionChecker->>IServerSettings: load configured accounts
IServerSettings-->>ImmichServerVersionChecker: return account settings
loop configured accounts
ImmichServerVersionChecker->>ImmichAPI: fetch server version with API key
ImmichAPI-->>ImmichServerVersionChecker: return server version
end
ImmichServerVersionChecker-->>Program: return compatibility result
Program->>Program: exit when validation fails
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ImmichFrame.Core/Logic/Pool/AlbumAssetsPool.cs`:
- Line 30: Update SearchAssetsAsync setups, verifications, and invocations to
pass the new key and slug arguments. Apply the corresponding changes in
AlbumAssetsPool.cs (30-30), AssetHelper.cs (26-26), FavoriteAssetsPool.cs
(31-31), PeopleAssetsPool.cs (39-39), and TagAssetsPool.cs (53-53), along with
the related AlbumAssetsPoolTests.cs, AllAssetsPoolTests.cs,
FavoriteAssetsPoolTests.cs, PersonAssetsPoolTests.cs, and TagAssetsPoolTests.cs
mocks.
In `@ImmichFrame.WebApi/Helpers/ImmichServerVersionChecker.cs`:
- Line 42: Update the accounts iteration in the server version checker to handle
a null IServerSettings.Accounts value by falling back to an empty sequence
before the foreach loop. Preserve the existing account-processing and graceful
logging behavior when accounts are configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0bb6360-1088-4677-a6c5-c4d62e2e74b5
📒 Files selected for processing (9)
ImmichFrame.Core/Helpers/AssetHelper.csImmichFrame.Core/Logic/Pool/AlbumAssetsPool.csImmichFrame.Core/Logic/Pool/FavoriteAssetsPool.csImmichFrame.Core/Logic/Pool/PeopleAssetsPool.csImmichFrame.Core/Logic/Pool/TagAssetsPool.csImmichFrame.Core/OpenAPIs/immich-openapi-specs.jsonImmichFrame.WebApi/Helpers/ImmichServerVersionChecker.csImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.csImmichFrame.WebApi/Program.cs
💤 Files with no reviewable changes (1)
- ImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.cs
|
|
||
| var allCompatible = true; | ||
|
|
||
| foreach (var account in accounts) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against a potential NullReferenceException.
If IServerSettings.Accounts resolves to null (e.g., if the accounts section is missing from the configuration file), the foreach loop will throw an uncaught NullReferenceException. This would crash the startup process abruptly, bypassing the graceful logging intended by this checker.
Consider using a null-coalescing operator to provide an empty fallback.
🛡️ Proposed fix
- foreach (var account in accounts)
+ foreach (var account in accounts ?? [])(Note: If using a C# version prior to 12, use accounts ?? Enumerable.Empty<IAccountSettings>() instead).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach (var account in accounts) | |
| foreach (var account in accounts ?? []) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ImmichFrame.WebApi/Helpers/ImmichServerVersionChecker.cs` at line 42, Update
the accounts iteration in the server version checker to handle a null
IServerSettings.Accounts value by falling back to an empty sequence before the
foreach loop. Preserve the existing account-processing and graceful logging
behavior when accounts are configured.
Summary by CodeRabbit
percentageLimitrange/type).