[CLIENT-3805] Add missing app_id attribute to Cluster class for extended metrics

.gitmodules

@@ -2,4 +2,4 @@
 	path = aerospike-client-c
 	# url = git@github.com:aerospike/aerospike-client-c.git
 	url = https://github.com/aerospike/aerospike-client-c.git
-	branch = client-3753-path-expressions-integration
+	branch = stage

aerospike_helpers/metrics/__init__.py

@@ -105,6 +105,8 @@ class Cluster:
 
     Attributes:
         cluster_name (Optional[str]): Expected cluster name for all nodes. May be :py:obj:`None`.
+        app_id (Optional[str]): Application identifier. Will be :py:obj:`None` if ``app_id`` is not set to a string in
+            the client config dictionary.
         invalid_node_count (int): Count of add node failures in the most recent cluster tend iteration.
         command_count (int): Command count. The value is cumulative and not reset per metrics interval.
         retry_count (int): Command retry count. There can be multiple retries for a single command.

doc/aerospike.rst

@@ -767,10 +767,17 @@ Only the `hosts` key is required; the rest of the keys are optional.
             Compress data for transmission if the object size is greater than a given number of bytes
 
             Default: ``0``, meaning 'never compress'
-        * **cluster_name** (:class:`str`)
-            Only server nodes matching this name will be used when determining the cluster name.
-        * **app_id** (:class:`str`)
-            Application identifier.
+        * **cluster_name** (:class:`Optional[str]`)
+            Expected cluster name. If set to a string value, the ``cluster_name`` must match the cluster-name field
+            in the service section in each server configuration. This ensures that the specified
+            seed nodes belong to the expected cluster on startup. If not, the client will refuse
+            to add the node to the client's view of the cluster.
+
+            Default: :py:obj:`None`
+        * **app_id** (:class:`Optional[str]`)
+            Application identifier. If not set to a string, uses the client's username by default.
+
+            Default: :py:obj:`None`
         * **rack_id** (:class:`int`)
             Rack id where this client instance resides.
 

src/main/aerospike.c

@@ -561,16 +561,14 @@ static struct module_constant_name_to_value module_constants[] = {
     EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(EXP_LOOPVAR_VALUE),
     EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(EXP_LOOPVAR_INDEX),
 
-    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_MATCHING_TREE),
-    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_VALUES),
+    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_TREE),
     EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(
-        CDT_SELECT_MAP_KEY_VALUES),
-    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_MAP_KEYS),
+        CDT_SELECT_LEAF_LIST_VALUE),
+    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(
+        CDT_SELECT_LEAF_MAP_VALUE),
+    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_LEAF_MAP_KEY),
     EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_SELECT_NO_FAIL),
 
-    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_MODIFY_NO_FAIL),
-    EXPOSE_AS_MACRO_WITHOUT_AS_PREFIX_AS_PUBLIC_FIELD(CDT_MODIFY_DEFAULT),
-
     // For aerospike_helpers to use. Not to be exposed in public API
     // TODO: move all internal constants used by aerospike_helpers to this loc
 

src/main/client/operate.c

