[PATCHES] Port ldb to Python 3

Petr Viktorin pviktori at redhat.com
Fri Aug 21 08:59:23 UTC 2015


On 08/17/2015 01:17 AM, Andrew Bartlett wrote:
> On Fri, 2015-08-14 at 13:07 +0200, Petr Viktorin wrote:
>>
>> Thanks.
>>
>> Here is a new version of the patchset. It includes a buildtools 
>> change
>> to replace the ABI tag by just ".py3". See the first patch.
>>
>> I've also moved the wscript patch to the end as discussed earlier in
>> this thread, and I've squashed in Stefan's const-correctness fixes.
> 
> In the tests, can you extend the api.py testsuite to cover the whole
> API?
> 
> I realise you didn't add the APIs in the first place, but I'm
> particularly concerned that the intersection between DNs and LdbValue
> elements (which mix up between UTF8 strings and binary values in this
> new world) could be a bit tricky.  We need tests that specifically aim
> for the difficult parts of this, for:
>  - ldb.Dn.{get,set}_{extended_,}component_{name,value}
>  - ldb.Dn.{get,set}_rdn_{name,value}
>  - ldb.Dn.get_casefold
>  - ldb.Dn.get_linearized
>  - ldb.Dn.canonical_str
> 
> (There are probably more missing tests, but these would seem to be the
> main string related tests).

Thanks for noticing!

I went with tests when checking my changes, so I overlooked that the
Dn.set_component family should take both strings and bytes, and
get_component should return text.

I added a function to register a dummy extension to make it possible to
write the extended_component tests.
I also fixed firstmodule and wrote tests for it.

These patches go on top of the earlier ones; I can squash if that would
make it easier for you to review.

-- 
Petr Viktorin
-------------- next part --------------
From f51aacc8dc4fd76abec140a6352a22b26c2f0c15 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Fri, 21 Aug 2015 10:07:17 +0200
Subject: [PATCH 1/3] pyldb: Prevent segfault when first module is NULL

Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 lib/ldb/pyldb.c             | 6 +++++-
 lib/ldb/tests/python/api.py | 9 +++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 308ecb7..d53a552 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -2047,7 +2047,11 @@ static PyObject *PyLdbModule_FromModule(struct ldb_module *mod)
 
 static PyObject *py_ldb_get_firstmodule(PyLdbObject *self, void *closure)
 {
-	return PyLdbModule_FromModule(pyldb_Ldb_AsLdbContext(self)->modules);
+	struct ldb_module *mod = pyldb_Ldb_AsLdbContext(self)->modules;
+	if (mod == NULL) {
+		Py_RETURN_NONE;
+	}
+	return PyLdbModule_FromModule(mod);
 }
 
 static PyGetSetDef py_ldb_getset[] = {
diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index 2a85c08..ff824e7 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -70,6 +70,15 @@ class SimpleLdb(TestCase):
         x = ldb.Ldb(filename())
         self.assertEqual("[<ldb module 'tdb'>]", repr(x.modules()))
 
+    def test_firstmodule_none(self):
+        x = ldb.Ldb()
+        self.assertEqual(x.firstmodule, None)
+
+    def test_firstmodule_tdb(self):
+        x = ldb.Ldb(filename())
+        mod = x.firstmodule
+        self.assertEqual(repr(mod), "<ldb module 'tdb'>")
+
     def test_search(self):
         l = ldb.Ldb(filename())
         self.assertEqual(len(l.search()), 0)
-- 
2.1.0


From baa8f52fa3ed9a770cb6f55c20e5aefc352a47e5 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Fri, 21 Aug 2015 10:10:28 +0200
Subject: [PATCH 2/3] pyldb: Fixes and Python3 compat for Dn component
 accessors

Use "s#"/"z#" argument specifiers in set_component and
set_extended_component instead of converting strings manually.
(Under Python 3, This means both text strings and bytes are accepted.)

Raise error on set_component(None), instead of crashing.

Return text strings from get_{extended}_component under Python 3.

Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 lib/ldb/pyldb.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index d53a552..d7f0f79 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -253,6 +253,11 @@ static PyObject *PyObject_FromLdbValue(const struct ldb_val *val)
 	return PyBytes_FromStringAndSize((const char *)val->data, val->length);
 }
 
