[WIP] Miq expression interp
parent:
- https://github.com/ManageIQ/manageiq/issues/22397
Made some progress
I wanted to see what our code looks like for preprocessing the whole tree up front. The code was parsing the same fields over and over for to_ruby. The thought is if to_ruby has all the data it needs, then an interpreter will have all the data it needs.
- don't lookup fields multiple times in to_ruby but rather once in the preprocessor
- don't lookup fields and validate in valid? but rather one time in the preprocessor
- embed field objets in the expr tree itself.
- drop
col_detailsand just use the cached field value from the preprocessor
There are still a few places to remove more Field.parse calls, but seems good enough.
From here we want to transition focus on Condition.subst.
This is where we spend the most of our time and create the most objects.
If we can verify that we don't use mode="tag_expr", then we can avoid the _to_ruby.gsub() { _subst}.gsub() { _subst } code and have the to_ruby produce the correct code in the first place.
@Fryguy do you know anything about mode="tag_expr" and if we would have customers still using that?
@Fryguy do you know anything about
mode="tag_expr"and if we would have customers still using that?
I did a little spelunking, and in 2008 @gtanzillo (Hi Greg!) introduced mode=tag_expr_v2 in 37762294131e0cc24e05adf13fa5bbc85fa19200 (pre-open-source), and replaced its usage in a bunch of seeded policies to use that. Below is an example diff of one of those changes where you can see the old format and the new format. Note that tag_expr_v2 is the MiqExpression format, as you can see in https://github.com/ManageIQ/manageiq/blob/master/app/models/condition.rb#L68 where it's used directly in MiqExpression.
Maybe @gtanzillo can remember anything here on this old format?
Even so, I'd be super-surprised if tag_expr still existed in any form for existing users/customers as 2008 was the big switch. git grep and org wide searches tell me this isn't used anymore and we can drop it.
@ vmdb/product/policies/env_prod_vm_host.yml:7 @ modifier: "allow"
who: "system"
what: "start"
rule:
- mode: "tag_expr"
- expr: |-
- <exist ref=host>/managed/environment/prod</exist> &&
- <exist>/managed/environment/prod</exist>
+ mode: "tag_expr_v2"
+ expr:
+ and:
+ - exist:
+ field: Host.managed-environment
+ value: prod
+ - exist:
+ field: Vm.managed-environment
+ value: prod
update:
- rebase
lib/miq_expression.rb
- [ ] :exclamation: - Line 335, Col 11 - Style/HashTransformValues - Prefer
transform_valuesovereach_with_object. - [ ] :exclamation: - Line 356, Col 14 - Performance/CollectionLiteralInLoop - Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.