Skip to content

Commit

Permalink
[Rollouts] Address feature branch comments (#12411)
Browse files Browse the repository at this point in the history
  • Loading branch information
themiswang committed Mar 8, 2024
1 parent 32a3b5a commit bb7bb5a
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 98 deletions.
46 changes: 0 additions & 46 deletions ClientApp/Podfile

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@import FirebaseCrashlyticsSwift;
#else // Swift Package Manager
#import <FirebaseCrashlytics/FirebaseCrashlytics-Swift.h>
#endif // Cocoapod
#endif // CocoaPods

@interface FIRCLSRolloutsPersistenceManager : NSObject <FIRCLSPersistenceLog>

Expand All @@ -26,4 +26,5 @@

- (void)updateRolloutsStateToPersistenceWithRollouts:(NSData *_Nonnull)rollouts
reportID:(NSString *_Nonnull)reportID;
- (void)debugLog:(NSString *_Nonnull)messages;
@end
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
#import "Crashlytics/Crashlytics/Helpers/FIRCLSLogger.h"
#import "Crashlytics/Crashlytics/Models/FIRCLSFileManager.h"
#import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"

#if SWIFT_PACKAGE
@import FirebaseCrashlyticsSwift;
#else // Swift Package Manager
#import <FirebaseCrashlytics/FirebaseCrashlytics-Swift.h>
#endif // Cocoapod
#endif // CocoaPods

@interface FIRCLSRolloutsPersistenceManager : NSObject <FIRCLSPersistenceLog>
@property(nonatomic, readonly) FIRCLSFileManager *fileManager;
Expand Down Expand Up @@ -58,4 +59,8 @@ - (void)updateRolloutsStateToPersistenceWithRollouts:(NSData *_Nonnull)rollouts
[rolloutsFile writeData:newLineData];
});
}

- (void)debugLog:(NSString *_Nonnull)messages {
FIRCLSDebugLog(messages);
}
@end
2 changes: 1 addition & 1 deletion Crashlytics/Crashlytics/FIRCrashlytics.m
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
@import FirebaseCrashlyticsSwift;
#else // Swift Package Manager
#import <FirebaseCrashlytics/FirebaseCrashlytics-Swift.h>
#endif // Cocoapod
#endif // CocoaPods

