[CLIENT-5052] Merge client config level and command level policy codepaths into one

src/include/policy.h

@@ -30,6 +30,8 @@
 #include <aerospike/as_metrics.h>
 #include <aerospike/as_string_operations.h>
 
+#include "types.h"
+
 enum Aerospike_serializer_values {
     SERIALIZER_NONE, /* default handler for serializer type */
     SERIALIZER_PYTHON,
@@ -269,8 +271,7 @@ as_status pyobject_to_policy_admin(AerospikeClient *self, as_error *err,
 as_status pyobject_to_policy_apply(AerospikeClient *self, as_error *err,
                                    PyObject *py_policy, as_policy_apply *policy,
                                    as_policy_apply **policy_p,
-                                   as_policy_apply *config_apply_policy,
-                                   as_exp **exp_list_p);
+                                   as_policy_apply *config_apply_policy);
 
 typedef enum {
     SECOND_AS_POLICY_WRITE,
@@ -288,49 +289,32 @@ pyobject_to_policy_info(as_error *err, PyObject *py_policy,
 as_status pyobject_to_policy_query(AerospikeClient *self, as_error *err,
                                    PyObject *py_policy, as_policy_query *policy,
                                    as_policy_query **policy_p,
-                                   as_policy_query *config_query_policy,
-                                   as_exp **exp_list_p);
-
-as_status pyobject_to_policy_read(AerospikeClient *self, as_error *err,
-                                  PyObject *py_policy, as_policy_read *policy,
-                                  as_policy_read **policy_p,
-                                  as_policy_read *config_read_policy,
-                                  as_exp **exp_list_p);
+                                   as_policy_query *config_query_policy);
 
 as_status pyobject_to_policy_remove(AerospikeClient *self, as_error *err,
                                     PyObject *py_policy,
                                     as_policy_remove *policy,
                                     as_policy_remove **policy_p,
-                                    as_policy_remove *config_remove_policy,
-                                    as_exp **exp_list_p);
+                                    as_policy_remove *config_remove_policy);
 
 // py_policy_also_supports_info_policy_fields only applies if self->validate_keys is true
 as_status
 pyobject_to_policy_scan(AerospikeClient *self, as_error *err,
                         PyObject *py_policy, as_policy_scan *policy,
                         as_policy_scan **policy_p,
-                        as_policy_scan *config_scan_policy, as_exp **exp_list_p,
+                        as_policy_scan *config_scan_policy,
                         bool py_policy_also_supports_info_policy_fields);
 
-// py_policy_also_supports_info_policy_fields only applies if self->validate_keys is true
-as_status pyobject_to_policy_write(
-    AerospikeClient *self, as_error *err, PyObject *py_policy,
-    as_policy_write *policy, as_policy_write **policy_p,
-    as_policy_write *config_write_policy, as_exp **exp_list_p,
-    bool py_policy_also_supports_info_policy_fields);
-
 as_status pyobject_to_policy_operate(AerospikeClient *self, as_error *err,
                                      PyObject *py_policy,
                                      as_policy_operate *policy,
                                      as_policy_operate **policy_p,
-                                     as_policy_operate *config_operate_policy,
-                                     as_exp **exp_list_p);
+                                     as_policy_operate *config_operate_policy);
 
 as_status pyobject_to_policy_batch(AerospikeClient *self, as_error *err,
                                    PyObject *py_policy, as_policy_batch *policy,
                                    as_policy_batch **policy_p,
-                                   as_policy_batch *config_batch_policy,
-                                   as_exp **exp_list_p);
+                                   as_policy_batch *config_batch_policy);
 
 as_status pyobject_to_map_policy(as_error *err, PyObject *py_policy,
                                  as_map_policy *policy, bool validate_keys);
