From 1eb103b70044c7a06ec029fcf375fc93baebd80e Mon Sep 17 00:00:00 2001 From: juliannguyen4 <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 14 Sep 2023 14:33:41 -0700 Subject: [PATCH 01/12] Add test for getting and putting key ordered map --- test/new_tests/test_get_put_keyordereddict.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/new_tests/test_get_put_keyordereddict.py diff --git a/test/new_tests/test_get_put_keyordereddict.py b/test/new_tests/test_get_put_keyordereddict.py new file mode 100644 index 000000000..a7a5ef118 --- /dev/null +++ b/test/new_tests/test_get_put_keyordereddict.py @@ -0,0 +1,19 @@ +from aerospike import KeyOrderedDict +import pytest + + +class TestGetPutOrderedDict: + @pytest.fixture(autouse=True) + def setup(self, request, as_connection): + pass + + def test_get_put_keyordereddict(self): + bins = { + "dict": KeyOrderedDict({"f": 6, "e": 5, "d": 4}) + } + key = ("test", "demo", 1) + self.as_connection.put(key, bins) + + _, _, res = self.as_connection.get(key) + assert res["dict"] == KeyOrderedDict({"f": 6, "e": 5, "d": 4}) + assert type(res["dict"]) == KeyOrderedDict From e7c3ac912bd7ff68ca3ac0b3fedd793839599e69 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:08:32 -0800 Subject: [PATCH 02/12] Convert as_orderedmap in C to KeyOrderedDict in Python --- src/main/conversions.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/conversions.c b/src/main/conversions.c index 248368819..b11fc4b49 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1662,6 +1662,11 @@ as_status map_to_pyobject(AerospikeClient *self, as_error *err, const as_map *map, PyObject **py_map) { *py_map = PyDict_New(); + // as_orderedmap has flags set to 1 + if (map->flags == 1) { + PyObject *key_ordered_dict_class = AerospikeKeyOrderedDict_Get_Type(); + *py_map = PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map); + } if (!*py_map) { return as_error_update(err, AEROSPIKE_ERR_CLIENT, From 75d41652fe88c3c2cb06556f66d30f7cf008de9f Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:10:22 -0800 Subject: [PATCH 03/12] PyObject_CallFunctionObjArgs args must end with NULL --- src/main/conversions.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/conversions.c b/src/main/conversions.c index b11fc4b49..ff11e19c3 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1665,7 +1665,8 @@ as_status map_to_pyobject(AerospikeClient *self, as_error *err, // as_orderedmap has flags set to 1 if (map->flags == 1) { PyObject *key_ordered_dict_class = AerospikeKeyOrderedDict_Get_Type(); - *py_map = PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map); + *py_map = + PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map, NULL); } if (!*py_map) { From bbde64674932222371c01286133ab3094354eef7 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:32:07 -0800 Subject: [PATCH 04/12] Add test cleanup code --- test/new_tests/test_get_put_keyordereddict.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/new_tests/test_get_put_keyordereddict.py b/test/new_tests/test_get_put_keyordereddict.py index a7a5ef118..56b018d9c 100644 --- a/test/new_tests/test_get_put_keyordereddict.py +++ b/test/new_tests/test_get_put_keyordereddict.py @@ -1,19 +1,24 @@ from aerospike import KeyOrderedDict import pytest +from aerospike import exception as e class TestGetPutOrderedDict: @pytest.fixture(autouse=True) def setup(self, request, as_connection): - pass + yield + try: + self.as_connection.remove(self.key) + except e.AerospikeError: + pass def test_get_put_keyordereddict(self): bins = { "dict": KeyOrderedDict({"f": 6, "e": 5, "d": 4}) } - key = ("test", "demo", 1) - self.as_connection.put(key, bins) + self.key = ("test", "demo", 1) + self.as_connection.put(self.key, bins) - _, _, res = self.as_connection.get(key) + _, _, res = self.as_connection.get(self.key) assert res["dict"] == KeyOrderedDict({"f": 6, "e": 5, "d": 4}) assert type(res["dict"]) == KeyOrderedDict From bb8b11b20c678f26fdd3c78dfe5881021f34834e Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:53:58 -0800 Subject: [PATCH 05/12] Add nested test case --- test/new_tests/test_get_put_keyordereddict.py | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/test/new_tests/test_get_put_keyordereddict.py b/test/new_tests/test_get_put_keyordereddict.py index 56b018d9c..f75d06107 100644 --- a/test/new_tests/test_get_put_keyordereddict.py +++ b/test/new_tests/test_get_put_keyordereddict.py @@ -12,13 +12,30 @@ def setup(self, request, as_connection): except e.AerospikeError: pass - def test_get_put_keyordereddict(self): + @pytest.mark.parametrize( + "bin_value", + [ + KeyOrderedDict({"f": 6, "e": 5, "d": 4}), + [ + KeyOrderedDict({"f": 6, "e": 5, "d": 4}) + ] + ], + ids=[ + "bin-level", + "nested" + ] + ) + def test_get_put_keyordereddict(self, bin_value): bins = { - "dict": KeyOrderedDict({"f": 6, "e": 5, "d": 4}) + "dict": bin_value } self.key = ("test", "demo", 1) self.as_connection.put(self.key, bins) _, _, res = self.as_connection.get(self.key) - assert res["dict"] == KeyOrderedDict({"f": 6, "e": 5, "d": 4}) - assert type(res["dict"]) == KeyOrderedDict + + assert res["dict"] == bin_value + if type(res["dict"]) == list: + assert type(res["dict"][0]) == KeyOrderedDict + else: + assert type(res["dict"]) == KeyOrderedDict From 116f5f52672cf2c356ecc3a265c61b160a6531a5 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Mon, 12 Feb 2024 14:25:14 -0800 Subject: [PATCH 06/12] Type init function's first parameter should be a PyObject* type --- src/main/key_ordered_dict/type.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/key_ordered_dict/type.c b/src/main/key_ordered_dict/type.c index c58df9b5d..01d7e4d40 100644 --- a/src/main/key_ordered_dict/type.c +++ b/src/main/key_ordered_dict/type.c @@ -29,8 +29,8 @@ static PyMethodDef AerospikeKeyOrderedDict_Type_Methods[] = {{NULL}}; * PYTHON TYPE HOOKS ******************************************************************************/ -static int AerospikeKeyOrderedDict_Type_Init(AerospikeQuery *self, - PyObject *args, PyObject *kwds) +static int AerospikeKeyOrderedDict_Type_Init(PyObject *self, PyObject *args, + PyObject *kwds) { return PyDict_Type.tp_init((PyObject *)self, args, kwds); } From 2c0303ae246ddc91f67ada6b498a9c51da1247d4 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Mon, 12 Feb 2024 14:40:24 -0800 Subject: [PATCH 07/12] Remove unnecessary cast to PyObject* --- src/main/key_ordered_dict/type.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/key_ordered_dict/type.c b/src/main/key_ordered_dict/type.c index 01d7e4d40..66dc4807e 100644 --- a/src/main/key_ordered_dict/type.c +++ b/src/main/key_ordered_dict/type.c @@ -32,7 +32,7 @@ static PyMethodDef AerospikeKeyOrderedDict_Type_Methods[] = {{NULL}}; static int AerospikeKeyOrderedDict_Type_Init(PyObject *self, PyObject *args, PyObject *kwds) { - return PyDict_Type.tp_init((PyObject *)self, args, kwds); + return PyDict_Type.tp_init(self, args, kwds); } /******************************************************************************* From 25154d1de9745a807268469c388e4a6de2f9799c Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 13 Feb 2024 09:55:16 -0800 Subject: [PATCH 08/12] Fix memory leak --- src/main/conversions.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/conversions.c b/src/main/conversions.c index ff11e19c3..44a0b3c41 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1667,6 +1667,7 @@ as_status map_to_pyobject(AerospikeClient *self, as_error *err, PyObject *key_ordered_dict_class = AerospikeKeyOrderedDict_Get_Type(); *py_map = PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map, NULL); + Py_DECREF(*py_map); } if (!*py_map) { From 96b1e3c2f71aeb4f2384e56834275f980103dd34 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 13 Feb 2024 10:49:36 -0800 Subject: [PATCH 09/12] Fix memory error and potential leak from reference cycle --- src/main/conversions.c | 18 +++++++++++------- src/main/key_ordered_dict/type.c | 31 +++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/main/conversions.c b/src/main/conversions.c index 44a0b3c41..4b54ed4fc 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1662,17 +1662,21 @@ as_status map_to_pyobject(AerospikeClient *self, as_error *err, const as_map *map, PyObject **py_map) { *py_map = PyDict_New(); + if (!*py_map) { + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Failed to allocate memory for dictionary."); + } + // as_orderedmap has flags set to 1 if (map->flags == 1) { PyObject *key_ordered_dict_class = AerospikeKeyOrderedDict_Get_Type(); - *py_map = + PyObject *py_keyordereddict = PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map, NULL); - Py_DECREF(*py_map); - } - - if (!*py_map) { - return as_error_update(err, AEROSPIKE_ERR_CLIENT, - "Failed to allocate memory for dictionary."); + if (py_keyordereddict == NULL) { + Py_DECREF(*py_map); + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Failed to create KeyOrderedDict instance."); + } } conversion_data convd = { diff --git a/src/main/key_ordered_dict/type.c b/src/main/key_ordered_dict/type.c index 66dc4807e..6b1182bdc 100644 --- a/src/main/key_ordered_dict/type.c +++ b/src/main/key_ordered_dict/type.c @@ -25,6 +25,14 @@ static PyMethodDef AerospikeKeyOrderedDict_Type_Methods[] = {{NULL}}; +static int local_traverse(PyObject *self, visitproc visit, void *arg) +{ + AerospikeKeyOrderedDict *self_keyordereddict = + (AerospikeKeyOrderedDict *)self; + Py_VISIT(&self_keyordereddict->dict); + return 0; +} + /******************************************************************************* * PYTHON TYPE HOOKS ******************************************************************************/ @@ -39,16 +47,19 @@ static int AerospikeKeyOrderedDict_Type_Init(PyObject *self, PyObject *args, * PYTHON TYPE DESCRIPTOR ******************************************************************************/ -static PyTypeObject AerospikeKeyOrderedDict_Type = { - PyVarObject_HEAD_INIT(NULL, 0).tp_name = "aerospike.KeyOrderedDict", - .tp_basicsize = sizeof(AerospikeKeyOrderedDict), - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_doc = "The KeyOrderedDict class is a dictionary that directly maps\n" - "to a key ordered map on the Aerospike server.\n" - "This assists in matching key ordered maps\n" - "through various read operations.\n", - .tp_methods = AerospikeKeyOrderedDict_Type_Methods, - .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init}; +static PyTypeObject + AerospikeKeyOrderedDict_Type = + {PyVarObject_HEAD_INIT(NULL, 0).tp_name = "aerospike.KeyOrderedDict", + .tp_basicsize = sizeof(AerospikeKeyOrderedDict), + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .tp_doc = + "The KeyOrderedDict class is a dictionary that directly maps\n" + "to a key ordered map on the Aerospike server.\n" + "This assists in matching key ordered maps\n" + "through various read operations.\n", + .tp_methods = AerospikeKeyOrderedDict_Type_Methods, + .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init}, + .tp_traverse = local_traverse; PyTypeObject *AerospikeKeyOrderedDict_Ready() { From 27babfedd8596718af9bc5ca5c30ecfd2bf5820c Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 13 Feb 2024 12:27:59 -0800 Subject: [PATCH 10/12] Fix syntax error --- src/main/key_ordered_dict/type.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/main/key_ordered_dict/type.c b/src/main/key_ordered_dict/type.c index 6b1182bdc..475919a65 100644 --- a/src/main/key_ordered_dict/type.c +++ b/src/main/key_ordered_dict/type.c @@ -47,19 +47,17 @@ static int AerospikeKeyOrderedDict_Type_Init(PyObject *self, PyObject *args, * PYTHON TYPE DESCRIPTOR ******************************************************************************/ -static PyTypeObject - AerospikeKeyOrderedDict_Type = - {PyVarObject_HEAD_INIT(NULL, 0).tp_name = "aerospike.KeyOrderedDict", - .tp_basicsize = sizeof(AerospikeKeyOrderedDict), - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, - .tp_doc = - "The KeyOrderedDict class is a dictionary that directly maps\n" - "to a key ordered map on the Aerospike server.\n" - "This assists in matching key ordered maps\n" - "through various read operations.\n", - .tp_methods = AerospikeKeyOrderedDict_Type_Methods, - .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init}, - .tp_traverse = local_traverse; +static PyTypeObject AerospikeKeyOrderedDict_Type = { + PyVarObject_HEAD_INIT(NULL, 0).tp_name = "aerospike.KeyOrderedDict", + .tp_basicsize = sizeof(AerospikeKeyOrderedDict), + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .tp_doc = "The KeyOrderedDict class is a dictionary that directly maps\n" + "to a key ordered map on the Aerospike server.\n" + "This assists in matching key ordered maps\n" + "through various read operations.\n", + .tp_methods = AerospikeKeyOrderedDict_Type_Methods, + .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init, + .tp_traverse = local_traverse}; PyTypeObject *AerospikeKeyOrderedDict_Ready() { From 046091e0cd0431a7f962a022ac9b9821332274cb Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:09:05 -0800 Subject: [PATCH 11/12] Need to set original PyObject* map to key ordered map. Always DECREF original map since it's no longer needed --- src/main/conversions.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/conversions.c b/src/main/conversions.c index 4b54ed4fc..fadcf3a21 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1672,11 +1672,12 @@ as_status map_to_pyobject(AerospikeClient *self, as_error *err, PyObject *key_ordered_dict_class = AerospikeKeyOrderedDict_Get_Type(); PyObject *py_keyordereddict = PyObject_CallFunctionObjArgs(key_ordered_dict_class, *py_map, NULL); + Py_DECREF(*py_map); if (py_keyordereddict == NULL) { - Py_DECREF(*py_map); return as_error_update(err, AEROSPIKE_ERR_CLIENT, "Failed to create KeyOrderedDict instance."); } + *py_map = py_keyordereddict; } conversion_data convd = { From fc0b09e906ed49bdc9a99b9d578b2e4f14554e08 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:14:15 -0800 Subject: [PATCH 12/12] Revert adding traverse function --- src/main/key_ordered_dict/type.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/key_ordered_dict/type.c b/src/main/key_ordered_dict/type.c index 475919a65..66dc4807e 100644 --- a/src/main/key_ordered_dict/type.c +++ b/src/main/key_ordered_dict/type.c @@ -25,14 +25,6 @@ static PyMethodDef AerospikeKeyOrderedDict_Type_Methods[] = {{NULL}}; -static int local_traverse(PyObject *self, visitproc visit, void *arg) -{ - AerospikeKeyOrderedDict *self_keyordereddict = - (AerospikeKeyOrderedDict *)self; - Py_VISIT(&self_keyordereddict->dict); - return 0; -} - /******************************************************************************* * PYTHON TYPE HOOKS ******************************************************************************/ @@ -56,8 +48,7 @@ static PyTypeObject AerospikeKeyOrderedDict_Type = { "This assists in matching key ordered maps\n" "through various read operations.\n", .tp_methods = AerospikeKeyOrderedDict_Type_Methods, - .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init, - .tp_traverse = local_traverse}; + .tp_init = (initproc)AerospikeKeyOrderedDict_Type_Init}; PyTypeObject *AerospikeKeyOrderedDict_Ready() {