diff --git a/src/include/client.h b/src/include/client.h index 4ba944791..800c4c64d 100644 --- a/src/include/client.h +++ b/src/include/client.h @@ -368,7 +368,7 @@ PyObject *AerospikeClient_Index_Expr_Create(AerospikeClient *self, /** * Create secondary cdt index * - * client.index_cdt_create(namespace, set, bin, index_name, ctx, policy) + * client.index_cdt_create(namespace, set, bin, index_type, index_datatype, index_name, ctx, policy) * */ PyObject *AerospikeClient_Index_Cdt_Create(AerospikeClient *self, diff --git a/src/include/exceptions.h b/src/include/exceptions.h index f57673c91..ffa34d1f6 100644 --- a/src/include/exceptions.h +++ b/src/include/exceptions.h @@ -23,7 +23,6 @@ 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, PyObject *py_tuple); diff --git a/src/main/client/sec_index.c b/src/main/client/sec_index.c index d07175871..1b22db225 100644 --- a/src/main/client/sec_index.c +++ b/src/main/client/sec_index.c @@ -191,6 +191,8 @@ PyObject *AerospikeClient_Index_Expr_Create(AerospikeClient *self, data_type, NULL, expr); } +#define CTX_PARSE_ERROR_MESSAGE "Unable to parse ctx" + /** ******************************************************************************************************* * Creates a cdt index for a bin in the Aerospike DB. @@ -219,9 +221,12 @@ PyObject *AerospikeClient_Index_Cdt_Create(AerospikeClient *self, PyObject *py_indextype = NULL; PyObject *py_datatype = NULL; PyObject *py_name = NULL; + PyObject *py_ctx = NULL; as_cdt_ctx ctx; bool ctx_in_use = false; + PyObject *py_ctx_dict = NULL; + PyObject *py_obj = NULL; as_index_datatype data_type; as_index_type index_type; @@ -247,31 +252,43 @@ PyObject *AerospikeClient_Index_Cdt_Create(AerospikeClient *self, goto CLEANUP; } + // TODO: this should be refactored by using a new helper function to parse a ctx list instead of get_cdt_ctx() + // which only parses a dictionary containing a ctx list + py_ctx_dict = PyDict_New(); + if (!py_ctx_dict) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, CTX_PARSE_ERROR_MESSAGE); + goto CLEANUP; + } + int retval = PyDict_SetItemString(py_ctx_dict, "ctx", py_ctx); + if (retval == -1) { + Py_DECREF(py_ctx_dict); + as_error_update(&err, AEROSPIKE_ERR_CLIENT, CTX_PARSE_ERROR_MESSAGE); + goto CLEANUP; + } + as_static_pool static_pool; memset(&static_pool, 0, sizeof(static_pool)); - if (get_cdt_ctx(self, &err, &ctx, py_ctx, &ctx_in_use, &static_pool, + if (get_cdt_ctx(self, &err, &ctx, py_ctx_dict, &ctx_in_use, &static_pool, SERIALIZER_PYTHON) != AEROSPIKE_OK) { goto CLEANUP; } - if (!ctx_in_use) { - goto CLEANUP; - } + // Even if this call fails, it will raise its own exception + // and the err object here will not be set. We don't raise an exception twice py_obj = createIndexWithDataAndCollectionType( self, py_policy, py_ns, py_set, py_bin, py_name, index_type, data_type, &ctx, NULL); as_cdt_ctx_destroy(&ctx); - return py_obj; - CLEANUP: - if (py_obj == NULL) { + Py_XDECREF(py_ctx_dict); + + if (err.code != AEROSPIKE_OK) { raise_exception_base(&err, Py_None, Py_None, Py_None, Py_None, py_name); return NULL; } - return py_obj; } diff --git a/src/main/query/where.c b/src/main/query/where.c index 23d66bc8d..f9eb0e37d 100644 --- a/src/main/query/where.c +++ b/src/main/query/where.c @@ -42,6 +42,8 @@ int64_t pyobject_to_int64(PyObject *py_obj) } } +#define CTX_PARSE_ERROR_MESSAGE "Unable to parse ctx" + // py_bin, py_val1, pyval2 are guaranteed to be non-NULL // The rest of the PyObject parameters can be NULL and are optional. // 3 cases for these optional parameters: @@ -58,18 +60,41 @@ static int AerospikeQuery_Where_Add(AerospikeQuery *self, PyObject *py_ctx, { as_error err; as_error_init(&err); + + // TODO: does static pool go out of scope? + as_static_pool static_pool; + memset(&static_pool, 0, sizeof(static_pool)); + as_cdt_ctx *pctx = NULL; bool ctx_in_use = false; + // Used to pass ctx into get_cdt_ctx() helper + // Declared here to make cleanup logic simpler + PyObject *py_ctx_dict = NULL; + + // Ctx is an optional parameter + if (py_ctx && !Py_IsNone(py_ctx)) { + // If user wanted to pass in an actual ctx + + // Glue code to pass into get_cdt_ctx() + py_ctx_dict = PyDict_New(); + if (!py_ctx_dict) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + CTX_PARSE_ERROR_MESSAGE); + goto error; + } + int retval = PyDict_SetItemString(py_ctx_dict, "ctx", py_ctx); + if (retval == -1) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + CTX_PARSE_ERROR_MESSAGE); + goto CLEANUP_PY_CTX_DICT_ON_ERROR; + } - if (py_ctx) { - // TODO: does static pool go out of scope? - as_static_pool static_pool; - memset(&static_pool, 0, sizeof(static_pool)); pctx = cf_malloc(sizeof(as_cdt_ctx)); memset(pctx, 0, sizeof(as_cdt_ctx)); - if (get_cdt_ctx(self->client, &err, pctx, py_ctx, &ctx_in_use, + + if (get_cdt_ctx(self->client, &err, pctx, py_ctx_dict, &ctx_in_use, &static_pool, SERIALIZER_PYTHON) != AEROSPIKE_OK) { - return err.code; + goto CLEANUP_AS_CTX_ON_ERROR; } } @@ -78,7 +103,7 @@ static int AerospikeQuery_Where_Add(AerospikeQuery *self, PyObject *py_ctx, as_status status = as_exp_new_from_pyobject(self->client, py_expr, &exp_list, &err, true); if (status != AEROSPIKE_OK) { - goto CLEANUP_CTX_ON_ERROR; + goto CLEANUP_AS_CTX_ON_ERROR; } } @@ -286,7 +311,7 @@ static int AerospikeQuery_Where_Add(AerospikeQuery *self, PyObject *py_ctx, as_exp_destroy(exp_list); } -CLEANUP_CTX_ON_ERROR: +CLEANUP_AS_CTX_ON_ERROR: // The ctx ends up not being used by as_query if (ctx_in_use) { as_cdt_ctx_destroy(pctx); @@ -295,6 +320,10 @@ static int AerospikeQuery_Where_Add(AerospikeQuery *self, PyObject *py_ctx, cf_free(pctx); } +CLEANUP_PY_CTX_DICT_ON_ERROR: + Py_XDECREF(py_ctx_dict); + +error: return 1; } diff --git a/test/new_tests/test_base_class.py b/test/new_tests/test_base_class.py index cfdc9c280..270d45812 100644 --- a/test/new_tests/test_base_class.py +++ b/test/new_tests/test_base_class.py @@ -178,9 +178,10 @@ def get_new_connection(add_config=None): if res is not None: break res = res.split(".") - # major_ver = res[0] - # minor_ver = res[1] - # print("major_ver:", major_ver, "minor_ver:", minor_ver) + TestBaseClass.major_ver = res[0] + TestBaseClass.minor_ver = res[1] + # print("major_ver:", TestBaseClass.major_ver, "minor_ver:", TestBaseClass.minor_ver) + return client @staticmethod diff --git a/test/new_tests/test_cdt_index.py b/test/new_tests/test_cdt_index.py index acf7beba0..b894b3d64 100644 --- a/test/new_tests/test_cdt_index.py +++ b/test/new_tests/test_cdt_index.py @@ -103,7 +103,7 @@ def test_pos_cdtindex_with_correct_parameters(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -148,7 +148,7 @@ def test_pos_cdtindex_with_listrank_correct_parameters(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_rank}, + ctx_list_rank, policy, ) @@ -169,7 +169,7 @@ def test_pos_cdtindex_with_listvalue_correct_parameters(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_value}, + ctx_list_value, policy, ) @@ -190,7 +190,7 @@ def test_pos_cdtindex_with_mapindex_correct_parameters(self): aerospike.INDEX_TYPE_MAPKEYS, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_map_index}, + ctx_map_index, policy, ) @@ -211,7 +211,7 @@ def test_pos_cdtindex_with_mapvalue_correct_parameters(self): aerospike.INDEX_TYPE_MAPVALUES, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_map_value}, + ctx_map_value, policy, ) @@ -232,7 +232,7 @@ def test_pos_cdtindex_with_maprankvalue_correct_parameters(self): aerospike.INDEX_TYPE_MAPVALUES, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_map_rank}, + ctx_map_rank, policy, ) @@ -254,7 +254,7 @@ def test_pos_cdtindex_with_correct_parameters1(self): aerospike.INDEX_TYPE_MAPVALUES, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_map_rank}, + ctx_map_rank, policy, ) @@ -275,7 +275,7 @@ def test_pos_cdtindex_with_correct_parameters_numeric(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -298,7 +298,7 @@ def test_pos_cdtindex_with_correct_parameters_set_length_extra(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) assert False @@ -319,7 +319,7 @@ def test_pos_cdtindex_with_incorrect_bin(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -339,7 +339,7 @@ def test_pos_create_same_cdtindex_multiple_times(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) if retobj == 0: @@ -351,7 +351,7 @@ def test_pos_create_same_cdtindex_multiple_times(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) except e.IndexFoundError: @@ -373,7 +373,7 @@ def test_pos_create_same_cdtindex_multiple_times_different_bin(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) if retobj == 0: @@ -385,7 +385,7 @@ def test_pos_create_same_cdtindex_multiple_times_different_bin(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) self.as_connection.index_remove("test", "test_string_list_cdt_index", policy) @@ -409,7 +409,7 @@ def test_pos_create_different_cdtindex_multiple_times_same_bin(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) if retobj == 0: @@ -421,7 +421,7 @@ def test_pos_create_different_cdtindex_multiple_times_same_bin(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index1", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) except e.IndexFoundError: @@ -444,7 +444,7 @@ def test_pos_createcdtindex_with_policy(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index_pol", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -464,7 +464,7 @@ def test_pos_createcdtindex_with_policystring(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -501,7 +501,7 @@ def test_pos_create_liststringindex_unicode_positive(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "uni_name_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -521,7 +521,7 @@ def test_pos_create_list_integer_index_unicode(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "uni_age_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -557,7 +557,7 @@ def test_neg_cdtindex_with_namespace_is_none(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -578,7 +578,7 @@ def test_neg_cdtindex_with_set_is_int(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) assert False @@ -601,7 +601,7 @@ def test_neg_cdtindex_with_set_is_none(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -624,7 +624,7 @@ def test_neg_cdtindex_with_bin_is_none(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -645,7 +645,7 @@ def test_neg_cdtindex_with_index_is_none(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, None, - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -667,7 +667,7 @@ def test_neg_cdtindex_with_incorrect_namespace(self): aerospike.INDEX_TYPE_DEFAULT, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -686,7 +686,7 @@ def test_neg_cdtindex_with_incorrect_set(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_NUMERIC, "test_numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -711,7 +711,7 @@ def test_neg_cdtindex_with_correct_parameters_no_connection(self): aerospike.INDEX_TYPE_LIST, aerospike.INDEX_STRING, "test_string_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, policy, ) @@ -726,3 +726,24 @@ def test_neg_cdtindex_with_no_paramters(self): self.as_connection.index_cdt_create() assert "argument 'ns' (pos 1)" in str(typeError.value) + + @pytest.mark.parametrize( + "ctx", + [ + None, + # Invalid type + {"ctx": 1} + ] + ) + def test_neg_cdtindex_with_invalid_ctx(self, ctx): + with pytest.raises(e.ParamError): + self.as_connection.index_cdt_create( + "test", + "demo", + "string_list", + aerospike.INDEX_TYPE_LIST, + aerospike.INDEX_STRING, + "test_string_list_cdt_index", + # Ctx must be a list + ctx + ) diff --git a/test/new_tests/test_query.py b/test/new_tests/test_query.py index 585871fe8..c65cbada9 100644 --- a/test/new_tests/test_query.py +++ b/test/new_tests/test_query.py @@ -124,7 +124,7 @@ def setupClass(self, as_connection): except e.IndexFoundError: pass - if (TestBaseClass.major_ver, TestBaseClass.minor_ver) >= (7, 0): + if (int(TestBaseClass.major_ver), int(TestBaseClass.minor_ver)) >= (7, 0): # These indexes are only used for server 7.0+ tests try: as_connection.index_list_create("test", "demo", "blob_list", aerospike.INDEX_BLOB, "blob_list_index") @@ -151,7 +151,7 @@ def setupClass(self, as_connection): aerospike.INDEX_TYPE_DEFAULT, aerospike.INDEX_NUMERIC, "numeric_list_cdt_index", - {"ctx": ctx_list_index}, + ctx_list_index, ) except e.IndexFoundError: pass @@ -164,7 +164,7 @@ def setupClass(self, as_connection): aerospike.INDEX_TYPE_DEFAULT, aerospike.INDEX_NUMERIC, "numeric_map_cdt_index", - {"ctx": ctx_map_index}, + ctx_map_index, ) except e.IndexFoundError: pass @@ -312,7 +312,8 @@ def test_query_with_correct_parameters_hi(self): """ query = self.as_connection.query("test", "demo") query.select("name", "test_age") - query.where(p.equals("test_age", 1)) + # Here we explicitly test that ctx accepts None + query.where(p.equals("test_age", 1), None) records = [] @@ -1053,7 +1054,7 @@ def test_query_with_list_cdt_ctx(self): query = self.as_connection.query("test", "demo") query.select("numeric_list") - query.where(p.range("numeric_list", aerospike.INDEX_TYPE_DEFAULT, 2, 4), {"ctx": ctx_list_index}) + query.where(p.range("numeric_list", aerospike.INDEX_TYPE_DEFAULT, 2, 4), ctx_list_index) records = [] @@ -1073,6 +1074,7 @@ def test_query_with_list_cdt_ctx_and_invalid_bin(self): Make sure that ctx is being cleaned up properly """ query = self.as_connection.query("test", "demo") + # Invalid bin with pytest.raises(e.ParamError): query.where(p.range(5, aerospike.INDEX_TYPE_DEFAULT, 2, 4), {"ctx": ctx_list_index}) @@ -1095,7 +1097,7 @@ def test_query_with_map_cdt_ctx(self): query = self.as_connection.query("test", "demo") query.select("numeric_map") - query.where(p.range("numeric_map", aerospike.INDEX_TYPE_DEFAULT, 2, 4), {"ctx": ctx_map_index}) + query.where(p.range("numeric_map", aerospike.INDEX_TYPE_DEFAULT, 2, 4), ctx_map_index) records = [] @@ -1110,6 +1112,15 @@ def callback(input_tuple): assert records assert len(records) == 3 + def test_query_with_invalid_list_cdt_ctx_dict(self): + """ + Invoke query() with cdt_ctx containing incorrect arguments + """ + query = self.as_connection.query("test", "demo") + + with pytest.raises(e.ParamError): + query.where(p.range("numeric_map", aerospike.INDEX_TYPE_DEFAULT, 2, 4), ['not a ctx list']) + def test_query_with_base64_cdt_ctx(self): bs_b4_cdt = self.as_connection.get_cdtctx_base64(ctx_list_index) assert bs_b4_cdt == "khAA"