#if TARGET_OS_IPHONE
#import <UIKit/UIKit.h>
Expand Down
2 changes: 1 addition & 1 deletion Crashlytics/Crashlytics/Handlers/FIRCLSException.mm
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ void FIRCLSExceptionRecord(FIRCLSExceptionType type,

// Create new report and copy into it the current state of custom keys and log and the sdk.log,
// binary_images.clsrecord, and metadata.clsrecord files.
// also copy rollouts.clsrecord if applicable.
// Also copy rollouts.clsrecord if applicable.
NSError *error = nil;
BOOL copied = [fileManager.underlyingFileManager copyItemAtPath:currentReportPath
toPath:newReportPath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Foundation
@objc(FIRCLSPersistenceLog)
public protocol CrashlyticsPersistenceLog {
func updateRolloutsStateToPersistence(rollouts: Data, reportID: String)
func debugLog(message: String)
}

@objc(FIRCLSRemoteConfigManager)
Expand Down Expand Up @@ -49,7 +50,7 @@ public class CrashlyticsRemoteConfigManager: NSObject {
_rolloutAssignment = normalizeRolloutAssignment(assignments: Array(rolloutsState.assignments))
lock.unlock()

// writring to persistence
// Writring to persistence
if let rolloutsData =
getRolloutsStateEncodedJsonData() {
persistenceDelegate.updateRolloutsStateToPersistence(
Expand All @@ -60,16 +61,20 @@ public class CrashlyticsRemoteConfigManager: NSObject {
}

/// Return string format: [{RolloutAssignment1}, {RolloutAssignment2}, {RolloutAssignment3}...]
/// This will get insert into each clsrcord for non-fatal events.
/// This will get inserted into each clsrcord for non-fatal events.
/// Return a string type because later `FIRCLSFileWriteStringUnquoted` takes string as input
@objc public func getRolloutAssignmentsEncodedJsonString() -> String? {
let encodeData = getRolloutAssignmentsEncodedJsonData()
if let data = encodeData {
return String(data: data, encoding: .utf8)
}

// TODO(themisw): Hook into core logging functions
debugPrint("Failed to serialize rollouts", encodeData ?? "nil")
let debugInfo = encodeData?.debugDescription ?? "nil"
persistenceDelegate.debugLog(message: String(
format: "Failed to serialize rollouts: %@",
arguments: [debugInfo]
))

return nil
}
}
Expand All @@ -78,7 +83,10 @@ private extension CrashlyticsRemoteConfigManager {
func normalizeRolloutAssignment(assignments: [RolloutAssignment]) -> [RolloutAssignment] {
var validatedAssignments = assignments
if assignments.count > CrashlyticsRemoteConfigManager.maxRolloutAssignments {
debugPrint("Rollouts excess the maximum number of assignments can pass to Crashlytics")
persistenceDelegate
.debugLog(
message: "Rollouts excess the maximum number of assignments can pass to Crashlytics"
)
validatedAssignments =
Array(assignments[..<CrashlyticsRemoteConfigManager.maxRolloutAssignments])
}
Expand Down
2 changes: 1 addition & 1 deletion Crashlytics/UnitTests/FIRCLSFileTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
@import FirebaseCrashlyticsSwift;
#else // Swift Package Manager
#import <FirebaseCrashlytics/FirebaseCrashlytics-Swift.h>
#endif // Cocoapod
#endif // CocoaPods

#import <XCTest/XCTest.h>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
@import FirebaseCrashlyticsSwift;
#else // Swift Package Manager
#import <FirebaseCrashlytics/FirebaseCrashlytics-Swift.h>
#endif // Cocoapod
#endif // CocoaPods

NSString *reportId = @"1234567";

Expand Down
2 changes: 1 addition & 1 deletion FirebaseCrashlytics.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Pod::Spec.new do |s|
s.dependency 'FirebaseCore', '~> 10.5'
s.dependency 'FirebaseInstallations', '~> 10.0'
s.dependency 'FirebaseSessions', '~> 10.5'
s.dependency 'FirebaseRemoteConfigInterop', '~> 10.20'
s.dependency 'FirebaseRemoteConfigInterop', '~> 10.23'
s.dependency 'PromisesObjC', '~> 2.1'
s.dependency 'GoogleDataTransport', '~> 9.2'
s.dependency 'GoogleUtilities/Environment', '~> 7.8'
Expand Down
2 changes: 1 addition & 1 deletion FirebaseRemoteConfig.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ app update.
s.dependency 'FirebaseInstallations', '~> 10.0'
s.dependency 'GoogleUtilities/Environment', '~> 7.8'
s.dependency 'GoogleUtilities/NSData+zlib', '~> 7.8'
s.dependency 'FirebaseRemoteConfigInterop', '~> 10.20'
s.dependency 'FirebaseRemoteConfigInterop', '~> 10.23'

s.test_spec 'unit' do |unit_tests|
unit_tests.scheme = { :code_coverage => true }
Expand Down
4 changes: 2 additions & 2 deletions FirebaseRemoteConfigInterop.podspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = 'FirebaseRemoteConfigInterop'
s.version = '10.20.0'
s.version = '10.23.0'
s.summary = 'Interfaces that allow other Firebase SDKs to use Remote Config functionality.'

s.description = <<-DESC
Expand All @@ -21,7 +21,7 @@ Pod::Spec.new do |s|
}

s.swift_version = '5.3'
s.cocoapods_version = '>= 1.4.0'
s.cocoapods_version = '>= 1.12.0'
s.prefix_header_file = false

s.social_media_url = 'https://twitter.com/Firebase'
Expand Down
1 change: 1 addition & 0 deletions IntegrationTesting/ClientApp/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ target 'ClientApp-CocoaPods' do
pod 'FirebaseAppCheck', :path => '../../'
pod 'FirebaseRemoteConfig', :path => '../../'
pod 'FirebaseRemoteConfigSwift', :path => '../../'
pod 'FirebaseRemoteConfigInterop', :path => '../../'
pod 'FirebaseAppDistribution', :path => '../../'
pod 'FirebaseAuth', :path => '../../'
pod 'FirebaseCrashlytics', :path => '../../'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ target 'CocoapodsIntegrationTest' do
pod 'FirebaseInstallations', :path => '../../'
pod 'FirebaseMessaging', :path => '../../'
pod 'FirebaseMessagingInterop', :path => '../../'
pod 'FirebaseRemoteConfigInterop', :path => '../../'
pod 'FirebasePerformance', :path => '../../'
pod 'FirebaseStorage', :path => '../../'
end
Expand Down

0 comments on commit bb7bb5a

Please sign in to comment.