Ticket 47921 - indirect cos does not reflect changes in the cos attribute

Bug Description:  Indirect cos results are incorrectly cached, so any changes
                  to entries that are indirect are not returned to the client.

Fix Description:  Do not cache the vattr result if it came from a indirect cos
                  definition.

https://fedorahosted.org/389/ticket/47921

Reviewed by: nhosoi(Thanks!)

dirsrvtests/tickets/ticket47921_test.py

@@ -0,0 +1,155 @@
+import os
+import sys
+import time
+import ldap
+import logging
+import pytest
+from lib389 import DirSrv, Entry, tools, tasks
+from lib389.tools import DirSrvTools
+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
+
+
[email protected](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()
+
+    # Clear out the tmp dir
+    standalone.clearTmpDir(__file__)
+
+    return TopologyStandalone(standalone)
+
+
+def test_ticket47921(topology):
+    '''
+    Test that indirect cos reflects the current value of the indirect entry
+    '''
+
+    INDIRECT_COS_DN = 'cn=cos definition,' + DEFAULT_SUFFIX
+    MANAGER_DN = 'uid=my manager,ou=people,' + DEFAULT_SUFFIX
+    USER_DN = 'uid=user,ou=people,' + DEFAULT_SUFFIX
+
+    # Add COS definition
+    try:
+        topology.standalone.add_s(Entry((INDIRECT_COS_DN,
+            {'objectclass': 'top cosSuperDefinition cosIndirectDefinition ldapSubEntry'.split(),
+             'cosIndirectSpecifier': 'manager',
+             'cosAttribute': 'roomnumber'
+            })))
+    except ldap.LDAPError, e:
+        log.fatal('Failed to add cos defintion, error: ' + e.message['desc'])
+        assert False
+
+    # Add manager entry
+    try:
+        topology.standalone.add_s(Entry((MANAGER_DN,
+            {'objectclass': 'top extensibleObject'.split(),
+             'uid': 'my manager',
+             'roomnumber': '1'
+            })))
+    except ldap.LDAPError, e:
+        log.fatal('Failed to add manager entry, error: ' + e.message['desc'])
+        assert False
+
+    # Add user entry
+    try:
+        topology.standalone.add_s(Entry((USER_DN,
+            {'objectclass': 'top person organizationalPerson inetorgperson'.split(),
+             'sn': 'last',
+             'cn': 'full',
+             'givenname': 'mark',
+             'uid': 'user',
+             'manager': MANAGER_DN
+            })))
+    except ldap.LDAPError, e:
+        log.fatal('Failed to add manager entry, error: ' + e.message['desc'])
+        assert False
+
+    # Test COS is working
+    try:
+        entry = topology.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE,
+                                             "uid=user",
+                                             ['roomnumber'])
+        if entry:
+            if entry[0].getValue('roomnumber') != '1':
+                log.fatal('COS is not working.')
+                assert False
+        else:
+            log.fatal('Failed to find user entry')
+            assert False
+    except ldap.LDAPError, e:
+        log.error('Failed to search for user entry: ' + e.message['desc'])
+        assert False
+
+    # Modify manager entry
+    try:
+        topology.standalone.modify_s(MANAGER_DN, [(ldap.MOD_REPLACE, 'roomnumber', '2')])
+    except ldap.LDAPError, e:
+        log.error('Failed to modify manager entry: ' + e.message['desc'])
+        assert False
+
+    # Confirm COS is returning the new value
+    try:
+        entry = topology.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE,
+                                             "uid=user",
+                                             ['roomnumber'])
+        if entry:
+            if entry[0].getValue('roomnumber') != '2':
+                log.fatal('COS is not working after manager update.')
+                assert False
+        else:
+            log.fatal('Failed to find user entry')
+            assert False
+    except ldap.LDAPError, e:
+        log.error('Failed to search for user entry: ' + e.message['desc'])
+        assert False
+
+    log.info('Test complete')
+
+
+def test_ticket47921_final(topology):
+    topology.standalone.delete()
+    log.info('Testcase PASSED')
+
+
+def run_isolated():
+    global installation1_prefix
+    installation1_prefix = None
+
+    topo = topology(True)
+    test_ticket47921(topo)
+    test_ticket47921_final(topo)
+
+
+if __name__ == '__main__':
+    run_isolated()
+

