[prev in list] [next in list] [prev in thread] [next in thread] 

List:       samba-technical
Subject:    [PATCH] Improve usnChanged handling
From:       Andrew Bartlett <abartlet () samba ! org>
Date:       2013-05-31 5:32:10
Message-ID: 1369978330.23610.116.camel () jesse
[Download RAW message or body]

I noticed this issue when looking into other DRS questions, and wrote a
test and fixed the issue.

Please review/push.

Andrew Bartlett
-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org


["0001-dsdb-Fix-behaviour-for-when-to-update-the-USN-when-t.patch" (0001-dsdb-Fix-behaviour-for-when-to-update-the-USN-when-t.patch)]

From c5ef7441aae0840e57562fd3cb4cb3116018a88b Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 31 May 2013 14:16:02 +1000
Subject: [PATCH 1/3] dsdb: Fix behaviour for when to update the USN when
 there is no change

This handles deletions and replacements with no value, or with an
exactly specified value, as well as modifies.

Andrew Bartlett
---
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 28 ++++++++++++++++++-------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c \
b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 98e60d7..ab058a2 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -1092,18 +1092,30 @@ static int replmd_update_rpmd_element(struct ldb_context \
*ldb,  el->name));
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
-
+	
 	if ((a->systemFlags & DS_FLAG_ATTR_NOT_REPLICATED) || (a->systemFlags & \
DS_FLAG_ATTR_IS_CONSTRUCTED)) {  return LDB_SUCCESS;
 	}
-
-	/* if the attribute's value haven't changed then return LDB_SUCCESS
-	 * Unless we have the provision control or if the attribute is
-	 * interSiteTopologyGenerator as this page explain: \
                http://support.microsoft.com/kb/224815
-	 * this attribute is periodicaly written by the DC responsible for the intersite \
                generation
-	 * in a given site
+	
+	/* if the attribute's value haven't changed, and this isn't
+	 * just a delete of everything then return LDB_SUCCESS Unless
+	 * we have the provision control or if the attribute is
+	 * interSiteTopologyGenerator as this page explain:
+	 * http://support.microsoft.com/kb/224815 this attribute is
+	 * periodicaly written by the DC responsible for the intersite
+	 * generation in a given site
+	 *
+	 * Unchanged could be deleting or replacing an already-gone
+	 * thing with an unconstrained delete/empty replace or a
+	 * replace with the same value, but not an add with the same
+	 * value because that could be about adding a duplicate (which
+	 * is for someone else to error out on).
 	 */
-	if (old_el != NULL && ldb_msg_element_compare(el, old_el) == 0) {
+	if (((LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE
+	      || LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE)
+	     && old_el == NULL && el->num_values == 0) ||
+	    (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_REPLACE 
+	     && old_el != NULL && ldb_msg_element_compare(el, old_el) == 0)) {
 		if (strcmp(el->name, "interSiteTopologyGenerator") != 0 &&
 		    !ldb_request_get_control(req, LDB_CONTROL_PROVISION_OID)) {
 			/*
-- 
1.7.11.7


["0002-dsdb-tests-ldap.py-Fix-quoting-of-print-statements.patch" (0002-dsdb-tests-ldap.py-Fix-quoting-of-print-statements.patch)]

From 107b6495e5bd533cbd348f11399c8b9d91a31d2c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 31 May 2013 11:15:51 +1000
Subject: [PATCH 2/3] dsdb-tests ldap.py: Fix quoting of print statements

While python didn't mind (oddly) it really confused my editor.

Andrew Bartlett
---
 source4/dsdb/tests/python/ldap.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py
index 5ca4c26..96331d2 100755
--- a/source4/dsdb/tests/python/ldap.py
+++ b/source4/dsdb/tests/python/ldap.py
@@ -712,7 +712,7 @@ class BasicTests(samba.tests.TestCase):
 
     def test_attribute_ranges(self):
         """Test attribute ranges"""
-        print "Test attribute ranges"""
+        print "Test attribute ranges"
 
         # Too short (min. 1)
         try:
@@ -833,7 +833,7 @@ class BasicTests(samba.tests.TestCase):
 
     def test_instanceType(self):
         """Tests the 'instanceType' attribute"""
-        print "Tests the 'instanceType' attribute"""
+        print "Tests the 'instanceType' attribute"
 
         # The instance type is single-valued
         try:
@@ -902,7 +902,7 @@ class BasicTests(samba.tests.TestCase):
 
     def test_distinguished_name(self):
         """Tests the 'distinguishedName' attribute"""
-        print "Tests the 'distinguishedName' attribute"""
+        print "Tests the 'distinguishedName' attribute"
 
         # The "dn" shortcut isn't supported
         m = Message()
@@ -982,7 +982,7 @@ class BasicTests(samba.tests.TestCase):
 
     def test_rdn_name(self):
         """Tests the RDN"""
-        print "Tests the RDN"""
+        print "Tests the RDN"
 
         # Search
 
@@ -1177,7 +1177,7 @@ objectClass: container
 
     def test_rename(self):
         """Tests the rename operation"""
-        print "Tests the rename operations"""
+        print "Tests the rename operations"
 
         try:
             # cannot rename to be a child of itself
@@ -1288,7 +1288,7 @@ objectClass: container
 
     def test_rename_twice(self):
         """Tests the rename operation twice - this corresponds to a past bug"""
-        print "Tests the rename twice operation"""
+        print "Tests the rename twice operation"
 
         self.ldb.add({
              "dn": "cn=ldaptestuser5,cn=users," + self.base_dn,
@@ -1576,7 +1576,7 @@ objectGUID: bd3480c9-58af-4cd8-92df-bc4a18b6e44d
 
     def test_wkguid(self):
         """Test Well known GUID behaviours (including DN+Binary)"""
-        print "Test Well known GUID behaviours (including DN+Binary)"""
+        print "Test Well known GUID behaviours (including DN+Binary)"
 
         res = self.ldb.search(base=("<WKGUID=ab1d30f3768811d1aded00c04fd8d5cd,%s>" % \
self.base_dn), scope=SCOPE_BASE, attrs=[])  self.assertEquals(len(res), 1)
@@ -1593,7 +1593,7 @@ objectGUID: bd3480c9-58af-4cd8-92df-bc4a18b6e44d
 
     def test_subschemasubentry(self):
         """Test subSchemaSubEntry appears when requested, but not when not \
                requested"""
-        print "Test subSchemaSubEntry"""
+        print "Test subSchemaSubEntry"
 
         res = self.ldb.search(base=self.base_dn, scope=SCOPE_BASE, \
attrs=["subSchemaSubEntry"])  self.assertEquals(len(res), 1)
@@ -2720,7 +2720,7 @@ nTSecurityDescriptor:: """ + desc_base64
 
     def test_dsheuristics(self):
         """Tests the 'dSHeuristics' attribute"""
-        print "Tests the 'dSHeuristics' attribute"""
+        print "Tests the 'dSHeuristics' attribute"
 
         # Get the current value to restore it later
         dsheuristics = self.ldb.get_dsheuristics()
@@ -2763,7 +2763,7 @@ nTSecurityDescriptor:: """ + desc_base64
 
     def test_operational(self):
         """Tests operational attributes"""
-        print "Tests operational attributes"""
+        print "Tests operational attributes"
 
         res = self.ldb.search(self.base_dn, scope=SCOPE_BASE,
                               attrs=["createTimeStamp", "modifyTimeStamp",
-- 
1.7.11.7


["0003-dsdb-tests-ldap.py-Add-test-for-usn-behaviour-on-cer.patch" (0003-dsdb-tests-ldap.py-Add-test-for-usn-behaviour-on-cer.patch)]

From 67855dd0647a41fe458f51488d1d6855a30e15af Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 31 May 2013 14:14:54 +1000
Subject: [PATCH 3/3] dsdb-tests ldap.py: Add test for usn behaviour on
 certain changes

This probes when the usn is updated, and when it is not.

Andrew Bartlett
---
 source4/dsdb/tests/python/ldap.py | 209 ++++++++++++++++++++++++++++----------
 1 file changed, 155 insertions(+), 54 deletions(-)

diff --git a/source4/dsdb/tests/python/ldap.py b/source4/dsdb/tests/python/ldap.py
index 96331d2..d3f6953 100755
--- a/source4/dsdb/tests/python/ldap.py
+++ b/source4/dsdb/tests/python/ldap.py
@@ -1327,60 +1327,6 @@ objectGUID: bd3480c9-58af-4cd8-92df-bc4a18b6e44d
             "dn": "cn=ldaptestcontainer," + self.base_dn,
             "objectClass": "container" })
 
-        res = ldb.search("cn=ldaptestcontainer," + self.base_dn,
-                         scope=SCOPE_BASE,
-                         attrs=["objectGUID", "uSNCreated", "uSNChanged", \
                "whenCreated", "whenChanged"])
-        self.assertTrue(len(res) == 1)
-        self.assertTrue("objectGUID" in res[0])
-        self.assertTrue("uSNCreated" in res[0])
-        self.assertTrue("uSNChanged" in res[0])
-        self.assertTrue("whenCreated" in res[0])
-        self.assertTrue("whenChanged" in res[0])
-
-        delete_force(self.ldb, "cn=ldaptestcontainer," + self.base_dn)
-
-        # All the following attributes are specificable on add operations
-        self.ldb.add({
-            "dn": "cn=ldaptestcontainer," + self.base_dn,
-            "objectClass": "container",
-            "uSNCreated" : "1",
-            "uSNChanged" : "1",
-            "whenCreated": timestring(long(time.time())),
-            "whenChanged": timestring(long(time.time())) })
-
-        res = ldb.search("cn=ldaptestcontainer," + self.base_dn,
-                         scope=SCOPE_BASE,
-                         attrs=["objectGUID", "uSNCreated", "uSNChanged", \
                "whenCreated", "whenChanged"])
-        self.assertTrue(len(res) == 1)
-        self.assertTrue("objectGUID" in res[0])
-        self.assertTrue("uSNCreated" in res[0])
-        self.assertFalse(res[0]["uSNCreated"][0] == "1") # these are corrected
-        self.assertTrue("uSNChanged" in res[0])
-        self.assertFalse(res[0]["uSNChanged"][0] == "1") # these are corrected
-
-        delete_force(self.ldb, "cn=ldaptestcontainer," + self.base_dn)
-
-        # All this attributes are specificable on add operations
-        self.ldb.add({
-            "dn": "cn=ldaptestcontainer," + self.base_dn,
-            "objectclass": "container",
-            "uSNCreated" : "1",
-            "uSNChanged" : "1",
-            "whenCreated": timestring(long(time.time())),
-            "whenChanged": timestring(long(time.time())) })
-
-        res = ldb.search("cn=ldaptestcontainer," + self.base_dn,
-                         scope=SCOPE_BASE,
-                         attrs=["objectGUID", "uSNCreated", "uSNChanged", \
                "whenCreated", "whenChanged"])
-        self.assertTrue(len(res) == 1)
-        self.assertTrue("objectGUID" in res[0])
-        self.assertTrue("uSNCreated" in res[0])
-        self.assertFalse(res[0]["uSNCreated"][0] == "1") # these are corrected
-        self.assertTrue("uSNChanged" in res[0])
-        self.assertFalse(res[0]["uSNChanged"][0] == "1") # these are corrected
-        self.assertTrue("whenCreated" in res[0])
-        self.assertTrue("whenChanged" in res[0])
-
         # The objectGUID cannot directly be changed
         try:
             self.ldb.modify_ldif("""
@@ -1469,6 +1415,161 @@ objectGUID: bd3480c9-58af-4cd8-92df-bc4a18b6e44d
         delete_force(self.ldb, "cn=parentguidtest,cn=testotherusers," + \
self.base_dn)  delete_force(self.ldb, "cn=testotherusers," + self.base_dn)
 
+    def test_usnChanged(self):
+        """Test usnChanged behaviour"""
+        print "Testing usnChanged behaviour\n"
+
+        self.ldb.add({
+            "dn": "cn=ldaptestcontainer," + self.base_dn,
+            "objectClass": "container" })
+
+        res = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["objectGUID", "uSNCreated", "uSNChanged", \
"whenCreated", "whenChanged", "description"]) +        self.assertTrue(len(res) == 1)
+        self.assertFalse("description" in res[0])
+        self.assertTrue("objectGUID" in res[0])
+        self.assertTrue("uSNCreated" in res[0])
+        self.assertTrue("uSNChanged" in res[0])
+        self.assertTrue("whenCreated" in res[0])
+        self.assertTrue("whenChanged" in res[0])
+
+        delete_force(self.ldb, "cn=ldaptestcontainer," + self.base_dn)
+
+        # All this attributes are specificable on add operations
+        self.ldb.add({
+            "dn": "cn=ldaptestcontainer," + self.base_dn,
+            "objectclass": "container",
+            "uSNCreated" : "1",
+            "uSNChanged" : "1",
+            "whenCreated": timestring(long(time.time())),
+            "whenChanged": timestring(long(time.time())) })
+
+        res = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["objectGUID", "uSNCreated", "uSNChanged", \
"whenCreated", "whenChanged", "description"]) +        self.assertTrue(len(res) == 1)
+        self.assertFalse("description" in res[0])
+        self.assertTrue("objectGUID" in res[0])
+        self.assertTrue("uSNCreated" in res[0])
+        self.assertFalse(res[0]["uSNCreated"][0] == "1") # these are corrected
+        self.assertTrue("uSNChanged" in res[0])
+        self.assertFalse(res[0]["uSNChanged"][0] == "1") # these are corrected
+        self.assertTrue("whenCreated" in res[0])
+        self.assertTrue("whenChanged" in res[0])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+replace: description
+""")
+        
+        res2 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertFalse("description" in res2[0])
+        self.assertEqual(res[0]["usnCreated"], res2[0]["usnCreated"])
+        self.assertEqual(res[0]["usnCreated"], res2[0]["usnChanged"])
+        self.assertEqual(res[0]["usnChanged"], res2[0]["usnChanged"])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+replace: description
+description: test
+""")
+        
+        res3 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertTrue("description" in res3[0])
+        self.assertEqual("test", str(res3[0]["description"][0]))
+        self.assertEqual(res[0]["usnCreated"], res3[0]["usnCreated"])
+        self.assertNotEqual(res[0]["usnCreated"], res3[0]["usnChanged"])
+        self.assertNotEqual(res[0]["usnChanged"], res3[0]["usnChanged"])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+replace: description
+description: test
+""")
+        
+        res4 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertTrue("description" in res4[0])
+        self.assertEqual("test", str(res4[0]["description"][0]))
+        self.assertEqual(res[0]["usnCreated"], res4[0]["usnCreated"])
+        self.assertNotEqual(res3[0]["usnCreated"], res4[0]["usnChanged"])
+        self.assertEqual(res3[0]["usnChanged"], res4[0]["usnChanged"])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+replace: description
+description: test2
+""")
+        
+        res5 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertTrue("description" in res5[0])
+        self.assertEqual("test2", str(res5[0]["description"][0]))
+        self.assertEqual(res[0]["usnCreated"], res5[0]["usnCreated"])
+        self.assertNotEqual(res3[0]["usnChanged"], res5[0]["usnChanged"])
+
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+delete: description
+description: test2
+""")
+        
+        res6 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertFalse("description" in res6[0])
+        self.assertEqual(res[0]["usnCreated"], res6[0]["usnCreated"])
+        self.assertNotEqual(res5[0]["usnChanged"], res6[0]["usnChanged"])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+add: description
+description: test3
+""")
+        
+        res7 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertTrue("description" in res7[0])
+        self.assertEqual("test3", str(res7[0]["description"][0]))
+        self.assertEqual(res[0]["usnCreated"], res7[0]["usnCreated"])
+        self.assertNotEqual(res6[0]["usnChanged"], res7[0]["usnChanged"])
+        
+        ldb.modify_ldif("""
+dn: cn=ldaptestcontainer,""" + self.base_dn + """
+changetype: modify
+delete: description
+""")
+        
+        res8 = ldb.search("cn=ldaptestcontainer," + self.base_dn,
+                         scope=SCOPE_BASE,
+                         attrs=["uSNCreated", "uSNChanged", "description"])
+        self.assertTrue(len(res) == 1)
+        self.assertFalse("description" in res8[0])
+        self.assertEqual(res[0]["usnCreated"], res8[0]["usnCreated"])
+        self.assertNotEqual(res7[0]["usnChanged"], res8[0]["usnChanged"])
+        
+        delete_force(self.ldb, "cn=ldaptestcontainer," + self.base_dn)
+
     def test_groupType_int32(self):
         """Test groupType (int32) behaviour (should appear to be casted to a 32 bit \
signed integer before comparsion)"""  print "Testing groupType (int32) behaviour\n"
-- 
1.7.11.7



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic