store keyexpr string in declared keyexpression;

get rid of zp_keyexpr_resolve;

docs/api.rst

@@ -300,7 +300,6 @@ Primitives
 .. autocfunction:: primitives.h::z_view_keyexpr_from_str_unchecked
 .. autocfunction:: primitives.h::z_view_keyexpr_from_str_autocanonize
 .. autocfunction:: primitives.h::z_keyexpr_to_string
-.. autocfunction:: primitives.h::zp_keyexpr_resolve
 .. autocfunction:: primitives.h::z_keyexpr_is_initialized
 .. autocfunction:: primitives.h::z_keyexpr_is_canon
 .. autocfunction:: primitives.h::zp_keyexpr_is_canon_null_terminated

include/zenoh-pico/api/primitives.h

@@ -116,10 +116,6 @@ int8_t z_view_keyexpr_from_str_autocanonize(z_view_keyexpr_t *keyexpr, char *nam
 /**
  * Gets a null-terminated string from a :c:type:`z_keyexpr_t`.
  *
- * If given keyexpr contains a declared keyexpr, the resulting owned string will be uninitialized.
- * In that case, the user must use :c:func:`zp_keyexpr_resolve` to resolve the nesting declarations
- * and get its full expanded representation.
- *
  * Parameters:
  *   keyexpr: Pointer to a loaned instance of :c:type:`z_keyexpr_t`.
  *   str: Pointer to an uninitialized :c:type:`z_owned_string_t`.
@@ -129,20 +125,6 @@ int8_t z_view_keyexpr_from_str_autocanonize(z_view_keyexpr_t *keyexpr, char *nam
  */
 int8_t z_keyexpr_to_string(const z_loaned_keyexpr_t *keyexpr, z_owned_string_t *str);
 
-/**
- * Builds a null-terminated string from a :c:type:`z_loaned_keyexpr_t` for a given
- * :c:type:`z_loaned_session_t`.
- *
- * Parameters:
- *   zs: Pointer to a :c:type:`z_loaned_session_t` to resolve the keyexpr.
- *   keyexpr: Pointer to a :c:type:`z_loaned_keyexpr_t` to be resolved.
- *   str: Pointer to an uninitialized :c:type:`z_owned_string_t`.
- *
- * Return:
- *   ``0`` if creation successful, ``negative value`` otherwise.
- */
-int8_t zp_keyexpr_resolve(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, z_owned_string_t *str);
-
 /**
  * Checks if a given keyexpr is valid.
  *

include/zenoh-pico/api/types.h

@@ -107,7 +107,6 @@ _Z_VIEW_TYPE(_z_string_t, string)
  *   - :c:func:`z_keyexpr_from_str`
  *   - :c:func:`z_keyexpr_is_initialized`
  *   - :c:func:`z_keyexpr_to_string`
- *   - :c:func:`zp_keyexpr_resolve`
  */
 _Z_OWNED_TYPE_VALUE(_z_keyexpr_t, keyexpr)
 _Z_LOANED_TYPE(_z_keyexpr_t, keyexpr)

include/zenoh-pico/protocol/keyexpr.h

@@ -29,6 +29,10 @@ int8_t _z_keyexpr_copy(_z_keyexpr_t *dst, const _z_keyexpr_t *src);
 _z_keyexpr_t _z_keyexpr_duplicate(_z_keyexpr_t src);
 _z_keyexpr_t _z_keyexpr_to_owned(_z_keyexpr_t src);
 _z_keyexpr_t _z_keyexpr_alias(_z_keyexpr_t src);
+/// Returns either keyexpr defined by id + mapping with null suffix if try_declared is true and id is non-zero,
+/// or keyexpr defined by its suffix only, with 0 id and no mapping. This is to be used only when forwarding
+/// keyexpr in user api to properly separate declard keyexpr from its suffix.
+_z_keyexpr_t _z_keyexpr_alias_from_user_defined(_z_keyexpr_t src, _Bool try_declared);
 _z_keyexpr_t _z_keyexpr_steal(_Z_MOVE(_z_keyexpr_t) src);
 static inline _z_keyexpr_t _z_keyexpr_null(void) {
     _z_keyexpr_t keyexpr = {0, {0}, NULL};

src/api/api.c

@@ -102,26 +102,14 @@ int8_t z_view_keyexpr_from_str_unchecked(z_view_keyexpr_t *keyexpr, const char *
 
 int8_t z_keyexpr_to_string(const z_loaned_keyexpr_t *keyexpr, z_owned_string_t *s) {
     int8_t ret = _Z_RES_OK;
-    if (keyexpr->_id == Z_RESOURCE_ID_NONE) {
-        s->_val = _z_string_make(keyexpr->_suffix);
-        if (!_z_string_check(&s->_val)) {
-            ret = _Z_ERR_SYSTEM_OUT_OF_MEMORY;
-        }
-    } else {
-        ret = _Z_ERR_GENERIC;
+    s->_val = _z_string_make(keyexpr->_suffix);
+    if (!_z_string_check(&s->_val)) {
+        ret = _Z_ERR_SYSTEM_OUT_OF_MEMORY;
     }
 
     return ret;
 }
 
-int8_t zp_keyexpr_resolve(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, z_owned_string_t *str) {
-    _z_keyexpr_t ekey = _z_get_expanded_key_from_key(&_Z_RC_IN_VAL(zs), keyexpr);
-    str->_val = _z_string_make((char *)ekey._suffix);  // ekey will be out of scope so
-                                                       //  - suffix can be safely casted as non-const
-                                                       //  - suffix does not need to be copied
-    return _z_string_check(&str->_val) ? _Z_RES_OK : _Z_ERR_SYSTEM_OUT_OF_MEMORY;
-}
-
 _Bool z_keyexpr_is_initialized(const z_loaned_keyexpr_t *keyexpr) {
     _Bool ret = false;
 
@@ -943,12 +931,14 @@ int8_t z_put(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, z_
         opt.attachment = options->attachment;
     }
 
-    ret = _z_write(&_Z_RC_IN_VAL(zs), *keyexpr, _z_bytes_from_owned_bytes(payload),
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
+
+    ret = _z_write(&_Z_RC_IN_VAL(zs), keyexpr_aliased, _z_bytes_from_owned_bytes(payload),
                    _z_encoding_from_owned(opt.encoding), Z_SAMPLE_KIND_PUT, opt.congestion_control, opt.priority,
                    opt.timestamp, _z_bytes_from_owned_bytes(opt.attachment));
 
     // Trigger local subscriptions
-    _z_trigger_local_subscriptions(&_Z_RC_IN_VAL(zs), *keyexpr, _z_bytes_from_owned_bytes(payload),
+    _z_trigger_local_subscriptions(&_Z_RC_IN_VAL(zs), keyexpr_aliased, _z_bytes_from_owned_bytes(payload),
                                    _z_n_qos_make(0, opt.congestion_control == Z_CONGESTION_CONTROL_BLOCK, opt.priority),
                                    _z_bytes_from_owned_bytes(opt.attachment));
     // Clean-up
@@ -981,16 +971,17 @@ void z_publisher_options_default(z_publisher_options_t *options) {
 
 int8_t z_declare_publisher(z_owned_publisher_t *pub, const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr,
                            const z_publisher_options_t *options) {
-    _z_keyexpr_t key = *keyexpr;
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
+    _z_keyexpr_t key = keyexpr_aliased;
 
     pub->_val = _z_publisher_null();
     // TODO: Currently, if resource declarations are done over multicast transports, the current protocol definition
     //       lacks a way to convey them to later-joining nodes. Thus, in the current version automatic
     //       resource declarations are only performed on unicast transports.
     if (_Z_RC_IN_VAL(zs)._tp._type == _Z_TRANSPORT_UNICAST_TYPE) {
-        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), keyexpr);
+        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), &keyexpr_aliased);
         if (r == NULL) {
-            uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), *keyexpr);
+            uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), keyexpr_aliased);
             key = _z_rid_with_suffix(id, NULL);
         }
     }
@@ -1090,6 +1081,8 @@ int8_t z_get(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, co
     void *ctx = callback->_val.context;
     callback->_val.context = NULL;
 
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
+
     z_get_options_t opt;
     z_get_options_default(&opt);
     if (options != NULL) {
@@ -1112,7 +1105,7 @@ int8_t z_get(const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr, co
     _z_value_t value = {.payload = _z_bytes_from_owned_bytes(opt.payload),
                         .encoding = _z_encoding_from_owned(opt.encoding)};
 
-    ret = _z_query(&_Z_RC_IN_VAL(zs), *keyexpr, parameters, opt.target, opt.consolidation.mode, value,
+    ret = _z_query(&_Z_RC_IN_VAL(zs), keyexpr_aliased, parameters, opt.target, opt.consolidation.mode, value,
                    callback->_val.call, callback->_val.drop, ctx, opt.timeout_ms,
                    _z_bytes_from_owned_bytes(opt.attachment));
     if (opt.payload != NULL) {
@@ -1164,15 +1157,16 @@ int8_t z_declare_queryable(z_owned_queryable_t *queryable, const z_loaned_sessio
     void *ctx = callback->_val.context;
     callback->_val.context = NULL;
 
-    _z_keyexpr_t key = *keyexpr;
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
+    _z_keyexpr_t key = keyexpr_aliased;
 
     // TODO: Currently, if resource declarations are done over multicast transports, the current protocol definition
     //       lacks a way to convey them to later-joining nodes. Thus, in the current version automatic
     //       resource declarations are only performed on unicast transports.
     if (_Z_RC_IN_VAL(zs)._tp._type == _Z_TRANSPORT_UNICAST_TYPE) {
-        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), keyexpr);
+        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), &keyexpr_aliased);
         if (r == NULL) {
-            uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), *keyexpr);
+            uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), keyexpr_aliased);
             key = _z_rid_with_suffix(id, NULL);
         }
     }
@@ -1200,6 +1194,7 @@ void z_query_reply_options_default(z_query_reply_options_t *options) {
 
 int8_t z_query_reply(const z_loaned_query_t *query, const z_loaned_keyexpr_t *keyexpr, z_owned_bytes_t *payload,
                      const z_query_reply_options_t *options) {
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
     z_query_reply_options_t opts;
     if (options == NULL) {
         z_query_reply_options_default(&opts);
@@ -1210,8 +1205,8 @@ int8_t z_query_reply(const z_loaned_query_t *query, const z_loaned_keyexpr_t *ke
     _z_value_t value = {.payload = _z_bytes_from_owned_bytes(payload),
                         .encoding = _z_encoding_from_owned(opts.encoding)};
 
-    int8_t ret =
-        _z_send_reply(&query->in->val, *keyexpr, value, Z_SAMPLE_KIND_PUT, _z_bytes_from_owned_bytes(opts.attachment));
+    int8_t ret = _z_send_reply(&query->in->val, keyexpr_aliased, value, Z_SAMPLE_KIND_PUT,
+                               _z_bytes_from_owned_bytes(opts.attachment));
     if (payload != NULL) {
         z_bytes_drop(payload);
     }
@@ -1232,8 +1227,19 @@ int8_t z_keyexpr_from_str(z_owned_keyexpr_t *key, const char *name) {
 }
 
 int8_t z_declare_keyexpr(z_owned_keyexpr_t *key, const z_loaned_session_t *zs, const z_loaned_keyexpr_t *keyexpr) {
-    uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), *keyexpr);
+    _z_keyexpr_t k = _z_keyexpr_alias_from_user_defined(*keyexpr, false);
+    uint16_t id = _z_declare_resource(&_Z_RC_IN_VAL(zs), k);
     key->_val = _z_rid_with_suffix(id, NULL);
+    if (keyexpr->_suffix) key->_val._suffix = _z_str_clone(keyexpr->_suffix);
+    // we still need to store the original suffix, for user needs
+    // (for example to compare keys or perform other operations on their string representation).
+    // Generally this breaks internal keyexpr representation, but is ok for user-defined keyexprs
+    // since they consist of 2 disjoint sets: either they have a non-nul suffix or non-trivial id/mapping.
+    // The resulting keyexpr can be separated later into valid internal keys using _z_keyexpr_alias_from_user_defined.
+    if (key->_val._suffix == NULL && keyexpr->_suffix != NULL) {
+        return _Z_ERR_SYSTEM_OUT_OF_MEMORY;
+    }
+    _z_keyexpr_set_owns_suffix(&key->_val, true);
     return _Z_RES_OK;
 }
 
@@ -1267,16 +1273,18 @@ int8_t z_declare_subscriber(z_owned_subscriber_t *sub, const z_loaned_session_t
     callback->_val.context = NULL;
     char *suffix = NULL;
 
-    _z_keyexpr_t key = *keyexpr;
+    _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
+    _z_keyexpr_t key = keyexpr_aliased;
+
     // TODO: Currently, if resource declarations are done over multicast transports, the current protocol definition
     //       lacks a way to convey them to later-joining nodes. Thus, in the current version automatic
     //       resource declarations are only performed on unicast transports.
     if (_Z_RC_IN_VAL(zs)._tp._type == _Z_TRANSPORT_UNICAST_TYPE) {
-        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), keyexpr);
+        _z_resource_t *r = _z_get_resource_by_key(&_Z_RC_IN_VAL(zs), &keyexpr_aliased);
         if (r == NULL) {
-            char *wild = strpbrk(keyexpr->_suffix, "*$");
+            char *wild = strpbrk(keyexpr_aliased._suffix, "*$");
             _Bool do_keydecl = true;
-            _z_keyexpr_t resource_key = *keyexpr;
+            _z_keyexpr_t resource_key = keyexpr_aliased;
             if (wild != NULL && wild != resource_key._suffix) {
                 wild -= 1;
                 size_t len = (size_t)(wild - resource_key._suffix);

src/protocol/keyexpr.c

@@ -89,6 +89,18 @@ _z_keyexpr_t _z_keyexpr_alias(_z_keyexpr_t src) {
     return alias;
 }
 
+_z_keyexpr_t _z_keyexpr_alias_from_user_defined(_z_keyexpr_t src, _Bool try_declared) {
+    if ((try_declared && src._id != Z_RESOURCE_ID_NONE) || src._suffix == NULL) {
+        return (_z_keyexpr_t){
+            ._id = src._id,
+            ._mapping = src._mapping,
+            ._suffix = NULL,
+        };
+    } else {
+        return _z_rname(src._suffix);
+    }
+}
+
 /*------------------ Canonize helpers ------------------*/
 zp_keyexpr_canon_status_t __zp_canon_prefix(const char *start, size_t *len) {
     zp_keyexpr_canon_status_t ret = Z_KEYEXPR_CANON_SUCCESS;

tests/z_api_alignment_test.c

@@ -67,11 +67,8 @@ void query_handler(const z_loaned_query_t *query, void *arg) {
     const z_loaned_keyexpr_t *query_ke = z_query_keyexpr(query);
     z_owned_string_t k_str;
     z_keyexpr_to_string(query_ke, &k_str);
-#ifdef ZENOH_PICO
-    if (z_check(k_str) == false) {
-        zp_keyexpr_resolve(*(const z_loaned_session_t **)arg, z_query_keyexpr(query), &k_str);
-    }
-#endif
+    (void)arg;
+    assert(z_check(k_str));
 
     z_view_string_t pred;
     z_query_parameters(query, &pred);
@@ -94,17 +91,14 @@ volatile unsigned int replies = 0;
 void reply_handler(const z_loaned_reply_t *reply, void *arg) {
     printf("%s\n", __func__);
     replies++;
+    (void)arg;
 
     if (z_reply_is_ok(reply)) {
         const z_loaned_sample_t *sample = z_reply_ok(reply);
 
         z_owned_string_t k_str;
         z_keyexpr_to_string(z_sample_keyexpr(sample), &k_str);
-#ifdef ZENOH_PICO
-        if (z_check(k_str) == false) {
-            zp_keyexpr_resolve(*(const z_loaned_session_t **)arg, z_sample_keyexpr(sample), &k_str);
-        }
-#endif
+        assert(z_check(k_str));
         z_drop(z_move(k_str));
     } else {
         const z_loaned_reply_err_t *_ret_zerr = z_reply_err(reply);
@@ -119,11 +113,8 @@ void data_handler(const z_loaned_sample_t *sample, void *arg) {
 
     z_owned_string_t k_str;
     z_keyexpr_to_string(z_sample_keyexpr(sample), &k_str);
-#ifdef ZENOH_PICO
-    if (z_check(k_str) == false) {
-        zp_keyexpr_resolve(*(const z_loaned_session_t **)arg, z_sample_keyexpr(sample), &k_str);
-    }
-#endif
+    assert(z_check(k_str));
+    (void)arg;
     z_drop(z_move(k_str));
 }