@@ -352,27 +336,82 @@ as_status pyobject_to_hll_policy(as_error *err, PyObject *py_policy,
 as_status pyobject_to_batch_write_policy(AerospikeClient *self, as_error *err,
                                          PyObject *py_policy,
                                          as_policy_batch_write *policy,
-                                         as_policy_batch_write **policy_p,
-                                         as_exp **exp_list_p);
+                                         as_policy_batch_write **policy_p);
 
 as_status pyobject_to_batch_read_policy(AerospikeClient *self, as_error *err,
                                         PyObject *py_policy,
                                         as_policy_batch_read *policy,
-                                        as_policy_batch_read **policy_p,
-                                        as_exp **exp_list_p);
+                                        as_policy_batch_read **policy_p);
 
 as_status pyobject_to_batch_apply_policy(AerospikeClient *self, as_error *err,
                                          PyObject *py_policy,
                                          as_policy_batch_apply *policy,
-                                         as_policy_batch_apply **policy_p,
-                                         as_exp **exp_list_p);
+                                         as_policy_batch_apply **policy_p);
 
 as_status pyobject_to_batch_remove_policy(AerospikeClient *self, as_error *err,
                                           PyObject *py_policy,
                                           as_policy_batch_remove *policy,
-                                          as_policy_batch_remove **policy_p,
-                                          as_exp **exp_list_p);
-
+                                          as_policy_batch_remove **policy_p);
+
+// These methods are used for setting config-level policies and are re-used for the helper methods
+// that set transaction-level policies.
+//
+// Overrides an as_policy_* *policy* fields using entries from a Python object *py_policy*.
+//
+// *policy* must point to a valid as_policy_* instance.
+//
+// If py_policy is NULL, do nothing. Else if it is not a Python dictionary, an error will be raised.
+// (We check if it's a dictionary here because *py_policy*'s type is not validated when parsing API arguments)
+//
+// We only need the AerospikeClient *self* for parsing Python client expressions and to tell whether to validate keys.
+//
+// *is_this_txn_policy* is required because there are fields that should only be set in *policy* for txn-level
+// policies.
+//
+// Returns AEROSPIKE_OK on success or another status code on error. On error, the err argument is populated.
+
+as_status as_policy_read_set_from_pyobject(AerospikeClient *self, as_error *err,
+                                           PyObject *py_policy,
+                                           as_policy_read *policy,
+                                           bool is_policy_txn_level);
+
+as_status as_policy_write_set_from_pyobject(AerospikeClient *self,
+                                            as_error *err, PyObject *py_policy,
+                                            as_policy_write *policy,
+                                            bool is_policy_txn_level);
+
+as_status as_policy_apply_set_from_pyobject(AerospikeClient *self,
+                                            as_error *err, PyObject *py_policy,
+                                            as_policy_apply *policy,
+                                            bool is_policy_txn_level);
+
+// These methods are used for setting transaction level policies
+// 1. Copies an config-level as_policy_* *src* to a txn-level *dst* policy
+// 2. Sets an as_policy_* *dst* from a Python object *py_policy*. to override the defaults from *src*
+// *py_policy* must be a Python dictionary, and *src* and *dst* must point to valid as_policy_* instances.
+// We only need the AerospikeClient *self* for parsing Python client expressions and to tell whether to validate keys.
+//
+// Returns AEROSPIKE_OK on success or another status code on error. On error, the err argument is populated.
+
+as_status as_policy_write_copy_and_set_from_pyobject(AerospikeClient *self,
+                                                     as_error *err,
+                                                     PyObject *py_policy,
+                                                     as_policy_write *dst,
+                                                     as_policy_write *src);
+
+as_status as_policy_read_copy_and_set_from_pyobject(AerospikeClient *self,
+                                                    as_error *err,
+                                                    PyObject *py_policy,
+                                                    as_policy_read *dst,
+                                                    as_policy_read *src);
+
+as_status as_policy_apply_copy_and_set_from_pyobject(AerospikeClient *self,
+                                                     as_error *err,
+                                                     PyObject *py_policy,
+                                                     as_policy_apply *dst,
+                                                     as_policy_apply *src);
+
+// TODO: make consistent
 // metrics_policy must be declared already
 // py_metrics_policy must be non-NULL
 // Returns non-zero integer value on error.

src/include/policy_config.h

@@ -19,6 +19,7 @@
 #include <aerospike/as_config.h>
 
 #include "macros.h"
+#include "types.h"
 
 as_status set_optional_uint32_property(uint32_t *target_ptr,
                                        PyObject *policy_dict, const char *name);
