feat: unresolved_configs in Release Config - #380
Conversation
|
Review changes with Changed Files |
WalkthroughThe release config URL now uses ChangesRelease config URL handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@airborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swift`:
- Around line 1059-1075: Remove the registered timeoutObserver before returning
from both URL-validation guard failures in the release-config download flow.
Ensure each failure path deregisters the observer before invoking
completionHandler, preventing the timeout callback from invoking
completionHandler again.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cb05cc4f-b06d-4a04-9ab5-0add20267bf3
📒 Files selected for processing (1)
airborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swift
| guard let baseUrl = URL(string: self.releaseConfigURL), | ||
| var urlComponents = URLComponents(url: baseUrl, resolvingAgainstBaseURL: false) else { | ||
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | ||
| return | ||
| } | ||
|
|
||
|
|
||
| var queryItems = urlComponents.queryItems ?? [] | ||
| if !queryItems.contains(where: { $0.name == "extended" }) { | ||
| queryItems.append(URLQueryItem(name: "extended", value: "true")) | ||
| } | ||
| urlComponents.queryItems = queryItems | ||
|
|
||
| guard let manifestUrl = urlComponents.url else { | ||
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove timeoutObserver before returning on guard failure.
Both guards at lines 1059-1063 and 1071-1074 call completionHandler and return early on failure. Neither removes timeoutObserver, which was registered at lines 1034-1055 and remains active after this function returns. If RELEASE_CONFIG_TIMEOUT_NOTIFICATION fires later, the observer closure calls completionHandler a second time with a tempManifest result. startDownload (lines 851-886) treats each completionHandler invocation as authoritative and mutates releaseConfigDownloadStatus, posts notifications, and calls updateConfig/tryDownloadingUpdate. A second invocation can drive that state machine twice with different results.
This leak pattern already existed for the original single-guard case. The new second guard adds a further return path with the same gap.
🔧 Proposed fix to remove the observer on both guard-failure paths
guard let baseUrl = URL(string: self.releaseConfigURL),
var urlComponents = URLComponents(url: baseUrl, resolvingAgainstBaseURL: false) else {
+ if let observer = timeoutObserver {
+ NotificationCenter.default.removeObserver(observer)
+ }
completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false)
return
}
var queryItems = urlComponents.queryItems ?? []
if !queryItems.contains(where: { $0.name == "extended" }) {
queryItems.append(URLQueryItem(name: "extended", value: "true"))
}
urlComponents.queryItems = queryItems
guard let manifestUrl = urlComponents.url else {
+ if let observer = timeoutObserver {
+ NotificationCenter.default.removeObserver(observer)
+ }
completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false)
return
}📝 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.
| guard let baseUrl = URL(string: self.releaseConfigURL), | |
| var urlComponents = URLComponents(url: baseUrl, resolvingAgainstBaseURL: false) else { | |
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | |
| return | |
| } | |
| var queryItems = urlComponents.queryItems ?? [] | |
| if !queryItems.contains(where: { $0.name == "extended" }) { | |
| queryItems.append(URLQueryItem(name: "extended", value: "true")) | |
| } | |
| urlComponents.queryItems = queryItems | |
| guard let manifestUrl = urlComponents.url else { | |
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | |
| return | |
| } | |
| guard let baseUrl = URL(string: self.releaseConfigURL), | |
| var urlComponents = URLComponents(url: baseUrl, resolvingAgainstBaseURL: false) else { | |
| if let observer = timeoutObserver { | |
| NotificationCenter.default.removeObserver(observer) | |
| } | |
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | |
| return | |
| } | |
| var queryItems = urlComponents.queryItems ?? [] | |
| if !queryItems.contains(where: { $0.name == "extended" }) { | |
| queryItems.append(URLQueryItem(name: "extended", value: "true")) | |
| } | |
| urlComponents.queryItems = queryItems | |
| guard let manifestUrl = urlComponents.url else { | |
| if let observer = timeoutObserver { | |
| NotificationCenter.default.removeObserver(observer) | |
| } | |
| completionHandler(nil, NSError(domain: "in.juspay.Airborne", code: 2, userInfo: [NSLocalizedDescriptionKey: "Invalid URL"]), false) | |
| return | |
| } |
🤖 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
`@airborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swift`
around lines 1059 - 1075, Remove the registered timeoutObserver before returning
from both URL-validation guard failures in the release-config download flow.
Ensure each failure path deregisters the observer before invoking
completionHandler, preventing the timeout callback from invoking
completionHandler again.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@airborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swift`:
- Line 932: Persist unresolved properties in the late-success response path that
calls utils.saveManifestToTemp(manifest), not only in the startDownload
completion path invoking updateUnresolvedProperties. Ensure the standalone
unresolved-properties cache is updated from the successfully received manifest
before or alongside saving the temp manifest, and add a regression test covering
a timed-out request that later succeeds and verifies the persisted values.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1633e0d1-a877-4360-900e-459d926ab4d0
📒 Files selected for processing (7)
airborne_docs/docs/react-native-sdk/reference/callbacks-and-events.mdairborne_docs/docs/react-native-sdk/reference/ios-api.mdairborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swiftairborne_sdk_iOS/hyper-ota/Airborne/AirborneSwiftCore/AJPFileUtil.swiftairborne_sdk_iOS/hyper-ota/Airborne/AirborneSwiftModel/AJPApplicationConstants.swiftairborne_sdk_iOS/hyper-ota/Airborne/AirborneSwiftModel/AJPApplicationManifest.swiftairborne_sdk_iOS/hyper-ota/AirborneTestAppTests/AJPReleaseConfigExtendedTests.swift
| self.downloadedApplicationManifest = manifest | ||
| self.releaseConfigDownloadStatus = .completed | ||
| self.cleanUpUnwantedFiles() | ||
| self.updateUnresolvedProperties(manifest?.unresolvedProperties) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Persist unresolved properties when a timed-out request later succeeds.
Line 932 only updates app-unresolved-properties.dat when the fetch invokes startDownload completion. A valid response received after the timeout takes the utils.saveManifestToTemp(manifest) path instead. initializeDefaults() does not read that temp manifest for unresolved properties. The next launch can therefore return stale or missing data despite the completed response.
Update the standalone cache when saving the late manifest. Add a regression test for this timeout path.
Proposed fix
} else {
if let manifest = manifest, manifestError == nil {
self.tracker.trackInfo("release_config_fetch_after_timeout", value: NSMutableDictionary(dictionary: ["version": manifest.config.version]))
utils.saveManifestToTemp(manifest)
+ self.updateUnresolvedProperties(manifest.unresolvedProperties)
}
}📝 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.
| self.updateUnresolvedProperties(manifest?.unresolvedProperties) | |
| } else { | |
| if let manifest = manifest, manifestError == nil { | |
| self.tracker.trackInfo("release_config_fetch_after_timeout", value: NSMutableDictionary(dictionary: ["version": manifest.config.version])) | |
| utils.saveManifestToTemp(manifest) | |
| self.updateUnresolvedProperties(manifest.unresolvedProperties) | |
| } | |
| } |
🤖 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
`@airborne_sdk_iOS/hyper-ota/Airborne/AirborneSwift/AJPApplicationManager.swift`
at line 932, Persist unresolved properties in the late-success response path
that calls utils.saveManifestToTemp(manifest), not only in the startDownload
completion path invoking updateUnresolvedProperties. Ensure the standalone
unresolved-properties cache is updated from the successfully received manifest
before or alongside saving the temp manifest, and add a regression test covering
a timed-out request that later succeeds and verifies the persisted values.
unresolved_configs in Release Config
1 participant
Summary by CodeRabbit
extended=true, appending or updating it while preserving any existing query parameters.unresolved_properties, which are cached across launches and returned through the iOS API.unresolved_propertiesfield and added an iOS-only lifecycle event for when it changes.extended=trueURL construction.