diff --git a/doc/aerospike.rst b/doc/aerospike.rst index a6a9c0f2e..a8e166057 100644 --- a/doc/aerospike.rst +++ b/doc/aerospike.rst @@ -344,7 +344,10 @@ Only the `hosts` key is required; the rest of the keys are optional. :columns: 1 * **validate_keys** (:class:`bool`) - (Optional) Validate keys passed into this config dictionary as well as any policy dictionaries. + (Optional) Validate keys passed into this config dictionary as well as any: + + - :ref:`aerospike_policies` + - :ref:`metadata_dict` If a key that is undefined in this documentation gets passed to a config or policy dictionary: diff --git a/src/include/conversions.h b/src/include/conversions.h index a7d11b297..4b8bb7299 100644 --- a/src/include/conversions.h +++ b/src/include/conversions.h @@ -183,8 +183,11 @@ void initialize_bin_for_strictypes(AerospikeClient *self, as_error *err, as_status bin_strict_type_checking(AerospikeClient *self, as_error *err, PyObject *py_bin, char **bin); -as_status check_and_set_meta(PyObject *py_meta, as_operations *ops, - as_error *err); +// Both as_operations and as_record have ttl and gen fields, +// so we have ttl and gen as separate parameters instead of accepting either as_operations or as_record +as_status check_and_set_meta(PyObject *py_meta, uint32_t *ttl_ref, + uint16_t *gen_ref, as_error *err, + bool validate_keys); as_status as_batch_read_results_to_pyobject(as_error *err, AerospikeClient *client, diff --git a/src/include/policy.h b/src/include/policy.h index 3fa9d4dab..cd9789553 100644 --- a/src/include/policy.h +++ b/src/include/policy.h @@ -332,3 +332,5 @@ typedef struct { } PyListenerData; void free_py_listener_data(PyListenerData *py_listener_data); + +#define POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE "policy" diff --git a/src/include/types.h b/src/include/types.h index 0a671d62f..7397e9aeb 100644 --- a/src/include/types.h +++ b/src/include/types.h @@ -147,6 +147,7 @@ extern PyObject *py_hll_policy_valid_keys; extern PyObject *py_info_and_write_policy_valid_keys; // scan.apply() takes in one policy parameter that accepts both write and info policy options extern PyObject *py_info_and_scan_policy_valid_keys; +extern PyObject *py_record_metadata_valid_keys; #define INVALID_DICTIONARY_KEY_ERROR_PART1 "is an invalid" #define INVALID_DICTIONARY_KEY_ERROR_PART2 "dictionary key" @@ -159,8 +160,7 @@ extern PyObject *py_info_and_scan_policy_valid_keys; // Return 0 and set err if dictionary has invalid keys // Return 1 if dictionary's keys are all valid // -// is_py_dict_a_policy is for error reporting only; -// If is_py_dict_a_policy is false, we are validating a client config dictionary +// adjective is for error reporting only extern int does_py_dict_contain_valid_keys(as_error *err, PyObject *py_dict, PyObject *py_set_of_valid_keys, - bool is_py_dict_a_policy); + const char *adjective); diff --git a/src/main/aerospike.c b/src/main/aerospike.c index 10ba45234..287183968 100644 --- a/src/main/aerospike.c +++ b/src/main/aerospike.c @@ -778,6 +778,8 @@ DEFINE_SET_OF_VALID_KEYS(hll_policy, "flags", NULL) DEFINE_SET_OF_VALID_KEYS(admin_policy, "timeout", NULL) +DEFINE_SET_OF_VALID_KEYS(record_metadata, "gen", "ttl", NULL) + // Use a struct to create pairs of pyobjects and list of strings defined above // When we initialize the module, we create sets for the valid keys that the client can use later @@ -817,7 +819,9 @@ static struct py_set_name_to_str_list py_set_name_to_str_lists[] = { PY_SET_NAME_TO_STR_LIST(list_policy_valid_keys), PY_SET_NAME_TO_STR_LIST(hll_policy_valid_keys), PY_SET_NAME_TO_STR_LIST(info_and_write_policy_valid_keys), - PY_SET_NAME_TO_STR_LIST(info_and_scan_policy_valid_keys)}; + PY_SET_NAME_TO_STR_LIST(info_and_scan_policy_valid_keys), + PY_SET_NAME_TO_STR_LIST(record_metadata_valid_keys), +}; // Return NULL if an exception is raised // Returns strong reference to new Python dictionary diff --git a/src/main/client/batch_write.c b/src/main/client/batch_write.c index c06cd9086..29ef4b10d 100644 --- a/src/main/client/batch_write.c +++ b/src/main/client/batch_write.c @@ -300,7 +300,8 @@ static PyObject *AerospikeClient_BatchWriteInvoke(AerospikeClient *self, ops = as_operations_new(py_ops_size); garb->ops_to_free = ops; - if (check_and_set_meta(py_meta, ops, err) != AEROSPIKE_OK) { + if (check_and_set_meta(py_meta, &ops->ttl, &ops->gen, err, + self->validate_keys) != AEROSPIKE_OK) { goto CLEANUP_ON_ERROR; } diff --git a/src/main/client/operate.c b/src/main/client/operate.c index e9c8a6a11..2910558c8 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -945,7 +945,8 @@ static PyObject *AerospikeClient_Operate_Invoke(AerospikeClient *self, memset(&static_pool, 0, sizeof(static_pool)); CHECK_CONNECTED(err); - if (check_and_set_meta(py_meta, &ops, err) != AEROSPIKE_OK) { + if (check_and_set_meta(py_meta, &ops.ttl, &ops.gen, err, + self->validate_keys) != AEROSPIKE_OK) { goto CLEANUP; } @@ -1116,7 +1117,8 @@ AerospikeClient_OperateOrdered_Invoke(AerospikeClient *self, as_error *err, } } - if (check_and_set_meta(py_meta, &ops, err) != AEROSPIKE_OK) { + if (check_and_set_meta(py_meta, &ops.ttl, &ops.gen, err, + self->validate_keys) != AEROSPIKE_OK) { goto CLEANUP; } diff --git a/src/main/client/put.c b/src/main/client/put.c index d153ec07a..4822b0d67 100644 --- a/src/main/client/put.c +++ b/src/main/client/put.c @@ -62,10 +62,6 @@ PyObject *AerospikeClient_Put_Invoke(AerospikeClient *self, PyObject *py_key, bool key_initialised = false; bool record_initialised = false; - // Initialize record - as_record_init(&rec, 0); - record_initialised = true; - as_static_pool static_pool; memset(&static_pool, 0, sizeof(static_pool)); @@ -98,6 +94,8 @@ PyObject *AerospikeClient_Put_Invoke(AerospikeClient *self, PyObject *py_key, goto CLEANUP; } + record_initialised = true; + // Convert python policy object to as_policy_write pyobject_to_policy_write(self, &err, py_policy, &write_policy, &write_policy_p, &self->as->config.policies.write, @@ -120,11 +118,9 @@ PyObject *AerospikeClient_Put_Invoke(AerospikeClient *self, PyObject *py_key, } if (key_initialised == true) { - // Destroy the key if it is initialised. as_key_destroy(&key); } if (record_initialised == true) { - // Destroy the record if it is initialised. as_record_destroy(&rec); } diff --git a/src/main/client/remove_bin.c b/src/main/client/remove_bin.c index ef3ee5010..0451d534b 100644 --- a/src/main/client/remove_bin.c +++ b/src/main/client/remove_bin.c @@ -104,43 +104,9 @@ AerospikeClient_RemoveBin_Invoke(AerospikeClient *self, PyObject *py_key, } } - if (py_meta && PyDict_Check(py_meta)) { - PyObject *py_gen = PyDict_GetItemString(py_meta, "gen"); - PyObject *py_ttl = PyDict_GetItemString(py_meta, "ttl"); - - if (py_ttl) { - if (PyLong_Check(py_ttl)) { - rec.ttl = (uint32_t)PyLong_AsLong(py_ttl); - if ((uint32_t)-1 == rec.ttl && PyErr_Occurred()) { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "integer value for ttl exceeds sys.maxsize"); - goto CLEANUP; - } - } - else { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Ttl should be an int or long"); - goto CLEANUP; - } - } - - if (py_gen) { - if (PyLong_Check(py_gen)) { - rec.gen = (uint16_t)PyLong_AsLongLong(py_gen); - if ((uint16_t)-1 == rec.gen && PyErr_Occurred()) { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "integer value for gen exceeds sys.maxsize"); - goto CLEANUP; - } - } - else { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Generation should be an int or long"); - goto CLEANUP; - } - } + check_and_set_meta(py_meta, &rec.ttl, &rec.gen, err, self->validate_keys); + if (err->code != AEROSPIKE_OK) { + goto CLEANUP; } Py_BEGIN_ALLOW_THREADS diff --git a/src/main/client/type.c b/src/main/client/type.c index 0b2864a4e..0dcb92de2 100644 --- a/src/main/client/type.c +++ b/src/main/client/type.c @@ -525,7 +525,7 @@ static PyObject *AerospikeClient_Type_New(PyTypeObject *type, PyObject *args, int does_py_dict_contain_valid_keys(as_error *err, PyObject *py_dict, PyObject *py_set_of_valid_keys, - bool is_py_dict_a_policy) + const char *adjective) { Py_ssize_t pos = 0; PyObject *py_key = NULL; @@ -540,8 +540,6 @@ int does_py_dict_contain_valid_keys(as_error *err, PyObject *py_dict, } else if (res == 0) { // Key is invalid - const char *adjective = - is_py_dict_a_policy ? "policy" : "client config"; // py_key may not be a string PyObject *py_error_msg = PyUnicode_FromFormat( INVALID_DICTIONARY_KEY_ERROR, py_key, adjective); @@ -567,6 +565,8 @@ int does_py_dict_contain_valid_keys(as_error *err, PyObject *py_dict, return -1; } +#define CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE "client config" + static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, PyObject *kwds) { @@ -624,7 +624,8 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - &constructor_err, py_config, py_client_config_valid_keys, false); + &constructor_err, py_config, py_client_config_valid_keys, + CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { goto RAISE_EXCEPTION_WITHOUT_AS_ERROR; } @@ -681,7 +682,7 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( &constructor_err, py_lua, py_client_config_lua_valid_keys, - false); + CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { goto RAISE_EXCEPTION_WITHOUT_AS_ERROR; } @@ -719,7 +720,7 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( &constructor_err, py_tls, py_client_config_tls_valid_keys, - false); + CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { goto RAISE_EXCEPTION_WITHOUT_AS_ERROR; } @@ -801,7 +802,7 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( &constructor_err, py_shm, py_client_config_shm_valid_keys, - false); + CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { goto RAISE_EXCEPTION_WITHOUT_AS_ERROR; } @@ -895,7 +896,8 @@ static int AerospikeClient_Type_Init(AerospikeClient *self, PyObject *args, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( &constructor_err, py_policies, - py_client_config_policies_valid_keys, false); + py_client_config_policies_valid_keys, + CLIENT_CONFIG_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { goto RAISE_EXCEPTION_WITHOUT_AS_ERROR; } diff --git a/src/main/conversions.c b/src/main/conversions.c index 65f006a96..0360b7099 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1390,118 +1390,69 @@ as_status as_record_init_from_pyobject(AerospikeClient *self, as_error *err, // this should never happen, but if it did... return as_error_update(err, AEROSPIKE_ERR_CLIENT, "record is null"); } - else if (PyDict_Check(py_bins_dict)) { - PyObject *py_bin_name = NULL, *py_bin_value = NULL; - Py_ssize_t pos = 0; - Py_ssize_t size = PyDict_Size(py_bins_dict); - const char *name; - - as_record_init(rec, size); - - while (PyDict_Next(py_bins_dict, &pos, &py_bin_name, &py_bin_value)) { - - if (!PyUnicode_Check(py_bin_name)) { - return as_error_update( - err, AEROSPIKE_ERR_CLIENT, - "A bin name must be a string or unicode string."); - } - - name = PyUnicode_AsUTF8(py_bin_name); - if (!name) { - return as_error_update( - err, AEROSPIKE_ERR_CLIENT, - "Unable to convert unicode object to C string"); - } + else if (!PyDict_Check(py_bins_dict)) { + return as_error_update(err, AEROSPIKE_ERR_PARAM, + "Record should be passed as bin-value pair"); + } - if (self->strict_types) { - if (strlen(name) > AS_BIN_NAME_MAX_LEN) { - return as_error_update( - err, AEROSPIKE_ERR_BIN_NAME, - "A bin name should not exceed 15 characters limit"); - } - } + PyObject *py_bin_name = NULL, *py_bin_value = NULL; + Py_ssize_t pos = 0; + Py_ssize_t size = PyDict_Size(py_bins_dict); + const char *name; - if (!py_bin_value) { - // this should never happen, but if it did... - return as_error_update(err, AEROSPIKE_ERR_CLIENT, - "record is null"); - } + as_record_init(rec, size); - as_val *val = NULL; - as_val_new_from_pyobject(self, err, py_bin_value, &val, static_pool, - serializer_type); - if (err->code != AEROSPIKE_OK) { - break; - } - bool success = as_record_set(rec, name, (as_bin_value *)val); - if (success == false) { - as_val_destroy(val); - return as_error_update(err, AEROSPIKE_ERR_BIN_NAME, - "Unable to set key-value pair"); - } + while (PyDict_Next(py_bins_dict, &pos, &py_bin_name, &py_bin_value)) { + if (!PyUnicode_Check(py_bin_name)) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "A bin name must be a string or unicode string."); + goto CLEANUP; } - if (py_meta && py_meta != Py_None) { - if (!PyDict_Check(py_meta)) { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "meta must be a dictionary"); - } - else { - PyObject *py_gen = PyDict_GetItemString(py_meta, "gen"); - PyObject *py_ttl = PyDict_GetItemString(py_meta, "ttl"); - - if (py_ttl) { - if (PyLong_Check(py_ttl)) { - rec->ttl = (uint32_t)PyLong_AsLong(py_ttl); - if (rec->ttl == (uint32_t)-1 && PyErr_Occurred()) { - if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "integer value exceeds sys.maxsize"); - } - } - } - else { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "TTL should be an int or long"); - } - } - else { - rec->ttl = AS_RECORD_CLIENT_DEFAULT_TTL; - } + name = PyUnicode_AsUTF8(py_bin_name); + if (!name) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to convert unicode object to C string"); + goto CLEANUP; + } - if (py_gen) { - if (PyLong_Check(py_gen)) { - // TODO: need to check that this value does not exceed an unsigned 16 bit integer - rec->gen = (uint16_t)PyLong_AsLong(py_gen); - if (rec->gen == (uint16_t)-1 && PyErr_Occurred()) { - if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - as_error_update( - err, AEROSPIKE_ERR_PARAM, - "integer value exceeds sys.maxsize"); - } - } - } - else { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Generation should be an int or long"); - } - } + if (self->strict_types) { + if (strlen(name) > AS_BIN_NAME_MAX_LEN) { + as_error_update( + err, AEROSPIKE_ERR_BIN_NAME, + "A bin name should not exceed 15 characters limit"); + goto CLEANUP; } } - else { - rec->ttl = AS_RECORD_CLIENT_DEFAULT_TTL; + + if (!py_bin_value) { + // this should never happen, but if it did... + as_error_update(err, AEROSPIKE_ERR_CLIENT, "record is null"); + goto CLEANUP; } + as_val *val = NULL; + as_val_new_from_pyobject(self, err, py_bin_value, &val, static_pool, + serializer_type); if (err->code != AEROSPIKE_OK) { - as_record_destroy(rec); + goto CLEANUP; + } + + bool success = as_record_set(rec, name, (as_bin_value *)val); + if (success == false) { + as_val_destroy(val); + as_error_update(err, AEROSPIKE_ERR_BIN_NAME, + "Unable to set key-value pair"); + goto CLEANUP; } - } - else { - as_error_update(err, AEROSPIKE_ERR_PARAM, - "Record should be passed as bin-value pair"); } + check_and_set_meta(py_meta, &rec->ttl, &rec->gen, err, self->validate_keys); + +CLEANUP: + if (err->code != AEROSPIKE_OK) { + as_record_destroy(rec); + } return err->code; } @@ -2527,11 +2478,26 @@ as_status bin_strict_type_checking(AerospikeClient *self, as_error *err, * Returns: error code. ******************************************************************************************************* */ -as_status check_and_set_meta(PyObject *py_meta, as_operations *ops, - as_error *err) +as_status check_and_set_meta(PyObject *py_meta, uint32_t *ttl_ref, + uint16_t *gen_ref, as_error *err, + bool validate_keys) { as_error_reset(err); if (py_meta && PyDict_Check(py_meta)) { + if (validate_keys) { + as_status retval = does_py_dict_contain_valid_keys( + err, py_meta, py_record_metadata_valid_keys, "record metadata"); + if (retval == -1) { + // This shouldn't happen, but if it did... + // TODO: wrong error message + return as_error_update(err, AEROSPIKE_ERR, + ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); + } + else if (retval == 0) { + return err->code; + } + } + PyObject *py_gen = PyDict_GetItemString(py_meta, "gen"); PyObject *py_ttl = PyDict_GetItemString(py_meta, "ttl"); uint32_t ttl = 0; @@ -2550,11 +2516,11 @@ as_status check_and_set_meta(PyObject *py_meta, as_operations *ops, err, AEROSPIKE_ERR_PARAM, "integer value for ttl exceeds sys.maxsize"); } - ops->ttl = ttl; + *ttl_ref = ttl; } else { // Metadata dict was present, but ttl field did not exist - ops->ttl = AS_RECORD_CLIENT_DEFAULT_TTL; + *ttl_ref = AS_RECORD_CLIENT_DEFAULT_TTL; } if (py_gen) { @@ -2572,7 +2538,7 @@ as_status check_and_set_meta(PyObject *py_meta, as_operations *ops, err, AEROSPIKE_ERR_PARAM, "integer value for gen exceeds sys.maxsize"); } - ops->gen = gen; + *gen_ref = gen; } } else if (py_meta && (py_meta != Py_None)) { @@ -2581,7 +2547,7 @@ as_status check_and_set_meta(PyObject *py_meta, as_operations *ops, } else { // Metadata dict was not set by user - ops->ttl = AS_RECORD_CLIENT_DEFAULT_TTL; + *ttl_ref = AS_RECORD_CLIENT_DEFAULT_TTL; } return err->code; } diff --git a/src/main/policy.c b/src/main/policy.c index fbc8617ac..e65baf423 100644 --- a/src/main/policy.c +++ b/src/main/policy.c @@ -255,7 +255,8 @@ as_status pyobject_to_policy_admin(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_admin_policy_valid_keys, true); + err, py_policy, py_admin_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -358,7 +359,8 @@ as_status pyobject_to_policy_apply(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_apply_policy_valid_keys, true); + err, py_policy, py_apply_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -426,7 +428,8 @@ pyobject_to_policy_info(as_error *err, PyObject *py_policy, } as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_policy_valid_keys, true); + err, py_policy, py_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -472,7 +475,8 @@ as_status pyobject_to_policy_query(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_query_policy_valid_keys, true); + err, py_policy, py_query_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -526,7 +530,8 @@ as_status pyobject_to_policy_read(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_read_policy_valid_keys, true); + err, py_policy, py_read_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -583,7 +588,8 @@ as_status pyobject_to_policy_remove(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_remove_policy_valid_keys, true); + err, py_policy, py_remove_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -646,7 +652,8 @@ as_status pyobject_to_policy_scan( } as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_policy_valid_keys, true); + err, py_policy, py_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -705,7 +712,8 @@ as_status pyobject_to_policy_write( } as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_policy_valid_keys, true); + err, py_policy, py_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -761,7 +769,8 @@ as_status pyobject_to_policy_operate(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_operate_policy_valid_keys, true); + err, py_policy, py_operate_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -821,7 +830,8 @@ as_status pyobject_to_policy_batch(AerospikeClient *self, as_error *err, if (py_policy && py_policy != Py_None) { if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_policy_valid_keys, true); + err, py_policy, py_batch_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -870,7 +880,8 @@ as_status pyobject_to_batch_write_policy(AerospikeClient *self, as_error *err, if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_write_policy_valid_keys, true); + err, py_policy, py_batch_write_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { // This shouldn't happen, but if it did... return as_error_update(err, AEROSPIKE_ERR, @@ -909,7 +920,8 @@ as_status pyobject_to_batch_read_policy(AerospikeClient *self, as_error *err, if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_read_policy_valid_keys, true); + err, py_policy, py_batch_read_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -944,7 +956,8 @@ as_status pyobject_to_batch_apply_policy(AerospikeClient *self, as_error *err, if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_apply_policy_valid_keys, true); + err, py_policy, py_batch_apply_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -981,7 +994,8 @@ as_status pyobject_to_batch_remove_policy(AerospikeClient *self, as_error *err, if (self->validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_remove_policy_valid_keys, true); + err, py_policy, py_batch_remove_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -1015,7 +1029,8 @@ as_status pyobject_to_bit_policy(as_error *err, PyObject *py_policy, if (validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_bit_policy_valid_keys, true); + err, py_policy, py_bit_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -1051,7 +1066,8 @@ as_status pyobject_to_map_policy(as_error *err, PyObject *py_policy, if (validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_map_policy_valid_keys, true); + err, py_policy, py_map_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -1107,7 +1123,8 @@ as_status pyobject_to_list_policy(as_error *err, PyObject *py_policy, if (validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_list_policy_valid_keys, true); + err, py_policy, py_list_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); @@ -1171,7 +1188,8 @@ as_status pyobject_to_hll_policy(as_error *err, PyObject *py_policy, if (validate_keys) { as_status retval = does_py_dict_contain_valid_keys( - err, py_policy, py_hll_policy_valid_keys, true); + err, py_policy, py_hll_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval == -1) { return as_error_update(err, AEROSPIKE_ERR, ERR_MSG_FAILED_TO_VALIDATE_POLICY_KEYS); diff --git a/src/main/policy_config.c b/src/main/policy_config.c index f6fe53081..217bc3a42 100644 --- a/src/main/policy_config.c +++ b/src/main/policy_config.c @@ -17,6 +17,7 @@ #include "policy_config.h" #include "types.h" +#include "policy.h" as_status set_optional_key(as_policy_key *target_ptr, PyObject *py_policy, const char *name); @@ -169,7 +170,8 @@ as_status set_read_policy(as_error *err, as_policy_read *read_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_read_policy_valid_keys, true); + err, py_policy, py_read_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -233,7 +235,8 @@ as_status set_write_policy(as_error *err, as_policy_write *write_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_write_policy_valid_keys, true); + err, py_policy, py_write_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -307,7 +310,8 @@ as_status set_apply_policy(as_error *err, as_policy_apply *apply_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_apply_policy_valid_keys, true); + err, py_policy, py_apply_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -364,7 +368,8 @@ as_status set_remove_policy(as_error *err, as_policy_remove *remove_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_remove_policy_valid_keys, true); + err, py_policy, py_remove_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -422,7 +427,8 @@ as_status set_query_policy(as_error *err, as_policy_query *query_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_query_policy_valid_keys, true); + err, py_policy, py_query_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -472,7 +478,8 @@ as_status set_scan_policy(as_error *err, as_policy_scan *scan_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_scan_policy_valid_keys, true); + err, py_policy, py_scan_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -518,7 +525,8 @@ as_status set_operate_policy(as_error *err, as_policy_operate *operate_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_operate_policy_valid_keys, true); + err, py_policy, py_operate_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -607,7 +615,8 @@ as_status set_batch_policy(as_error *err, as_policy_batch *batch_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_policy_valid_keys, true); + err, py_policy, py_batch_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -675,7 +684,8 @@ as_status set_info_policy(as_error *err, as_policy_info *info_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_info_policy_valid_keys, true); + err, py_policy, py_info_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -710,7 +720,8 @@ as_status set_admin_policy(as_error *err, as_policy_admin *admin_policy, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_admin_policy_valid_keys, true); + err, py_policy, py_admin_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -744,7 +755,8 @@ as_status set_batch_apply_policy(as_error *err, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_apply_policy_valid_keys, true); + err, py_policy, py_batch_apply_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -791,7 +803,8 @@ as_status set_batch_write_policy(as_error *err, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_write_policy_valid_keys, true); + err, py_policy, py_batch_write_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } @@ -849,7 +862,8 @@ as_status set_batch_remove_policy(as_error *err, if (validate_keys) { int retval = does_py_dict_contain_valid_keys( - err, py_policy, py_batch_remove_policy_valid_keys, true); + err, py_policy, py_batch_remove_policy_valid_keys, + POLICY_DICTIONARY_ADJECTIVE_FOR_ERROR_MESSAGE); if (retval != 1) { return AEROSPIKE_ERR_PARAM; } diff --git a/test/new_tests/test_compress.py b/test/new_tests/test_compress.py index 2d1df705d..bce492b12 100644 --- a/test/new_tests/test_compress.py +++ b/test/new_tests/test_compress.py @@ -57,7 +57,7 @@ def test_put_get_with_compress_policy(self): policy = {"compress": True} - self.as_connection.put(key, rec, policy) + self.as_connection.put(key, rec, policy=policy) _, _, bins = self.as_connection.get(key, policy) assert bins["val"] == expected @@ -106,7 +106,7 @@ def test_read_with_compress_policy(self, put_data): policy = {"key": aerospike.POLICY_KEY_SEND, "compress": True} - self.as_connection.put(key, rec, policy) + self.as_connection.put(key, rec, policy=policy) key, _, bins = self.as_connection.get(key, policy) diff --git a/test/new_tests/test_get_put.py b/test/new_tests/test_get_put.py index d7ab778bc..628078a5b 100644 --- a/test/new_tests/test_get_put.py +++ b/test/new_tests/test_get_put.py @@ -75,7 +75,7 @@ def test_pos_get_initkey_with_digest(self, put_data): policy = {"key": aerospike.POLICY_KEY_DIGEST} - put_data(self.as_connection, key, rec, policy) + put_data(self.as_connection, key, rec, _policy=policy) key, _, bins = self.as_connection.get(key, policy) @@ -109,7 +109,7 @@ def test_pos_get_initkey_with_policy_send(self, put_data): policy = {"key": aerospike.POLICY_KEY_SEND} - put_data(self.as_connection, key, rec, policy) + put_data(self.as_connection, key, rec, _policy=policy) key, _, bins = self.as_connection.get(key, policy) diff --git a/test/new_tests/test_hll.py b/test/new_tests/test_hll.py index 69707c613..a3fec9b86 100644 --- a/test/new_tests/test_hll.py +++ b/test/new_tests/test_hll.py @@ -444,7 +444,7 @@ def test_neg_hll_update(self, bin, policy, expected_result): ops = [hll_operations.hll_add(bin, ["key1", "key2", "key3"], policy=policy)] with pytest.raises(expected_result): - self.as_connection.operate(self.test_keys[0], ops, policy) + self.as_connection.operate(self.test_keys[0], ops) def test_pos_hll_update(self): """ diff --git a/test/new_tests/test_new_constructor.py b/test/new_tests/test_new_constructor.py index 45f1d057a..d9233c5b1 100644 --- a/test/new_tests/test_new_constructor.py +++ b/test/new_tests/test_new_constructor.py @@ -325,6 +325,8 @@ def test_config_level_misc_options(): except: pass +KEY = ("test", "demo", 0) + class TestConfigTTL: NEW_TTL = 9000 @@ -335,7 +337,7 @@ def config_ttl_setup(self, policy_name: str): "ttl": self.NEW_TTL } self.client = aerospike.client(config) - self.key = ("test", "demo", 0) + self.client.put(KEY, {"a": "a", "b": "b"}) if "apply" in policy_name: self.client.udf_put("test_record_udf.lua") @@ -352,12 +354,14 @@ def config_ttl_setup(self, policy_name: str): pass try: - self.client.remove(self.key) + self.client.remove(KEY) except e.RecordNotFound: pass + self.client.close() + def check_ttl(self): - _, meta = self.client.exists(self.key) + _, meta = self.client.exists(KEY) clock_skew_tolerance_secs = 50 assert meta["ttl"] in range(self.NEW_TTL - clock_skew_tolerance_secs, self.NEW_TTL + clock_skew_tolerance_secs) @@ -367,16 +371,17 @@ def check_ttl(self): [None, {"ttl": aerospike.TTL_CLIENT_DEFAULT}, {"gen": 10}], ids=["no metadata", "metadata with special ttl value", "metadata without ttl"] ) - def test_setting_write_ttl(self, config_ttl_setup, meta): - self.client.put(self.key, bins={"a": 1}, meta=meta) + @pytest.mark.parametrize("api_method, kwargs", [ + (aerospike.Client.put, {"key": KEY, "bins": {"a": 1}}), + (aerospike.Client.remove_bin, {"key": KEY, "list": ["a"]}), + ]) + def test_setting_write_ttl(self, config_ttl_setup, meta, api_method, kwargs): + api_method(self.client, **kwargs, meta=meta) self.check_ttl() @pytest.mark.parametrize("policy_name", ["operate"]) @pytest.mark.parametrize( "meta", - # The reason we also test a metadata dict without ttl for operate() - # is the codepath that handles the metadata dict for operate() is different - # from that for put() [None, {"ttl": aerospike.TTL_CLIENT_DEFAULT}, {"gen": 10}], ids=["no metadata", "metadata with special ttl value", "metadata without ttl"] ) @@ -384,17 +389,17 @@ def test_setting_operate_ttl(self, config_ttl_setup, meta): ops = [ operations.write("a", 1) ] - self.client.operate(self.key, ops, meta=meta) + self.client.operate(KEY, ops, meta=meta) self.check_ttl() @pytest.mark.parametrize("policy_name", ["apply"]) def test_setting_apply_ttl(self, config_ttl_setup): # Setup - self.client.put(self.key, {"bin": "a"}) + self.client.put(KEY, {"bin": "a"}) - # Call without setting the ttl in the transaction's apply policy + # Call without setting the ttl in the command's apply policy # Args: bin name, str - self.client.apply(self.key, module="test_record_udf", function="bin_udf_operation_string", args=["bin", "a"]) + self.client.apply(KEY, module="test_record_udf", function="bin_udf_operation_string", args=["bin", "a"]) self.check_ttl() @pytest.mark.parametrize("policy_name", ["batch_write"]) @@ -408,7 +413,7 @@ def test_setting_batch_write_ttl_with_batch_write(self, config_ttl_setup, meta): operations.write("bin", 1) ] batch_records = BatchRecords([ - Write(self.key, ops=ops, meta=meta) + Write(KEY, ops=ops, meta=meta) ]) brs = self.client.batch_write(batch_records) # assert brs.result == 0 @@ -426,7 +431,7 @@ def test_setting_batch_write_ttl_with_batch_operate(self, config_ttl_setup, ttl) ops = [ operations.write("bin", 1) ] - keys = [self.key] + keys = [KEY] brs = self.client.batch_operate(keys, ops, ttl=ttl) # assert brs.result == 0 for br in brs.batch_records: @@ -437,11 +442,11 @@ def test_setting_batch_write_ttl_with_batch_operate(self, config_ttl_setup, ttl) @pytest.mark.parametrize("policy_name", ["batch_apply"]) def test_setting_batch_apply_ttl(self, config_ttl_setup): # Setup - self.client.put(self.key, {"bin": "a"}) + self.client.put(KEY, {"bin": "a"}) # Call without setting the ttl in batch_apply()'s batch apply policy keys = [ - self.key + KEY ] self.client.batch_apply(keys, module="test_record_udf", function="bin_udf_operation_string", args=["bin", "a"]) self.check_ttl() @@ -449,7 +454,7 @@ def test_setting_batch_apply_ttl(self, config_ttl_setup): @pytest.mark.parametrize("policy_name", ["scan"]) def test_setting_scan_ttl(self, config_ttl_setup): # Setup - self.client.put(self.key, {"bin": "a"}) + self.client.put(KEY, {"bin": "a"}) # Tell scan to use client config's scan policy ttl scan = self.client.scan("test", "demo") @@ -467,7 +472,7 @@ def test_setting_scan_ttl(self, config_ttl_setup): @pytest.mark.parametrize("policy_name", ["write"]) def test_query_client_default_ttl(self, config_ttl_setup): # Setup - self.client.put(self.key, {"bin": "a"}, meta={"ttl": 90}) + self.client.put(KEY, {"bin": "a"}, meta={"ttl": 90}) # Tell scan to use client config's write policy ttl query = self.client.query("test", "demo") diff --git a/test/new_tests/test_remove_bin.py b/test/new_tests/test_remove_bin.py index 9cd216afc..6714f053f 100644 --- a/test/new_tests/test_remove_bin.py +++ b/test/new_tests/test_remove_bin.py @@ -268,11 +268,8 @@ def test_neg_remove_bin_with_incorrect_meta(self): "key": aerospike.POLICY_KEY_SEND, "gen": aerospike.POLICY_GEN_IGNORE, } - try: - self.as_connection.remove_bin(key, ["age"], policy) - - except (e.ClusterError, e.RecordNotFound): - pass + with pytest.raises(e.ParamError): + self.as_connection.remove_bin(key, ["age"], meta=2, policy=policy) def test_neg_remove_bin_with_incorrect_policy(self): """ @@ -427,3 +424,11 @@ def test_remove_bin_with_gen_too_large(self, put_data): meta = {"gen": 2**65, "ttl": 2} with pytest.raises(e.ClientError): self.as_connection.remove_bin(key, ["age"], meta=meta) + + # TODO: this does not fail as expected + # def test_remove_bin_with_bin_name_too_long(self, put_data): + # key = ("test", "demo", 1) + # record = {"Name": "Herry", "age": 60} + # put_data(self.as_connection, key, record) + # with pytest.raises(e.BinNameError): + # self.as_connection.remove_bin(key, ["a" * 16]) diff --git a/test/new_tests/test_transaction_level_policies.py b/test/new_tests/test_transaction_level_policies.py index 228ab5ef9..0ad2b163c 100644 --- a/test/new_tests/test_transaction_level_policies.py +++ b/test/new_tests/test_transaction_level_policies.py @@ -2,42 +2,12 @@ from aerospike import exception as e import time -from aerospike_helpers.operations import operations, bitwise_operations, map_operations, list_operations, hll_operations from aerospike_helpers.batch import records as br from .test_base_class import TestBaseClass -from contextlib import nullcontext -import aerospike -from typing import Callable +from aerospike_helpers.operations import operations SKIP_MSG = "read_touch_ttl_percent only supported on server 7.1 or higher" KEY = ("test", "demo", 1) -OPS_LIST = [ - operations.read(bin_name="a") -] -INVALID_POLICY = {"a": "key"} -BATCHRECORDS_WITH_INVALID_BATCH_READ_POLICY = br.BatchRecords( - [ - br.Read( - key=("test", "demo", 1), - ops=[ - operations.read("count"), - ], - policy=INVALID_POLICY - ) - ] -) -OPS_LIST_WITH_INVALID_BIT_POLICY = [ - bitwise_operations.bit_not("bin", bit_offset=0, bit_size=0, policy={"a": "key"}) -] -OPS_LIST_WITH_INVALID_MAP_POLICY = [ - map_operations.map_put("bin", "map_key", "map_value", map_policy={"a": "key"}) -] -OPS_LIST_WITH_INVALID_LIST_POLICY = [ - list_operations.list_append("bin_name", "list_item_value", policy={"a": "key"}) -] -OPS_LIST_WITH_INVALID_HLL_POLICY = [ - hll_operations.hll_add("bin_name", values=[1, 2, 3], policy={"a": "key"}) -] class TestReadTouchTTLPercent: @@ -133,63 +103,3 @@ def test_batch_write(self): self.as_connection.batch_write(batch_records) time.sleep(self.delay) self.as_connection.get(KEY) - - # Test all policy code paths for invalid policy keys - # This codepath is only for command (e.g transaction)-level policies. Config level policies - # have a separate codepath. - # Read and write command policies with invalid keys already covered by other test cases, so - # we don't include test cases for them here. - @pytest.mark.parametrize( - "api_method, kwargs, context_if_validate_keys_is_false", - [ - (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST, "policy": INVALID_POLICY}, nullcontext()), - # User doesn't exist - (aerospike.Client.admin_query_user_info, {"user": "asdf", "policy": INVALID_POLICY}, pytest.raises((e.InvalidUser, e.SecurityNotSupported))), - (aerospike.Client.info_all, {"command": "status", "policy": INVALID_POLICY}, nullcontext()), - # UDF doesn't exist on server - (aerospike.Client.apply, {"key": KEY, "module": "module", "function": "function", "args": [], "policy": INVALID_POLICY}, pytest.raises(e.UDFError)), - (aerospike.Scan.results, {"policy": INVALID_POLICY}, nullcontext()), - (aerospike.Query.results, {"policy": INVALID_POLICY}, nullcontext()), - (aerospike.Client.remove, {"key": KEY, "policy": INVALID_POLICY}, nullcontext()), - # Batch policy - (aerospike.Client.batch_read, {"keys": [KEY], "policy": INVALID_POLICY}, nullcontext()), - # Batch write policy - (aerospike.Client.batch_operate, {"keys": [KEY], "ops": OPS_LIST, "policy_batch": INVALID_POLICY}, nullcontext()), - # Batch read policy - (aerospike.Client.batch_write, {"batch_records": BATCHRECORDS_WITH_INVALID_BATCH_READ_POLICY}, nullcontext()), - # Batch apply policy - (aerospike.Client.batch_apply, {"keys": [KEY], "module": "module", "function": "function", "args": [], "policy_batch_apply": INVALID_POLICY}, nullcontext()), - # Batch remove policy - (aerospike.Client.batch_remove, {"keys": [KEY], "policy_batch_remove": INVALID_POLICY}, nullcontext()), - # If not validating keys, we don't really care what server error is reported - # We only care that we are not validating keys if the feature flag is disabled - # Bit policy - (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_BIT_POLICY}, pytest.raises(e.ServerError)), - # The map and list operations create a new bin with a new list/map if it doesn't exist - # Map policy - (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_MAP_POLICY}, nullcontext()), - # List policy - (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_LIST_POLICY}, nullcontext()), - # HLL policy - (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_HLL_POLICY}, pytest.raises(e.ServerError)), - ] - ) - def test_invalid_policy_keys(self, api_method: Callable, kwargs: dict, context_if_validate_keys_is_false): - if self.config["validate_keys"]: - context = pytest.raises(e.ParamError) - EXPECTED_ERROR_MESSAGE = '\"a\" is an invalid policy dictionary key' - else: - context = context_if_validate_keys_is_false - - if api_method.__objclass__ == aerospike.Scan: - invoker = self.as_connection.scan("test", "demo") - elif api_method.__objclass__ == aerospike.Query: - invoker = self.as_connection.query("test", "demo") - else: - invoker = self.as_connection - - with context as excinfo: - api_method(invoker, **kwargs) - - if self.config["validate_keys"]: - assert EXPECTED_ERROR_MESSAGE in excinfo.value.msg diff --git a/test/new_tests/test_validate_keys.py b/test/new_tests/test_validate_keys.py new file mode 100644 index 000000000..9c9959f04 --- /dev/null +++ b/test/new_tests/test_validate_keys.py @@ -0,0 +1,128 @@ +import aerospike +from aerospike_helpers.batch import records as br +from aerospike_helpers.operations import operations, bitwise_operations, map_operations, list_operations, hll_operations +from aerospike import exception as e + +import pytest +from contextlib import nullcontext +from typing import Callable + +KEY = ("test", "demo", 1) +OPS_LIST = [ + operations.read(bin_name="a") +] + +INVALID_POLICY = {"a": "key"} +BATCHRECORDS_WITH_INVALID_BATCH_READ_POLICY = br.BatchRecords( + [ + br.Read( + key=("test", "demo", 1), + ops=[ + operations.read("count"), + ], + policy=INVALID_POLICY + ) + ] +) +OPS_LIST_WITH_INVALID_BIT_POLICY = [ + bitwise_operations.bit_not("bin", bit_offset=0, bit_size=0, policy={"a": "key"}) +] +OPS_LIST_WITH_INVALID_MAP_POLICY = [ + map_operations.map_put("bin", "map_key", "map_value", map_policy={"a": "key"}) +] +OPS_LIST_WITH_INVALID_LIST_POLICY = [ + list_operations.list_append("bin_name", "list_item_value", policy={"a": "key"}) +] +OPS_LIST_WITH_INVALID_HLL_POLICY = [ + hll_operations.hll_add("bin_name", values=[1, 2, 3], policy={"a": "key"}) +] + +EXPECTED_CONTEXT_IF_VALIDATE_KEYS_ENABLED = pytest.raises(e.ParamError) + + +class TestValidateKeys: + @pytest.fixture(autouse=True, scope="class") + def setup(self, as_connection): + as_connection.put(KEY, {"a": "a"}) + + yield + + try: + as_connection.remove(KEY) + except e.RecordNotFound: + pass + + # Test all policy code paths for invalid policy keys + # This codepath is only for command (e.g transaction)-level policies. Config level policies + # have a separate codepath. + # Read and write command policies with invalid keys already covered by other test cases, so + # we don't include test cases for them here. + @pytest.mark.parametrize( + "api_method, kwargs, context_if_validate_keys_is_false", + [ + (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST, "policy": INVALID_POLICY}, nullcontext()), + # User doesn't exist + (aerospike.Client.admin_query_user_info, {"user": "asdf", "policy": INVALID_POLICY}, pytest.raises((e.InvalidUser, e.SecurityNotSupported))), + (aerospike.Client.info_all, {"command": "status", "policy": INVALID_POLICY}, nullcontext()), + # UDF doesn't exist on server + (aerospike.Client.apply, {"key": KEY, "module": "module", "function": "function", "args": [], "policy": INVALID_POLICY}, pytest.raises(e.UDFError)), + (aerospike.Scan.results, {"policy": INVALID_POLICY}, nullcontext()), + (aerospike.Query.results, {"policy": INVALID_POLICY}, nullcontext()), + (aerospike.Client.remove, {"key": KEY, "policy": INVALID_POLICY}, nullcontext()), + # Batch policy + (aerospike.Client.batch_read, {"keys": [KEY], "policy": INVALID_POLICY}, nullcontext()), + # Batch write policy + (aerospike.Client.batch_operate, {"keys": [KEY], "ops": OPS_LIST, "policy_batch_write": INVALID_POLICY}, nullcontext()), + # Batch read policy + (aerospike.Client.batch_write, {"batch_records": BATCHRECORDS_WITH_INVALID_BATCH_READ_POLICY}, nullcontext()), + # Batch apply policy + (aerospike.Client.batch_apply, {"keys": [KEY], "module": "module", "function": "function", "args": [], "policy_batch_apply": INVALID_POLICY}, nullcontext()), + # Batch remove policy + (aerospike.Client.batch_remove, {"keys": [KEY], "policy_batch_remove": INVALID_POLICY}, nullcontext()), + # If not validating keys, we don't really care what server error is reported + # We only care that we are not validating keys if the feature flag is disabled + # Bit policy + (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_BIT_POLICY}, pytest.raises(e.ServerError)), + # The map and list operations create a new bin with a new list/map if it doesn't exist + # Map policy + (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_MAP_POLICY}, nullcontext()), + # List policy + (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_LIST_POLICY}, nullcontext()), + # HLL policy + (aerospike.Client.operate, {"key": KEY, "list": OPS_LIST_WITH_INVALID_HLL_POLICY}, pytest.raises(e.ServerError)), + ] + ) + def test_invalid_policy_keys(self, api_method: Callable, kwargs: dict, context_if_validate_keys_is_false): + if self.config["validate_keys"]: + EXPECTED_ERROR_MESSAGE = '\"a\" is an invalid policy dictionary key' + context = EXPECTED_CONTEXT_IF_VALIDATE_KEYS_ENABLED + else: + context = context_if_validate_keys_is_false + + if api_method.__objclass__ == aerospike.Scan: + invoker = self.as_connection.scan("test", "demo") + elif api_method.__objclass__ == aerospike.Query: + invoker = self.as_connection.query("test", "demo") + else: + invoker = self.as_connection + + with context as excinfo: + api_method(invoker, **kwargs) + + if self.config["validate_keys"]: + assert EXPECTED_ERROR_MESSAGE in excinfo.value.msg + + def test_invalid_metadata_dictionary_key(self): + INVALID_METADATA_KEY = "generation" + if self.config["validate_keys"]: + EXPECTED_ERROR_MESSAGE = f"\"{INVALID_METADATA_KEY}\" is an invalid record metadata dictionary key" + context = EXPECTED_CONTEXT_IF_VALIDATE_KEYS_ENABLED + else: + context = nullcontext() + + meta = {"ttl": 30, INVALID_METADATA_KEY: 1} + with context as excinfo: + self.as_connection.put(key=KEY, bins={"a": 1}, meta=meta) + + if self.config["validate_keys"]: + assert EXPECTED_ERROR_MESSAGE in excinfo.value.msg