[rc-swift] RemoteConfig.swift #14309
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated by 🚫 Danger |
Apple API Diff ReportCommit: 54267c8 FirebaseRemoteConfigType Definitions[REMOVED] FIRRemoteConfigFetchAndActivateCompletion[REMOVED] FIRRemoteConfigFetchAndActivateCompletionObjective-C:
- typedef void ( ^ FIRRemoteConfigFetchAndActivateCompletion )( FIRRemoteConfigFetchAndActivateStatus , NSError * _Nullable )[REMOVED] FIRRemoteConfigFetchCompletion[REMOVED] FIRRemoteConfigFetchCompletionObjective-C:
- typedef void ( ^ FIRRemoteConfigFetchCompletion )( FIRRemoteConfigFetchStatus , NSError * _Nullable )[REMOVED] FIRRemoteConfigUpdateCompletion[REMOVED] FIRRemoteConfigUpdateCompletion[REMOVED] EnumerationsSwift:
- typealias RemoteConfigError . Code . _ErrorType = RemoteConfigError
- case unknown = 8001
- case throttled = 8002
- case internalError = 8003
- enum RemoteConfigFetchAndActivateStatus : Int , @unchecked Sendable
- case successFetchedFromRemote = 0
- case successUsingPreFetchedData = 1
- case error = 2
- enum RemoteConfigFetchStatus : Int , @unchecked Sendable
- case noFetchYet = 0
- case success = 1
- case failure = 2
- case throttled = 3
- enum RemoteConfigSource : Int , @unchecked Sendable
- case remote = 0
- case ` default ` = 1
- case ` static ` = 2
- typealias RemoteConfigUpdateError . Code . _ErrorType = RemoteConfigUpdateError
- case streamError = 8001
- case notFetched = 8002
- case messageInvalid = 8003
- case unavailable = 8004
Objective-C:
- enum FIRRemoteConfigError : NSInteger {}
- FIRRemoteConfigErrorUnknown = 8001
- FIRRemoteConfigErrorThrottled = 8002
- FIRRemoteConfigErrorInternalError = 8003
- enum FIRRemoteConfigFetchAndActivateStatus : NSInteger {}
- FIRRemoteConfigFetchAndActivateStatusSuccessFetchedFromRemote
- FIRRemoteConfigFetchAndActivateStatusSuccessUsingPreFetchedData
- FIRRemoteConfigFetchAndActivateStatusError
- enum FIRRemoteConfigFetchStatus : NSInteger {}
- FIRRemoteConfigFetchStatusNoFetchYet
- FIRRemoteConfigFetchStatusSuccess
- FIRRemoteConfigFetchStatusFailure
- FIRRemoteConfigFetchStatusThrottled
- enum FIRRemoteConfigSource : NSInteger {}
- FIRRemoteConfigSourceRemote
- FIRRemoteConfigSourceDefault
- FIRRemoteConfigSourceStatic
- enum FIRRemoteConfigUpdateError : NSInteger {}
- FIRRemoteConfigUpdateErrorStreamError = 8001
- FIRRemoteConfigUpdateErrorNotFetched = 8002
- FIRRemoteConfigUpdateErrorMessageInvalid = 8003
- FIRRemoteConfigUpdateErrorUnavailable = 8004Constants[REMOVED] FIRRemoteConfigErrorDomain[REMOVED] FIRRemoteConfigErrorDomainSwift:
- let RemoteConfigErrorDomain : String
Objective-C:
- extern NS_SWIFT_NAME ( RemoteConfigErrorDomain ) NSString * const FIRRemoteConfigErrorDomain[REMOVED] FIRRemoteConfigUpdateErrorDomain[REMOVED] FIRRemoteConfigUpdateErrorDomainSwift:
- let RemoteConfigUpdateErrorDomain : String
Objective-C:
- extern NS_SWIFT_NAME ( RemoteConfigUpdateErrorDomain ) NSString * const FIRRemoteConfigUpdateErrorDomain[REMOVED] ClassesSwift:
- var lastFetchTime : Date ? { get }
- var lastFetchStatus : RemoteConfigFetchStatus { get }
- var configSettings : RemoteConfigSettings { get set }
- class func remoteConfig () -> RemoteConfig
- class func remoteConfig ( app : FIRApp ) -> RemoteConfig
- func ensureInitialized () async throws
- func fetch () async throws -> RemoteConfigFetchStatus
- func fetch ( withExpirationDuration expirationDuration : TimeInterval ) async throws -> RemoteConfigFetchStatus
- func fetchAndActivate () async throws -> RemoteConfigFetchAndActivateStatus
- func activate () async throws -> Bool
- subscript ( key : String ) -> FIRRemoteConfigValue { get }
- func configValue ( forKey key : String ?) -> FIRRemoteConfigValue
- func configValue ( forKey key : String ?, source : RemoteConfigSource ) -> FIRRemoteConfigValue
- func allKeys ( from source : RemoteConfigSource ) -> [ String ]
- func keys ( withPrefix prefix : String ?) -> Set < String >
- func setDefaults ( _ defaults : [ String : NSObject ]?)
- func setDefaults ( fromPlist fileName : String ?)
- func defaultValue ( forKey key : String ?) -> FIRRemoteConfigValue ?
- func addOnConfigUpdateListener ( remoteConfigUpdateCompletion listener : @escaping ( FIRRemoteConfigUpdate ?, ( any Error )?) -> Void ) -> FIRConfigUpdateListenerRegistration
- var configRealtime : RCNConfigRealtime { get set }
- var settings : RCNConfigSettings { get }
- init ( appName : String , firOptions options : FIROptions , namespace FIRNamespace : String , dbManager DBManager : RCNConfigDBManager , configContent : RCNConfigContent , analytics : ( any FIRAnalyticsInterop )?)
- init ( appName : String , firOptions options : FIROptions , namespace FIRNamespace : String , dbManager DBManager : RCNConfigDBManager , configContent : RCNConfigContent , userDefaults : UserDefaults ?, analytics : ( any FIRAnalyticsInterop )?, configFetch : RCNConfigFetch ?, configRealtime : RCNConfigRealtime ?)
- func addInteropSubscriber ( _ subscriber : any FIRRolloutsStateSubscriber )
- class RemoteConfigSettings : NSObject
- var minimumFetchInterval : TimeInterval { get set }
- var fetchTimeout : TimeInterval { get set }
Objective-C:
- @property ( nonatomic , strong , readonly , nullable ) NSDate * lastFetchTime ;
- @property ( nonatomic , readonly ) FIRRemoteConfigFetchStatus lastFetchStatus ;
- @property ( nonatomic , strong , nonnull ) FIRRemoteConfigSettings * configSettings ;
- + ( nonnull FIRRemoteConfig * ) remoteConfig ;
- + ( nonnull FIRRemoteConfig * ) remoteConfigWithApp :( nonnull FIRApp * ) app ;
- - ( nonnull instancetype ) init ;
- - ( void ) ensureInitializedWithCompletionHandler : ( void ( ^ _Nonnull )( NSError * _Nullable )) completionHandler ;
- - ( void ) fetchWithCompletionHandler : ( void ( ^ _Nullable )( FIRRemoteConfigFetchStatus , NSError * _Nullable )) completionHandler ;
- - ( void ) fetchWithExpirationDuration :( NSTimeInterval ) expirationDuration completionHandler : ( void ( ^ _Nullable )( FIRRemoteConfigFetchStatus , NSError * _Nullable )) completionHandler ;
- - ( void ) fetchAndActivateWithCompletionHandler : ( void ( ^ _Nullable )( FIRRemoteConfigFetchAndActivateStatus , NSError * _Nullable )) completionHandler ;
- - ( void ) activateWithCompletion : ( void ( ^ _Nullable )( BOOL , NSError * _Nullable )) completion ;
- - ( nonnull FIRRemoteConfigValue * ) objectForKeyedSubscript : ( nonnull NSString * ) key ;
- - ( nonnull FIRRemoteConfigValue * ) configValueForKey :( nullable NSString * ) key ;
- - ( nonnull FIRRemoteConfigValue * ) configValueForKey :( nullable NSString * ) key source : ( FIRRemoteConfigSource ) source ;
- - ( nonnull NSArray < NSString *> * ) allKeysFromSource : ( FIRRemoteConfigSource ) source ;
- - ( nonnull NSSet < NSString *> * ) keysWithPrefix :( nullable NSString * ) prefix ;
- - ( void ) setDefaults :( nullable NSDictionary < NSString * , NSObject *> * ) defaults ;
- - ( void ) setDefaultsFromPlistFileName :( nullable NSString * ) fileName ;
- - ( nullable FIRRemoteConfigValue * ) defaultValueForKey :( nullable NSString * ) key ;
- - ( FIRConfigUpdateListenerRegistration * _Nonnull ) addOnConfigUpdateListener : ( FIRRemoteConfigUpdateCompletion _Nonnull ) listener ;
- @property ( nonatomic , readwrite , strong , nonnull ) RCNConfigRealtime * configRealtime
- @property ( nonatomic , readonly , strong ) RCNConfigSettings * settings
- - ( nonnull instancetype ) initWithAppName :( nonnull NSString * ) appName FIROptions :( nonnull FIROptions * ) options namespace :( nonnull NSString * ) FIRNamespace DBManager :( nonnull RCNConfigDBManager * ) DBManager configContent :( nonnull RCNConfigContent * ) configContent analytics :( nullable id < FIRAnalyticsInterop > ) analytics ;
- - ( instancetype ) initWithAppName :( NSString * ) appName FIROptions :( FIROptions * ) options namespace :( NSString * ) FIRNamespace DBManager :( RCNConfigDBManager * ) DBManager configContent :( RCNConfigContent * ) configContent userDefaults :( nullable NSUserDefaults * ) userDefaults analytics :( nullable id < FIRAnalyticsInterop > ) analytics configFetch :( nullable RCNConfigFetch * ) configFetch configRealtime :( nullable RCNConfigRealtime * ) configRealtime ;
- - ( void ) addRemoteConfigInteropSubscriber : ( id < FIRRolloutsStateSubscriber > _Nonnull ) subscriber ;
- @interface FIRRemoteConfigSettings : NSObject
- @property ( nonatomic ) NSTimeInterval minimumFetchInterval ;
- @property ( nonatomic ) NSTimeInterval fetchTimeout ; |
ncooke3
reviewed
Jan 16, 2025
FirebaseRemoteConfig/Tests/Swift/SwiftAPI/FirebaseRemoteConfigSwift_APIBuildTests.swift
Outdated
Show resolved
Hide resolved
| configRealtime: nil | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
init() was made unavailable in ObjC. Can clients use the inherited init()? If so, we may need to add:
/// Unavailable. Use `remoteConfig()` instead.
@available(*, unavailable)
@objc public init() {}There was a problem hiding this comment.
Hmm, deferring for now.
ncooke3
reviewed
Jan 16, 2025
ncooke3
reviewed
Jan 16, 2025
| RCLog.debug("I-RCN987366", | ||
| "Successfully read configSettings. Minimum Fetch Interval: " + | ||
| "\(minimumFetchInterval), Fetch timeout: \(fetchTimeout)") | ||
| return settings |
There was a problem hiding this comment.
The ObjC implementation would recreate a network session before returning the settings.
/// The NSURLSession needs to be recreated whenever the fetch timeout may be updated.
[_configFetch recreateNetworkSession];
return settings;There was a problem hiding this comment.
I don't understand why the get needs to do this, but will add it back for now to be consistent.
Comment on lines
391
to
416
| NSError( | ||
| domain: ConfigConstants.RemoteConfigErrorDomain, | ||
| code: RemoteConfigError.internalError.rawValue, | ||
| userInfo: [NSLocalizedDescriptionKey: "Timed out waiting for database load."] | ||
| ) |
There was a problem hiding this comment.
Should this be an RC-typed error ?
Comment on lines
402
to
435
| @objc public func addListener(_ listener: @escaping RemoteConfigListener) { | ||
| queue.async { | ||
| self.listeners.append(listener) | ||
| } | ||
| } | ||
|
|
||
| private func callListeners(key: String, config: [String: RemoteConfigValue]) { | ||
| queue.async { [weak self] in | ||
| guard let self = self else { return } | ||
| for listener in self.listeners { | ||
| listener(key, config) | ||
| } | ||
| } |
There was a problem hiding this comment.
No changes required, but the ObjC impl. synchronized with a lock on the listeners property rather than the serial queue.
There was a problem hiding this comment.
Hmm, I can't think of advantages of a separate dispatch_async for each listener
paulb777
commented
Jan 16, 2025
Comment on lines
25
to
31
| public let FIRNamespaceGoogleMobilePlatform = "firebase" | ||
|
|
||
| public let FIRRemoteConfigThrottledEndTimeInSecondsKey = "error_throttled_end_time_seconds" | ||
|
|
||
| public let FIRRemoteConfigActivateNotification = | ||
| Notification.Name("FIRRemoteConfigActivateNotification") |
There was a problem hiding this comment.
Yes for some. Restored objc version of FIRRemoteConfigThrottledEndTimeInSecondsKey and FIRNamespaceGoogleMobilePlatform
|
@ncooke3 I've only processed your first batch so far - will start on second batch now. |
paulb777
commented
Jan 16, 2025
| RCLog.debug("I-RCN987366", | ||
| "Successfully read configSettings. Minimum Fetch Interval: " + | ||
| "\(minimumFetchInterval), Fetch timeout: \(fetchTimeout)") | ||
| return settings |
There was a problem hiding this comment.
I don't understand why the get needs to do this, but will add it back for now to be consistent.
Comment on lines
402
to
435
| @objc public func addListener(_ listener: @escaping RemoteConfigListener) { | ||
| queue.async { | ||
| self.listeners.append(listener) | ||
| } | ||
| } | ||
|
|
||
| private func callListeners(key: String, config: [String: RemoteConfigValue]) { | ||
| queue.async { [weak self] in | ||
| guard let self = self else { return } | ||
| for listener in self.listeners { | ||
| listener(key, config) | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmm, I can't think of advantages of a separate dispatch_async for each listener
|
I'm going to merge here and rebase with |
ncooke3
reviewed
Jan 17, 2025
| ((RemoteConfigFetchAndActivateStatus, Error?) -> Void)? = | ||
| nil) { | ||
| fetch { [weak self] fetchStatus, error in | ||
| guard let self = self else { return } |
There was a problem hiding this comment.
Suggested change
| guard let self = self else { return } | |
| guard let self else { return } |
Comment on lines
+515
to
+542
| _ = try await fetch() | ||
| do { | ||
| try await activate() | ||
| return .successFetchedFromRemote | ||
| } catch { | ||
| return .successUsingPreFetchedData | ||
| } |
There was a problem hiding this comment.
This impl. looks good to me, but it may be worth it to wrap a continuation around a call to the completion handler API below. So both API's share the same impl.
| /// - Parameter completion Activate operation callback with changed and error parameters. | ||
| @objc public func activate(completion: ((Bool, Error?) -> Void)? = nil) { | ||
| queue.async { [weak self] in | ||
| guard let self = self else { |
There was a problem hiding this comment.
Suggested change
| guard let self = self else { | |
| guard let self else { |
Comment on lines
+613
to
+618
| RCLog.error("I-RCN000068", "Internal error activating config.") | ||
| if let completion { | ||
| DispatchQueue.main.async { | ||
| completion(false, error) | ||
| } | ||
| } |
There was a problem hiding this comment.
probably doesn't matter, but ObjC logged after the async dispatch
if let completion {
DispatchQueue.main.async {
completion(false, error)
}
}
RCLog.error("I-RCN000068", "Internal error activating config.")| ) | ||
| } | ||
|
|
||
| // MARK: helpers |
Comment on lines
+775
to
+787
| var keys = Set<String>() | ||
| queue.sync { | ||
| let fullyQualifiedNamespace = self.fullyQualifiedNamespace(FIRNamespace) | ||
|
|
||
| if let config = configContent.activeConfig()[fullyQualifiedNamespace] { | ||
| if let prefix = prefix, !prefix.isEmpty { | ||
| keys = Set(config.keys.filter { $0.hasPrefix(prefix) }) | ||
| } else { | ||
| keys = Set(config.keys) | ||
| } | ||
| } | ||
| } | ||
| return keys |
There was a problem hiding this comment.
Probably could return the value from the sync block, rather than passing it through the context.
Comment on lines
+793
to
+801
| var count = 0 | ||
| queue.sync { | ||
| let fullyQualifiedNamespace = self.fullyQualifiedNamespace(FIRNamespace) | ||
|
|
||
| if let config = configContent.activeConfig()[fullyQualifiedNamespace] as? NSDictionary { | ||
| count = config.countByEnumerating(with: state, objects: buffer, count: len) | ||
| } | ||
| } | ||
| return count |
There was a problem hiding this comment.
Probably could return the value from the sync block, rather than passing it through the context.
Comment on lines
+806
to
+807
| /// Sets config defaults for parameter keys and values in the default namespace config. | ||
| /// - Parameter defaults A dictionary mapping a NSString * key to a NSObject * value. |
Comment on lines
+850
to
+862
| let fullyQualifiedNamespace = self.fullyQualifiedNamespace(FIRNamespace) | ||
| var value: RemoteConfigValue? | ||
| queue.sync { | ||
| if let config = configContent.defaultConfig()[fullyQualifiedNamespace] { | ||
| value = config[key] | ||
| if let value, value.source != .default { | ||
| RCLog.error("I-RCN000002", | ||
| "Key \(key) should come from source: \(RemoteConfigSource.default.rawValue)" + | ||
| "instead coming from source: \(value.source.rawValue)") | ||
| } | ||
| } | ||
| } | ||
| return value |
There was a problem hiding this comment.
Probably could return the value from the sync block, rather than passing it through the context.
| /// `RemoteConfig.remoteConfig().setDefaults(fromPlist: "defaultSamples")` | ||
| @objc(setDefaultsFromPlistFileName:) | ||
| public func setDefaults(fromPlist fileName: String?) { | ||
| guard let fileName = fileName, !fileName.isEmpty else { |
There was a problem hiding this comment.
guard let fileName, !fileName.isEmpty else {
|
Agree with all comments. |
ncooke3
added a commit
that referenced
this pull request
Jan 22, 2025
ncooke3
added a commit
that referenced
this pull request
Jan 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FIRRemoteConfig.m -> RemoteConfig.swift
This is a first pass sufficient to pass the fake console integration tests. Open questions include: