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

Allow copying databases from custom assets path #94

Merged
merged 4 commits into from
May 12, 2024

Conversation

ospfranco
Copy link
Contributor

No description provided.

@ospfranco ospfranco merged commit e429db7 into main May 12, 2024
5 checks passed

// Create the output file in the documents directory
val databasesFolder =
context.getDatabasePath("defaultDatabase")
.absolutePath
.replace("defaultDatabase", "")

val outputFile = File(databasesFolder, "$name.$extension")
val outputFile = File(databasesFolder, filename)

if (outputFile.exists()) {

Choose a reason for hiding this comment

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

Would it not be more efficient to check if the output file exists before even opening the input stream, so that it is only opened if we will actually use it? This doesn't matter if we are overwriting the file, but in the case where we're not and the file already exists, it would make a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think opening a stream makes such a difference, but I will move it.

if(overwrite) {
outputFile.delete()
} else {
promise.resolve(true)

Choose a reason for hiding this comment

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

Shouldn't the input stream be closed here before resolving the promise and returning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved by moving the instantiation down

extension: string
): boolean => {
return NativeModules.OPSQLite.moveAssetsDatabase(dbName, extension);
export const moveAssetsDatabase = (args: {

Choose a reason for hiding this comment

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

This method's native implementation is now async so the definition here needs to be updated to reflect that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah, missed that. Thanks!

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.

2 participants