From 8d12782289ddac5ecee49f2c6040976bc3b6b0e7 Mon Sep 17 00:00:00 2001 From: Ishan Bhanuka Date: Sun, 23 Jun 2024 02:08:41 -0400 Subject: [PATCH] JNI bindings for FileStorage (#61) * Remove unnecessary clone trait * Add jni bindings for FileStorage * Manually convert BTreeMap to java LinkedHashMap * Add feature flag * implemented java wrapper Signed-off-by: pushkarm029 * fix Signed-off-by: pushkarm029 * fix Signed-off-by: pushkarm029 * Update folder structure and instructions * added java wrapper tests Signed-off-by: pushkarm029 * fix readme and makefile Signed-off-by: pushkarm029 * ci rebase Signed-off-by: pushkarm029 * fix Signed-off-by: pushkarm029 * removed main Signed-off-by: pushkarm029 * instead of throwing err, return err as string Signed-off-by: pushkarm029 * Refactor module structure * Fix fmt * fix Signed-off-by: pushkarm029 * add tests for exception Signed-off-by: pushkarm029 * tests fix Signed-off-by: pushkarm029 * fix Signed-off-by: pushkarm029 * fix Signed-off-by: pushkarm029 --------- Signed-off-by: pushkarm029 Co-authored-by: pushkarm029 --- .github/workflows/build.yml | 33 ++++ .gitignore | 1 + fs-storage/Cargo.toml | 9 +- fs-storage/Makefile | 52 ++++++ fs-storage/README.md | 12 ++ fs-storage/src/base_storage.rs | 5 + fs-storage/src/jni/file_storage.rs | 217 ++++++++++++++++++++++++++ fs-storage/src/jni/mod.rs | 1 + fs-storage/src/lib.rs | 2 + fs-storage/tests/.gitignore | 1 + fs-storage/tests/FileStorage.java | 61 ++++++++ fs-storage/tests/FileStorageTest.java | 153 ++++++++++++++++++ fs-storage/tests/SyncStatus.java | 11 ++ 13 files changed, 556 insertions(+), 2 deletions(-) create mode 100644 fs-storage/Makefile create mode 100644 fs-storage/src/jni/file_storage.rs create mode 100644 fs-storage/src/jni/mod.rs create mode 100644 fs-storage/tests/.gitignore create mode 100644 fs-storage/tests/FileStorage.java create mode 100644 fs-storage/tests/FileStorageTest.java create mode 100644 fs-storage/tests/SyncStatus.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cddbfab4..5922651f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -41,6 +41,17 @@ jobs: - name: Build Release run: cargo build --verbose --release + - name: Install JDK + uses: actions/setup-java@v4.2.1 + with: + distribution: 'temurin' + java-version: '22' + + - name: Java tests + run: | + cd fs-storage + make test-linux + windows: name: Test on Windows runs-on: windows-latest @@ -54,6 +65,17 @@ jobs: - name: Run tests run: cargo test --workspace --verbose + - name: Install JDK + uses: actions/setup-java@v4.2.1 + with: + distribution: 'temurin' + java-version: '22' + + - name: Java tests + run: | + cd fs-storage + make test-windows + mac-intel: name: Test on macOS Intel runs-on: macos-14 @@ -66,3 +88,14 @@ jobs: - name: Run tests run: cargo test --workspace --verbose + + - name: Install JDK + uses: actions/setup-java@v4.2.1 + with: + distribution: 'temurin' + java-version: '22' + + - name: Java tests + run: | + cd fs-storage + make test-mac diff --git a/.gitignore b/.gitignore index ff80aa87..bc757308 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ target Cargo.lock **/app_id +*.class \ No newline at end of file diff --git a/fs-storage/Cargo.toml b/fs-storage/Cargo.toml index c637c5a7..e6714d18 100644 --- a/fs-storage/Cargo.toml +++ b/fs-storage/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [lib] name = "fs_storage" -crate-type = ["rlib"] +crate-type = ["rlib", "cdylib"] bench = false [[example]] @@ -15,7 +15,8 @@ name = "cli" log = { version = "0.4.17", features = ["release_max_level_off"] } serde_json = "1.0.82" serde = { version = "1.0.138", features = ["derive"] } - +jni = { version = "0.21.1", optional = true } +jnix = { version = "0.5.1", features = ["derive"] } data-error = { path = "../data-error" } @@ -23,3 +24,7 @@ data-error = { path = "../data-error" } [dev-dependencies] anyhow = "1.0.81" tempdir = "0.3.7" + +[features] +default = ["jni-bindings"] +jni-bindings = ["jni"] diff --git a/fs-storage/Makefile b/fs-storage/Makefile new file mode 100644 index 00000000..a7d173ef --- /dev/null +++ b/fs-storage/Makefile @@ -0,0 +1,52 @@ +CARGO_CMD = cargo +JAR_FILE := junit.jar +URL := https://repo1.maven.org/maven2/org/junit/platform/junit-platform-console-standalone/1.9.3/junit-platform-console-standalone-1.9.3.jar + +.PHONY: test-linux +test-linux: build-rust download-test-lib java-test-linux + +.PHONY: test-mac +test-mac: build-rust download-test-lib java-test-mac + +.PHONY: test-windows +test-windows: build-rust download-test-lib java-test-windows + +.PHONY: build-rust +build-rust: + $(CARGO_CMD) build + +.PHONY: download-test-lib +download-test-lib: + cd tests && \ + if [ ! -f "$(JAR_FILE)" ]; then \ + curl -o "$(JAR_FILE)" "$(URL)"; \ + else \ + echo "$(JAR_FILE) already exists."; \ + fi + +.PHONY: java-test-linux +java-test-linux: + (cd ../target/debug && \ + export LD_LIBRARY_PATH=$$PWD && \ + cd ../../fs-storage/tests && \ + javac -d out FileStorage.java && \ + javac -d out -cp out:$(JAR_FILE) FileStorageTest.java && \ + RUST_BACKTRACE=1 java -jar $(JAR_FILE) --class-path out --scan-class-path) + +.PHONY: java-test-mac +java-test-mac: + (cd ../target/debug && \ + export DYLD_LIBRARY_PATH=$$PWD && \ + cd ../../fs-storage/tests && \ + javac -d out FileStorage.java && \ + javac -d out -cp out:$(JAR_FILE) FileStorageTest.java && \ + RUST_BACKTRACE=1 java -jar $(JAR_FILE) --class-path out --scan-class-path) + +.PHONY: java-test-windows +java-test-windows: + (cd ../target/debug && \ + export LIBRARY_PATH="$$PWD" && \ + cd ../../fs-storage/tests && \ + javac -d out FileStorage.java && \ + javac -d out -cp "out;$(JAR_FILE)" FileStorageTest.java && \ + RUST_BACKTRACE=1 java -Djava.library.path=$$LIBRARY_PATH -jar $(JAR_FILE) --class-path out --scan-class-path) \ No newline at end of file diff --git a/fs-storage/README.md b/fs-storage/README.md index cfd68569..7351dede 100644 --- a/fs-storage/README.md +++ b/fs-storage/README.md @@ -44,3 +44,15 @@ key2: value2 ```bash cargo run --example cli read /tmp/z ``` + +## Java Wrapper +```java +javac -h . FileStorage.java +javac FileStorage.java +LD_LIBRARY_PATH=/target/debug && java FileStorage.java +``` + +## Steps to test Java Wrapper +```bash +cd tests && make test +``` \ No newline at end of file diff --git a/fs-storage/src/base_storage.rs b/fs-storage/src/base_storage.rs index 8a4b1f20..1b1bd70d 100644 --- a/fs-storage/src/base_storage.rs +++ b/fs-storage/src/base_storage.rs @@ -1,7 +1,12 @@ use data_error::Result; use std::collections::BTreeMap; +#[cfg(feature = "jni-bindings")] +use jnix::{FromJava, IntoJava}; + #[derive(Debug, PartialEq, PartialOrd, Ord, Eq, Clone)] +#[cfg_attr(feature = "jni-bindings", derive(FromJava, IntoJava))] +#[jnix(class_name = "SyncStatus")] /// Represents the synchronization status of the storage. pub enum SyncStatus { /// No synchronization needed. diff --git a/fs-storage/src/jni/file_storage.rs b/fs-storage/src/jni/file_storage.rs new file mode 100644 index 00000000..265e90a7 --- /dev/null +++ b/fs-storage/src/jni/file_storage.rs @@ -0,0 +1,217 @@ +use crate::base_storage::SyncStatus; +use jni::signature::ReturnType; +use std::collections::BTreeMap; +use std::path::Path; +// This is the interface to the JVM that we'll call the majority of our +// methods on. +use jni::JNIEnv; + +// These objects are what you should use as arguments to your native +// function. They carry extra lifetime information to prevent them escaping +// this context and getting used after being GC'd. +use jni::objects::{JClass, JObject, 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}; +use jnix::{IntoJava, JnixEnv}; + +use crate::base_storage::BaseStorage; + +use crate::file_storage::FileStorage; + +impl FileStorage { + fn from_jlong<'a>(value: jlong) -> &'a mut Self { + unsafe { &mut *(value as *mut FileStorage) } + } +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_create<'local>( + mut env: JNIEnv<'local>, + _class: JClass, + label: JString<'local>, + path: JString<'local>, +) -> jlong { + let label: String = env + .get_string(&label) + .expect("Couldn't get label!") + .into(); + let path: String = env + .get_string(&path) + .expect("Couldn't get path!") + .into(); + + let file_storage: FileStorage = + FileStorage::new(label, Path::new(&path)).unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .expect("Failed to throw RuntimeException"); + FileStorage::new("".to_string(), Path::new("")).unwrap() + }); + Box::into_raw(Box::new(file_storage)) as jlong +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_set<'local>( + mut env: JNIEnv<'local>, + _class: JClass, + id: JString<'local>, + value: JString<'local>, + file_storage_ptr: jlong, +) { + let id: String = env.get_string(&id).expect("msg").into(); + let value: String = env.get_string(&value).expect("msg").into(); + + FileStorage::from_jlong(file_storage_ptr).set(id, value); +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_remove<'local>( + mut env: JNIEnv<'local>, + _class: JClass, + id: JString<'local>, + file_storage_ptr: jlong, +) { + let id: String = env.get_string(&id).unwrap().into(); + FileStorage::from_jlong(file_storage_ptr) + .remove(&id) + .unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .unwrap(); + }); +} + +// A JNI function called from Java that creates a `MyData` Rust type, converts it to a Java +// type and returns it. +#[no_mangle] +#[allow(non_snake_case)] +pub extern "system" fn Java_FileStorage_syncStatus<'env>( + env: jnix::jni::JNIEnv<'env>, + _this: jnix::jni::objects::JObject<'env>, + file_storage_ptr: jnix::jni::sys::jlong, +) -> jnix::jni::objects::JObject<'env> { + let env = JnixEnv::from(env); + let sync_status = FileStorage::from_jlong(file_storage_ptr) + .sync_status() + .unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", err.to_string()) + .unwrap(); + SyncStatus::InSync + }); + + sync_status.into_java(&env).forget() +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_sync( + mut env: JNIEnv<'_>, + _class: JClass, + file_storage_ptr: jlong, +) { + FileStorage::from_jlong(file_storage_ptr) + .sync() + .unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .unwrap(); + }); +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_readFS( + mut env: JNIEnv<'_>, + _class: JClass, + file_storage_ptr: jlong, +) -> jobject { + let data: BTreeMap = + match FileStorage::from_jlong(file_storage_ptr).read_fs() { + Ok(data) => data.clone(), + Err(err) => { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .expect("Failed to throw RuntimeException"); + return JObject::null().into_raw(); + } + }; + + // Create a new LinkedHashMap object + let linked_hash_map_class = + env.find_class("java/util/LinkedHashMap").unwrap(); + let linked_hash_map = env + .new_object(linked_hash_map_class, "()V", &[]) + .unwrap(); + + // Get the put method ID + let put_method_id = env + .get_method_id( + "java/util/LinkedHashMap", + "put", + "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;", + ) + .unwrap(); + + // Insert each key-value pair from the BTreeMap into the LinkedHashMap + for (key, value) in data { + let j_key = env.new_string(key).unwrap(); + let j_value = env.new_string(value).unwrap(); + let j_key = JValue::from(&j_key).as_jni(); + let j_value = JValue::from(&j_value).as_jni(); + unsafe { + env.call_method_unchecked( + &linked_hash_map, + put_method_id, + ReturnType::Object, + &[j_key, j_value], + ) + .unwrap() + }; + } + + // Return the LinkedHashMap as a raw pointer + linked_hash_map.as_raw() +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_writeFS( + mut env: JNIEnv<'_>, + _class: JClass, + file_storage_ptr: jlong, +) { + FileStorage::from_jlong(file_storage_ptr) + .write_fs() + .unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &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( + mut env: JNIEnv<'_>, + _class: JClass, + file_storage_ptr: jlong, +) { + let file_storage = unsafe { + Box::from_raw(file_storage_ptr as *mut FileStorage) + }; + file_storage.erase().unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .unwrap(); + }); +} + +#[no_mangle] +pub extern "system" fn Java_FileStorage_merge( + mut env: JNIEnv<'_>, + _class: JClass, + file_storage_ptr: jlong, + other_file_storage_ptr: jlong, +) { + FileStorage::from_jlong(file_storage_ptr) + .merge_from(FileStorage::from_jlong(other_file_storage_ptr)) + .unwrap_or_else(|err| { + env.throw_new("java/lang/RuntimeException", &err.to_string()) + .unwrap(); + }); +} diff --git a/fs-storage/src/jni/mod.rs b/fs-storage/src/jni/mod.rs new file mode 100644 index 00000000..b30d884f --- /dev/null +++ b/fs-storage/src/jni/mod.rs @@ -0,0 +1 @@ +pub mod file_storage; diff --git a/fs-storage/src/lib.rs b/fs-storage/src/lib.rs index d31b27b6..adaeafe4 100644 --- a/fs-storage/src/lib.rs +++ b/fs-storage/src/lib.rs @@ -1,5 +1,7 @@ pub mod base_storage; pub mod file_storage; +#[cfg(feature = "jni-bindings")] +pub mod jni; pub mod monoid; mod utils; pub const ARK_FOLDER: &str = ".ark"; diff --git a/fs-storage/tests/.gitignore b/fs-storage/tests/.gitignore new file mode 100644 index 00000000..f23b9489 --- /dev/null +++ b/fs-storage/tests/.gitignore @@ -0,0 +1 @@ +*.jar \ No newline at end of file diff --git a/fs-storage/tests/FileStorage.java b/fs-storage/tests/FileStorage.java new file mode 100644 index 00000000..132e6799 --- /dev/null +++ b/fs-storage/tests/FileStorage.java @@ -0,0 +1,61 @@ +public class FileStorage { + private long fileStoragePtr; + + static { + System.loadLibrary("fs_storage"); + } + + private static native long create(String label, String path); + + private static native void set(String id, String value, long file_storage_ptr); + + private static native void remove(String id, long file_storage_ptr); + + private static native void sync(long file_storage_ptr); + + private static native SyncStatus syncStatus(long file_storage_ptr); + + private static native Object readFS(long file_storage_ptr); + + private static native void writeFS(long file_storage_ptr); + + private static native void erase(long 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); + } + + public void set(String id, String value) { + set(id, value, this.fileStoragePtr); + } + + public void remove(String id) { + remove(id, this.fileStoragePtr); + } + + public void sync() { + sync(this.fileStoragePtr); + } + + public SyncStatus syncStatus() { + return syncStatus(this.fileStoragePtr); + } + + public Object readFS() { + return readFS(this.fileStoragePtr); + } + + public void writeFS() { + writeFS(this.fileStoragePtr); + } + + public void erase() { + erase(this.fileStoragePtr); + } + + public void merge(FileStorage other) { + merge(this.fileStoragePtr, other.fileStoragePtr); + } +} diff --git a/fs-storage/tests/FileStorageTest.java b/fs-storage/tests/FileStorageTest.java new file mode 100644 index 00000000..f9844747 --- /dev/null +++ b/fs-storage/tests/FileStorageTest.java @@ -0,0 +1,153 @@ +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.nio.file.Path; +import java.util.LinkedHashMap; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class FileStorageTest { + FileStorage fileStorage = new FileStorage("test", "test.txt"); + + @TempDir + Path tempDir; + + @Test + public void testFileStorageWriteRead() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + + fileStorage.set("key1", "value1"); + fileStorage.set("key2", "value2"); + + fileStorage.remove("key1"); + fileStorage.writeFS(); + + @SuppressWarnings("unchecked") + LinkedHashMap data = (LinkedHashMap) fileStorage.readFS(); + assertEquals(1, data.size()); + assertEquals("value2", data.get("key2")); + } + + @Test + public void testFileStorageAutoDelete() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + + fileStorage.set("key1", "value1"); + fileStorage.set("key1", "value2"); + fileStorage.writeFS(); + + File file = storagePath.toFile(); + assertTrue(file.exists()); + + fileStorage.erase(); + assertFalse(file.exists()); + } + + @Test + public void testFileStorageNeedsSyncing() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + fileStorage.writeFS(); + assertEquals(SyncStatus.InSync, fileStorage.syncStatus()); + fileStorage.set("key1", "value1"); + assertEquals(SyncStatus.StorageStale, fileStorage.syncStatus()); + fileStorage.writeFS(); + assertEquals(SyncStatus.InSync, fileStorage.syncStatus()); + } + + @Test + public void testFileStorageMonoidCombine() { + Path storagePath1 = tempDir.resolve("test1.txt"); + Path storagePath2 = tempDir.resolve("test2.txt"); + FileStorage fileStorage1 = new FileStorage("test1", storagePath1.toString()); + FileStorage fileStorage2 = new FileStorage("test2", storagePath2.toString()); + + fileStorage1.set("key1", "2"); + fileStorage1.set("key2", "6"); + + fileStorage2.set("key1", "3"); + fileStorage2.set("key3", "9"); + + fileStorage1.merge(fileStorage2); + fileStorage1.writeFS(); + + @SuppressWarnings("unchecked") + LinkedHashMap data = (LinkedHashMap) fileStorage1.readFS(); + assertEquals(3, data.size()); + assertEquals("23", data.get("key1")); + assertEquals("6", data.get("key2")); + assertEquals("9", data.get("key3")); + } + + @Test + public void testFileStorageMainScenario() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + + fileStorage.set("key", "value"); + fileStorage.set("key", "value1"); + fileStorage.set("key1", "value"); + + fileStorage.remove("key"); + + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + fileStorage.writeFS(); + + @SuppressWarnings("unchecked") + LinkedHashMap data = (LinkedHashMap) fileStorage.readFS(); + assertEquals(1, data.size()); + assertEquals("value", data.get("key1")); + + fileStorage.erase(); + File file = storagePath.toFile(); + assertFalse(file.exists()); + } + + @Test + public void testRemoveException() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + Exception exception = assertThrows(RuntimeException.class, () -> fileStorage.remove("invalid_id")); + assertTrue(exception.getMessage().matches("Storage error.*")); + } + + @Test + public void testSyncException() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + Exception exception = assertThrows(RuntimeException.class, () -> fileStorage.sync()); + assertTrue(exception.getMessage().matches("IO error.*")); + } + + @Test + public void testCreateException() { + Path storagePath = tempDir.resolve(""); + Exception exception = assertThrows(RuntimeException.class, () -> new FileStorage("", storagePath.toString())); + assertTrue(exception.getMessage().matches("IO error.*")); + } + + @Test + public void testEraseException() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + Exception exception = assertThrows(RuntimeException.class, () -> fileStorage.erase()); + assertTrue(exception.getMessage().matches("Storage error.*")); + } + + @Test + public void testReadException() { + Path storagePath = tempDir.resolve("test.txt"); + FileStorage fileStorage = new FileStorage("test", storagePath.toString()); + Exception exception = assertThrows(RuntimeException.class, () -> fileStorage.readFS()); + assertTrue(exception.getMessage().matches("Storage error.*")); + } +} \ No newline at end of file diff --git a/fs-storage/tests/SyncStatus.java b/fs-storage/tests/SyncStatus.java new file mode 100644 index 00000000..8f2cd623 --- /dev/null +++ b/fs-storage/tests/SyncStatus.java @@ -0,0 +1,11 @@ +/// Represents the synchronization status of the storage. +public enum SyncStatus { + /// No synchronization needed. + InSync, + /// In-memory key-value mapping is stale. + MappingStale, + /// External file system storage is stale. + StorageStale, + /// In-memory key-value mapping and external file system storage diverge. + Diverge, +}