From 10f5afdd21e517908c61ce6a43fe2c191c5ee092 Mon Sep 17 00:00:00 2001 From: Darius Maitia Date: Fri, 30 Aug 2024 21:23:51 -0300 Subject: [PATCH 1/4] refactor(config): returning a Result when attempting to create a config, and associating it with a native config. --- examples/src/main/kotlin/io.zenoh/Config.kt | 4 +- zenoh-jni/src/config.rs | 107 ++++++++++++++++++ zenoh-jni/src/lib.rs | 1 + zenoh-jni/src/scouting.rs | 49 ++------ zenoh-jni/src/session.rs | 21 ++-- .../src/commonMain/kotlin/io/zenoh/Config.kt | 42 ++++--- .../kotlin/io/zenoh/jni/JNIConfig.kt | 76 +++++++++++++ .../kotlin/io/zenoh/jni/JNIScout.kt | 9 +- .../kotlin/io/zenoh/jni/JNISession.kt | 21 +--- .../kotlin/io/zenoh/jni/JNIZBytes.kt | 16 ++- .../commonTest/kotlin/io/zenoh/ConfigTest.kt | 38 +++---- .../commonTest/kotlin/io/zenoh/SessionTest.kt | 6 - 12 files changed, 267 insertions(+), 123 deletions(-) create mode 100644 zenoh-jni/src/config.rs create mode 100644 zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIConfig.kt diff --git a/examples/src/main/kotlin/io.zenoh/Config.kt b/examples/src/main/kotlin/io.zenoh/Config.kt index 2bc2903a5..875cbfc98 100644 --- a/examples/src/main/kotlin/io.zenoh/Config.kt +++ b/examples/src/main/kotlin/io.zenoh/Config.kt @@ -60,14 +60,14 @@ internal fun loadConfig( Config.default() } else { configFile?.let { - Config.from(path = Path(it)) + Config.from(path = Path(it)).getOrThrow() } ?: run { val connect = Connect(connectEndpoints) val listen = Listen(listenEndpoints) val scouting = Scouting(Multicast(!noMulticastScouting)) val configData = ConfigData(connect, listen, mode, scouting) val jsonConfig = Json.encodeToJsonElement(configData) - Config.from(jsonConfig) + Config.from(jsonConfig).getOrThrow() } } } diff --git a/zenoh-jni/src/config.rs b/zenoh-jni/src/config.rs new file mode 100644 index 000000000..0cf3d9a7b --- /dev/null +++ b/zenoh-jni/src/config.rs @@ -0,0 +1,107 @@ +// +// Copyright (c) 2023 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +use std::{ptr::null, sync::Arc}; + +use jni::{ + objects::{JClass, JString}, + JNIEnv, +}; +use zenoh::Config; + +use crate::errors::Result; +use crate::{session_error, throw_exception, utils::decode_string}; + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadDefaultConfigViaJNI( + _env: JNIEnv, + _class: JClass, +) -> *const Config { + let config = Config::default(); + Arc::into_raw(Arc::new(config)) +} + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadConfigFileViaJNI( + mut env: JNIEnv, + _class: JClass, + config_path: JString, +) -> *const Config { + || -> Result<*const Config> { + let config_file_path = decode_string(&mut env, &config_path)?; + let config = Config::from_file(config_file_path).map_err(|err| session_error!(err))?; + Ok(Arc::into_raw(Arc::new(config))) + }() + .unwrap_or_else(|err| { + throw_exception!(env, err); + null() + }) +} + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadJsonConfigViaJNI( + mut env: JNIEnv, + _class: JClass, + json_config: JString, +) -> *const Config { + || -> Result<*const Config> { + let json_config = decode_string(&mut env, &json_config)?; + let mut deserializer = + json5::Deserializer::from_str(&json_config).map_err(|err| session_error!(err))?; + let config = Config::from_deserializer(&mut deserializer).map_err(|err| match err { + Ok(c) => session_error!("Invalid configuration: {}", c), + Err(e) => session_error!("JSON error: {}", e), + })?; + Ok(Arc::into_raw(Arc::new(config))) + }() + .unwrap_or_else(|err| { + throw_exception!(env, err); + null() + }) +} + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadYamlConfigViaJNI( + mut env: JNIEnv, + _class: JClass, + yaml_config: JString, +) -> *const Config { + || -> Result<*const Config> { + let yaml_config = decode_string(&mut env, &yaml_config)?; + let deserializer = serde_yaml::Deserializer::from_str(&yaml_config); + let config = Config::from_deserializer(deserializer).map_err(|err| match err { + Ok(c) => session_error!("Invalid configuration: {}", c), + Err(e) => session_error!("YAML error: {}", e), + })?; + Ok(Arc::into_raw(Arc::new(config))) + }() + .unwrap_or_else(|err| { + throw_exception!(env, err); + null() + }) +} + +#[no_mangle] +#[allow(non_snake_case)] +pub(crate) unsafe extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_freePtrViaJNI( + _env: JNIEnv, + _: JClass, + config_ptr: *const Config, +) { + Arc::from_raw(config_ptr); +} diff --git a/zenoh-jni/src/lib.rs b/zenoh-jni/src/lib.rs index 406d20e22..5f402e934 100644 --- a/zenoh-jni/src/lib.rs +++ b/zenoh-jni/src/lib.rs @@ -12,6 +12,7 @@ // ZettaScale Zenoh Team, // +mod config; mod errors; mod key_expr; mod logger; diff --git a/zenoh-jni/src/scouting.rs b/zenoh-jni/src/scouting.rs index 356bfc5ab..59a429bbe 100644 --- a/zenoh-jni/src/scouting.rs +++ b/zenoh-jni/src/scouting.rs @@ -15,14 +15,14 @@ use std::{ptr::null, sync::Arc}; use jni::{ - objects::{JClass, JList, JObject, JString, JValue}, + objects::{JClass, JList, JObject, JValue}, sys::jint, JNIEnv, }; use zenoh::{config::WhatAmIMatcher, prelude::Wait}; use zenoh::{scouting::Scout, Config}; -use crate::{errors::Result, throw_exception, utils::decode_string}; +use crate::{errors::Result, throw_exception}; use crate::{ session_error, utils::{get_callback_global_ref, get_java_vm}, @@ -33,14 +33,7 @@ use crate::{ /// # Params /// - `whatAmI`: Ordinal value of the WhatAmI enum. /// - `callback`: Callback to be executed whenever a hello message is received. -/// - `config_string`: Optional embedded configuration as a string. -/// - `format`: format of the `config_string` param. -/// - `config_path`: Optional path to a config file. -/// -/// Note: Either the config_string or the config_path or None can be provided. -/// If none is provided, then the default configuration is loaded. Otherwise -/// it's the config_string or the config_path that are loaded. This consistency -/// logic is granted by the kotlin layer. +/// - `config_ptr`: Optional config pointer. /// /// Returns a pointer to the scout, which must be freed afterwards. /// If starting the scout fails, an exception is thrown on the JVM, and a null pointer is returned. @@ -52,43 +45,19 @@ pub unsafe extern "C" fn Java_io_zenoh_jni_JNIScout_00024Companion_scoutViaJNI( _class: JClass, whatAmI: jint, callback: JObject, - config_string: /*nullable=*/ JString, - format: jint, - config_path: /*nullable=*/ JString, + config_ptr: /*nullable=*/ *const Config, ) -> *const Scout<()> { || -> Result<*const Scout<()>> { let callback_global_ref = get_callback_global_ref(&mut env, callback)?; let java_vm = Arc::new(get_java_vm(&mut env)?); let whatAmIMatcher: WhatAmIMatcher = (whatAmI as u8).try_into().unwrap(); // The validity of the operation is guaranteed on the kotlin layer. - let config = if config_string.is_null() && config_path.is_null() { + let config = if config_ptr.is_null() { Config::default() - } else if !config_string.is_null() { - let string_config = decode_string(&mut env, &config_string)?; - match format { - 0 /*YAML*/ => { - let deserializer = serde_yaml::Deserializer::from_str(&string_config); - Config::from_deserializer(deserializer).map_err(|err| match err { - Ok(c) => session_error!("Invalid configuration: {}", c), - Err(e) => session_error!("YAML error: {}", e), - })? - } - 1 | 2 /*JSON | JSON5*/ => { - let mut deserializer = - json5::Deserializer::from_str(&string_config).map_err(|err| session_error!(err))?; - Config::from_deserializer(&mut deserializer).map_err(|err| match err { - Ok(c) => session_error!("Invalid configuration: {}", c), - Err(e) => session_error!("JSON error: {}", e), - })? - } - _ => { - // This can never happen unless the Config.Format enum on Kotlin is wrongly modified! - Err(session_error!("Unexpected error: attempting to decode a config with a format other than Json, - Json5 or Yaml. Check Config.Format for eventual modifications..."))? - } - } } else { - let config_file_path = decode_string(&mut env, &config_path)?; - Config::from_file(config_file_path).map_err(|err| session_error!(err))? + let arc_cfg = Arc::from_raw(config_ptr); + let config_clone = arc_cfg.as_ref().clone(); + std::mem::forget(arc_cfg); + config_clone }; zenoh::scout(whatAmIMatcher, config) .callback(move |hello| { diff --git a/zenoh-jni/src/session.rs b/zenoh-jni/src/session.rs index fd38aed41..a0be09de7 100644 --- a/zenoh-jni/src/session.rs +++ b/zenoh-jni/src/session.rs @@ -49,12 +49,12 @@ use zenoh::session::{Session, SessionDeclarations}; /// #[no_mangle] #[allow(non_snake_case)] -pub extern "C" fn Java_io_zenoh_jni_JNISession_openSessionViaJNI( +pub unsafe extern "C" fn Java_io_zenoh_jni_JNISession_openSessionViaJNI( mut env: JNIEnv, _class: JClass, - config_path: /*nullable*/ JString, + config_ptr: *const Config, ) -> *const Session { - let session = open_session(&mut env, config_path); + let session = open_session(config_ptr); match session { Ok(session) => Arc::into_raw(Arc::new(session)), Err(err) => { @@ -69,16 +69,13 @@ pub extern "C" fn Java_io_zenoh_jni_JNISession_openSessionViaJNI( /// /// If the config path provided is null then the default configuration is loaded. /// -fn open_session(env: &mut JNIEnv, config_path: JString) -> Result { - let config = if config_path.is_null() { - Config::default() - } else { - let config_file_path = decode_string(env, &config_path)?; - Config::from_file(config_file_path).map_err(|err| session_error!(err))? - }; - zenoh::open(config) +unsafe fn open_session(config_ptr: *const Config) -> Result { + let config = Arc::from_raw(config_ptr); + let result = zenoh::open(config.as_ref().clone()) .wait() - .map_err(|err| session_error!(err)) + .map_err(|err| session_error!(err)); + mem::forget(config); + return result; } /// Open a Zenoh session with a JSON configuration. diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt index a4c17a52a..8286ad9ef 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt @@ -14,6 +14,7 @@ package io.zenoh +import io.zenoh.jni.JNIConfig import java.io.File import java.nio.file.Path import kotlinx.serialization.json.JsonElement @@ -96,13 +97,7 @@ import kotlinx.serialization.json.JsonElement * @property config Raw string configuration, (supported types: JSON5, JSON and YAML). * @property format [Format] of the configuration. */ -class Config private constructor(internal val path: Path? = null, internal val config: String? = null, val format: Format? = null) { - - enum class Format { - YAML, - JSON, - JSON5 - } +class Config internal constructor(internal val jniConfig: JNIConfig) { companion object { @@ -110,7 +105,7 @@ class Config private constructor(internal val path: Path? = null, internal val c * Returns the default config. */ fun default(): Config { - return Config() + return JNIConfig.loadDefaultConfig() } /** @@ -118,8 +113,8 @@ class Config private constructor(internal val path: Path? = null, internal val c * * @param file The zenoh config file. */ - fun from(file: File): Config { - return Config(path = file.toPath()) + fun from(file: File): Result { + return JNIConfig.loadConfigFile(file) } /** @@ -127,15 +122,20 @@ class Config private constructor(internal val path: Path? = null, internal val c * * @param path The zenoh config file path. */ - fun from(path: Path): Config { - return Config(path) + fun from(path: Path): Result { + return JNIConfig.loadConfigFile(path) } - /** - * Loads the configuration from the [config] param. The [Format] needs to be specified explicitly. - */ - fun from(config: String, format: Format): Config { - return Config(config = config, format = format) + fun fromJson(config: String): Result { + return JNIConfig.loadJsonConfig(config) + } + + fun fromJson5(config: String): Result { + return JNIConfig.loadJson5Config(config) + } + + fun fromYaml(config: String): Result { + return JNIConfig.loadYamlConfig(config) } /** @@ -143,6 +143,12 @@ class Config private constructor(internal val path: Path? = null, internal val c * * @param jsonElement The zenoh config as a [JsonElement]. */ - fun from(jsonElement: JsonElement) = Config(config = jsonElement.toString(), format = Format.JSON) + fun from(jsonElement: JsonElement): Result { + return JNIConfig.loadJsonConfig(jsonElement.toString()) + } + } + + protected fun finalize() { + jniConfig.close() } } diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIConfig.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIConfig.kt new file mode 100644 index 000000000..0cd6be7b6 --- /dev/null +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIConfig.kt @@ -0,0 +1,76 @@ +// +// Copyright (c) 2023 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +package io.zenoh.jni + +import io.zenoh.Config +import io.zenoh.ZenohLoad +import java.io.File +import java.nio.file.Path + +internal class JNIConfig(internal val ptr: Long) { + + companion object { + + init { + ZenohLoad + } + + fun loadDefaultConfig(): Config { + val cfgPtr = loadDefaultConfigViaJNI() + return Config(JNIConfig(cfgPtr)) + } + + fun loadConfigFile(path: Path): Result = runCatching { + val cfgPtr = loadConfigFileViaJNI(path.toString()) + Config(JNIConfig(cfgPtr)) + } + + fun loadConfigFile(file: File): Result = loadConfigFile(file.toPath()) + + fun loadJsonConfig(rawConfig: String): Result = runCatching { + val cfgPtr = loadJsonConfigViaJNI(rawConfig) + Config(JNIConfig(cfgPtr)) + } + + fun loadJson5Config(rawConfig: String): Result = runCatching { + val cfgPtr = loadJsonConfigViaJNI(rawConfig) + Config(JNIConfig(cfgPtr)) + } + + fun loadYamlConfig(rawConfig: String): Result = runCatching { + val cfgPtr = loadYamlConfigViaJNI(rawConfig) + Config(JNIConfig(cfgPtr)) + } + + @Throws + private external fun loadDefaultConfigViaJNI(): Long + + @Throws + private external fun loadConfigFileViaJNI(path: String): Long + + @Throws + private external fun loadJsonConfigViaJNI(rawConfig: String): Long + + @Throws + private external fun loadYamlConfigViaJNI(rawConfig: String): Long + + /** Frees the underlying native config. */ + private external fun freePtrViaJNI(ptr: Long) + } + + fun close() { + freePtrViaJNI(ptr) + } +} diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIScout.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIScout.kt index 67b75cab4..7a5f0e4f3 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIScout.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIScout.kt @@ -40,10 +40,7 @@ class JNIScout(private val ptr: Long) { callback.run(Hello(WhatAmI.fromInt(whatAmI2), ZenohID(id), locators)) } val binaryWhatAmI: Int = whatAmI.map { it.value }.reduce { acc, it -> acc or it } - val ptr = scoutViaJNI( - binaryWhatAmI, scoutCallback, config?.config, config?.format?.ordinal ?: 0, - config?.path?.toString() - ) + val ptr = scoutViaJNI(binaryWhatAmI, scoutCallback, config?.jniConfig?.ptr) return Scout(receiver, JNIScout(ptr)) } @@ -51,9 +48,7 @@ class JNIScout(private val ptr: Long) { private external fun scoutViaJNI( whatAmI: Int, callback: JNIScoutCallback, - config: String?, - format: Int, - path: String? + configPtr: Long?, ): Long @Throws(Exception::class) diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNISession.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNISession.kt index 1af8d3c62..90c03302a 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNISession.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNISession.kt @@ -53,17 +53,8 @@ internal class JNISession { /* Pointer to the underlying Rust zenoh session. */ private var sessionPtr: AtomicLong = AtomicLong(0) - fun open(config: Config?): Result = runCatching { - val session = when { - config == null -> openSessionViaJNI(null) - config.config != null -> { - when (config.format) { - Config.Format.YAML -> openSessionWithYamlConfigViaJNI(config.config) - else -> openSessionWithJsonConfigViaJNI(config.config) - } - } - else -> openSessionViaJNI(config.path?.toString()) - } + fun open(config: Config): Result = runCatching { + val session = openSessionViaJNI(config.jniConfig.ptr) sessionPtr.set(session) } @@ -267,13 +258,7 @@ internal class JNISession { } @Throws(Exception::class) - private external fun openSessionViaJNI(configFilePath: String?): Long - - @Throws(Exception::class) - private external fun openSessionWithJsonConfigViaJNI(jsonConfig: String): Long - - @Throws(Exception::class) - private external fun openSessionWithYamlConfigViaJNI(yamlConfig: String): Long + private external fun openSessionViaJNI(configPtr: Long): Long @Throws(Exception::class) private external fun closeSessionViaJNI(ptr: Long) diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIZBytes.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIZBytes.kt index 804cd07d5..eb1d78ce4 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIZBytes.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/jni/JNIZBytes.kt @@ -1,3 +1,17 @@ +// +// Copyright (c) 2023 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + package io.zenoh.jni import io.zenoh.ZenohLoad @@ -33,4 +47,4 @@ object JNIZBytes { private external fun deserializeIntoMapViaJNI(payload: ByteArray): Map private external fun deserializeIntoListViaJNI(payload: ByteArray): List -} \ No newline at end of file +} diff --git a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt index d40591578..d5ce4156d 100644 --- a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt +++ b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt @@ -28,7 +28,7 @@ class ConfigTest { val TEST_KEY_EXP = "example/testing/keyexpr".intoKeyExpr().getOrThrow() } - private val json5ClientConfig = Config.from( + private val json5ClientConfig = Config.fromJson5( config = """ { mode: "peer", @@ -41,10 +41,10 @@ class ConfigTest { } } } - """.trimIndent(), format = Config.Format.JSON5 - ) + """.trimIndent() + ).getOrThrow() - private val json5ServerConfig = Config.from( + private val json5ServerConfig = Config.fromJson5( config = """ { mode: "peer", @@ -57,11 +57,11 @@ class ConfigTest { } } } - """.trimIndent(), format = Config.Format.JSON5 - ) + """.trimIndent() + ).getOrThrow() - private val jsonClientConfig = Config.from( + private val jsonClientConfig = Config.fromJson( config = """ { "mode": "peer", @@ -74,11 +74,11 @@ class ConfigTest { } } } - """.trimIndent(), format = Config.Format.JSON - ) + """.trimIndent() + ).getOrThrow() - private val jsonServerConfig = Config.from( + private val jsonServerConfig = Config.fromJson( config = """ { "mode": "peer", @@ -91,11 +91,11 @@ class ConfigTest { } } } - """.trimIndent(), format = Config.Format.JSON - ) + """.trimIndent() + ).getOrThrow() - private val yamlClientConfig = Config.from( + private val yamlClientConfig = Config.fromYaml( config = """ mode: peer connect: @@ -104,11 +104,11 @@ class ConfigTest { scouting: multicast: enabled: false - """.trimIndent(), format = Config.Format.YAML - ) + """.trimIndent() + ).getOrThrow() - private val yamlServerConfig = Config.from( + private val yamlServerConfig = Config.fromYaml( config = """ mode: peer listen: @@ -117,8 +117,8 @@ class ConfigTest { scouting: multicast: enabled: false - """.trimIndent(), format = Config.Format.YAML - ) + """.trimIndent() + ).getOrThrow() private fun runSessionTest(clientConfig: Config, serverConfig: Config) { @@ -186,6 +186,6 @@ class ConfigTest { } """.trimIndent() ) - runSessionTest(Config.from(clientConfigJson), Config.from(serverConfigJson)) + runSessionTest(Config.from(clientConfigJson).getOrThrow(), Config.from(serverConfigJson).getOrThrow()) } } diff --git a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/SessionTest.kt b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/SessionTest.kt index 47909d473..742451e68 100644 --- a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/SessionTest.kt +++ b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/SessionTest.kt @@ -42,12 +42,6 @@ class SessionTest { assertFalse(session.isOpen()) } - @Test - fun sessionOpeningFailure() { - val invalidConfig = Config.from(path = Path.of("invalid")) - assertFailsWith { Session.open(invalidConfig).getOrThrow() } - } - @Test fun sessionClose_succeedsDespiteNotFreeingAllDeclarations() { val session = Session.open(Config.default()).getOrThrow() From 0574178c92d555a0e2e9976e8d8068a39cca89ae Mon Sep 17 00:00:00 2001 From: Darius Maitia Date: Mon, 2 Sep 2024 10:54:59 -0300 Subject: [PATCH 2/4] refactor(config): - renaming Config.from to Config.fromFile - Adding tests - Adding or improving KDocs on Config --- examples/src/main/kotlin/io.zenoh/Config.kt | 4 +- .../src/commonMain/kotlin/io/zenoh/Config.kt | 147 ++++++++++++-- .../commonTest/kotlin/io/zenoh/ConfigTest.kt | 190 +++++++++++++----- 3 files changed, 280 insertions(+), 61 deletions(-) diff --git a/examples/src/main/kotlin/io.zenoh/Config.kt b/examples/src/main/kotlin/io.zenoh/Config.kt index 875cbfc98..77b52cba2 100644 --- a/examples/src/main/kotlin/io.zenoh/Config.kt +++ b/examples/src/main/kotlin/io.zenoh/Config.kt @@ -60,14 +60,14 @@ internal fun loadConfig( Config.default() } else { configFile?.let { - Config.from(path = Path(it)).getOrThrow() + Config.fromFile(path = Path(it)).getOrThrow() } ?: run { val connect = Connect(connectEndpoints) val listen = Listen(listenEndpoints) val scouting = Scouting(Multicast(!noMulticastScouting)) val configData = ConfigData(connect, listen, mode, scouting) val jsonConfig = Json.encodeToJsonElement(configData) - Config.from(jsonConfig).getOrThrow() + Config.fromFile(jsonConfig).getOrThrow() } } } diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt index 8286ad9ef..a87febed0 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt @@ -30,7 +30,36 @@ import kotlinx.serialization.json.JsonElement * * Either way, the supported formats are `yaml`, `json` and `json5`. * - * ## Example: + * A default configuration can be loaded using [Config.default]. + * + * ## Examples: + * + * ### Loading default config: + * + * ```kotlin + * val config = Config.default() + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * ### Loading from file + * + * Using [Path]: + * ```kotlin + * val config = Config.fromFile(Path("example/path/config.json5")).getOrThrow() + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * + * or alternatively, using [File] + * ```kotlin + * val config = Config.fromFile(File("example/path/config.json5")).getOrThrow() + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * ### Embedded string configuration * - Json5 * ```kotlin * val json5config = """ @@ -46,7 +75,7 @@ import kotlinx.serialization.json.JsonElement * } * } * """.trimIndent() - * val config = Config(config = json5Config, format = Config.Format.JSON5) + * val config = Config.fromJson5(json5config).getOrThrow() * Session.open(config).onSuccess { * // ... * } @@ -67,7 +96,7 @@ import kotlinx.serialization.json.JsonElement * } * } * """.trimIndent() - * val config = Config(config = json5Config, format = Config.Format.JSON) + * val config = Config.fromJson(jsonConfig).getOrThrow() * Session.open(config).onSuccess { * // ... * } @@ -84,7 +113,7 @@ import kotlinx.serialization.json.JsonElement * multicast: * enabled: false * """.trimIndent() - * val config = Config(config = yamlConfig, format = Config.Format.YAML) + * val config = Config.fromYaml(yamlConfig).getOrThrow() * Session.open(config).onSuccess { * // ... * } @@ -92,10 +121,6 @@ import kotlinx.serialization.json.JsonElement * * Visit the [default configuration](https://github.com/eclipse-zenoh/zenoh/blob/main/DEFAULT_CONFIG.json5) for more * information on the Zenoh config parameters. - * - * @property path The path to the configuration file (supported types: JSON5, JSON and YAML). - * @property config Raw string configuration, (supported types: JSON5, JSON and YAML). - * @property format [Format] of the configuration. */ class Config internal constructor(internal val jniConfig: JNIConfig) { @@ -111,29 +136,125 @@ class Config internal constructor(internal val jniConfig: JNIConfig) { /** * Loads the configuration from the [File] specified. * - * @param file The zenoh config file. + * @param file The Zenoh config file. Supported types are: JSON, JSON5 and YAML. + * Note the format is determined after the file extension. + * @return A result with the [Config]. */ - fun from(file: File): Result { + fun fromFile(file: File): Result { return JNIConfig.loadConfigFile(file) } /** * Loads the configuration from the [Path] specified. * - * @param path The zenoh config file path. + * @param path Path to the Zenoh config file. Supported types are: JSON, JSON5 and YAML. + * Note the format is determined after the file extension. + * @return A result with the [Config]. */ - fun from(path: Path): Result { + fun fromFile(path: Path): Result { return JNIConfig.loadConfigFile(path) } + /** + * Loads the configuration from json-formatted string. + * + * Example: + * ```kotlin + * val config = Config.fromJson( + * config = """ + * { + * "mode": "peer", + * "connect": { + * "endpoints": ["tcp/localhost:7450"] + * }, + * "scouting": { + * "multicast": { + * "enabled": false + * } + * } + * } + * """.trimIndent() + * ).getOrThrow() + * + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * + * Visit the [default configuration](https://github.com/eclipse-zenoh/zenoh/blob/main/DEFAULT_CONFIG.json5) for more + * information on the Zenoh config parameters. + * + * @param config Json formatted config. + * @return A result with the [Config]. + */ fun fromJson(config: String): Result { return JNIConfig.loadJsonConfig(config) } + /** + * Loads the configuration from json5-formatted string. + * + * Example: + * ```kotlin + * val config = Config.fromJson5( + * config = """ + * { + * mode: "peer", + * connect: { + * endpoints: ["tcp/localhost:7450"], + * }, + * scouting: { + * multicast: { + * enabled: false, + * } + * } + * } + * """.trimIndent() + * ).getOrThrow() + * + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * + * Visit the [default configuration](https://github.com/eclipse-zenoh/zenoh/blob/main/DEFAULT_CONFIG.json5) for more + * information on the Zenoh config parameters. + * + * @param config Json5 formatted config + * @return A result with the [Config]. + */ fun fromJson5(config: String): Result { return JNIConfig.loadJson5Config(config) } + /** + * Loads the configuration from yaml-formatted string. + * + * Example: + * ```kotlin + * val config = Config.fromYaml( + * config = """ + * mode: peer + * connect: + * endpoints: + * - tcp/localhost:7450 + * scouting: + * multicast: + * enabled: false + * """.trimIndent() + * ).getOrThrow() + * + * Session.open(config).onSuccess { + * // ... + * } + * ``` + * + * Visit the [default configuration](https://github.com/eclipse-zenoh/zenoh/blob/main/DEFAULT_CONFIG.json5) for more + * information on the Zenoh config parameters. + * + * @param config Yaml formatted config + * @return A result with the [Config]. + */ fun fromYaml(config: String): Result { return JNIConfig.loadYamlConfig(config) } @@ -143,7 +264,7 @@ class Config internal constructor(internal val jniConfig: JNIConfig) { * * @param jsonElement The zenoh config as a [JsonElement]. */ - fun from(jsonElement: JsonElement): Result { + fun fromFile(jsonElement: JsonElement): Result { return JNIConfig.loadJsonConfig(jsonElement.toString()) } } diff --git a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt index d5ce4156d..4e566b037 100644 --- a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt +++ b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt @@ -13,23 +13,26 @@ // package io.zenoh +import io.zenoh.exceptions.SessionException import io.zenoh.keyexpr.intoKeyExpr import io.zenoh.protocol.into import io.zenoh.sample.Sample import kotlinx.coroutines.runBlocking import kotlinx.coroutines.delay import kotlinx.serialization.json.Json +import org.junit.jupiter.api.assertThrows +import java.io.File import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertTrue class ConfigTest { companion object { val TEST_KEY_EXP = "example/testing/keyexpr".intoKeyExpr().getOrThrow() } - private val json5ClientConfig = Config.fromJson5( - config = """ + private val json5ClientConfigString = """ { mode: "peer", connect: { @@ -42,10 +45,8 @@ class ConfigTest { } } """.trimIndent() - ).getOrThrow() - private val json5ServerConfig = Config.fromJson5( - config = """ + private val json5ServerConfigString = """ { mode: "peer", listen: { @@ -58,11 +59,8 @@ class ConfigTest { } } """.trimIndent() - ).getOrThrow() - - private val jsonClientConfig = Config.fromJson( - config = """ + private val jsonClientConfigString = """ { "mode": "peer", "connect": { @@ -75,11 +73,8 @@ class ConfigTest { } } """.trimIndent() - ).getOrThrow() - - private val jsonServerConfig = Config.fromJson( - config = """ + private val jsonServerConfigString = """ { "mode": "peer", "listen": { @@ -92,11 +87,8 @@ class ConfigTest { } } """.trimIndent() - ).getOrThrow() - - private val yamlClientConfig = Config.fromYaml( - config = """ + private val yamlClientConfigString = """ mode: peer connect: endpoints: @@ -105,11 +97,8 @@ class ConfigTest { multicast: enabled: false """.trimIndent() - ).getOrThrow() - - private val yamlServerConfig = Config.fromYaml( - config = """ + private val yamlServerConfigString = """ mode: peer listen: endpoints: @@ -118,8 +107,30 @@ class ConfigTest { multicast: enabled: false """.trimIndent() + + private val json5ClientConfig = Config.fromJson5( + config = json5ClientConfigString + ).getOrThrow() + + private val json5ServerConfig = Config.fromJson5( + config = json5ServerConfigString + ).getOrThrow() + + private val jsonClientConfig = Config.fromJson( + config = jsonClientConfigString + ).getOrThrow() + + private val jsonServerConfig = Config.fromJson( + config = jsonServerConfigString ).getOrThrow() + private val yamlServerConfig = Config.fromYaml( + config = yamlServerConfigString + ).getOrThrow() + + private val yamlClientConfig = Config.fromYaml( + config = yamlClientConfigString + ).getOrThrow() private fun runSessionTest(clientConfig: Config, serverConfig: Config) { runBlocking { @@ -157,35 +168,122 @@ class ConfigTest { @Test fun `test config loads from JsonElement`() { val clientConfigJson = Json.parseToJsonElement( - """ - { - "mode": "peer", - "connect": { - "endpoints": ["tcp/localhost:7450"] - }, - "scouting": { - "multicast": { - "enabled": false - } - } - } - """.trimIndent() + jsonClientConfigString ) val serverConfigJson = Json.parseToJsonElement( - """ + jsonServerConfigString + ) + runSessionTest(Config.fromFile(clientConfigJson).getOrThrow(), Config.fromFile(serverConfigJson).getOrThrow()) + } + + @Test + fun `test default config`() { + val config = Config.default() + val session = Session.open(config).getOrThrow() + session.close() + } + + @Test + fun `test config returns result failure with ill formated json`() { + val illFormatedConfig = """ { - "mode": "peer", - "listen": { - "endpoints": ["tcp/localhost:7450"] - }, - "scouting": { - "multicast": { - "enabled": false - } - } + mode: "peer", + connect: { + endpoints: ["tcp/localhost:7450"], } """.trimIndent() - ) - runSessionTest(Config.from(clientConfigJson).getOrThrow(), Config.from(serverConfigJson).getOrThrow()) + val config = Config.fromJson(illFormatedConfig) + assertTrue(config.isFailure) + assertThrows { config.getOrThrow() } + } + + @Test + fun `test config returns result failure with ill formated yaml`() { + val illFormatedConfig = """ + mode: peer + connect: + endpoints: + - tcp/localhost:7450 + scouting + """.trimIndent() + val config = Config.fromJson(illFormatedConfig) + assertTrue(config.isFailure) + assertThrows { config.getOrThrow() } + } + + @Test + fun `test config loads from JSON file`() { + val clientConfigFile = File.createTempFile("clientConfig", ".json") + val serverConfigFile = File.createTempFile("serverConfig", ".json") + + try { + clientConfigFile.writeText(jsonClientConfigString) + serverConfigFile.writeText(jsonServerConfigString) + + val loadedClientConfig = Config.fromFile(clientConfigFile).getOrThrow() + val loadedServerConfig = Config.fromFile(serverConfigFile).getOrThrow() + + runSessionTest(loadedClientConfig, loadedServerConfig) + } finally { + clientConfigFile.delete() + serverConfigFile.delete() + } + } + + @Test + fun `test config loads from YAML file`() { + val clientConfigFile = File.createTempFile("clientConfig", ".yaml") + val serverConfigFile = File.createTempFile("serverConfig", ".yaml") + + try { + clientConfigFile.writeText(yamlClientConfigString) + serverConfigFile.writeText(yamlServerConfigString) + + val loadedClientConfig = Config.fromFile(clientConfigFile).getOrThrow() + val loadedServerConfig = Config.fromFile(serverConfigFile).getOrThrow() + + runSessionTest(loadedClientConfig, loadedServerConfig) + } finally { + clientConfigFile.delete() + serverConfigFile.delete() + } + } + + @Test + fun `test config loads from JSON5 file`() { + val clientConfigFile = File.createTempFile("clientConfig", ".json5") + val serverConfigFile = File.createTempFile("serverConfig", ".json5") + + try { + clientConfigFile.writeText(json5ClientConfigString) + serverConfigFile.writeText(json5ServerConfigString) + + val loadedClientConfig = Config.fromFile(clientConfigFile).getOrThrow() + val loadedServerConfig = Config.fromFile(serverConfigFile).getOrThrow() + + runSessionTest(loadedClientConfig, loadedServerConfig) + } finally { + clientConfigFile.delete() + serverConfigFile.delete() + } + } + + @Test + fun `test config loads from JSON5 file providing path`() { + val clientConfigFile = File.createTempFile("clientConfig", ".json5") + val serverConfigFile = File.createTempFile("serverConfig", ".json5") + + try { + clientConfigFile.writeText(json5ClientConfigString) + serverConfigFile.writeText(json5ServerConfigString) + + val loadedClientConfig = Config.fromFile(clientConfigFile.toPath()).getOrThrow() + val loadedServerConfig = Config.fromFile(serverConfigFile.toPath()).getOrThrow() + + runSessionTest(loadedClientConfig, loadedServerConfig) + } finally { + clientConfigFile.delete() + serverConfigFile.delete() + } } } From 72619a533a71803fc50d6d83c1e4c7a961c64b75 Mon Sep 17 00:00:00 2001 From: Darius Maitia Date: Mon, 2 Sep 2024 11:21:12 -0300 Subject: [PATCH 3/4] refactor(config): fix load from JSON element + adding docs to zenoh-jni --- zenoh-jni/src/config.rs | 26 +++++++++++++++++++ .../src/commonMain/kotlin/io/zenoh/Config.kt | 2 +- .../commonTest/kotlin/io/zenoh/ConfigTest.kt | 5 +++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/zenoh-jni/src/config.rs b/zenoh-jni/src/config.rs index 0cf3d9a7b..ec0c5002f 100644 --- a/zenoh-jni/src/config.rs +++ b/zenoh-jni/src/config.rs @@ -23,6 +23,11 @@ use zenoh::Config; use crate::errors::Result; use crate::{session_error, throw_exception, utils::decode_string}; +/// Loads the default configuration, returning a raw pointer to it. +/// +/// The pointer to the config is expected to be freed later on upon the destruction of the +/// Kotlin Config instance. +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadDefaultConfigViaJNI( @@ -33,6 +38,12 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadDefaultConfigVi Arc::into_raw(Arc::new(config)) } +/// Loads the config from a file, returning a pointer to the loaded config in case of success. +/// In case of failure, an exception is thrown via JNI. +/// +/// The pointer to the config is expected to be freed later on upon the destruction of the +/// Kotlin Config instance. +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadConfigFileViaJNI( @@ -51,6 +62,12 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadConfigFileViaJN }) } +/// Loads the config from a json/json5 formatted string, returning a pointer to the loaded config +/// in case of success. In case of failure, an exception is thrown via JNI. +/// +/// The pointer to the config is expected to be freed later on upon the destruction of the +/// Kotlin Config instance. +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadJsonConfigViaJNI( @@ -74,6 +91,12 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadJsonConfigViaJN }) } +/// Loads the config from a yaml-formatted string, returning a pointer to the loaded config +/// in case of success. In case of failure, an exception is thrown via JNI. +/// +/// The pointer to the config is expected to be freed later on upon the destruction of the +/// Kotlin Config instance. +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadYamlConfigViaJNI( @@ -96,6 +119,9 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadYamlConfigViaJN }) } +/// Frees the pointer to the config. The pointer should be valid and should have been obtained through +/// one of the preceding `load` functions. This function should be called upon destruction of the kotlin +/// Config instance. #[no_mangle] #[allow(non_snake_case)] pub(crate) unsafe extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_freePtrViaJNI( diff --git a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt index a87febed0..90435cabc 100644 --- a/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt +++ b/zenoh-kotlin/src/commonMain/kotlin/io/zenoh/Config.kt @@ -264,7 +264,7 @@ class Config internal constructor(internal val jniConfig: JNIConfig) { * * @param jsonElement The zenoh config as a [JsonElement]. */ - fun fromFile(jsonElement: JsonElement): Result { + fun fromJsonElement(jsonElement: JsonElement): Result { return JNIConfig.loadJsonConfig(jsonElement.toString()) } } diff --git a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt index 4e566b037..687e479a7 100644 --- a/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt +++ b/zenoh-kotlin/src/commonTest/kotlin/io/zenoh/ConfigTest.kt @@ -173,7 +173,10 @@ class ConfigTest { val serverConfigJson = Json.parseToJsonElement( jsonServerConfigString ) - runSessionTest(Config.fromFile(clientConfigJson).getOrThrow(), Config.fromFile(serverConfigJson).getOrThrow()) + runSessionTest( + Config.fromJsonElement(clientConfigJson).getOrThrow(), + Config.fromJsonElement(serverConfigJson).getOrThrow() + ) } @Test From e95863a50c3ee422b9ea8d157b0cfe03593a584a Mon Sep 17 00:00:00 2001 From: Darius Maitia Date: Mon, 2 Sep 2024 11:57:27 -0300 Subject: [PATCH 4/4] Cargo fmt + cargo clippy --- zenoh-jni/src/config.rs | 18 +++++++++--------- zenoh-jni/src/session.rs | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/zenoh-jni/src/config.rs b/zenoh-jni/src/config.rs index ec0c5002f..0edf1420e 100644 --- a/zenoh-jni/src/config.rs +++ b/zenoh-jni/src/config.rs @@ -24,10 +24,10 @@ use crate::errors::Result; use crate::{session_error, throw_exception, utils::decode_string}; /// Loads the default configuration, returning a raw pointer to it. -/// +/// /// The pointer to the config is expected to be freed later on upon the destruction of the /// Kotlin Config instance. -/// +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadDefaultConfigViaJNI( @@ -40,10 +40,10 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadDefaultConfigVi /// Loads the config from a file, returning a pointer to the loaded config in case of success. /// In case of failure, an exception is thrown via JNI. -/// +/// /// The pointer to the config is expected to be freed later on upon the destruction of the /// Kotlin Config instance. -/// +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadConfigFileViaJNI( @@ -64,10 +64,10 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadConfigFileViaJN /// Loads the config from a json/json5 formatted string, returning a pointer to the loaded config /// in case of success. In case of failure, an exception is thrown via JNI. -/// +/// /// The pointer to the config is expected to be freed later on upon the destruction of the /// Kotlin Config instance. -/// +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadJsonConfigViaJNI( @@ -93,10 +93,10 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadJsonConfigViaJN /// Loads the config from a yaml-formatted string, returning a pointer to the loaded config /// in case of success. In case of failure, an exception is thrown via JNI. -/// +/// /// The pointer to the config is expected to be freed later on upon the destruction of the /// Kotlin Config instance. -/// +/// #[no_mangle] #[allow(non_snake_case)] pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadYamlConfigViaJNI( @@ -119,7 +119,7 @@ pub extern "C" fn Java_io_zenoh_jni_JNIConfig_00024Companion_loadYamlConfigViaJN }) } -/// Frees the pointer to the config. The pointer should be valid and should have been obtained through +/// Frees the pointer to the config. The pointer should be valid and should have been obtained through /// one of the preceding `load` functions. This function should be called upon destruction of the kotlin /// Config instance. #[no_mangle] diff --git a/zenoh-jni/src/session.rs b/zenoh-jni/src/session.rs index a0be09de7..dcc8a47dc 100644 --- a/zenoh-jni/src/session.rs +++ b/zenoh-jni/src/session.rs @@ -75,7 +75,7 @@ unsafe fn open_session(config_ptr: *const Config) -> Result { .wait() .map_err(|err| session_error!(err)); mem::forget(config); - return result; + result } /// Open a Zenoh session with a JSON configuration.