diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 557861970..af5f051f8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -362,7 +362,7 @@ jobs: container-name: aerospike - name: Run tests - run: python -m pytest ./new_tests -vv + run: python -m pytest ./new_tests -vv -W error::pytest.PytestUnraisableExceptionWarning working-directory: test test-lowest-supported-server: @@ -413,12 +413,12 @@ jobs: - uses: actions/setup-python@v2 with: - python-version: ${{ env.LOWEST_SUPPORTED_PY_VERSION }} + python-version: "3.13" architecture: 'x64' - uses: actions/download-artifact@v4 with: - name: wheel-${{ env.LOWEST_SUPPORTED_PY_VERSION }} + name: wheel-3.13 - name: Install client run: pip install *.whl @@ -433,7 +433,7 @@ jobs: docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }} - name: Run tests - run: python -m pytest ./new_tests/test_{mrt_functionality,admin_*}.py + run: python -m pytest ./new_tests/test_{mrt_functionality,admin_*,compress}.py -W error::pytest.PytestUnraisableExceptionWarning working-directory: test - name: Show logs if failed diff --git a/src/include/conversions.h b/src/include/conversions.h index d91b60761..cd7c4a728 100644 --- a/src/include/conversions.h +++ b/src/include/conversions.h @@ -232,9 +232,15 @@ PyObject *create_py_cluster_from_as_cluster(as_error *error_p, PyObject *create_py_node_from_as_node(as_error *error_p, struct as_node_s *node); +// Checks if pyobject is an instance of a class type defined in aerospike_helpers or one of its submodules +// If expected_submodule_name is NULL, the type is expected to be defined directly in the aerospike_helpers package +// If is_subclass_instance is true, we expect the instance's type to directly inherit from aerospike_helpers.. +// We need this method because classes defined natively in Python are heap-allocated types: +// https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_name bool is_pyobj_correct_as_helpers_type(PyObject *obj, const char *expected_submodule_name, - const char *expected_type_name); + const char *expected_type_name, + bool is_subclass_instance); PyObject *create_class_instance_from_module(as_error *error_p, const char *module_name, const char *class_name, diff --git a/src/include/exceptions.h b/src/include/exceptions.h index f2778307d..f57673c91 100644 --- a/src/include/exceptions.h +++ b/src/include/exceptions.h @@ -20,6 +20,9 @@ PyObject *AerospikeException_New(void); void raise_exception(as_error *err); +void raise_exception_base(as_error *err, PyObject *py_key, PyObject *py_bin, + PyObject *py_module, PyObject *py_func, + PyObject *py_name); PyObject *raise_exception_old(as_error *err); void remove_exception(as_error *err); void set_aerospike_exc_attrs_using_tuple_of_attrs(PyObject *py_exc, diff --git a/src/main/client/apply.c b/src/main/client/apply.c index e342241cf..d5b08ae0e 100644 --- a/src/main/client/apply.c +++ b/src/main/client/apply.c @@ -168,24 +168,8 @@ PyObject *AerospikeClient_Apply_Invoke(AerospikeClient *self, PyObject *py_key, as_val_destroy(result); if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", py_module); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", py_function); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, py_module, py_function, + Py_None); return NULL; } diff --git a/src/main/client/batch_write.c b/src/main/client/batch_write.c index 233a2eac2..96d1855d7 100644 --- a/src/main/client/batch_write.c +++ b/src/main/client/batch_write.c @@ -166,17 +166,21 @@ static PyObject *AerospikeClient_BatchWriteInvoke(AerospikeClient *self, } } - // TODO check that py_object is an instance of class + if (!is_pyobj_correct_as_helpers_type(py_obj, "batch.records", + "BatchRecords", false)) { + as_error_update(err, AEROSPIKE_ERR_PARAM, + "batch_records must be an " + "aerospike_helpers.batch.records.BatchRecords " + "instance"); + goto CLEANUP4; + } py_batch_records = PyObject_GetAttrString(py_obj, FIELD_NAME_BATCH_RECORDS); if (py_batch_records == NULL || !PyList_Check(py_batch_records)) { as_error_update(err, AEROSPIKE_ERR_PARAM, "%s must be a list of BatchRecord", FIELD_NAME_BATCH_RECORDS); - if (py_batch_records) { - Py_DECREF(py_batch_records); - } - goto CLEANUP4; + goto CLEANUP3; } py_batch_records_size = PyList_Size(py_batch_records); @@ -193,13 +197,20 @@ static PyObject *AerospikeClient_BatchWriteInvoke(AerospikeClient *self, for (Py_ssize_t i = 0; i < py_batch_records_size; i++) { garbage *garb = as_vector_get(&garbage_list, i); PyObject *py_batch_record = PyList_GetItem(py_batch_records, i); - // TODO check that this is an instance/subclass on BatchRecord if (py_batch_record == NULL) { as_error_update( err, AEROSPIKE_ERR_PARAM, "py_batch_record is NULL, %s must be a list of BatchRecord", FIELD_NAME_BATCH_RECORDS); - goto CLEANUP4; + goto CLEANUP3; + } + + if (is_pyobj_correct_as_helpers_type(py_batch_record, "batch.records", + "BatchRecord", true) == false) { + as_error_update( + err, AEROSPIKE_ERR_PARAM, + "batch_record must be a BatchRecord class instance"); + goto CLEANUP3; } // extract as_batch_base_record fields diff --git a/src/main/client/exists.c b/src/main/client/exists.c index d835b324a..610353060 100644 --- a/src/main/client/exists.c +++ b/src/main/client/exists.c @@ -136,18 +136,7 @@ extern PyObject *AerospikeClient_Exists_Invoke(AerospikeClient *self, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); } return py_result; diff --git a/src/main/client/get.c b/src/main/client/get.c index 41a43f525..4faf63b6b 100644 --- a/src/main/client/get.c +++ b/src/main/client/get.c @@ -133,18 +133,7 @@ PyObject *AerospikeClient_Get_Invoke(AerospikeClient *self, PyObject *py_key, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } diff --git a/src/main/client/info.c b/src/main/client/info.c index be6095b60..1f86fe281 100644 --- a/src/main/client/info.c +++ b/src/main/client/info.c @@ -96,22 +96,12 @@ static bool AerospikeClient_InfoAll_each(as_error *err, const as_node *node, CLEANUP: if (udata_ptr->error.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&udata_ptr->error, &py_err); - PyObject *exception_type = raise_exception_old(&udata_ptr->error); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception(&udata_ptr->error); PyGILState_Release(gil_state); return false; } if (err->code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(err, &py_err); - PyObject *exception_type = raise_exception_old(err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception(err); PyGILState_Release(gil_state); return false; } @@ -211,13 +201,7 @@ static PyObject *AerospikeClient_InfoAll_Invoke(AerospikeClient *self, Py_DECREF(py_ustr); } if (info_callback_udata.error.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&info_callback_udata.error, &py_err); - PyObject *exception_type = - raise_exception_old(&info_callback_udata.error); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception(&info_callback_udata.error); if (py_nodes) { Py_DECREF(py_nodes); } diff --git a/src/main/client/operate.c b/src/main/client/operate.c index dd40bdce5..2083a0f83 100644 --- a/src/main/client/operate.c +++ b/src/main/client/operate.c @@ -76,18 +76,7 @@ static inline bool isExprOp(int op); #define EXCEPTION_ON_ERROR() \ if (err.code != AEROSPIKE_OK) { \ - PyObject *py_err = NULL; \ - error_to_pyobject(&err, &py_err); \ - PyObject *exception_type = raise_exception_old(&err); \ - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); \ - if (PyObject_HasAttrString(exception_type, "key")) { \ - PyObject_SetAttrString(exception_type, "key", py_key); \ - } \ - if (PyObject_HasAttrString(exception_type, "bin")) { \ - PyObject_SetAttrString(exception_type, "bin", py_bin); \ - } \ - PyErr_SetObject(exception_type, py_err); \ - Py_DECREF(py_err); \ + raise_exception_base(&err, py_key, py_bin, Py_None, Py_None, Py_None); \ return NULL; \ } @@ -1211,15 +1200,7 @@ PyObject *AerospikeClient_OperateOrdered(AerospikeClient *self, PyObject *args, CLEANUP: if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } return py_result; diff --git a/src/main/client/put.c b/src/main/client/put.c index 116e1a1b8..fa7910bb7 100644 --- a/src/main/client/put.c +++ b/src/main/client/put.c @@ -130,18 +130,7 @@ PyObject *AerospikeClient_Put_Invoke(AerospikeClient *self, PyObject *py_key, // If an error occurred, tell Python. if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", py_bins); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, py_bins, Py_None, Py_None, Py_None); return NULL; } diff --git a/src/main/client/query.c b/src/main/client/query.c index 7929216cf..fcf69ea8f 100644 --- a/src/main/client/query.c +++ b/src/main/client/query.c @@ -376,23 +376,40 @@ static PyObject *AerospikeClient_QueryApply_Invoke( } PyObject *py_op = PyTuple_GetItem(py_predicate, 0); - PyObject *py_op_data = PyTuple_GetItem(py_predicate, 1); + if (!py_op) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Failed to get predicate elements"); + goto CLEANUP; + } - if (!py_op || !py_op_data) { + PyObject *py_op_data = PyTuple_GetItem(py_predicate, 1); + if (!py_op_data) { as_error_update(&err, AEROSPIKE_ERR_CLIENT, "Failed to get predicate elements"); goto CLEANUP; } - if (!PyLong_Check(py_op_data)) { + else if (!PyLong_Check(py_op_data)) { as_error_update(&err, AEROSPIKE_ERR_PARAM, "Invalid Predicate"); goto CLEANUP; } - as_predicate_type op = (as_predicate_type)PyLong_AsLong(py_op); + long op = PyLong_AsLong(py_op); + if (op == -1 && PyErr_Occurred()) { + as_error_update(&err, AEROSPIKE_ERR_PARAM, + "unknown predicate type"); + goto CLEANUP; + } + as_index_datatype op_data = (as_index_datatype)PyLong_AsLong(py_op_data); + if (op == -1 && PyErr_Occurred()) { + as_error_update(&err, AEROSPIKE_ERR_PARAM, + "unknown index data type"); + goto CLEANUP; + } + rc = query_where_add( - &query_ptr, op, op_data, + &query_ptr, (as_predicate_type)op, op_data, size > 2 ? PyTuple_GetItem(py_predicate, 2) : Py_None, size > 3 ? PyTuple_GetItem(py_predicate, 3) : Py_None, size > 4 ? PyTuple_GetItem(py_predicate, 4) : Py_None, diff --git a/src/main/client/remove.c b/src/main/client/remove.c index b7266742b..0b76b6b35 100644 --- a/src/main/client/remove.c +++ b/src/main/client/remove.c @@ -133,18 +133,7 @@ PyObject *AerospikeClient_Remove_Invoke(AerospikeClient *self, PyObject *py_key, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } diff --git a/src/main/client/remove_bin.c b/src/main/client/remove_bin.c index ca627841f..1444f5747 100644 --- a/src/main/client/remove_bin.c +++ b/src/main/client/remove_bin.c @@ -160,18 +160,7 @@ AerospikeClient_RemoveBin_Invoke(AerospikeClient *self, PyObject *py_key, } if (err->code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(err, &py_err); - PyObject *exception_type = raise_exception_old(err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } return PyLong_FromLong(0); @@ -197,7 +186,6 @@ PyObject *AerospikeClient_RemoveBin(AerospikeClient *self, PyObject *args, PyObject *py_key = NULL; PyObject *py_policy = NULL; PyObject *py_binList = NULL; - PyObject *py_result = NULL; PyObject *py_meta = NULL; as_error err; @@ -236,20 +224,6 @@ PyObject *AerospikeClient_RemoveBin(AerospikeClient *self, PyObject *args, CLEANUP: - if (err.code != AEROSPIKE_OK || !py_result) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); - return NULL; - } + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } diff --git a/src/main/client/sec_index.c b/src/main/client/sec_index.c index d9129c23e..b3103d935 100644 --- a/src/main/client/sec_index.c +++ b/src/main/client/sec_index.c @@ -229,15 +229,7 @@ PyObject *AerospikeClient_Index_Cdt_Create(AerospikeClient *self, CLEANUP: if (py_obj == NULL) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "name")) { - PyObject_SetAttrString(exception_type, "name", py_name); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, py_name); return NULL; } @@ -331,15 +323,7 @@ PyObject *AerospikeClient_Index_Remove(AerospikeClient *self, PyObject *args, Py_DECREF(py_ustr_name); } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "name")) { - PyObject_SetAttrString(exception_type, "name", py_name); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, py_name); return NULL; } diff --git a/src/main/client/select.c b/src/main/client/select.c index 5e6f55995..92d1978fe 100644 --- a/src/main/client/select.c +++ b/src/main/client/select.c @@ -182,18 +182,7 @@ PyObject *AerospikeClient_Select_Invoke(AerospikeClient *self, PyObject *py_key, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "key")) { - PyObject_SetAttrString(exception_type, "key", py_key); - } - if (PyObject_HasAttrString(exception_type, "bin")) { - PyObject_SetAttrString(exception_type, "bin", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, py_key, Py_None, Py_None, Py_None, Py_None); return NULL; } diff --git a/src/main/client/udf.c b/src/main/client/udf.c index d161b2726..a24ffdd14 100644 --- a/src/main/client/udf.c +++ b/src/main/client/udf.c @@ -273,18 +273,7 @@ PyObject *AerospikeClient_UDF_Put(AerospikeClient *self, PyObject *args, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", Py_None); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, Py_None); return NULL; } @@ -370,18 +359,8 @@ PyObject *AerospikeClient_UDF_Remove(AerospikeClient *self, PyObject *args, Py_DECREF(py_ustr); } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", py_filename); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, py_filename, Py_None, + Py_None); return NULL; } @@ -463,18 +442,7 @@ PyObject *AerospikeClient_UDF_List(AerospikeClient *self, PyObject *args, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", Py_None); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, Py_None); return NULL; } @@ -579,18 +547,8 @@ PyObject *AerospikeClient_UDF_Get_UDF(AerospikeClient *self, PyObject *args, as_udf_file_destroy(&file); } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", py_module); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, py_module, Py_None, + Py_None); return NULL; } diff --git a/src/main/conversions.c b/src/main/conversions.c index fbfbadc8d..f2e282e39 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1018,14 +1018,16 @@ PyObject *create_class_instance_from_module(as_error *error_p, return py_instance; } -// Checks if pyobject is a certain type defined in aerospike_helpers or one of its submodules -// If expected_submodule_name is NULL, the type is expected to be defined directly in the aerospike_helpers package bool is_pyobj_correct_as_helpers_type(PyObject *obj, const char *expected_submodule_name, - const char *expected_type_name) + const char *expected_type_name, + bool is_subclass_instance) { - if (strcmp(obj->ob_type->tp_name, expected_type_name)) { - // Expected class name does not match object's class name + if (obj->ob_type->tp_dict == NULL) { + // Unable to get type's __module__ attribute. + // In Python 3.12+, this would happen if obj was a native Python type + // https://docs.python.org/3.12/c-api/typeobj.html#c.PyTypeObject.tp_dict + // so the object would not be the correct type, anyways return false; } @@ -1054,7 +1056,8 @@ bool is_pyobj_correct_as_helpers_type(PyObject *obj, retval = false; goto CLEANUP2; } - char *pyobj_submodule = strtok(NULL, delimiters); + // Get rest of submodule after parent aerospike_helpers package + char *pyobj_submodule = strchr(module_name, '.'); if (pyobj_submodule) { // Python object belongs in a aerospike_helpers submodule if (!expected_submodule_name) { @@ -1062,14 +1065,15 @@ bool is_pyobj_correct_as_helpers_type(PyObject *obj, retval = false; goto CLEANUP2; } - else if (strcmp(pyobj_submodule, expected_submodule_name)) { + // We want the string after the . + else if (strcmp(pyobj_submodule + 1, expected_submodule_name)) { // But it doesn't match the expected submodule retval = false; goto CLEANUP2; } } else { - // Python object belongs in the aerospike_helpers parent submodule + // Python object belongs in the aerospike_helpers parent module if (expected_submodule_name) { // But it is expected to belong to an aerospike_helpers submodule retval = false; @@ -1077,6 +1081,19 @@ bool is_pyobj_correct_as_helpers_type(PyObject *obj, } } + if (!is_subclass_instance) { + if (strcmp(obj->ob_type->tp_name, expected_type_name)) { + // object's class does not match expected class + return false; + } + } + else { + if (strcmp(obj->ob_type->tp_base->tp_name, expected_type_name)) { + // object's parent class does not match expected class + return false; + } + } + CLEANUP2: free(module_name_cpy); CLEANUP1: @@ -1155,7 +1172,8 @@ as_status as_val_new_from_pyobject(AerospikeClient *self, as_error *err, } *val = (as_val *)bytes; - if (is_pyobj_correct_as_helpers_type(py_obj, NULL, "HyperLogLog")) { + if (is_pyobj_correct_as_helpers_type(py_obj, NULL, "HyperLogLog", + false)) { bytes->type = AS_BYTES_HLL; } } @@ -1431,6 +1449,7 @@ as_status pyobject_to_key(as_error *err, PyObject *py_keytuple, as_key *key) as_key *returnResult = key; if (py_key && py_key != Py_None) { + // TODO: refactor using as_val_new_from_pyobject if (PyUnicode_Check(py_key)) { PyObject *py_ustr = PyUnicode_AsUTF8String(py_key); char *k = PyBytes_AsString(py_ustr); @@ -2335,6 +2354,7 @@ void initialize_bin_for_strictypes(AerospikeClient *self, as_error *err, strcpy(binop_bin->name, bin); } +// TODO: dead code as_status bin_strict_type_checking(AerospikeClient *self, as_error *err, PyObject *py_bin, char **bin) { diff --git a/src/main/exception.c b/src/main/exception.c index 8b80b8d39..d39edf041 100644 --- a/src/main/exception.c +++ b/src/main/exception.c @@ -26,7 +26,7 @@ #include "exception_types.h" #include "macros.h" -static PyObject *py_module; +static PyObject *py_exc_module; #define SUBMODULE_NAME "exception" @@ -275,8 +275,8 @@ PyObject *AerospikeException_New(void) NULL, NULL, NULL}; - py_module = PyModule_Create(&moduledef); - if (py_module == NULL) { + py_exc_module = PyModule_Create(&moduledef); + if (py_exc_module == NULL) { return NULL; } @@ -291,7 +291,7 @@ PyObject *AerospikeException_New(void) PyObject *py_base_class = NULL; if (exception_def.base_class_name != NULL) { py_base_class = PyObject_GetAttrString( - py_module, exception_def.base_class_name); + py_exc_module, exception_def.base_class_name); if (py_base_class == NULL) { goto MODULE_CLEANUP_ON_ERROR; } @@ -346,7 +346,7 @@ PyObject *AerospikeException_New(void) goto EXC_CLASS_CLEANUP_ON_ERROR; } - retval = PyModule_AddObject(py_module, exception_def.class_name, + retval = PyModule_AddObject(py_exc_module, exception_def.class_name, py_exception_class); if (retval == -1) { goto EXC_CLASS_CLEANUP_ON_ERROR; @@ -358,10 +358,10 @@ PyObject *AerospikeException_New(void) goto MODULE_CLEANUP_ON_ERROR; } - return py_module; + return py_exc_module; MODULE_CLEANUP_ON_ERROR: - Py_DECREF(py_module); + Py_DECREF(py_exc_module); return NULL; } @@ -369,7 +369,7 @@ void remove_exception(as_error *err) { PyObject *py_key = NULL, *py_value = NULL; Py_ssize_t pos = 0; - PyObject *py_module_dict = PyModule_GetDict(py_module); + PyObject *py_module_dict = PyModule_GetDict(py_exc_module); while (PyDict_Next(py_module_dict, &pos, &py_key, &py_value)) { Py_DECREF(py_value); @@ -398,14 +398,29 @@ void set_aerospike_exc_attrs_using_tuple_of_attrs(PyObject *py_exc, // TODO: idea. Use python dict to map error code to exception void raise_exception(as_error *err) { - PyObject *py_key = NULL, *py_value = NULL; + raise_exception_base(err, NULL, NULL, NULL, NULL, NULL); +} + +void raise_exception_base(as_error *err, PyObject *py_as_key, PyObject *py_bin, + PyObject *py_module, PyObject *py_func, + PyObject *py_name) +{ +// If there was an exception already raised, we need to chain it to the one we're raising now +#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 12 + PyObject *py_prev_exc = PyErr_GetRaisedException(); +#else + PyObject *py_prev_type, *py_prev_value, *py_prev_traceback; + PyErr_Fetch(&py_prev_type, &py_prev_value, &py_prev_traceback); +#endif + + PyObject *py_unused = NULL, *py_exc_class = NULL; Py_ssize_t pos = 0; - PyObject *py_module_dict = PyModule_GetDict(py_module); + PyObject *py_module_dict = PyModule_GetDict(py_exc_module); bool found = false; - while (PyDict_Next(py_module_dict, &pos, &py_key, &py_value)) { - if (PyObject_HasAttrString(py_value, "code")) { - PyObject *py_code = PyObject_GetAttrString(py_value, "code"); + while (PyDict_Next(py_module_dict, &pos, &py_unused, &py_exc_class)) { + if (PyObject_HasAttrString(py_exc_class, "code")) { + PyObject *py_code = PyObject_GetAttrString(py_exc_class, "code"); if (py_code == Py_None) { continue; } @@ -420,79 +435,55 @@ void raise_exception(as_error *err) PyObject *base_exception = PyDict_GetItemString(py_module_dict, "AerospikeError"); if (base_exception) { - py_value = base_exception; + py_exc_class = base_exception; + } + } + + const char *extra_attrs[] = {"key", "bin", "module", "func", "name"}; + PyObject *py_extra_attrs[] = {py_as_key, py_bin, py_module, py_func, + py_name}; + for (unsigned long i = 0; + i < sizeof(py_extra_attrs) / sizeof(py_extra_attrs[0]); i++) { + PyObject *py_exc_extra_attr = + PyObject_GetAttrString(py_exc_class, extra_attrs[i]); + if (py_exc_extra_attr) { + PyObject_SetAttrString(py_exc_class, extra_attrs[i], + py_extra_attrs[i]); + } + else if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + // We are sure that we want to ignore this + PyErr_Clear(); + } + else { + // This happens if the code that converts a C client error to a Python exception fails. + // The caller of this function should be returning because of an exception anyways + goto CHAIN_PREV_EXC_AND_RETURN; } } // Convert borrowed reference of exception class to strong reference - Py_INCREF(py_value); + Py_INCREF(py_exc_class); // Convert C error to Python exception PyObject *py_err = NULL; error_to_pyobject(err, &py_err); - set_aerospike_exc_attrs_using_tuple_of_attrs(py_value, py_err); + set_aerospike_exc_attrs_using_tuple_of_attrs(py_exc_class, py_err); // Raise exception - PyErr_SetObject(py_value, py_err); - - Py_DECREF(py_value); + PyErr_SetObject(py_exc_class, py_err); + Py_DECREF(py_exc_class); Py_DECREF(py_err); -} - -PyObject *raise_exception_old(as_error *err) -{ - PyObject *py_key = NULL, *py_value = NULL; - Py_ssize_t pos = 0; - PyObject *py_module_dict = PyModule_GetDict(py_module); - bool found = false; - while (PyDict_Next(py_module_dict, &pos, &py_key, &py_value)) { - if (PyObject_HasAttrString(py_value, "code")) { - PyObject *py_code = PyObject_GetAttrString(py_value, "code"); - if (py_code == Py_None) { - continue; - } - if (err->code == PyLong_AsLong(py_code)) { - found = true; - PyObject *py_attr = NULL; - py_attr = PyUnicode_FromString(err->message); - PyObject_SetAttrString(py_value, "msg", py_attr); - Py_DECREF(py_attr); - - // as_error.file is a char* so this may be null - if (err->file) { - py_attr = PyUnicode_FromString(err->file); - PyObject_SetAttrString(py_value, "file", py_attr); - Py_DECREF(py_attr); - } - else { - PyObject_SetAttrString(py_value, "file", Py_None); - } - // If the line is 0, set it as None - if (err->line > 0) { - py_attr = PyLong_FromUnsignedLong(err->line); - PyObject_SetAttrString(py_value, "line", py_attr); - Py_DECREF(py_attr); - } - else { - PyObject_SetAttrString(py_value, "line", Py_None); - } - - PyObject_SetAttrString(py_value, "in_doubt", - PyBool_FromLong(err->in_doubt)); - - break; - } - Py_DECREF(py_code); - } +CHAIN_PREV_EXC_AND_RETURN: +#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 12 + if (py_prev_exc) { + // Like PyErr_SetRaisedException, which steals a reference to the parameter + _PyErr_ChainExceptions1(py_prev_exc); } - // We haven't found the right exception, just use AerospikeError - if (!found) { - PyObject *base_exception = - PyDict_GetItemString(py_module_dict, "AerospikeError"); - if (base_exception) { - py_value = base_exception; - } +#else + if (py_prev_type) { + // Like PyErr_Restore, which steals a reference to the parameter + _PyErr_ChainExceptions(py_prev_type, py_prev_value, py_prev_traceback); } - return py_value; +#endif } diff --git a/src/main/policy.c b/src/main/policy.c index 8775166e6..577568e6d 100644 --- a/src/main/policy.c +++ b/src/main/policy.c @@ -1138,7 +1138,7 @@ set_as_metrics_listeners_using_pyobject(as_error *err, } if (!is_pyobj_correct_as_helpers_type(py_metricslisteners, "metrics", - "MetricsListeners")) { + "MetricsListeners", false)) { as_error_update(err, AEROSPIKE_ERR_PARAM, INVALID_ATTR_TYPE_ERROR_MSG, "metrics_listeners", "aerospike_helpers.metrics.MetricsListeners"); @@ -1218,7 +1218,7 @@ init_and_set_as_metrics_policy_using_pyobject(as_error *err, } if (!is_pyobj_correct_as_helpers_type(py_metrics_policy, "metrics", - "MetricsPolicy")) { + "MetricsPolicy", false)) { return as_error_update( err, AEROSPIKE_ERR_PARAM, "policy parameter must be an aerospike_helpers.MetricsPolicy type"); diff --git a/src/main/query/apply.c b/src/main/query/apply.c index ca7d84273..7c5176a4c 100644 --- a/src/main/query/apply.c +++ b/src/main/query/apply.c @@ -147,18 +147,8 @@ AerospikeQuery *AerospikeQuery_Apply(AerospikeQuery *self, PyObject *args, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", py_module); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", py_function); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, py_module, py_function, + Py_None); return NULL; } diff --git a/src/main/query/foreach.c b/src/main/query/foreach.c index 7aac11d21..19d996f8f 100644 --- a/src/main/query/foreach.c +++ b/src/main/query/foreach.c @@ -244,22 +244,14 @@ PyObject *AerospikeQuery_Foreach(AerospikeQuery *self, PyObject *args, self->query.apply.arglist = NULL; if (err.code != AEROSPIKE_OK || data.error.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - PyObject *exception_type = NULL; if (err.code != AEROSPIKE_OK) { - error_to_pyobject(&err, &py_err); - exception_type = raise_exception_old(&err); + raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, + Py_None); } if (data.error.code != AEROSPIKE_OK) { - error_to_pyobject(&data.error, &py_err); - exception_type = raise_exception_old(&data.error); + raise_exception_base(&data.error, Py_None, Py_None, Py_None, + Py_None, Py_None); } - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "name")) { - PyObject_SetAttrString(exception_type, "name", Py_None); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); return NULL; } diff --git a/src/main/scan/apply.c b/src/main/scan/apply.c index 9a88cb02b..0a8cb4fa7 100644 --- a/src/main/scan/apply.c +++ b/src/main/scan/apply.c @@ -145,18 +145,8 @@ AerospikeScan *AerospikeScan_Apply(AerospikeScan *self, PyObject *args, } if (err.code != AEROSPIKE_OK) { - PyObject *py_err = NULL; - error_to_pyobject(&err, &py_err); - PyObject *exception_type = raise_exception_old(&err); - set_aerospike_exc_attrs_using_tuple_of_attrs(exception_type, py_err); - if (PyObject_HasAttrString(exception_type, "module")) { - PyObject_SetAttrString(exception_type, "module", py_module); - } - if (PyObject_HasAttrString(exception_type, "func")) { - PyObject_SetAttrString(exception_type, "func", py_function); - } - PyErr_SetObject(exception_type, py_err); - Py_DECREF(py_err); + raise_exception_base(&err, Py_None, Py_None, py_module, py_function, + Py_None); return NULL; } diff --git a/test/new_tests/test_invalid_conf.py b/test/new_tests/test_invalid_conf.py index 1b4cbf321..62fce1132 100644 --- a/test/new_tests/test_invalid_conf.py +++ b/test/new_tests/test_invalid_conf.py @@ -9,6 +9,8 @@ def test_no_config(self): with pytest.raises(e.ParamError) as err: aerospike.client() assert "No config argument" in err.value.msg + # Test exception chaining + assert type(err.value.__context__) == TypeError def test_config_not_dict(self): with pytest.raises(e.ParamError) as err: diff --git a/test/new_tests/test_query_apply.py b/test/new_tests/test_query_apply.py index 1ecc356c5..dc3ded8cb 100644 --- a/test/new_tests/test_query_apply.py +++ b/test/new_tests/test_query_apply.py @@ -319,6 +319,8 @@ def test_query_apply_with_correct_parameters_without_connection(self): ("age"), ("age", 1), ("age", 1, 5), + (None, 1, "bin"), # Invalid predicate type + (1, None, "bin"), # Invalid index data type (1, 1, "bin"), # start of a valid predicate ), )