ldap/servers/plugins/cos/cos_cache.c

@@ -284,7 +284,7 @@ void cos_cache_backend_state_change(void *handle, char *be_name,
 static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_ValueSet** results,int *type_name_disposition, char** actual_type_name, int flags, int *free_flags, void *hint);
 static int cos_cache_vattr_compare(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_Value *test_this, int* result, int flags, void *hint);
 static int cos_cache_vattr_types(vattr_sp_handle *handle,Slapi_Entry *e,vattr_type_list_context *type_context,int flags);
-static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *ops);
+static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *ops, int *indirect_cos);
 
 /* 
 	compares s2 to s1 starting from end of string until the beginning of either
@@ -2096,8 +2096,9 @@ static int cos_cache_attrval_exists(cosAttrValue *pAttrs, const char *val)
 
 static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_ValueSet** results,int *type_name_disposition, char** actual_type_name, int flags, int *free_flags, void *hint)
 {
-	int ret = -1;
 	cos_cache *pCache = 0;
+	int indirect_cos = 0;
+	int ret = -1;
 
 	LDAPDebug( LDAP_DEBUG_TRACE, "--> cos_cache_vattr_get\n",0,0,0);
 	
@@ -2108,10 +2109,15 @@ static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_
 		goto bail;
 	}
 
-	ret = cos_cache_query_attr(pCache, c, e, type, results, NULL, NULL, NULL);
+	ret = cos_cache_query_attr(pCache, c, e, type, results, NULL, NULL, NULL, &indirect_cos);
 	if(ret == 0)
 	{
-        *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES | SLAPI_VIRTUALATTRS_VALUES_CACHEABLE;
+		if(indirect_cos){
+			/* we can't cache indirect cos */
+			*free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES;
+		} else {
+			*free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES | SLAPI_VIRTUALATTRS_VALUES_CACHEABLE;
+		}
         *actual_type_name = slapi_ch_strdup(type);
 		*type_name_disposition = SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_EXACTLY_OR_ALIAS;
 	}
@@ -2138,7 +2144,7 @@ static int cos_cache_vattr_compare(vattr_sp_handle *handle, vattr_context *c, Sl
 		goto bail;
 	}
 
-	ret = cos_cache_query_attr(pCache, c, e, type, NULL, test_this, result, NULL);
+	ret = cos_cache_query_attr(pCache, c, e, type, NULL, test_this, result, NULL, NULL);
 
 	cos_cache_release(pCache);
 
@@ -2179,7 +2185,7 @@ static int cos_cache_vattr_types(vattr_sp_handle *handle,Slapi_Entry *e,
 			lastattr = pCache->ppAttrIndex[index]->pAttrName;
 
 			if(1 == cos_cache_query_attr(pCache, NULL, e, lastattr, NULL, NULL,
-											 NULL, &props))
+											 NULL, &props, NULL))
 			{
 				/* entry contains this attr */
 				vattr_type_thang thang = {0};
@@ -2223,7 +2229,10 @@ bail:
 	overriding and allow the DS logic to pick it up by denying knowledge
 	of attribute
 */
-static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *props)
+static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context,
+                                Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr,
+                                Slapi_Value *test_this, int *result, int *props,
+                                int *indirect_cos)
 {
 	int ret = -1;
 	cosCache *pCache = (cosCache*)ptheCache;
@@ -2420,6 +2429,9 @@ static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Sl
 								if (cos_cache_follow_pointer( context, (char*)slapi_value_get_string(indirectdn),
 									type, &tmp_vals, test_this, result, pointer_flags) == 0)
 								{
+									if(indirect_cos){
+										*indirect_cos = 1;
+									}
 									hit = 1;
 									/* If the caller requested values, set them.  We need
 									 * to append values when we follow multiple pointers DNs. */