Skip to content

Create [bundle ID] folder during migration if none exists #303

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 4 commits into from
Feb 28, 2020
Merged

Create [bundle ID] folder during migration if none exists #303

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd83239
create [bundle ID] folder during migration if none exists
joonmanji Feb 19, 2020
4a0d756
typo
joonmanji Feb 20, 2020
fdfcf40
PR review changes
joonmanji Feb 25, 2020
dc9de89
PR review changes + fix indent
joonmanji Feb 25, 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
27 changes: 22 additions & 5 deletions ios/RNCAsyncStorage.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,32 @@ static void RCTStorageDirectoryCleanupOld(NSString *oldDirectoryPath)
}
}

static void _createStorageDirectory(NSString *storageDirectory, NSError **error)
{
[[NSFileManager defaultManager] createDirectoryAtPath:storageDirectory
withIntermediateDirectories:YES
attributes:nil
error:error];
}

static void RCTStorageDirectoryMigrate(NSString *oldDirectoryPath, NSString *newDirectoryPath, BOOL shouldCleanupOldDirectory)
{
NSError *error;
// Migrate data by copying old storage directory to new storage directory location
if (![[NSFileManager defaultManager] copyItemAtPath:oldDirectoryPath toPath:newDirectoryPath error:&error]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we put a fileExistsAtPath: check before this and if false, call _createStorageDirectory. Then add an "error != nil" to the copyItemAtPath: check to make sure the fileExistsAtPath succeeded and leave the rest of the method unchanged?

Seems a little messy to try to copy and recursively respond to that failure instead of checking if we should create first, putting ourselves in a consistent state in all cases and going from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey chris.. i'm down to make this change but i was reading on the apple docs that its better to attempt an operation and respond than to try to predict on how an operation will go based on the file system's current state. https://developer.apple.com/documentation/foundation/nsfilemanager/1415645-fileexistsatpath

since this is a race condition issue and we are the only thing writing to that folder, it'll be okay.
i'll test and try to get something up within today

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know about that doc. I think what you're doing is good then!

RCTStorageDirectoryMigrationLogError(@"Failed to copy old storage directory to new storage directory location during migration", error);
// the new storage directory "Application Support/[bundleID]/RCTAsyncLocalStorage_V1" seems unable to migrate
// because folder "Application Support/[bundleID]" doesn't exist.. create this folder and attempt folder copying again
if (error != nil && error.code == 4 && [newDirectoryPath isEqualToString:RCTGetStorageDirectory()]) {
NSError *error = nil;
_createStorageDirectory(RCTCreateStorageDirectoryPath(@""), &error);
if (error == nil) {
RCTStorageDirectoryMigrate(oldDirectoryPath, newDirectoryPath, shouldCleanupOldDirectory);
} else {
RCTStorageDirectoryMigrationLogError(@"Failed to create storage directory during migration.", error);
}
} else {
RCTStorageDirectoryMigrationLogError(@"Failed to copy old storage directory to new storage directory location during migration", error);
}
} else if (shouldCleanupOldDirectory) {
// If copying succeeds, remove old storage directory
RCTStorageDirectoryCleanupOld(oldDirectoryPath);
Expand Down Expand Up @@ -354,10 +374,7 @@ - (NSDictionary *)_ensureSetup

NSError *error = nil;
if (!RCTHasCreatedStorageDirectory) {
[[NSFileManager defaultManager] createDirectoryAtPath:RCTGetStorageDirectory()
withIntermediateDirectories:YES
attributes:nil
error:&error];
_createStorageDirectory(RCTGetStorageDirectory(), &error);
if (error) {
return RCTMakeError(@"Failed to create storage directory.", error, nil);
}
Expand Down