Skip to content

Commit 26638f6

Browse files
committed
Fix advanced mapping merge behavior and add comprehensive test
- Fix set_advanced_mapping to properly merge with existing mappings - Add test for existing mappings preservation - Maintain SDK consistency with other configuration methods
1 parent 75fb0b0 commit 26638f6

2 files changed

Lines changed: 45 additions & 4 deletions

File tree

cterasdk/edge/directoryservice.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,14 @@ def set_advanced_mapping(self, mappings):
108108
raise CTERAException('Failed to configure advanced mapping. Not connected to directory services.')
109109

110110
domains = self.domains()
111-
advanced_mapping = self._edge.api.get('/config/fileservices/cifs/idMapping/map')
112-
advanced_mapping = []
111+
existing_mappings = self._edge.api.get('/config/fileservices/cifs/idMapping/map')
112+
mapping_dict = {mapping.domainFlatName: mapping for mapping in existing_mappings}
113113
for mapping in mappings:
114114
if mapping.domainFlatName in domains:
115-
advanced_mapping.append(mapping)
115+
mapping_dict[mapping.domainFlatName] = mapping
116116
else:
117117
logger.warning('Invalid mapping. Could not find domain. %s', {'domain': mapping.domainFlatName})
118+
advanced_mapping = list(mapping_dict.values())
118119

119120
logger.debug('Updating advanced mapping. %s', {
120121
'domains': [mapping.domainFlatName for mapping in advanced_mapping]

tests/ut/edge/test_directory_service.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ def setUp(self):
1717
self._password = 'password'
1818
self._workgroup = "CTERA"
1919
self._domain_flat_name = "CTERA"
20+
self._domain_flat_name_2 = "MYDOMAIN"
2021
self._mapping_min = 2000000
2122
self._mapping_max = 5000000
23+
self._mapping_min_2 = 1000000
24+
self._mapping_max_2 = 1500000
2225
self._dc = '192.168.0.1'
2326
self._ports = [389, 3268, 445]
2427

@@ -144,7 +147,10 @@ def test_set_advanced_mapping(self):
144147
advanced_mapping = [
145148
TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max)
146149
]
147-
self._init_filer(get_response=0, execute_response=execute_response)
150+
existing_mappings = []
151+
get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings)
152+
self._init_filer(execute_response=execute_response)
153+
self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect)
148154
directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping)
149155
self._filer.api.get.assert_has_calls([
150156
mock.call('/status/fileservices/cifs/joinStatus'),
@@ -155,6 +161,30 @@ def test_set_advanced_mapping(self):
155161
actual_param = self._filer.api.put.call_args[0][1]
156162
self._assert_equal_objects(advanced_mapping[0], actual_param[0])
157163

164+
def test_set_advanced_mapping_with_existing_mappings(self):
165+
execute_response = TestEdgeDirectoryService._create_get_domains_response(self._domain_flat_name, self._domain_flat_name_2)
166+
advanced_mapping = [
167+
TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name, self._mapping_min, self._mapping_max)
168+
]
169+
existing_mappings = [
170+
TestEdgeDirectoryService._get_advanced_mapping_object(self._domain_flat_name_2, self._mapping_min_2, self._mapping_max_2)
171+
]
172+
get_response_side_effect = TestEdgeDirectoryService._get_set_advanced_mapping_side_effect(0, existing_mappings)
173+
self._init_filer(execute_response=execute_response)
174+
self._filer.api.get = mock.MagicMock(side_effect=get_response_side_effect)
175+
directoryservice.DirectoryService(self._filer).set_advanced_mapping(advanced_mapping)
176+
self._filer.api.get.assert_has_calls([
177+
mock.call('/status/fileservices/cifs/joinStatus'),
178+
mock.call('/config/fileservices/cifs/idMapping/map')
179+
])
180+
self._filer.api.execute.assert_called_once_with('/status/fileservices/cifs', 'enumDiscoveredDomains')
181+
self._filer.api.put.assert_called_once_with('/config/fileservices/cifs/idMapping/map', mock.ANY)
182+
actual_param = self._filer.api.put.call_args[0][1]
183+
self.assertEqual(len(actual_param), 2)
184+
domain_names = [mapping.domainFlatName for mapping in actual_param]
185+
self.assertIn(self._domain_flat_name, domain_names)
186+
self.assertIn(self._domain_flat_name_2, domain_names)
187+
158188
def test_set_advanced_mapping_raise(self):
159189
self._init_filer(get_response=1)
160190
with self.assertRaises(exceptions.CTERAException) as error:
@@ -253,3 +283,13 @@ def get_response(path):
253283
return cifs_param
254284
return None
255285
return get_response
286+
287+
@staticmethod
288+
def _get_set_advanced_mapping_side_effect(join_status, existing_mappings):
289+
def get_response(path):
290+
if path == '/status/fileservices/cifs/joinStatus':
291+
return join_status
292+
if path == '/config/fileservices/cifs/idMapping/map':
293+
return existing_mappings
294+
return None
295+
return get_response

0 commit comments

Comments
 (0)