Skip to content

Commit

Permalink
Fixes issue # 1887 - Mock Modify Instance property case sensitivity.
Browse files Browse the repository at this point in the history
Fixes issue with case sensitivity of property names in modify instance.

Note that we refactored code slightly in the method to eliminate an
unnecessary pair of list comprehenshions and make internal var
names clearer.

Adds test for the documented condtion and removes one duplicated
test.
  • Loading branch information
KSchopmeyer committed Oct 12, 2019
1 parent 4e82ab6 commit a9a69af
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 22 deletions.
10 changes: 8 additions & 2 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,17 @@ Released: not yet

* Test: Fixed args of WBEMOperation methods in mock unit tests & function tests.

* Code: Fixed pywbem_mock issue where CreateInstance was not handling the case
sensitivityof property cases if the instance property name case was different than the
* Code: Fixed pywbem_mock issue where CreateInstance was not handling the case
sensitivity of property cases if the instance property name case was different than the
class property name case. While not legally incorrect the created instance
looks bad. See issue #1883

* Code: Fixed pywbem_mock issue where ModifyInstance not handling case
sensitivity of property cases if the instance property name case was
different than the class property name case. Modify failed if
the case of property names did not match. Fixed the case test error and
put the class defined proerty name into the modified instance. See issue #1887

**Enhancements:**

* Changed GetCentralInstances methodology in WBEMServer.get_central_instances()
Expand Down
30 changes: 16 additions & 14 deletions pywbem_mock/_wbemconnection_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -2286,11 +2286,6 @@ def _fake_modifyinstance(self, namespace, **params):
modified_instance.classname, namespace))
raise

# get key properties and all class props
cl_props = [p.name for p in six.itervalues(target_class.properties)]
key_props = [p.name for p in six.itervalues(target_class.properties)
if 'key' in p.qualifiers]

# Get original instance in repo. Does not copy the orig instance.
mod_inst_path = modified_instance.path.copy()
if modified_instance.path.namespace is None:
Expand All @@ -2313,26 +2308,27 @@ def _fake_modifyinstance(self, namespace, **params):
# are in the class
if property_list:
for p in property_list:
if p not in cl_props:
if p not in target_class.properties:
raise CIMError(
CIM_ERR_INVALID_PARAMETER,
_format("Property {0!A} in PropertyList not in class "
"{1!A}", p, modified_instance.classname))
for p in modified_instance:
if p not in cl_props:
if p not in target_class.properties:
raise CIMError(
CIM_ERR_INVALID_PARAMETER,
_format("Property {0!A} in ModifiedInstance not in class "
"{1!A}", p, modified_instance.classname))

# Set the class value for properties in the property list but not
# in the modified_instance. This sets just the value component.
mod_inst_props = set(modified_instance.keys())
new_props = mod_inst_props.difference(set(cl_props))
if new_props:
for new_prop in new_props:
modified_instance[new_prop] = \
target_class.properties[new_prop].value
mod_inst_props = set([k.lower() for k in modified_instance.keys()])
cl_props = [pn.lower() for pn in target_class.properties]
chngd_props = mod_inst_props.difference(set(cl_props))
if chngd_props:
for prop in chngd_props:
modified_instance[prop] = \
target_class.properties[prop].value

# Remove all properties that do not change value between original
# instance and modified instance
Expand All @@ -2341,6 +2337,8 @@ def _fake_modifyinstance(self, namespace, **params):
del modified_instance[p]

# Confirm no key properties in remaining modified instance
key_props = [p.name for p in six.itervalues(target_class.properties)
if 'key' in p.qualifiers]
for p in key_props:
if p in modified_instance:
raise CIMError(
Expand All @@ -2350,7 +2348,7 @@ def _fake_modifyinstance(self, namespace, **params):

# Remove any properties from modified instance not in the property_list
if property_list:
for p in list(modified_instance):
for p in list(modified_instance): # create list before loop
if p not in property_list:
del modified_instance[p]

Expand All @@ -2375,6 +2373,10 @@ def _fake_modifyinstance(self, namespace, **params):
"or other attributes do not match: "
"instance={1!A}, class={2!A}",
pname, iprop, cprop))
# If case of modified_instance property != case of class property
# change the name in the modified_instance
if iprop.name != cprop.name:
modified_instance.properties[iprop.name].name = cprop.name

# Modify the value of properties in the repo with those from
# modified instance
Expand Down
21 changes: 15 additions & 6 deletions tests/unittest/pywbem_mock/test_wbemconnection_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -3744,6 +3744,8 @@ def test_createinstance_lite(self, conn_lite, tst_instances, ns):
# change any property that is different
[0, ['cimfoo_sub', 'newval'], None, True],
# change any property that is different with prop case different
[0, ['CIMFOO_SUB', 'newval'], None, True],
# change this property only
[0, ['cimfoo_sub', 'newval'], ['cimfoo_sub'], True],
# Duplicate in property list
Expand All @@ -3766,11 +3768,10 @@ def test_createinstance_lite(self, conn_lite, tst_instances, ns):
# change any property that is different, no prop list
[0, [('cimfoo_sub', 'newval'),
('cimfoo_sub_sub', 'newval2'), ], None, True],
# change any property that is different, no prop list, p case diff
[0, [('CIMFOO_SUB', 'newvalx'),
('CIMFOO_SUB_sub', 'newval2x'), ], None, True],
# Invalid change, Key property
# change any property that is different, no prop list
[0, ['InstanceID', 'newval'], None, CIMError(CIM_ERR_NOT_FOUND)],
# Invalid change, Key property
# change any property that is different, no prop list
[0, ['InstanceID', 'newval'], None, CIMError(CIM_ERR_NOT_FOUND)],
# Bad namespace. Depends on special code in path
[2, ['cimfoo_sub', 'newval'], None,
Expand Down Expand Up @@ -3838,12 +3839,20 @@ def test_modifyinstance(self, conn, ns, sp, nv, pl, exp_resp,
conn.ModifyInstance(modify_instance, PropertyList=pl)

rtn_instance = conn.GetInstance(modify_instance.path)
tst_class = conn.GetClass(modify_instance.path.classname,
LocalOnly=False)
if exp_resp:
for p in rtn_instance:
assert rtn_instance[p] == modify_instance[p]
assert rtn_instance.properties[p] == \
modify_instance.properties[p]
assert rtn_instance.properties[p].name == \
tst_class.properties[p].name
else:
for p in rtn_instance:
assert rtn_instance[p] == orig_instance[p]
assert rtn_instance.properties[p] == \
orig_instance.properties[p]
assert rtn_instance.properties[p].name == \
tst_class.properties[p].name

def test_modifyinstance_lite(self, conn_lite, tst_instances):
# pylint: disable=no-self-use
Expand Down

0 comments on commit a9a69af

Please sign in to comment.