Skip to content

Fix tvOS storage issues. #6658

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 8, 2020
Merged

Fix tvOS storage issues. #6658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
03be979
Fix tvOS storage issues.
ryanwilson Oct 5, 2020
c1e2dae
Fixed changed dir name for RC
ryanwilson Oct 5, 2020
7a74299
Fixed test naming, too.
ryanwilson Oct 5, 2020
e85f3e6
Fix the documentation comment for GoogleUtilities.
ryanwilson Oct 5, 2020
f836d19
Updated changelog.
ryanwilson Oct 5, 2020
d0da96a
Review feedback
ryanwilson Oct 5, 2020
e89b159
Merge branch 'master' into rw-tvos-fixes
ryanwilson Oct 6, 2020
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
1 change: 1 addition & 0 deletions FirebaseRemoteConfig/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- [fixed] Updated `numberValue` to be nonnull to align with current behavior. (#6623)
- [removed] Removed deprecated APIs `isDeveloperModeEnabled`, `initWithDeveloperModeEnabled:developerModeEnabled`, `activateWithCompletionHandler:completionHandler`, `activateFetched`, `configValueForKey:namespace`, `configValueForKey:namespace:source`, `allKeysFromSource:namespace`, `keysWithPrefix:namespace`, `setDefaults:namespace`, `setDefaultsFromPlistFileName:namespace`, `defaultValueForKey:namespace`. (#6637)
- [fixed] Completion handler for `fetchAndActivateWithCompletionHandler` is now run on the main thread. (#5897)
- [fixed] Fixed database creation on tvOS. (#6612)

# v4.9.1
- [fixed] Fix an `attempt to insert nil object` crash in `fetchWithExpirationDuration:`. (#6522)
Expand Down
13 changes: 8 additions & 5 deletions FirebaseRemoteConfig/Sources/RCNConfigDBManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
static BOOL gIsNewDatabase;
/// SQLite file name in versions 0, 1 and 2.
static NSString *const RCNDatabaseName = @"RemoteConfig.sqlite3";
/// The application support sub-directory that the Remote Config database resides in.
static NSString *const RCNRemoteConfigApplicationSupportSubDirectory = @"Google/RemoteConfig";
/// The storage sub-directory that the Remote Config database resides in.
static NSString *const RCNRemoteConfigStorageSubDirectory = @"Google/RemoteConfig";

/// Remote Config database path for deprecated V0 version.
static NSString *RemoteConfigPathForOldDatabaseV0() {
Expand All @@ -46,11 +46,14 @@

/// Remote Config database path for current database.
static NSString *RemoteConfigPathForDatabase(void) {
#if TARGET_OS_TV
NSArray *dirPaths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *dirPaths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *appSupportPath = dirPaths.firstObject;
NSArray *components =
@[ appSupportPath, RCNRemoteConfigApplicationSupportSubDirectory, RCNDatabaseName ];
#endif
NSString *storageDirPath = dirPaths.firstObject;
NSArray *components = @[ storageDirPath, RCNRemoteConfigStorageSubDirectory, RCNDatabaseName ];
return [NSString pathWithComponents:components];
}

Expand Down
14 changes: 9 additions & 5 deletions FirebaseRemoteConfig/Tests/Unit/RCNTestUtilities.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
NSString *const RCNTestsDefaultFIRAppName = @"__FIRAPP_DEFAULT";
NSString *const RCNTestsSecondFIRAppName = @"secondFIRApp";

/// The application support sub-directory that the Remote Config database resides in.
static NSString *const RCNRemoteConfigApplicationSupportSubDirectory = @"Google/RemoteConfig";
/// The storage sub-directory that the Remote Config database resides in.
static NSString *const RCNRemoteConfigStorageSubDirectory = @"Google/RemoteConfig";

@implementation RCNTestUtilities

Expand All @@ -39,11 +39,15 @@ + (NSString *)generatedTestAppNameForTest:(NSString *)testName {

/// Remote Config database path for test version
+ (NSString *)remoteConfigPathForTestDatabase {
NSArray<NSString *> *dirPaths =
#if TARGET_OS_TV
NSArray *dirPaths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *dirPaths =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It seems like this condition appears in the code enough times to justify creating a method in core or google utilities to encapsulate it. Not necessary should be a part of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a reasonable idea, I do hesitate because I also want to make sure people are making intentional decisions about where they store things and recognize that this could be writing to a cache. Ideally I'd like to see this pop up in danger or something - if you're running tvOS tests and the active target has the NSApplicationSupportDirectory symbol, throw a warning/error.

NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *appSupportPath = dirPaths.firstObject;
#endif
NSString *storageDirPath = dirPaths.firstObject;
NSArray<NSString *> *components = @[
appSupportPath, RCNRemoteConfigApplicationSupportSubDirectory,
storageDirPath, RCNRemoteConfigStorageSubDirectory,
[NSString stringWithFormat:@"test-%f.sqlite3", [[NSDate date] timeIntervalSince1970] * 1000]
];
NSString *dbPath = [NSString pathWithComponents:components];
Expand Down
14 changes: 9 additions & 5 deletions FirebaseSegmentation/Sources/SEGDatabaseManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

/// SQLite file name.
static NSString *const kDatabaseName = @"FirebaseSegmentation.sqlite3";
/// The application support sub-directory that the Segmentation database resides in.
static NSString *const kApplicationSupportSubDirectory = @"Google/FirebaseSegmentation";
/// The storage sub-directory that the Segmentation database resides in.
static NSString *const kSegmentationStorageSubDirectory = @"Google/FirebaseSegmentation";
/// Column names
static NSString *const kMainTableName = @"main";
static NSString *const kMainTableColumnApplicationIdentifier = @"firebase_app_identifier";
Expand Down Expand Up @@ -220,11 +220,15 @@ - (NSDictionary *)loadMainTable {

/// Returns the current version of the Remote Config database.
+ (NSString *)pathForSegmentationDatabase {
NSArray<NSString *> *dirPaths =
#if TARGET_OS_TV
NSArray *dirPaths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *dirPaths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *appSupportPath = dirPaths.firstObject;
#endif
NSString *storageDir = dirPaths.firstObject;
NSArray<NSString *> *components =
@[ appSupportPath, kApplicationSupportSubDirectory, kDatabaseName ];
@[ storageDir, kSegmentationStorageSubDirectory, kDatabaseName ];
return [NSString pathWithComponents:components];
}

Expand Down
8 changes: 6 additions & 2 deletions FirebaseSegmentation/Tests/Unit/SEGDatabaseManagerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,16 @@ - (void)testDatabaseInsertAndRead {

#pragma mark Helpers
- (NSString *)pathForSegmentationTestDatabase {
#if TARGET_OS_TV
NSArray *dirPaths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *dirPaths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *appSupportPath = dirPaths.firstObject;
#endif
NSString *storageDir = dirPaths.firstObject;
NSString *databaseName =
[NSString stringWithFormat:@"FirebaseSegmentation-test-%d.sqlite3", (arc4random() % 100)];
NSArray *components = @[ appSupportPath, @"Google/FirebaseSegmentation", databaseName ];
NSArray *components = @[ storageDir, @"Google/FirebaseSegmentation", databaseName ];
NSString *dbPath = [NSString pathWithComponents:components];
NSLog(@"Created test database at: %@", dbPath);
return dbPath;
Expand Down
1 change: 1 addition & 0 deletions GoogleDataTransport/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased
- Legacy pre Xcode 10 compatibility checks removed. (#6486)
- `GDTCORDirectorySizeTracker` crash fixed. (#6540)
- Fixed writing heartbeat to disk on tvOS devices. (#6658)
Copy link
Member

Choose a reason for hiding this comment

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

@ryanwilson I don't see any changes that go with this CHANGELOG update


# v7.4.0
- Limit disk space consumed by GoogleDataTransport to store events. (#6365)
Expand Down
9 changes: 7 additions & 2 deletions GoogleUtilities/Environment/GULHeartbeatDateStorage.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,17 @@ - (NSURL *)fileURL {
return _fileURL;
}

/** Returns the URL path of the Application Support folder.
* @return the URL path of Application Support.
/** Returns the URL path of the directory for heartbeat storage data.
* @return the URL path of the directory for heartbeat storage data.
*/
+ (NSURL *)directoryPathURL {
#if TARGET_OS_TV
NSArray<NSString *> *paths =
NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the fix. I created #6662 for a specific tvOS implementation to improve reliability of the heartbeat.

#else
NSArray<NSString *> *paths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
#endif
NSArray<NSString *> *components = @[ paths.lastObject, @"Google/FIRApp" ];
NSString *directoryString = [NSString pathWithComponents:components];
NSURL *directoryURL = [NSURL fileURLWithPath:directoryString];
Expand Down
9 changes: 6 additions & 3 deletions GoogleUtilities/Network/GULNetworkURLSession.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,15 @@ - (instancetype)initWithNetworkLoggerDelegate:(id<GULNetworkLoggerDelegate>)netw
self = [super init];
if (self) {
// Create URL to the directory where all temporary files to upload have to be stored.
#if TARGET_OS_TV
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *paths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *applicationSupportDirectory = paths.firstObject;
#endif
NSString *storageDirectory = paths.firstObject;
NSArray *tempPathComponents = @[
applicationSupportDirectory, kGULNetworkApplicationSupportSubdirectory,
kGULNetworkTempDirectoryName
storageDirectory, kGULNetworkApplicationSupportSubdirectory, kGULNetworkTempDirectoryName
];
_networkDirectoryURL = [NSURL fileURLWithPathComponents:tempPathComponents];
_sessionID = [NSString stringWithFormat:@"%@-%@", kGULNetworkBackgroundSessionConfigIDPrefix,
Expand Down
28 changes: 19 additions & 9 deletions GoogleUtilities/Tests/Unit/Environment/GULHeartbeatDateStorageTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ @interface GULHeartbeatDateStorageTest : XCTestCase
@implementation GULHeartbeatDateStorageTest

- (void)setUp {
NSString *documentsPath = [NSSearchPathForDirectoriesInDomains(
NSApplicationSupportDirectory, NSUserDomainMask, YES) firstObject];
XCTAssertNotNil(documentsPath);
NSURL *documentsURL = [NSURL fileURLWithPath:documentsPath];
#if TARGET_OS_TV
NSArray *path = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *path =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
#endif
NSString *rootPath = [path firstObject];
XCTAssertNotNil(rootPath);
NSURL *rootURL = [NSURL fileURLWithPath:rootPath];

NSError *error;
if (![documentsURL checkResourceIsReachableAndReturnError:&error]) {
XCTAssert([[NSFileManager defaultManager] createDirectoryAtURL:documentsURL
if (![rootURL checkResourceIsReachableAndReturnError:&error]) {
XCTAssert([[NSFileManager defaultManager] createDirectoryAtURL:rootURL
withIntermediateDirectories:YES
attributes:nil
error:&error],
Expand Down Expand Up @@ -69,9 +74,14 @@ - (void)assertInitializationDoesNotAccessFileSystem {
}

- (NSURL *)heartbeatFileURL {
NSString *documentsPath = [NSSearchPathForDirectoriesInDomains(
NSApplicationSupportDirectory, NSUserDomainMask, YES) firstObject];
NSArray<NSString *> *components = @[ documentsPath, @"Google/FIRApp", kTestFileName ];
#if TARGET_OS_TV
NSArray *path = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *path =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
#endif
NSString *rootPath = [path firstObject];
NSArray<NSString *> *components = @[ rootPath, @"Google/FIRApp", kTestFileName ];
NSString *fileString = [NSString pathWithComponents:components];
NSURL *fileURL = [NSURL fileURLWithPath:fileString];
return fileURL;
Expand Down
4 changes: 4 additions & 0 deletions GoogleUtilities/Tests/Unit/Network/GULNetworkTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,8 +897,12 @@ - (void)testRemoveExpiredFiles {

GULNetworkURLSession *session = [[GULNetworkURLSession alloc]
initWithNetworkLoggerDelegate:(id<GULNetworkLoggerDelegate>)_network];
#if TARGET_OS_TV
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
#else
NSArray *paths =
NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
#endif
NSString *applicationSupportDirectory = paths.firstObject;
NSArray *tempPathComponents = @[
applicationSupportDirectory, kGULNetworkApplicationSupportSubdirectory,
Expand Down