Blob Blame History Raw
From 3b3faee01e645577ad77ff4f38429a9e0806231b Mon Sep 17 00:00:00 2001
From: Simon Pichugin <simon.pichugin@gmail.com>
Date: Tue, 16 Jun 2020 20:35:05 +0200
Subject: [PATCH] Issue 51157 - Reindex task may create abandoned index file

Bug Description: Recreating an index for the same attribute but changing
the case of for example 1 letter, results in abandoned indexfile.

Fix Decsription: Add a test case to a newly created 'indexes' test suite.
When we remove the index config from the backend, - remove the attribute
info from LDBM instance attributes.

https://pagure.io/389-ds-base/issue/51157

Reviewed by: firstyear, mreynolds (Thanks!)
---
 dirsrvtests/tests/suites/indexes/__init__.py  |   3 +
 .../tests/suites/indexes/regression_test.py   | 125 ++++++++++++++++++
 ldap/servers/slapd/back-ldbm/ldbm_attr.c      |   7 +
 .../slapd/back-ldbm/ldbm_index_config.c       |   3 +
 .../servers/slapd/back-ldbm/proto-back-ldbm.h |   1 +
 5 files changed, 139 insertions(+)
 create mode 100644 dirsrvtests/tests/suites/indexes/__init__.py
 create mode 100644 dirsrvtests/tests/suites/indexes/regression_test.py

diff --git a/dirsrvtests/tests/suites/indexes/__init__.py b/dirsrvtests/tests/suites/indexes/__init__.py
new file mode 100644
index 000000000..04441667e
--- /dev/null
+++ b/dirsrvtests/tests/suites/indexes/__init__.py
@@ -0,0 +1,3 @@
+"""
+   :Requirement: 389-ds-base: Indexes
+"""
diff --git a/dirsrvtests/tests/suites/indexes/regression_test.py b/dirsrvtests/tests/suites/indexes/regression_test.py
new file mode 100644
index 000000000..1a71f16e9
--- /dev/null
+++ b/dirsrvtests/tests/suites/indexes/regression_test.py
@@ -0,0 +1,125 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2020 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import time
+import os
+import pytest
+import ldap
+from lib389._constants import DEFAULT_BENAME, DEFAULT_SUFFIX
+from lib389.index import Indexes
+from lib389.backend import Backends
+from lib389.idm.user import UserAccounts
+from lib389.topologies import topology_st as topo
+
+pytestmark = pytest.mark.tier1
+
+
+def test_reindex_task_creates_abandoned_index_file(topo):
+    """
+    Recreating an index for the same attribute but changing
+    the case of for example 1 letter, results in abandoned indexfile
+
+    :id: 07ae5274-481a-4fa8-8074-e0de50d89ac6
+    :setup: Standalone instance
+    :steps:
+        1. Create a user object with additional attributes:
+           objectClass: mozillaabpersonalpha
+           mozillaCustom1: xyz
+        2. Add an index entry mozillacustom1
+        3. Reindex the backend
+        4. Check the content of the index (after it has been flushed to disk) mozillacustom1.db
+        5. Remove the index
+        6. Notice the mozillacustom1.db is removed
+        7. Recreate the index but now use the exact case as mentioned in the schema
+        8. Reindex the backend
+        9. Check the content of the index (after it has been flushed to disk) mozillaCustom1.db
+        10. Check that an ldapsearch does not return a result (mozillacustom1=xyz)
+        11. Check that an ldapsearch returns the results (mozillaCustom1=xyz)
+        12. Restart the instance
+        13. Notice that an ldapsearch does not return a result(mozillacustom1=xyz)
+        15. Check that an ldapsearch does not return a result (mozillacustom1=xyz)
+        16. Check that an ldapsearch returns the results (mozillaCustom1=xyz)
+        17. Reindex the backend
+        18. Notice the second indexfile for this attribute
+        19. Check the content of the index (after it has been flushed to disk) no mozillacustom1.db
+        20. Check the content of the index (after it has been flushed to disk) mozillaCustom1.db
+    :expectedresults:
+        1. Should Success.
+        2. Should Success.
+        3. Should Success.
+        4. Should Success.
+        5. Should Success.
+        6. Should Success.
+        7. Should Success.
+        8. Should Success.
+        9. Should Success.
+        10. Should Success.
+        11. Should Success.
+        12. Should Success.
+        13. Should Success.
+        14. Should Success.
+        15. Should Success.
+        16. Should Success.
+        17. Should Success.
+        18. Should Success.
+        19. Should Success.
+        20. Should Success.
+    """
+
+    inst = topo.standalone
+    attr_name = "mozillaCustom1"
+    attr_value = "xyz"
+
+    users = UserAccounts(inst, DEFAULT_SUFFIX)
+    user = users.create_test_user()
+    user.add("objectClass", "mozillaabpersonalpha")
+    user.add(attr_name, attr_value)
+
+    backends = Backends(inst)
+    backend = backends.get(DEFAULT_BENAME)
+    indexes = backend.get_indexes()
+    index = indexes.create(properties={
+        'cn': attr_name.lower(),
+        'nsSystemIndex': 'false',
+        'nsIndexType': ['eq', 'pres']
+        })
+
+    backend.reindex()
+    time.sleep(3)
+    assert os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name.lower()}.db")
+    index.delete()
+    assert not os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name.lower()}.db")
+
+    index = indexes.create(properties={
+        'cn': attr_name,
+        'nsSystemIndex': 'false',
+        'nsIndexType': ['eq', 'pres']
+        })
+
+    backend.reindex()
+    time.sleep(3)
+    assert not os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name.lower()}.db")
+    assert os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name}.db")
+
+    entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, f"{attr_name}={attr_value}")
+    assert len(entries) > 0
+    inst.restart()
+    entries = inst.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, f"{attr_name}={attr_value}")
+    assert len(entries) > 0
+
+    backend.reindex()
+    time.sleep(3)
+    assert not os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name.lower()}.db")
+    assert os.path.exists(f"{inst.ds_paths.db_home_dir}/{DEFAULT_BENAME}/{attr_name}.db")
+
+
+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/ldbm_attr.c b/ldap/servers/slapd/back-ldbm/ldbm_attr.c
index f0d418572..688c4f137 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_attr.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_attr.c
@@ -98,6 +98,13 @@ ainfo_cmp(
     return (strcasecmp(a->ai_type, b->ai_type));
 }
 
