Inconsistent attribute names inside Automate Engine #13440
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@tinaafitz @gmcculloug |
https://bugzilla.redhat.com/show_bug.cgi?id=1408482 Inside the Automate Engine we convert the attribute name for simple data types to lowercase, but for vmdb or Array types we dont convert the attribute name to lowercase. This PR converts all attribute names to lowercase. This is inconsistent. e.g. VM::VM=100&Name=fred would yield 2 attribute names VM and name. The "name" got converted to lowercase but VM didn't. When the methods access these variables using $evm.root we internally convert it to lowercase at access time $evm.root['VM'] gets converted to $evm.root['vm'] $evm.root['vm'] gets converted to $evm.root['vm'] $evm.root['Name'] gets converted to $evm.root['Name'] If the caller passed in VM::VM=100&Name=fred $evm.root['VM'] fails because the method layer converts it to lowercase and internally it has preserved its original case This is not that prevalent since most of the attribute names that embed the class type are generated internally and they always lowercase the name. We have seen this manifest itself when using Tag controls using dialogs and the user names the tag with mixed case e.g. TagControl would be stored in the engine as dialog_TagControl but when the methods try to access $evm.root['dialog_TagControl'] the method layer would lowercase it and not find the attribute.
mkanoor
force-pushed
the
bugzilla_1408482
branch
from
637a044 to
e6bff3a
Compare
gmcculloug
reviewed
Jan 11, 2017
| expect(result["vms"].length).to eq(1) | ||
| expect(result["name"]).to eq("fred") | ||
| expect(result["Name"]).to be_nil | ||
| expect(result["VMs"]).to be_nil |
There was a problem hiding this comment.
Minor, but can you move this check for VMs next to the vms tests so they are grouped together. Makes it clearer what you are testing.
|
One minor comment on the spec both otherwise this looks good. |
tinaafitz
reviewed
Jan 11, 2017
| @@ -273,7 +273,7 @@ def process_args_attribute(args, args_key) | |||
| key, klass = get_key_name_and_klass_from_key(args_key) | |||
| value = args.delete(args_key) | |||
| args["#{key}_id"] = value unless @attributes.key?(key) | |||
| args[key] = MiqAeObject.convert_value_based_on_datatype(value, klass) | |||
| args[key.downcase] = MiqAeObject.convert_value_based_on_datatype(value, klass) | |||
There was a problem hiding this comment.
@mkanoor The key is already lower case here.
|
Checked commits mkanoor/manageiq@c3379ae~...18c82f1 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
|
Backported to Euwe via #13545 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Inside the Automate Engine we convert the attribute name for
simple data types to lowercase, but for Array types we dont
convert the attribute name to lowercase.
This PR converts all attribute names to lowercase.
This is inconsistent.
e.g. Array::VM=VM::100&Name=fred
would yield 2 attribute names VM and name. The "name" got converted to
lowercase but VM didn't.
When the methods access these variables using $evm.root we internally
convert it to lowercase at access time
$evm.root['VM'] gets converted to $evm.root['vm']
$evm.root['vm'] gets converted to $evm.root['vm']
$evm.root['Name'] gets converted to $evm.root['name']
If the caller passed in Array::VM=VM::100&Name=fred
$evm.root['VM'] fails because the method layer converts it to lowercase
and internally it has preserved its original case
This is not that prevalent since most of the attribute names that embed
the class type are generated internally and they always lowercase the
name. We have seen this manifest itself when using Tag controls using
dialogs and the user names the tag with mixed case e.g.
TagControl would be stored in the engine as dialog_TagControl but when
the methods try to access $evm.root['dialog_TagControl'] the method
layer would lowercase it and not find the attribute.
Links
Steps for Testing/QA
Screen Shot attached
