[WIP] prune down get_homonymic

[kbrock]

Right now, this is opening a conversation with @lfu

As far as I can tell, get_homonymic is basically find_best_match_by

• get_homonymic there are 2 very different signatures depending upon if you have a relative_path or a fqname.
• find_best_match_by doesn't check that the domains are enabled.
• get_homonymic typically doesn't check if the domain is enabled (expected the default to be to check).
• get_homonymic handles urls - not sure if this is a valid use case.
• find_best_match_by needs to be split out concepts of visible_by_user(), domain_priority_order, best_match (with a where), and find_by (use a first)
• find_best_match_by allows wild characters for some models. Do we really need this? Where is this actually valid?
• the lookup by user concept feels like it belongs in rbac and not here.

Could we consolidate this functionality and put into the models, RelativePathMixin, or MiqAeDatastore.

Since this is basically an enhanced find_by, and you need to pass self.class into the get_homonymic methods, I'm leaning towards moving it into RelativePathMixin

There are a few changes in the models, but they are basically consolidating to one method call.

Wanted to know where I should consolidate this stuff before I go all in with these changes.

There is a lot of code around homonymic and it is very inefficient the way it looks up domains and sorts the methods/instances/...

Usage:

• get_homonymic is used in 3 places in the ui, 1 of those calls being to the generic klass.
• find_best_match_by is called in 1 place byMiqAeDomainSearch- the call uses a genericklass`.

[miq-bot]

Checked commit https://github.com/kbrock/manageiq-automation_engine/commit/eb121c15f93170ddd76f9d8bd287bfe12f626ac5 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint 1 file checked, 5 offenses detected

app/models/miq_ae_datastore.rb

• [ ] :warning: - Line 268, Col 57 - Lint/UselessAssignment - Useless assignment to variable - enabled.
• [ ] :exclamation: - Line 257, Col 8 - Performance/RegexpMatch - Use match? instead of =~ when MatchData is not used.
• [ ] :exclamation: - Line 267, Col 24 - Layout/SpaceAfterComma - Space missing after comma.
• [ ] :exclamation: - Line 267, Col 30 - Layout/SpaceAfterComma - Space missing after comma.
• [ ] :exclamation: - Line 268, Col 72 - Style/HashSyntax - Use hash rockets syntax.

[coveralls]

Pull Request Test Coverage Report for Build 4351

• 4 of 15 (26.67%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 85.52%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
app/models/miq_ae_datastore.rb	4	15	26.67%
<!--	Total:	4	15	26.67%	-->

Totals
Change from base Build 4325:	-0.2%
Covered Lines:	5032
Relevant Lines:	5884

---

💛 - Coveralls

[miq-bot]

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

[kbrock]

@Fryguy I'm guessing we want to close all automate performance efforts?

Lucy and I were making headway (e.g.: #439) mostly in the realm of deleting inconsistent lookup logic so we could just pass objects around. Having Domain in core and Namespace in this repo made this work tricky.

Underlying issue:

• methods accept a path string
• use the path to lookup an object
• do work
• return a path.
• Then the next method used the path or a child path
• looks up an object,
• ...

Even though domains are very ancestry and includes friendly, they currently are not used and basically force N+1's.

My knowledge is rusty, and I don't have Lucy as a resource. I also am a little gunshy introducing issues in bugs

Verifying: we want to just close these efforts

[Fryguy]

Yeah if we're moving towards workflows let's not put effort into it