+void
+attrinfo_delete_from_tree(backend *be, struct attrinfo *ai)
+{
+    ldbm_instance *inst = (ldbm_instance *)be->be_instance_info;
+    avl_delete(&inst->inst_attrs, ai, ainfo_cmp);
+}
+
 /*
  * Called when a duplicate "index" line is encountered.
  *
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_index_config.c b/ldap/servers/slapd/back-ldbm/ldbm_index_config.c
index 720f93036..9722d0ce7 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_index_config.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_index_config.c
@@ -201,7 +201,10 @@ ldbm_instance_index_config_delete_callback(Slapi_PBlock *pb,
             *returncode = LDAP_UNWILLING_TO_PERFORM;
             rc = SLAPI_DSE_CALLBACK_ERROR;
         }
+        attrinfo_delete_from_tree(inst->inst_be, ainfo);
     }
+    /* Free attrinfo structure */
+    attrinfo_delete(&ainfo);
 bail:
     return rc;
 }
diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
index a07acee5e..4d2524fd9 100644
--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
@@ -21,6 +21,7 @@
  */
 struct attrinfo *attrinfo_new(void);
 void attrinfo_delete(struct attrinfo **pp);
+void attrinfo_delete_from_tree(backend *be, struct attrinfo *ai);
 void ainfo_get(backend *be, char *type, struct attrinfo **at);
 void attr_masks(backend *be, char *type, int *indexmask, int *syntaxmask);
 void attr_masks_ex(backend *be, char *type, int *indexmask, int *syntaxmask, struct attrinfo **at);
-- 
2.26.2