@@ -586,7 +586,7 @@ as_status add_op(AerospikeClient *self, as_error *err,
 
         if (operation == AS_OPERATOR_CDT_READ) {
             operation_succeeded =
-                as_operations_select_by_path(ops, bin, ctx_ref, flags);
+                as_operations_cdt_select(ops, bin, ctx_ref, flags);
         }
         else if (operation == AS_OPERATOR_CDT_MODIFY) {
             PyObject *py_expr = NULL;
@@ -610,7 +610,7 @@ as_status add_op(AerospikeClient *self, as_error *err,
             }
 
             operation_succeeded =
-                as_operations_modify_by_path(ops, bin, ctx_ref, mod_exp, flags);
+                as_operations_cdt_apply(ops, bin, ctx_ref, mod_exp, flags);
         }
 
         break;

src/main/client/type.c

@@ -1219,7 +1219,7 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args,
 
     PyObject *py_app_id = NULL;
     retval = PyDict_GetItemStringRef(py_config, "app_id", &py_app_id);
-    if (retval == 1) {
+    if (retval == 1 && !Py_IsNone(py_app_id)) {
         const char *str = convert_pyobject_to_str(py_app_id);
         if (!str) {
             Py_DECREF(py_app_id);

src/main/conversions.c

@@ -1054,6 +1054,24 @@ PyObject *create_py_cluster_from_as_cluster(as_error *error_p,
         PyObject_SetAttrString(py_cluster, "cluster_name", Py_None);
     }
 
+    // App Id is optional (declared in client config)
+    PyObject *py_app_id = NULL;
+    if (cluster->app_id) {
+        py_app_id = PyUnicode_FromString(cluster->app_id);
+        if (!py_app_id) {
+            goto error;
+        }
+    }
+    else {
+        py_app_id = Py_NewRef(Py_None);
+    }
+
+    int retval = PyObject_SetAttrString(py_cluster, "app_id", py_app_id);
+    Py_DECREF(py_app_id);
+    if (retval == -1) {
+        goto error;
+    }
+
     PyObject *py_invalid_node_count =
         PyLong_FromUnsignedLong(cluster->invalid_node_count);
     PyObject_SetAttrString(py_cluster, "invalid_node_count",
@@ -2507,7 +2525,7 @@ as_status get_cdt_ctx(AerospikeClient *self, as_error *err, as_cdt_ctx *cdt_ctx,
         }
         else if (item_type == AS_CDT_CTX_EXP) {
             if (Py_IsNone(py_extra_args)) {
-                as_cdt_ctx_add_all_children(cdt_ctx);
+                as_cdt_ctx_add_all(cdt_ctx);
             }
             else {
                 PyObject *py_expr = NULL;
@@ -2527,7 +2545,7 @@ as_status get_cdt_ctx(AerospikeClient *self, as_error *err, as_cdt_ctx *cdt_ctx,
                 }
 
                 // This C client call memcpy's the expr's contents
-                as_cdt_ctx_add_all_children_with_filter(cdt_ctx, expr);
+                as_cdt_ctx_add_exp(cdt_ctx, expr);
                 as_exp_destroy(expr);
             }
         }

src/main/convert_expressions.c

@@ -206,10 +206,9 @@ static as_status get_expr_size(int *size_to_alloc, int *intermediate_exprs_size,
 {
 
     static const int EXPR_SIZES[] = {
-        [_AS_EXP_CODE_CALL_SELECT] =
-            EXP_SZ(as_exp_select_by_path(NULL, 0, 0, NIL)),
+        [_AS_EXP_CODE_CALL_SELECT] = EXP_SZ(as_exp_cdt_select(NULL, 0, 0, NIL)),
         [_AS_EXP_CODE_CALL_APPLY] =
-            EXP_SZ(as_exp_modify_by_path(NULL, 0, NULL, 0, NIL)),
+            EXP_SZ(as_exp_cdt_apply(NULL, 0, NULL, 0, NIL)),
         [BIN] = EXP_SZ(as_exp_bin_int(0)),
         [_AS_EXP_CODE_AS_VAL] = EXP_SZ(as_exp_val(NULL)),
         [_AS_EXP_LOOPVAR_FLOAT] = EXP_SZ(as_exp_loopvar_float(0)),
@@ -1675,12 +1674,12 @@ add_expr_macros(AerospikeClient *self, as_static_pool *static_pool,
                     return err->code;
                 }
 
-                APPEND_ARRAY(1, as_exp_modify_by_path(temp_expr->ctx, lval1,
-                                                      mod_exp, lval2, NIL));
+                APPEND_ARRAY(1, as_exp_cdt_apply(temp_expr->ctx, lval1, mod_exp,
+                                                 lval2, NIL));
             }
             else {
-                APPEND_ARRAY(1, as_exp_select_by_path(temp_expr->ctx, lval1,
-                                                      lval2, NIL));
+                APPEND_ARRAY(
+                    1, as_exp_cdt_select(temp_expr->ctx, lval1, lval2, NIL));
             }
             break;
         default:

test/new_tests/test_connect.py

@@ -147,18 +147,28 @@ def test_connect_positive_shm_not_enabled(self):
             assert client.is_connected()
             assert client.shm_key() is None
 
-    def test_connect_positive_cluster_name(self):
+    @pytest.mark.parametrize(
+        "cluster_name",
+        [
+            None,
+            # This test case is for code coverage purposes
+            "invalid-cluster-name"
+        ]
+    )
+    def test_connect_with_cluster_name(self, cluster_name):
         """
-        Invoke connect() giving a cluster name. This is just a usage test (doesn't care if the server's cluster name
-        matches or not)
+        Invoke connect() giving a cluster name
         """
         config = self.connection_config.copy()
-        config["cluster_name"] = "test-cluster"
+        config["cluster_name"] = cluster_name
+
+        if cluster_name is None:
+            cm = nullcontext()
+        else:
+            cm = pytest.raises(e.ClientError)
 
-        try:
-            self.client = aerospike.client(config).connect()
-        except e.ClientError:
-            pass
+        with cm:
+            self.client = aerospike.client(config)
 
     def test_connect_positive_reconnect(self):
         """

test/new_tests/test_metrics.py

@@ -7,6 +7,8 @@
 import time
 from typing import Optional
 import re
+import aerospike
+from .test_base_class import TestBaseClass
 from importlib.metadata import version
 
 # Flags for testing callbacks
@@ -160,7 +162,19 @@ def test_metrics_writer(self):
             for item in metrics_log_filenames:
                 os.remove(item)
 
-    def test_setting_metrics_policy_custom_settings(self):
+    @pytest.fixture(scope="function", params=[None, "my_app"])
+    def get_client_and_app_id(self, request):
+        config = TestBaseClass.get_connection_config()
+        config["app_id"] = request.param
+        client = aerospike.client(config)
+
+        yield request.param, client
+
+        client.close()
+
+    def test_setting_metrics_policy_custom_settings(self, get_client_and_app_id):
+        app_id, client = get_client_and_app_id
+
         self.metrics_log_folder = "./metrics-logs"
 
         # Save bucket count for testing later
@@ -175,9 +189,9 @@ def test_setting_metrics_policy_custom_settings(self):
             labels={"a": "b"},
         )
 
-        self.as_connection.enable_metrics(policy=policy)
+        client.enable_metrics(policy=policy)
         time.sleep(3)
-        self.as_connection.disable_metrics()
+        client.disable_metrics()
 
         # These callbacks should've been called
         assert enable_triggered is True
@@ -194,6 +208,11 @@ def test_setting_metrics_policy_custom_settings(self):
             assert type(cluster.command_count) == int
             assert type(cluster.retry_count) == int
             assert type(cluster.nodes) == list
+            if type(app_id) == str:
+                assert cluster.app_id == app_id
+            else:
+                # Or None if the app_id is not set
+                assert cluster.app_id is None
             # Also check the Node and ConnectionStats objects in the Cluster object were populated
             for node in cluster.nodes:
                 assert type(node) == Node