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

Made Realm.compactRealmFile() more failure resilient. #955

Merged
merged 13 commits into from
Apr 16, 2015
Merged

Conversation

cmelchior
Copy link
Contributor

@emanuelez @kneth @bmunkholm

@kneth
Copy link
Contributor

kneth commented Mar 12, 2015

We should definitely start using core's compact() method.

@cmelchior
Copy link
Contributor Author

I have added cores compact method and we now use that instead
@emanuelez @kneth

Realm realm = null;
String path = realmFile.getAbsolutePath();
if (openRealms.get(path.hashCode()).get() > 0) {
throw new IllegalStateException("Cannot compact a open Realm");
Copy link
Contributor

Choose a reason for hiding this comment

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

a -> an

@kneth
Copy link
Contributor

kneth commented Mar 13, 2015

Besides my comments: 👍

@@ -1717,32 +1716,28 @@ public static synchronized boolean deleteRealmFile(Context context, String fileN
*
* @param context an Android {@link android.content.Context}
* @param fileName the name of the file to compact
* @param key Key for opening a encrypted Realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

a -> an

} catch (IllegalStateException expected) {
return;
}
fail();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you move fail() inside the try block, the return isn't needed.

@timanglade
Copy link
Contributor

Will repeat this during the “how to talk about Realm” presentation.

“Realm” is always capitalized i.e. “a Realm file” but in some context it’s easier to refer to the file extension, which is lowercase. For example:

  • “You can compact a Realm file”
  • “look for the .realm file in your Documents folder“

@kneth
Copy link
Contributor

kneth commented Apr 15, 2015

I have disabled compacting encrypted Realms as it fails on some devices.

@cmelchior @emanuelez @tgoyne @finnschiermer

@@ -1,4 +1,5 @@
0.81
* Realm.compactRealmFile() now also works for encrypted Realms.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, no :)

kneth pushed a commit that referenced this pull request Apr 16, 2015
Made Realm.compactRealmFile() more failure resilient.
@kneth kneth merged commit da535c6 into master Apr 16, 2015
@finnschiermer
Copy link

This look like a testcase error: when calling compact on the SharedGroup, it must be in the attached state (file must have been opened). In the testcases you close SharedGroup right before calling compact. I guess compact() should detect and assert.

@kneth
Copy link
Contributor

kneth commented May 18, 2015

@finnschiermer The Realm.compactRealmFile() creates a SharedGroup object (without implicit transactions), compacts the file, and closes the SharedGroup.

@finnschiermer
Copy link

okidoki

@emanuelez emanuelez deleted the cm-bug-compactfile branch October 16, 2015 09:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compactRealmFile() doesn't cleanup properly
6 participants