Convert mgp_list to PyTuple instead of PyList

Summary: It's much safer to give users immutable types.

Reviewers: ipaljak

Reviewed By: ipaljak

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2705
This commit is contained in:
Teon Banek 2020-03-03 13:13:09 +01:00
parent 32b35f6f88
commit 456267249e
2 changed files with 31 additions and 26 deletions
src/query/procedure
tests/unit

View File

@ -387,6 +387,22 @@ struct PyQueryModule {
mgp_module *module;
};
py::Object MgpListToPyTuple(const mgp_list *list, PyGraph *py_graph) {
CHECK(list);
CHECK(py_graph);
const size_t len = mgp_list_size(list);
py::Object py_tuple(PyTuple_New(len));
if (!py_tuple) return nullptr;
for (size_t i = 0; i < len; ++i) {
auto elem = MgpValueToPyObject(*mgp_list_at(list, i), py_graph);
if (!elem) return nullptr;
// Explicitly convert `py_tuple`, which is `py::Object`, via static_cast.
// Then the macro will cast it to `PyTuple *`.
PyTuple_SET_ITEM(static_cast<PyObject *>(py_tuple), i, elem.Steal());
}
return py_tuple;
}
PyObject *PyQueryModuleAddReadProcedure(PyQueryModule *self, PyObject *cb) {
CHECK(self->module);
if (!PyCallable_Check(cb)) {
@ -1089,29 +1105,18 @@ py::Object MgpValueToPyObject(const mgp_value &value, PyGraph *py_graph) {
return py::Object(PyFloat_FromDouble(mgp_value_get_double(&value)));
case MGP_VALUE_TYPE_STRING:
return py::Object(PyUnicode_FromString(mgp_value_get_string(&value)));
case MGP_VALUE_TYPE_LIST: {
const auto *list = mgp_value_get_list(&value);
const size_t len = mgp_list_size(list);
py::Object py_list(PyList_New(len));
CHECK(py_list);
for (size_t i = 0; i < len; ++i) {
auto elem = MgpValueToPyObject(*mgp_list_at(list, i), py_graph);
CHECK(elem);
// Explicitly convert `py_list`, which is `py::Object`, via static_cast.
// Then the macro will cast it to `PyList *`.
PyList_SET_ITEM(static_cast<PyObject *>(py_list), i, elem.Steal());
}
return py_list;
}
case MGP_VALUE_TYPE_LIST:
return MgpListToPyTuple(mgp_value_get_list(&value), py_graph);
case MGP_VALUE_TYPE_MAP: {
const auto *map = mgp_value_get_map(&value);
py::Object py_dict(PyDict_New());
CHECK(py_dict);
if (!py_dict) return nullptr;
for (const auto &[key, val] : map->items) {
auto py_val = MgpValueToPyObject(val, py_graph);
CHECK(py_val);
if (!py_val) return nullptr;
// Unlike PyList_SET_ITEM, PyDict_SetItem does not steal the value.
CHECK(PyDict_SetItemString(py_dict, key.c_str(), py_val) == 0);
if (PyDict_SetItemString(py_dict, key.c_str(), py_val) != 0)
return nullptr;
}
return py_dict;
}

View File

@ -33,7 +33,7 @@ TEST(PyModule, MgpValueToPyObject) {
static_cast<PyObject *>(py_graph)));
mgp_value_destroy(map_val);
// We should now have in Python:
// {"list": [None, False, True, 42, 0.1, "some text"]}
// {"list": (None, False, True, 42, 0.1, "some text")}
ASSERT_TRUE(PyDict_Check(py_dict));
EXPECT_EQ(PyDict_Size(py_dict), 1);
PyObject *key = nullptr;
@ -42,18 +42,18 @@ TEST(PyModule, MgpValueToPyObject) {
while (PyDict_Next(py_dict, &pos, &key, &value)) {
ASSERT_TRUE(PyUnicode_Check(key));
EXPECT_EQ(std::string(PyUnicode_AsUTF8(key)), "list");
ASSERT_TRUE(PyList_Check(value));
ASSERT_EQ(PyList_Size(value), 6);
EXPECT_EQ(PyList_GetItem(value, 0), Py_None);
EXPECT_EQ(PyList_GetItem(value, 1), Py_False);
EXPECT_EQ(PyList_GetItem(value, 2), Py_True);
auto *py_long = PyList_GetItem(value, 3);
ASSERT_TRUE(PyTuple_Check(value));
ASSERT_EQ(PyTuple_Size(value), 6);
EXPECT_EQ(PyTuple_GetItem(value, 0), Py_None);
EXPECT_EQ(PyTuple_GetItem(value, 1), Py_False);
EXPECT_EQ(PyTuple_GetItem(value, 2), Py_True);
auto *py_long = PyTuple_GetItem(value, 3);
ASSERT_TRUE(PyLong_Check(py_long));
EXPECT_EQ(PyLong_AsLong(py_long), 42);
auto *py_float = PyList_GetItem(value, 4);
auto *py_float = PyTuple_GetItem(value, 4);
ASSERT_TRUE(PyFloat_Check(py_float));
EXPECT_EQ(PyFloat_AsDouble(py_float), 0.1);
auto *py_str = PyList_GetItem(value, 5);
auto *py_str = PyTuple_GetItem(value, 5);
ASSERT_TRUE(PyUnicode_Check(py_str));
EXPECT_EQ(std::string(PyUnicode_AsUTF8(py_str)), "some text");
}