+static PyObject *PyStr_FromLdbValue(const struct ldb_val *val)
+{
+	return PyStr_FromStringAndSize((const char *)val->data, val->length);
+}
+
 /**
  * Create a Python object from a ldb_result.
  *
@@ -481,23 +486,19 @@ static PyObject *py_ldb_dn_get_extended_component(PyLdbDnObject *self, PyObject
 static PyObject *py_ldb_dn_set_extended_component(PyLdbDnObject *self, PyObject *args)
 {
 	char *name;
-	PyObject *value;
-	int err, result;
-	Py_ssize_t size;
+	int err;
+	uint8_t *value;
+	int size = 0;
 
-	if (!PyArg_ParseTuple(args, "sO", &name, &value))
+	if (!PyArg_ParseTuple(args, "sz#", &name, (const char**)&value, &size))
 		return NULL;
 
-	if (value == Py_None) {
+	if (value == NULL) {
 		err = ldb_dn_set_extended_component(self->dn, name, NULL);
 	} else {
 		struct ldb_val val;
-		result = PyBytes_AsStringAndSize(value, (char **) &val.data, &size);
+		val.data = (uint8_t *)value;
 		val.length = size;
-		if (result != 0) {
-			PyErr_SetString(PyExc_TypeError, "Expected a bytestring argument");
-			return NULL;
-		}
 		err = ldb_dn_set_extended_component(self->dn, name, &val);
 	}
 
@@ -665,29 +666,22 @@ static PyObject *py_ldb_dn_get_component_value(PyLdbDnObject *self, PyObject *ar
 		Py_RETURN_NONE;
 	}
 
-	return PyObject_FromLdbValue(val);
+	return PyStr_FromLdbValue(val);
 }
 
 static PyObject *py_ldb_dn_set_component(PyLdbDnObject *self, PyObject *args)
 {
 	unsigned int num = 0;
-	char *name = NULL;
-	PyObject *value = Py_None;
+	char *name = NULL, *value = NULL;
 	struct ldb_val val = { NULL, };
-	int err, ret;
-	Py_ssize_t size;
+	int err;
+	Py_ssize_t size = 0;
 
-	if (!PyArg_ParseTuple(args, "IsO", &num, &name, &value))
+	if (!PyArg_ParseTuple(args, "Iss#", &num, &name, &value, &size))
 		return NULL;
 
-	if (value != Py_None) {
-		ret = PyBytes_AsStringAndSize(value, (char **) &val.data, &size);
-		if (ret != 0) {
-			PyErr_SetString(PyExc_TypeError, "Expected a bytestring argument");
-			return NULL;
-		}
-		val.length = size;
-	}
+	val.data = (unsigned char*) value;
+	val.length = size;
 
 	err = ldb_dn_set_component(self->dn, num, name, val);
 	if (err != LDB_SUCCESS) {
@@ -725,7 +719,7 @@ static PyObject *py_ldb_dn_get_rdn_value(PyLdbDnObject *self)
 		Py_RETURN_NONE;
 	}
 
-	return PyObject_FromLdbValue(val);
+	return PyStr_FromLdbValue(val);
 }
 
 static PyMethodDef py_ldb_dn_methods[] = {
-- 
2.1.0


From e9071fdae990a3dc2b7d618b240051518e3e1274 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori at redhat.com>
Date: Fri, 21 Aug 2015 10:22:22 +0200
Subject: [PATCH 3/3] pyldb: Improve test coverage

Add tests for:
 - ldb.Dn.{get,set}_{extended_,}component_{name,value}
 - ldb.Dn.{get,set}_rdn_{name,value}
 - ldb.Dn.get_casefold
 - ldb.Dn.get_linearized
 - ldb.Dn.canonical_str

Add negative test for Dn.__contains__

Add a helper function to register a dummy DN extension for testing.

Signed-off-by: Petr Viktorin <pviktori at redhat.com>
---
 lib/ldb/pyldb.c             | 26 ++++++++++++++
 lib/ldb/tests/python/api.py | 83 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index d7f0f79..e783694 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -30,6 +30,7 @@
 
 #include <Python.h>
 #include "ldb_private.h"
+#include "ldb_handlers.h"
 #include "pyldb.h"
 
 void initldb(void);
@@ -1933,6 +1934,28 @@ static PyObject *py_ldb_sequence_number(PyLdbObject *self, PyObject *args)
 
 	return PyLong_FromLongLong(value);
 }
+
+
+static const struct ldb_dn_extended_syntax test_dn_syntax = {
+	.name             = "TEST",
+	.read_fn          = ldb_handler_copy,
+	.write_clear_fn   = ldb_handler_copy,
+	.write_hex_fn     = ldb_handler_copy,
+};
+
+static PyObject *py_ldb_register_test_extensions(PyLdbObject *self)
+{
+	struct ldb_context *ldb = pyldb_Ldb_AsLdbContext(self);
+	int ret;
+
+	ret = ldb_dn_extended_add_syntax(ldb, LDB_ATTR_FLAG_FIXED, &test_dn_syntax);
+
+	PyErr_LDB_ERROR_IS_ERR_RAISE(PyExc_LdbError, ret, ldb);
+
+	Py_RETURN_NONE;
+}
+
+
 static PyMethodDef py_ldb_methods[] = {
 	{ "set_debug", (PyCFunction)py_ldb_set_debug, METH_VARARGS, 
 		"S.set_debug(callback) -> None\n"
@@ -2022,6 +2045,9 @@ static PyMethodDef py_ldb_methods[] = {
 	{ "sequence_number", (PyCFunction)py_ldb_sequence_number, METH_VARARGS,
 		"S.sequence_number(type) -> value\n"
 		"Return the value of the sequence according to the requested type" },
+	{ "_register_test_extensions", (PyCFunction)py_ldb_register_test_extensions, METH_NOARGS,
+		"S._register_test_extensions() -> None\n"
+		"Register internal extensions used in testing" },
 	{ NULL },
 };
 
diff --git a/lib/ldb/tests/python/api.py b/lib/ldb/tests/python/api.py
index ff824e7..0bce207 100755
--- a/lib/ldb/tests/python/api.py
+++ b/lib/ldb/tests/python/api.py
@@ -135,6 +135,7 @@ class SimpleLdb(TestCase):
         l.add(m)
         try:
             self.assertTrue(ldb.Dn(l, "dc=foo3") in l)
+            self.assertFalse(ldb.Dn(l, "dc=foo4") in l)
         finally:
             l.delete(m.dn)
 
@@ -591,6 +592,88 @@ class DnTests(TestCase):
         self.assertFalse(dn3.is_child_of(dn2))
         self.assertFalse(dn1.is_child_of(dn4))
 
+    def test_get_component_name(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_component_name(0), 'cn')
+        self.assertEqual(dn.get_component_name(1), 'dc')
+        self.assertEqual(dn.get_component_name(2), None)
+        self.assertEqual(dn.get_component_name(-1), None)
+
+    def test_get_component_value(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_component_value(0), 'foo')
+        self.assertEqual(dn.get_component_value(1), 'base')
+        self.assertEqual(dn.get_component_name(2), None)
+        self.assertEqual(dn.get_component_name(-1), None)
+
+    def test_set_component(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        dn.set_component(0, 'cn', 'bar')
+        self.assertEqual(str(dn), "cn=bar,dc=base")
+        dn.set_component(1, 'o', 'asep')
+        self.assertEqual(str(dn), "cn=bar,o=asep")
+        self.assertRaises(TypeError, dn.set_component, 2, 'dc', 'base')
+        self.assertEqual(str(dn), "cn=bar,o=asep")
+        dn.set_component(1, 'o', 'a,b+c')
+        self.assertEqual(str(dn), r"cn=bar,o=a\,b\+c")
+
+    def test_set_component_bytes(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        dn.set_component(0, 'cn', b'bar')
+        self.assertEqual(str(dn), "cn=bar,dc=base")
+        dn.set_component(1, 'o', b'asep')
+        self.assertEqual(str(dn), "cn=bar,o=asep")
+
+    def test_set_component_none(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,cn=bar,dc=base")
+        self.assertRaises(TypeError, dn.set_component, 1, 'cn', None)
+
+    def test_get_extended_component_null(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,cn=bar,dc=base")
+        self.assertEqual(dn.get_extended_component("TEST"), None)
+
+    def test_get_extended_component(self):
+        self.ldb._register_test_extensions()
+        dn = ldb.Dn(self.ldb, "<TEST=foo>;cn=bar,dc=base")
+        self.assertEqual(dn.get_extended_component("TEST"), b"foo")
+
+    def test_set_extended_component(self):
+        self.ldb._register_test_extensions()
+        dn = ldb.Dn(self.ldb, "dc=base")
+        dn.set_extended_component("TEST", "foo")
+        self.assertEqual(dn.get_extended_component("TEST"), b"foo")
+        dn.set_extended_component("TEST", b"bar")
+        self.assertEqual(dn.get_extended_component("TEST"), b"bar")
+
+    def test_extended_str(self):
+        self.ldb._register_test_extensions()
+        dn = ldb.Dn(self.ldb, "<TEST=foo>;cn=bar,dc=base")
+        self.assertEqual(dn.extended_str(), "<TEST=foo>;cn=bar,dc=base")
+
+    def test_get_rdn_name(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_rdn_name(), 'cn')
+
+    def test_get_rdn_value(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_rdn_value(), 'foo')
+
+    def test_get_casefold(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_casefold(), 'CN=FOO,DC=BASE')
+
+    def test_get_linearized(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertEqual(dn.get_linearized(), 'cn=foo,dc=base')
+
+    def test_is_null(self):
+        dn = ldb.Dn(self.ldb, "cn=foo,dc=base")
+        self.assertFalse(dn.is_null())
+
+        dn = ldb.Dn(self.ldb, '')
+        self.assertTrue(dn.is_null())
+
+
 class LdbMsgTests(TestCase):
 
     def setUp(self):
-- 
2.1.0



More information about the samba-technical mailing list