Skip to content

refactor: Added ability to inject custom directory for DataStoreFile#612

Open
artesmichael wants to merge 5 commits intooptimizely:masterfrom
artesmichael:artesmichael/make-data-store-more-flexible
Open

refactor: Added ability to inject custom directory for DataStoreFile#612
artesmichael wants to merge 5 commits intooptimizely:masterfrom
artesmichael:artesmichael/make-data-store-more-flexible

Conversation

@artesmichael
Copy link

Summary

  • Extended DataStoreFile to allow for injection of custom FileManager.SearchPathDirectory
  • Extended DefaultEventDispatcher to allow for injection of custom FileManager.SearchPathDirectory for DataStoreType.file

Users have been raising the issue that they see these technical files inside of their Files app, because the directory for DataStoreFile defaults to .documentsDirectory, which is visible to users. This change allows us to update the directory to .applicationSupportDirectory in DefaultEventDispatcher using DataStoreType.file(directory: .applicationSupportDirectory) and in DefaultDatafileHandler by overriding open func createDataStore(sdkKey: String) -> OPTDataStore.

Test plan

  • Added a new test to verify the custom directory is used by DataStoreFile

Issues

  • None

@artesmichael
Copy link
Author

Hi @muzahidul-opti, could you please review this PR and let me know if any changes are required?

@100hp4ever
Copy link

lgtm

@muzahidul-opti
Copy link
Contributor

Hi @muzahidul-opti, could you please review this PR and let me know if any changes are required?

Sure, we will review it.

@artesmichael
Copy link
Author

Hi @muzahidul-opti, are there any updates on this?

Copy link
Contributor

@muzahidul-opti muzahidul-opti left a comment

Choose a reason for hiding this comment

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

Looks good to me, CI failing could you please fix it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants