From f82ad8baa4003e0c58711f11321513e821a7fc6f Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Mon, 7 Dec 2015 16:45:06 -0500 Subject: [PATCH 367/368] Ticket 48370 - The 'eq' index does not get updated properly when deleting and re-adding attributes in the same modify operation Bug Description: If you delete several values of the same attribute, and add at least one of them back in the same operation, the equality index does not get updated. Fix Description: Modify the logic of the index code to update the index if at least one of the values in the entry changes. Also did pep8 cleanup of create_test.py https://fedorahosted.org/389/ticket/48370 Reviewed by: wibrown(Thanks!) (cherry picked from commit 63b80b5c31ebda51445c662903a28e2a79ebe60a) (cherry picked from commit 5cd8f73205007ecbd44ae2fbfb5bcdf7e39c3d6e) (cherry picked from commit 8e49d6db6973078396b869ab4ed59c565d7010a9) (cherry picked from commit c7cf0001002da1bcabe0371d9511a014d8e2b16f) --- dirsrvtests/tickets/ticket48370_test.py | 236 ++++++++++++++++++++++++++++++++ ldap/servers/slapd/back-ldbm/index.c | 29 ++-- 2 files changed, 247 insertions(+), 18 deletions(-) create mode 100644 dirsrvtests/tickets/ticket48370_test.py diff --git a/dirsrvtests/tickets/ticket48370_test.py b/dirsrvtests/tickets/ticket48370_test.py new file mode 100644 index 0000000..f5b1f47 --- /dev/null +++ b/dirsrvtests/tickets/ticket48370_test.py @@ -0,0 +1,236 @@ +import os +import ldap +import logging +import pytest +from lib389 import DirSrv, Entry +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from lib389.utils import * + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + +installation1_prefix = None + + +class TopologyStandalone(object): + def __init__(self, standalone): + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + global installation1_prefix + if installation1_prefix: + args_instance[SER_DEPLOYED_DIR] = installation1_prefix + + # Creating standalone instance ... + standalone = DirSrv(verbose=False) + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + instance_standalone = standalone.exists() + if instance_standalone: + standalone.delete() + standalone.create() + standalone.open() + + # Delete each instance in the end + def fin(): + standalone.delete() + request.addfinalizer(fin) + + # Clear out the tmp dir + standalone.clearTmpDir(__file__) + + return TopologyStandalone(standalone) + + +def test_ticket48370(topology): + """ + Deleting attirbute values and readding a value does not properly update + the pres index. The values are not actually deleted from the index + """ + + DN = 'uid=user0099,' + DEFAULT_SUFFIX + + # + # Add an entry + # + topology.standalone.add_s(Entry((DN, { + 'objectclass': ['top', 'person', + 'organizationalPerson', + 'inetorgperson', + 'posixAccount'], + 'givenname': 'test', + 'sn': 'user', + 'loginshell': '/bin/bash', + 'uidNumber': '10099', + 'gidNumber': '10099', + 'gecos': 'Test User', + 'mail': ['user0099@dev.null', + 'alias@dev.null', + 'user0099@redhat.com'], + 'cn': 'Test User', + 'homeDirectory': '/home/user0099', + 'uid': 'admin2', + 'userpassword': 'password'}))) + + # + # Perform modify (delete & add mail attributes) + # + try: + topology.standalone.modify_s(DN, [(ldap.MOD_DELETE, + 'mail', + 'user0099@dev.null'), + (ldap.MOD_DELETE, + 'mail', + 'alias@dev.null'), + (ldap.MOD_ADD, + 'mail', 'user0099@dev.null')]) + except ldap.LDAPError as e: + log.fatal('Failedto modify user: ' + str(e)) + assert False + + # + # Search using deleted attribute value- no entries should be returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=alias@dev.null') + if entry: + log.fatal('Entry incorrectly returned') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + # + # Search using existing attribute value - the entry should be returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=user0099@dev.null') + if entry is None: + log.fatal('Entry not found, but it should have been') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + # + # Delete the last values + # + try: + topology.standalone.modify_s(DN, [(ldap.MOD_DELETE, + 'mail', + 'user0099@dev.null'), + (ldap.MOD_DELETE, + 'mail', + 'user0099@redhat.com') + ]) + except ldap.LDAPError as e: + log.fatal('Failed to modify user: ' + str(e)) + assert False + + # + # Search using deleted attribute value - no entries should be returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=user0099@redhat.com') + if entry: + log.fatal('Entry incorrectly returned') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + # + # Make sure presence index is correctly updated - no entries should be + # returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=*') + if entry: + log.fatal('Entry incorrectly returned') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + # + # Now add the attributes back, and lets run a different set of tests with + # a different number of attributes + # + try: + topology.standalone.modify_s(DN, [(ldap.MOD_ADD, + 'mail', + ['user0099@dev.null', + 'alias@dev.null'])]) + except ldap.LDAPError as e: + log.fatal('Failedto modify user: ' + str(e)) + assert False + + # + # Remove and readd some attibutes + # + try: + topology.standalone.modify_s(DN, [(ldap.MOD_DELETE, + 'mail', + 'alias@dev.null'), + (ldap.MOD_DELETE, + 'mail', + 'user0099@dev.null'), + (ldap.MOD_ADD, + 'mail', 'user0099@dev.null')]) + except ldap.LDAPError as e: + log.fatal('Failedto modify user: ' + str(e)) + assert False + + # + # Search using deleted attribute value - no entries should be returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=alias@dev.null') + if entry: + log.fatal('Entry incorrectly returned') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + # + # Search using existing attribute value - the entry should be returned + # + try: + entry = topology.standalone.search_s(DEFAULT_SUFFIX, + ldap.SCOPE_SUBTREE, + 'mail=user0099@dev.null') + if entry is None: + log.fatal('Entry not found, but it should have been') + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search for user: ' + str(e)) + assert False + + log.info('Test PASSED') + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE) diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c index 90d1d23..0b41ce4 100644 --- a/ldap/servers/slapd/back-ldbm/index.c +++ b/ldap/servers/slapd/back-ldbm/index.c @@ -740,31 +740,24 @@ index_add_mods( flags = BE_INDEX_DEL|BE_INDEX_PRESENCE|BE_INDEX_EQUALITY; } else { flags = BE_INDEX_DEL; - - /* If the same value doesn't exist in a subtype, set - * BE_INDEX_EQUALITY flag so the equality index is - * removed. - */ curr_attr = NULL; slapi_entry_attr_find(olde->ep_entry, - mods[i]->mod_type, &curr_attr); + mods[i]->mod_type, + &curr_attr); if (curr_attr) { - int found = 0; for (j = 0; mods_valueArray[j] != NULL; j++ ) { - if ( slapi_valueset_find(curr_attr, all_vals, mods_valueArray[j])) { - /* The same value found in evals. - * We don't touch the equality index. */ - found = 1; + if ( !slapi_valueset_find(curr_attr, all_vals, mods_valueArray[j]) ) { + /* + * If the mod del value is not found in all_vals + * we need to update the equality index as the + * final value(s) have changed + */ + if (!(flags & BE_INDEX_EQUALITY)) { + flags |= BE_INDEX_EQUALITY; + } break; } } - /* - * to-be-deleted curr_attr does not exist in the - * new value set evals. So, we can remove it. - */ - if (!found && !(flags & BE_INDEX_EQUALITY)) { - flags |= BE_INDEX_EQUALITY; - } } } -- 2.4.3