-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Changes from 3 commits
703512b
7af8aa7
a403a0a
22c6f26
6216a8a
dabadec
ee0ff69
d29179c
458dcba
b69d6ca
880ed79
9ba0f1e
b4dfc2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -268,3 +268,14 @@ JNIEXPORT jstring JNICALL Java_io_realm_internal_SharedGroup_nativeGetDefaultRep | |
return 0; | ||
#endif | ||
} | ||
|
||
JNIEXPORT jboolean JNICALL Java_io_realm_internal_SharedGroup_nativeCompact( | ||
JNIEnv* env, jobject, jlong native_ptr) | ||
{ | ||
TR_ENTER_PTR(native_ptr) | ||
try { | ||
return SG(native_ptr)->compact(); // throws | ||
} | ||
CATCH_STD() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add |
||
return false; | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -917,11 +917,32 @@ public void testWriteCopyTo() throws IOException { | |
} | ||
} | ||
|
||
|
||
public void testCompactRealmFileThrowsIfOpen() throws IOException { | ||
try { | ||
Realm.compactRealmFile(getContext()); | ||
} catch (IllegalStateException expected) { | ||
return; | ||
} | ||
fail(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you move |
||
} | ||
|
||
public void testCompactEncryptedRealmFile() { | ||
String REALM_NAME = "enc.realm"; | ||
Realm.deleteRealmFile(getContext(), REALM_NAME); | ||
byte[] key = new byte[64]; | ||
new Random(42).nextBytes(key); | ||
Realm realm = Realm.getInstance(getContext(), REALM_NAME, key); | ||
realm.close(); | ||
assertTrue(Realm.compactRealmFile(getContext(), REALM_NAME, key)); | ||
} | ||
|
||
public void testCompactRealmFile() throws IOException { | ||
final String copyRealm = "copy.realm"; | ||
fileCopy( | ||
new File(getContext().getFilesDir(), Realm.DEFAULT_REALM_NAME), | ||
new File(getContext().getFilesDir(), copyRealm)); | ||
new File(getContext().getFilesDir(), Realm.DEFAULT_REALM_NAME), | ||
new File(getContext().getFilesDir(), copyRealm) | ||
); | ||
long before = new File(getContext().getFilesDir(), copyRealm).length(); | ||
assertTrue(Realm.compactRealmFile(getContext())); | ||
long after = new File(getContext().getFilesDir(), copyRealm).length(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,6 @@ | |
import android.os.Looper; | ||
import android.os.Message; | ||
import android.util.JsonReader; | ||
import android.util.Log; | ||
|
||
import org.json.JSONArray; | ||
import org.json.JSONException; | ||
|
@@ -1699,7 +1698,7 @@ public static synchronized boolean deleteRealmFile(Context context, String fileN | |
boolean deleteResult = fileToDelete.delete(); | ||
if (!deleteResult) { | ||
result = false; | ||
Log.w(TAG, "Could not delete the file " + fileToDelete); | ||
RealmLog.w("Could not delete the file " + fileToDelete); | ||
} | ||
} | ||
} | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a -> an |
||
* @return true if successful, false if any file operation failed | ||
* | ||
* @throws IllegalStateException if trying to compact a Realm that is already open | ||
*/ | ||
public static synchronized boolean compactRealmFile(Context context, String fileName) { | ||
public static synchronized boolean compactRealmFile(Context context, String fileName, byte[] key) { | ||
File realmFile = new File(context.getFilesDir(), fileName); | ||
File tmpFile = new File( | ||
context.getFilesDir(), | ||
String.valueOf(System.currentTimeMillis()) + UUID.randomUUID() + ".realm"); | ||
|
||
Realm realm = null; | ||
String path = realmFile.getAbsolutePath(); | ||
if (openRealms.get(path.hashCode()).get() > 0) { | ||
throw new IllegalStateException("Cannot compact a open Realm"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a -> an |
||
} | ||
SharedGroup sharedGroup = null; | ||
boolean result = false; | ||
try { | ||
realm = Realm.getInstance(context, fileName); | ||
realm.writeCopyTo(tmpFile); | ||
if (!realmFile.delete()) { | ||
return false; | ||
} | ||
if (!tmpFile.renameTo(realmFile)) { | ||
return false; | ||
} | ||
} catch (IOException e) { | ||
return false; | ||
sharedGroup = new SharedGroup(path, false, key); | ||
result = sharedGroup.compact(); | ||
} finally { | ||
if (realm != null) { | ||
realm.close(); | ||
if (sharedGroup != null) { | ||
sharedGroup.close(); | ||
} | ||
} | ||
return true; | ||
return result; | ||
} | ||
|
||
/** | ||
|
@@ -1756,9 +1751,30 @@ public static synchronized boolean compactRealmFile(Context context, String file | |
* | ||
* @param context an Android {@link android.content.Context} | ||
* @return true if successful, false if any file operation failed | ||
* | ||
* @throws IllegalStateException if trying to compact a Realm that is already open | ||
*/ | ||
public static boolean compactRealmFile(Context context) { | ||
return compactRealmFile(context, DEFAULT_REALM_NAME); | ||
return compactRealmFile(context, DEFAULT_REALM_NAME, null); | ||
} | ||
|
||
/** | ||
* Compact a realm file. A realm file usually contain free/unused space. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Realm is written with capital R in our documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually not always. @timanglade made some guidelines once - maybe he can share those again. In this case I wouldn't capitalize (but that's just based on intuition, not guideliens :-) ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's impossible to argue against intuition but the documentation for |
||
* This method removes this free space and the file size is thereby reduced. | ||
* Objects within the realm files are untouched. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. realm -> Realm |
||
* <p> | ||
* The file must be closed before this method is called.<br> | ||
* The file system should have free space for at least a copy of the realm file.<br> | ||
* The realm file is left untouched if any file operation fails.<br> | ||
* | ||
* @param context an Android {@link android.content.Context} | ||
* @param fileName the name of the file to compact | ||
* @return true if successful, false if any file operation failed | ||
* | ||
* @throws IllegalStateException if trying to compact a Realm that is already open | ||
*/ | ||
public static synchronized boolean compactRealmFile(Context context, String fileName) { | ||
return compactRealmFile(context, fileName, null); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, no :)