@@ -39,10 +40,15 @@ as_status set_optional_gen(as_policy_gen *target_ptr, PyObject *py_policy,
 as_status set_optional_exists(as_policy_exists *target_ptr, PyObject *py_policy,
                               const char *name);
 
+// Although set_subpolicies is called by AerospikeClient's init method, and it takes in AerospikeClient as a param,
+// it should be safe to use because set_subpolicies only reads from self->validate_keys in this case
+// We know that self->validate_keys is initialized by the time we call this.
+// TODO: refactor set_subpolicies to not depend on AerospikeClient
+//
 // This only sets the err object if an invalid dictionary key is passed
 // On error, return an error code
-as_status set_subpolicies(as_error *err, as_config *config,
-                          PyObject *py_policies, int validate_keys);
+as_status set_subpolicies(AerospikeClient *self, as_error *err,
+                          as_config *config, PyObject *py_policies);
 as_status set_read_policy(as_error *err, as_policy_read *read_policy,
                           PyObject *py_policy, int validate_keys);
 as_status set_write_policy(as_error *err, as_policy_write *write_policy,

src/main/client/apply.c

@@ -63,9 +63,6 @@ PyObject *AerospikeClient_Apply_Invoke(AerospikeClient *self, PyObject *py_key,
     PyObject *py_umodule = NULL;
     PyObject *py_ufunction = NULL;
 
-    // For converting expressions.
-    as_exp *exp_list_p = NULL;
-
     as_static_pool static_pool;
     memset(&static_pool, 0, sizeof(static_pool));
     // Initialisation flags
@@ -108,11 +105,21 @@ PyObject *AerospikeClient_Apply_Invoke(AerospikeClient *self, PyObject *py_key,
     }
 
     // Convert python policy object to as_policy_apply
-    pyobject_to_policy_apply(self, &err, py_policy, &apply_policy,
-                             &apply_policy_p, &self->as->config.policies.apply,
-                             &exp_list_p);
-    if (err.code != AEROSPIKE_OK) {
-        goto CLEANUP;
+    // This is a double check if py_policy is NULL where as_policy_apply_set_from_pyobject
+    // also checks the same thing before setting the apply policy.
+    // But this is because for the client config dictionary, as_policy_apply_set_from_pyobject is the only
+    // place where we check if py_policy is NULL before setting apply policy.
+    //
+    // Also, I didn't want to set the pointer to the apply policy in this helper function as well to make
+    // the latter more simple
+    if (py_policy) {
+        as_policy_apply_copy_and_set_from_pyobject(
+            self, &err, py_policy, &apply_policy,
+            &self->as->config.policies.apply);
+        if (err.code != AEROSPIKE_OK) {
+            goto CLEANUP;
+        }
+        apply_policy_p = &apply_policy;
     }
 
     if (PyUnicode_Check(py_module)) {
@@ -147,8 +154,8 @@ PyObject *AerospikeClient_Apply_Invoke(AerospikeClient *self, PyObject *py_key,
     }
 
 CLEANUP:
-    if (exp_list_p) {
-        as_exp_destroy(exp_list_p);
+    if (apply_policy_p) {
+        as_exp_destroy(apply_policy_p->base.filter_exp);
     }
 
     if (py_umodule) {
@@ -209,6 +216,10 @@ PyObject *AerospikeClient_Apply(AerospikeClient *self, PyObject *args,
         return NULL;
     }
 
+    if (py_policy == Py_None) {
+        py_policy = NULL;
+    }
+
     // Invoke Operation
     return AerospikeClient_Apply_Invoke(self, py_key, py_module, py_function,
                                         py_arglist, py_policy);

src/main/client/batch_apply.c

@@ -124,8 +124,6 @@ static PyObject *AerospikeClient_Batch_Apply_Invoke(
     as_batch_init(&batch, 0);
 
     // For expressions conversion.
-    as_exp *batch_exp_list_p = NULL;
-    as_exp *batch_apply_exp_list_p = NULL;
 
     PyObject *br_instance = NULL;
 
@@ -177,19 +175,17 @@ static PyObject *AerospikeClient_Batch_Apply_Invoke(
            sizeof(as_key) * processed_key_count);
 
     if (py_policy_batch) {
-        if (pyobject_to_policy_batch(self, err, py_policy_batch, &policy_batch,
-                                     &policy_batch_p,
-                                     &self->as->config.policies.batch,
-                                     &batch_exp_list_p) != AEROSPIKE_OK) {
+        if (pyobject_to_policy_batch(
+                self, err, py_policy_batch, &policy_batch, &policy_batch_p,
+                &self->as->config.policies.batch) != AEROSPIKE_OK) {
             goto CLEANUP;
         }
     }
 
     if (py_policy_batch_apply) {
         if (pyobject_to_batch_apply_policy(
                 self, err, py_policy_batch_apply, &policy_batch_apply,
-                &policy_batch_apply_p,
-                &batch_apply_exp_list_p) != AEROSPIKE_OK) {
+                &policy_batch_apply_p) != AEROSPIKE_OK) {
             goto CLEANUP;
         }
     }
@@ -270,12 +266,12 @@ static PyObject *AerospikeClient_Batch_Apply_Invoke(
         as_list_destroy(arglist);
     }
 
-    if (batch_exp_list_p) {
-        as_exp_destroy(batch_exp_list_p);
+    if (policy_batch_p) {
+        as_exp_destroy(policy_batch_p->base.filter_exp);
     }
 
-    if (batch_apply_exp_list_p) {
-        as_exp_destroy(batch_apply_exp_list_p);
+    if (policy_batch_apply_p) {
+        as_exp_destroy(policy_batch_apply_p->filter_exp);
     }
 
     as_batch_destroy(&batch);

src/main/client/batch_operate.c

@@ -130,11 +130,6 @@ static PyObject *AerospikeClient_Batch_Operate_Invoke(
     as_batch batch;
     as_batch_init(&batch, 0);
 
-    // For expressions conversion.
-    as_exp *batch_exp_list_p = NULL;
-
-    as_exp *batch_write_exp_list_p = NULL;
-
     as_vector *unicodeStrVector = as_vector_create(sizeof(char *), 128);
 
     as_static_pool static_pool;
@@ -207,19 +202,17 @@ static PyObject *AerospikeClient_Batch_Operate_Invoke(
            sizeof(as_key) * processed_key_count);
 
     if (py_policy_batch) {
-        if (pyobject_to_policy_batch(self, err, py_policy_batch, &policy_batch,
-                                     &policy_batch_p,
-                                     &self->as->config.policies.batch,
-                                     &batch_exp_list_p) != AEROSPIKE_OK) {
+        if (pyobject_to_policy_batch(
+                self, err, py_policy_batch, &policy_batch, &policy_batch_p,
+                &self->as->config.policies.batch) != AEROSPIKE_OK) {
             goto CLEANUP;
         }
     }
 
     if (py_policy_batch_write) {
         if (pyobject_to_batch_write_policy(
                 self, err, py_policy_batch_write, &policy_batch_write,
-                &policy_batch_write_p,
-                &batch_write_exp_list_p) != AEROSPIKE_OK) {
+                &policy_batch_write_p) != AEROSPIKE_OK) {
             goto CLEANUP;
         }
     }
@@ -300,12 +293,12 @@ static PyObject *AerospikeClient_Batch_Operate_Invoke(
         free(as_vector_get_ptr(unicodeStrVector, i));
     }
 
-    if (batch_exp_list_p) {
-        as_exp_destroy(batch_exp_list_p);
+    if (policy_batch_p) {
+        as_exp_destroy(policy_batch_p->base.filter_exp);
     }
 
-    if (batch_write_exp_list_p) {
-        as_exp_destroy(batch_write_exp_list_p);
+    if (policy_batch_write_p) {
+        as_exp_destroy(policy_batch_write_p->filter_exp);
     }
 
     as_vector_destroy(unicodeStrVector);

src/main/client/batch_read.c

@@ -153,14 +153,10 @@ PyObject *AerospikeClient_BatchRead(AerospikeClient *self, PyObject *args,
     as_policy_batch policy_batch;
     as_policy_batch *policy_batch_p = NULL;
 
-    // For expressions conversion.
-    as_exp *batch_exp_list_p = NULL;
-
     if (py_policy_batch) {
-        if (pyobject_to_policy_batch(self, &err, py_policy_batch, &policy_batch,
-                                     &policy_batch_p,
-                                     &self->as->config.policies.batch,
-                                     &batch_exp_list_p) != AEROSPIKE_OK) {
+        if (pyobject_to_policy_batch(
+                self, &err, py_policy_batch, &policy_batch, &policy_batch_p,
+                &self->as->config.policies.batch) != AEROSPIKE_OK) {
             goto CLEANUP3;
         }
     }
@@ -287,8 +283,8 @@ PyObject *AerospikeClient_BatchRead(AerospikeClient *self, PyObject *args,
 
     as_batch_destroy(&batch);
 
-    if (batch_exp_list_p) {
-        as_exp_destroy(batch_exp_list_p);
+    if (policy_batch_p) {
+        as_exp_destroy(policy_batch_p->base.filter_exp);
     }
 
 CLEANUP2: