Skip to content
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

tests #12133

Closed
wants to merge 2 commits into from
Closed

tests #12133

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions Crashlytics/Crashlytics/FIRCrashlytics.m
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#import <GoogleDataTransport/GoogleDataTransport.h>

@import FirebaseSessions;
@import FirebaseRemoteConfigInterop;

#if TARGET_OS_IPHONE
#import <UIKit/UIKit.h>
Expand Down Expand Up @@ -104,7 +105,8 @@ - (instancetype)initWithApp:(FIRApp *)app
appInfo:(NSDictionary *)appInfo
installations:(FIRInstallations *)installations
analytics:(id<FIRAnalyticsInterop>)analytics
sessions:(id<FIRSessionsProvider>)sessions {
sessions:(id<FIRSessionsProvider>)sessions
remoteConfig:(id<FIRRemoteConfigInterop>)remoteConfig {
self = [super init];

if (self) {
Expand Down Expand Up @@ -206,6 +208,10 @@ + (void)load {
FIRDependency *sessionsDep =
[FIRDependency dependencyWithProtocol:@protocol(FIRSessionsProvider)];

FIRDependency *remoteConfigDep =
[FIRDependency dependencyWithProtocol:@protocol(FIRRemoteConfigInterop)];

// break point on this also with rc creatioBlock and check the order
FIRComponentCreationBlock creationBlock =
^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) {
if (!container.app.isDefaultApp) {
Expand All @@ -215,6 +221,11 @@ + (void)load {

id<FIRAnalyticsInterop> analytics = FIR_COMPONENT(FIRAnalyticsInterop, container);
id<FIRSessionsProvider> sessions = FIR_COMPONENT(FIRSessionsProvider, container);
id<FIRRemoteConfigInterop> remoteConfig = FIR_COMPONENT(FIRRemoteConfigInterop, container);

if (remoteConfig) {
[remoteConfig registerRolloutsStateSubscriber:@"fire-cls" subscriber:self];
}

FIRInstallations *installations = [FIRInstallations installationsWithApp:container.app];

Expand All @@ -224,13 +235,14 @@ + (void)load {
appInfo:NSBundle.mainBundle.infoDictionary
installations:installations
analytics:analytics
sessions:sessions];
sessions:sessions
remoteConfig:remoteConfig];
};

FIRComponent *component =
[FIRComponent componentWithProtocol:@protocol(FIRCrashlyticsInstanceProvider)
instantiationTiming:FIRInstantiationTimingEagerInDefaultApp
dependencies:@[ analyticsDep, sessionsDep ]
dependencies:@[ analyticsDep, sessionsDep, remoteConfigDep ]
creationBlock:creationBlock];
return @[ component ];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#import "FIRCrashlyticsReport.h"
#import "FIRExceptionModel.h"

@protocol FIRRolloutsStateSubscriber;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we import this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this can be imported.

Here's an example where AppCheck imports the AppCheckInterop header in order to declare conformance with the FIRAppCheckProtocol protocol from the interop SDK.

Copy link
Member

@ncooke3 ncooke3 Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs to be imported per https://stackoverflow.com/a/11533804/9331576. Were you able to get things building in its current state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I see in codebase that we have some pattern for forward declaring a protocol, for example FireAuth: https://github.com/firebase/firebase-ios-sdk/blob/master/FirebaseAuth/Sources/Backend/FIRAuthRequestConfiguration.h#L24. I am able to get things build for now. But I'll change to import if it's safer.

#if __has_include(<Crashlytics/Crashlytics.h>)
#warning "FirebaseCrashlytics and Crashlytics are not compatible \
in the same app because including multiple crash reporters can \
Expand All @@ -35,7 +35,7 @@ NS_ASSUME_NONNULL_BEGIN
* we suggest using a wrapper class or a protocol extension.
*/
NS_SWIFT_NAME(Crashlytics)
@interface FIRCrashlytics : NSObject
@interface FIRCrashlytics : NSObject <FIRRolloutsStateSubscriber>

/** :nodoc: */
- (instancetype)init NS_UNAVAILABLE;
Expand Down
14 changes: 14 additions & 0 deletions FirebaseRemoteConfig/Interop/RemoteConfigInterop.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
Comment on lines +1 to +6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.


import Foundation

@objc(FIRRemoteConfigInterop)
public protocol RemoteConfigInterop {
func registerRolloutsStateSubscriber(_ namespace: String,
subscriber: RolloutsStateSubscriber?)
}
39 changes: 39 additions & 0 deletions FirebaseRemoteConfig/Interop/RolloutAssignment.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
Comment on lines +1 to +6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.


import Foundation

@objc(FIRRolloutAssignment)
public class RolloutAssignment: NSObject {
@objc public var rolloutId: String
@objc public var variantId: String
@objc public var templateVersion: String
@objc public var parameterKey: String
@objc public var parameterValue: String

public init(rolloutId: String, variantId: String, templateVersion: String, parameterKey: String,
parameterValue: String) {
self.rolloutId = rolloutId
self.variantId = variantId
self.templateVersion = templateVersion
self.parameterKey = parameterKey
self.parameterValue = parameterValue
super.init()
}
}

@objc(FIRRolloutsState)
public class RolloutsState: NSObject {
@objc public var assignments: Set<RolloutAssignment> = Set()

public init(assignmentList: [RolloutAssignment]) {
for assignment in assignmentList {
assignments.insert(assignment)
}
super.init()
}
}
13 changes: 13 additions & 0 deletions FirebaseRemoteConfig/Interop/RolloutsStateSubscriber.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
Comment on lines +1 to +6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//
// File.swift
//
//
// Created by Themis Wang on 2023-11-16.
//
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.


import Foundation

@objc(FIRRolloutsStateSubscriber)
public protocol RolloutsStateSubscriber {
func onRolloutsStateChanged(_ rolloutsState: RolloutsState)
}
9 changes: 9 additions & 0 deletions FirebaseRemoteConfig/Sources/FIRRemoteConfig.m
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#import "FirebaseRemoteConfig/Sources/RCNDevice.h"
#import "FirebaseRemoteConfig/Sources/RCNPersonalization.h"

@import FirebaseRemoteConfigInterop;

/// Remote Config Error Domain.
/// TODO: Rename according to obj-c style for constants.
NSString *const FIRRemoteConfigErrorDomain = @"com.google.remoteconfig.ErrorDomain";
Expand Down Expand Up @@ -73,6 +75,7 @@ @implementation FIRRemoteConfig {
dispatch_queue_t _queue;
NSString *_appName;
NSMutableArray *_listeners;
NSMutableArray *_subscribers;
}

static NSMutableDictionary<NSString *, NSMutableDictionary<NSString *, FIRRemoteConfig *> *>
Expand Down Expand Up @@ -176,6 +179,8 @@ - (instancetype)initWithAppName:(NSString *)appName

[_settings loadConfigFromMetadataTable];

_subscribers = [[NSMutableArray alloc] init];

if (analytics) {
_listeners = [[NSMutableArray alloc] init];
RCNPersonalization *personalization =
Expand Down Expand Up @@ -613,4 +618,8 @@ - (FIRConfigUpdateListenerRegistration *)addOnConfigUpdateListener:
return [self->_configRealtime addConfigUpdateListener:listener];
}

- (void)addRemoteConfigInteropSubscriber:(id<FIRRolloutsStateSubscriber>)subscriber {
[self->_subscribers addObject:subscriber];
}

@end
4 changes: 3 additions & 1 deletion FirebaseRemoteConfig/Sources/FIRRemoteConfigComponent.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import <Foundation/Foundation.h>

#import "FirebaseCore/Extension/FirebaseCoreInternal.h"
@import FirebaseRemoteConfigInterop;

@class FIRApp;
@class FIRRemoteConfig;
Expand All @@ -37,7 +38,8 @@ NS_ASSUME_NONNULL_BEGIN

/// A concrete implementation for FIRRemoteConfigInterop to create Remote Config instances and
/// register with Core's component system.
@interface FIRRemoteConfigComponent : NSObject <FIRRemoteConfigProvider, FIRLibrary>
@interface FIRRemoteConfigComponent
: NSObject <FIRRemoteConfigProvider, FIRLibrary, FIRRemoteConfigInterop>

/// The FIRApp that instances will be set up with.
@property(nonatomic, weak, readonly) FIRApp *app;
Expand Down
41 changes: 40 additions & 1 deletion FirebaseRemoteConfig/Sources/FIRRemoteConfigComponent.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@
#import "Interop/Analytics/Public/FIRAnalyticsInterop.h"

@implementation FIRRemoteConfigComponent
static FIRRemoteConfigComponent *_sharedDefaultAppRemoteConfigSingleton = nil;

+ (FIRRemoteConfigComponent *)sharedDefaultAppRemoteConfigSingletonWithDefaultApp:(FIRApp *)app {
Copy link
Contributor Author

@themiswang themiswang Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need some strategy to create a singleton for the default app Provider and Interop. They will return the same object.

Copy link
Contributor Author

@themiswang themiswang Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked RC Component from Crashlytics and FirePerf, confirm they have the same RC Component: https://screenshot.googleplex.com/9NXgjPbmtxoSrSe

@synchronized([FIRRemoteConfigComponent class]) {
if (!_sharedDefaultAppRemoteConfigSingleton) {
_sharedDefaultAppRemoteConfigSingleton = [[self alloc] initWithApp:app];
}
return _sharedDefaultAppRemoteConfigSingleton;
}
return nil;
}

/// Default method for retrieving a Remote Config instance, or creating one if it doesn't exist.
- (FIRRemoteConfig *)remoteConfigForNamespace:(NSString *)remoteConfigNamespace {
Expand Down Expand Up @@ -60,6 +71,7 @@ - (FIRRemoteConfig *)remoteConfigForNamespace:(NSString *)remoteConfigNamespace
FIRApp *app = self.app;
id<FIRAnalyticsInterop> analytics =
app.isDefaultApp ? FIR_COMPONENT(FIRAnalyticsInterop, app.container) : nil;

instance = [[FIRRemoteConfig alloc] initWithAppName:app.name
FIROptions:app.options
namespace:remoteConfigNamespace
Expand Down Expand Up @@ -95,16 +107,43 @@ + (void)load {
+ (NSArray<FIRComponent *> *)componentsToRegister {
FIRDependency *analyticsDep = [FIRDependency dependencyWithProtocol:@protocol(FIRAnalyticsInterop)
isRequired:NO];

FIRComponent *rcProvider = [FIRComponent
componentWithProtocol:@protocol(FIRRemoteConfigProvider)
instantiationTiming:FIRInstantiationTimingAlwaysEager
dependencies:@[ analyticsDep ]
creationBlock:^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) {
// Cache the component so instances of Remote Config are cached.
*isCacheable = YES;
if (container.app.isDefaultApp) {
return [FIRRemoteConfigComponent
sharedDefaultAppRemoteConfigSingletonWithDefaultApp:container.app];
}
return [[FIRRemoteConfigComponent alloc] initWithApp:container.app];
}];
return @[ rcProvider ];

// Crashlytics only works on default app, the only shared component between Provider and Interop
// is the component for default app
FIRComponent *rcInterop = [FIRComponent
componentWithProtocol:@protocol(FIRRemoteConfigInterop)
instantiationTiming:FIRInstantiationTimingAlwaysEager
dependencies:@[]
creationBlock:^id _Nullable(FIRComponentContainer *container, BOOL *isCacheable) {
// Cache the component so instances of Remote Config are cached.
*isCacheable = YES;
if (container.app.isDefaultApp) {
return [FIRRemoteConfigComponent
sharedDefaultAppRemoteConfigSingletonWithDefaultApp:container.app];
}
return nil;
}];
return @[ rcProvider, rcInterop ];
}

- (void)registerRolloutsStateSubscriber:(nonnull NSString *)nameSpace
subscriber:(nullable id<FIRRolloutsStateSubscriber>)subscriber {
// adding the registered subscriber reference to the namespace instance
[self.instances[nameSpace] addRemoteConfigInteropSubscriber:subscriber];
}

themiswang marked this conversation as resolved.
Show resolved Hide resolved
@end
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
@class RCNConfigDBManager;
@class RCNConfigFetch;
@class RCNConfigRealtime;

@protocol FIRRemoteConfigComponentDelegate;
@protocol FIRAnalyticsInterop;

NS_ASSUME_NONNULL_BEGIN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import <Foundation/Foundation.h>

@class FIRApp;
@protocol FIRRolloutsStateSubscriber;

/// The Firebase Remote Config service default namespace, to be used if the API method does not
/// specify a different namespace. Use the default namespace if configuring from the Google Firebase
Expand Down Expand Up @@ -150,6 +151,8 @@ NS_SWIFT_NAME(RemoteConfigValue)
@property(nonatomic, readonly, nullable) id JSONValue NS_SWIFT_NAME(jsonValue);
/// Identifies the source of the fetched value.
@property(nonatomic, readonly) FIRRemoteConfigSource source;

@property(nonatomic, readonly, nonnull) NSMutableArray<id<FIRRolloutsStateSubscriber>> *subscribers;
@end

#pragma mark - FIRRemoteConfigSettings
Expand Down Expand Up @@ -357,4 +360,5 @@ typedef void (^FIRRemoteConfigUpdateCompletion)(FIRRemoteConfigUpdate *_Nullable
(FIRRemoteConfigUpdateCompletion _Nonnull)listener
NS_SWIFT_NAME(addOnConfigUpdateListener(remoteConfigUpdateCompletion:));

- (void)addRemoteConfigInteropSubscriber:(nonnull id<FIRRolloutsStateSubscriber>)subscriber;
@end
25 changes: 20 additions & 5 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,16 @@ let package = Package(
),
.target(
name: "FirebaseCrashlytics",
dependencies: ["FirebaseCore", "FirebaseInstallations", "FirebaseSessions",
.product(name: "GoogleDataTransport", package: "GoogleDataTransport"),
.product(name: "GULEnvironment", package: "GoogleUtilities"),
.product(name: "FBLPromises", package: "Promises"),
.product(name: "nanopb", package: "nanopb")],
dependencies: [
"FirebaseCore",
"FirebaseInstallations",
"FirebaseSessions",
"FirebaseRemoteConfigInterop",
.product(name: "GoogleDataTransport", package: "GoogleDataTransport"),
.product(name: "GULEnvironment", package: "GoogleUtilities"),
.product(name: "FBLPromises", package: "Promises"),
.product(name: "nanopb", package: "nanopb"),
],
path: "Crashlytics",
exclude: [
"run",
Expand Down Expand Up @@ -957,6 +962,7 @@ let package = Package(
"FirebaseCore",
"FirebaseABTesting",
"FirebaseInstallations",
"FirebaseRemoteConfigInterop",
.product(name: "GULNSData", package: "GoogleUtilities"),
],
path: "FirebaseRemoteConfig/Sources",
Expand Down Expand Up @@ -1028,6 +1034,15 @@ let package = Package(
.headerSearchPath("../../../"),
]
),
// Internal headers only for consuming from other SDK.
.target(
name: "FirebaseRemoteConfigInterop",
path: "FirebaseRemoteConfig/Interop",
publicHeadersPath: ".",
cSettings: [
.headerSearchPath("../../"),
]
),

// MARK: - Firebase Sessions

Expand Down
Loading