From e3e824269c38684669ab6b6d2f2c0b33eb0a0793 Mon Sep 17 00:00:00 2001 From: pushkarm029 Date: Tue, 11 Jun 2024 22:53:47 +0530 Subject: [PATCH] fix Signed-off-by: pushkarm029 --- fs-storage/src/jni/file_storage.rs | 101 +++++++++++++------------- fs-storage/tests/FileStorage.java | 58 +++++++++++---- fs-storage/tests/FileStorageTest.java | 35 +++------ 3 files changed, 104 insertions(+), 90 deletions(-) diff --git a/fs-storage/src/jni/file_storage.rs b/fs-storage/src/jni/file_storage.rs index 227e3a2..d5c0b98 100644 --- a/fs-storage/src/jni/file_storage.rs +++ b/fs-storage/src/jni/file_storage.rs @@ -14,7 +14,7 @@ use jni::objects::{JClass, JString, JValue}; // This is just a pointer. We'll be returning it from our function. We // can't return one of the objects with lifetime information because the // lifetime checker won't let us. -use jni::sys::{jlong, jobject, jstring}; +use jni::sys::{jboolean, jlong, jobject}; use crate::base_storage::BaseStorage; @@ -67,36 +67,35 @@ pub extern "system" fn Java_FileStorage_remove<'local>( _class: JClass, id: JString<'local>, file_storage_ptr: jlong, -) -> jstring { +) { let id: String = env.get_string(&id).unwrap().into(); - match FileStorage::from_jlong(file_storage_ptr).remove(&id) { - Ok(()) => env.new_string("").unwrap().into_raw(), - Err(err) => env - .new_string(err.to_string()) - .unwrap() - .into_raw(), - } + FileStorage::from_jlong(file_storage_ptr) + .remove(&id) + .unwrap_or_else(|err| { + let error_class = env + .find_class("java/lang/RuntimeException") + .unwrap(); + env.throw_new(error_class, &err.to_string()) + .unwrap(); + }); } #[no_mangle] pub extern "system" fn Java_FileStorage_needsSyncing( - env: JNIEnv<'_>, + mut env: JNIEnv<'_>, _class: JClass, file_storage_ptr: jlong, -) -> jstring { +) -> jboolean { match FileStorage::from_jlong(file_storage_ptr).needs_syncing() { - Ok(updated) => { - let result = if updated { - "true" - } else { - "false" - }; - env.new_string(result).unwrap().into_raw() + Ok(updated) => updated as jboolean, + Err(err) => { + let error_class = env + .find_class("java/lang/RuntimeException") + .unwrap(); + env.throw_new(error_class, &err.to_string()) + .unwrap(); + false as jboolean } - Err(err) => env - .new_string(err.to_string()) - .unwrap() - .into_raw(), } } @@ -150,53 +149,55 @@ pub extern "system" fn Java_FileStorage_readFS( #[no_mangle] pub extern "system" fn Java_FileStorage_writeFS( - env: JNIEnv, + mut env: JNIEnv<'_>, _class: JClass, file_storage_ptr: jlong, -) -> jstring { - match FileStorage::from_jlong(file_storage_ptr).write_fs() { - Ok(()) => env.new_string("").unwrap().into_raw(), - Err(err) => env - .new_string(err.to_string()) - .unwrap() - .into_raw(), - } +) { + FileStorage::from_jlong(file_storage_ptr) + .write_fs() + .unwrap_or_else(|err| { + let error_class = env + .find_class("java/lang/RuntimeException") + .unwrap(); + env.throw_new(error_class, &err.to_string()) + .unwrap(); + }); } #[allow(clippy::suspicious_doc_comments)] ///! Safety: The FileStorage instance is dropped after this call #[no_mangle] pub extern "system" fn Java_FileStorage_erase( - env: JNIEnv, + mut env: JNIEnv<'_>, _class: JClass, file_storage_ptr: jlong, -) -> jstring { +) { let file_storage = unsafe { Box::from_raw(file_storage_ptr as *mut FileStorage) }; - match file_storage.erase() { - Ok(()) => env.new_string("").unwrap().into_raw(), - Err(err) => env - .new_string(err.to_string()) - .unwrap() - .into_raw(), - } + file_storage.erase().unwrap_or_else(|err| { + let error_class = env + .find_class("java/lang/RuntimeException") + .unwrap(); + env.throw_new(error_class, &err.to_string()) + .unwrap(); + }); } #[no_mangle] pub extern "system" fn Java_FileStorage_merge( - env: JNIEnv, + mut env: JNIEnv<'_>, _class: JClass, file_storage_ptr: jlong, other_file_storage_ptr: jlong, -) -> jstring { - match FileStorage::from_jlong(file_storage_ptr) +) { + FileStorage::from_jlong(file_storage_ptr) .merge_from(FileStorage::from_jlong(other_file_storage_ptr)) - { - Ok(()) => env.new_string("").unwrap().into_raw(), - Err(err) => env - .new_string(err.to_string()) - .unwrap() - .into_raw(), - } + .unwrap_or_else(|err| { + let error_class = env + .find_class("java/lang/RuntimeException") + .unwrap(); + env.throw_new(error_class, &err.to_string()) + .unwrap(); + }); } diff --git a/fs-storage/tests/FileStorage.java b/fs-storage/tests/FileStorage.java index 39a6154..0578546 100644 --- a/fs-storage/tests/FileStorage.java +++ b/fs-storage/tests/FileStorage.java @@ -9,17 +9,17 @@ public class FileStorage { private static native void set(String id, String value, long file_storage_ptr); - private static native String remove(String id, long file_storage_ptr); + private static native void remove(String id, long file_storage_ptr); - private static native String needsSyncing(long file_storage_ptr); + private static native boolean needsSyncing(long file_storage_ptr); private static native Object readFS(long file_storage_ptr); - private static native String writeFS(long file_storage_ptr); + private static native void writeFS(long file_storage_ptr); - private static native String erase(long file_storage_ptr); + private static native void erase(long file_storage_ptr); - private static native String merge(long file_storage_ptr, long other_file_storage_ptr); + private static native void merge(long file_storage_ptr, long other_file_storage_ptr); public FileStorage(String label, String path) { this.fileStoragePtr = create(label, path); @@ -29,27 +29,53 @@ public void set(String id, String value) { set(id, value, this.fileStoragePtr); } - public String remove(String id) { - return remove(id, this.fileStoragePtr); + public void remove(String id) { + try { + remove(id, this.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error removing file storage: " + e.getMessage()); + } } - public String needsSyncing() { - return needsSyncing(this.fileStoragePtr); + public boolean needsSyncing() { + try { + return needsSyncing(this.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error checking if file storage needs syncing: " + e.getMessage()); + return false; + } } public Object readFS() { - return readFS(this.fileStoragePtr); + try { + return readFS(this.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error reading file storage: " + e.getMessage()); + return null; + } } - public String writeFS() { - return writeFS(this.fileStoragePtr); + public void writeFS() { + try { + writeFS(this.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error writing file storage: " + e.getMessage()); + } } - public String erase() { - return erase(this.fileStoragePtr); + public void erase() { + try { + erase(this.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error erasing file storage: " + e.getMessage()); + } } - public String merge(FileStorage other) { - return merge(this.fileStoragePtr, other.fileStoragePtr); + public void merge(FileStorage other) { + try { + merge(this.fileStoragePtr, other.fileStoragePtr); + } catch (RuntimeException e) { + System.err.println("Error merging file storage: " + e.getMessage()); + } } } diff --git a/fs-storage/tests/FileStorageTest.java b/fs-storage/tests/FileStorageTest.java index fd24a6b..dc47fad 100644 --- a/fs-storage/tests/FileStorageTest.java +++ b/fs-storage/tests/FileStorageTest.java @@ -22,8 +22,7 @@ public void testFileStorageWriteRead() { fileStorage.set("key1", "value1"); fileStorage.set("key2", "value2"); - String err = fileStorage.remove("key1"); - assertTrue(err.isEmpty()); + fileStorage.remove("key1"); @SuppressWarnings("unchecked") LinkedHashMap data = (LinkedHashMap) fileStorage.readFS(); @@ -39,34 +38,27 @@ public void testFileStorageAutoDelete() { fileStorage.set("key1", "value1"); fileStorage.set("key1", "value2"); - String err = fileStorage.writeFS(); - assertTrue(err.isEmpty()); + fileStorage.writeFS(); File file = storagePath.toFile(); assertTrue(file.exists()); - err = fileStorage.erase(); - assertTrue(err.isEmpty()); + fileStorage.erase(); assertFalse(file.exists()); } - // problem @Test public void testFileStorageNeedsSyncing() { String label = "test"; Path storagePath = tempDir.resolve("test.txt"); FileStorage fileStorage = new FileStorage(label, storagePath.toString()); fileStorage.writeFS(); - String result = fileStorage.needsSyncing(); - assertEquals("false", result); + assertFalse(fileStorage.needsSyncing()); fileStorage.set("key1", "value1"); // // FAIL: don't why it is still false // assertTrue(fileStorage.needsSyncing()); - - String err = fileStorage.writeFS(); - assertTrue(err.isEmpty()); - result = fileStorage.needsSyncing(); - assertEquals("false", result); + fileStorage.writeFS(); + assertFalse(fileStorage.needsSyncing()); } @Test @@ -82,10 +74,8 @@ public void testFileStorageMonoidCombine() { fileStorage2.set("key1", "3"); fileStorage2.set("key3", "9"); - String err = fileStorage1.merge(fileStorage2); - assertTrue(err.isEmpty()); - err = fileStorage1.writeFS(); - assertTrue(err.isEmpty()); + fileStorage1.merge(fileStorage2); + fileStorage1.writeFS(); @SuppressWarnings("unchecked") LinkedHashMap data = (LinkedHashMap) fileStorage1.readFS(); @@ -106,8 +96,7 @@ public void testFileStorageMainScenario() { fileStorage.set("key", "value1"); fileStorage.set("key1", "value"); - String err = fileStorage.remove("key"); - assertTrue(err.isEmpty()); + fileStorage.remove("key"); // Sleep for 1 second try { @@ -117,16 +106,14 @@ public void testFileStorageMainScenario() { } - err = fileStorage.writeFS(); - assertTrue(err.isEmpty()); + fileStorage.writeFS(); @SuppressWarnings("unchecked") LinkedHashMap data = (LinkedHashMap) fileStorage.readFS(); assertEquals(1, data.size()); assertEquals("value", data.get("key1")); - err = fileStorage.erase(); - assertTrue(err.isEmpty()); + fileStorage.erase(); File file = storagePath.toFile(); assertFalse(file.exists()); }