Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented May 23, 2014

Improve the algorithm Console::ModelHelper#in_groups_by_tag to consider application types for grouping under a tag even for a type that has previously matched a tag that had only one match.

in_groups_by_tag takes a list of application types and tags and returns two arrays: One with the application types grouped tags that they match and one with application types that have no tags that more than one application type matches.

The algorithm iterates over the array of tags that is passed into it. For each tag in this array, the algorithm pulls out each type that matches the tag. After iterating over the array of types while matching them against the iterator tag, the algorithm either creates a new grouping of all types that matched the tag if there were more than one, or else the algorithm puts any matching type in the "other" group.

The problem is that jbosseap-6 has the "xpaas" tag, which comes before the "java" tag in the array of tags provided to in_groups_by_tag. Thus the jbosseap-6 cartridge is found to match the "xpaas" tag before the "java" tag is considered. However, jbosseap-6 is the only cartridge with the "xpaas" tag, so it is placed on the "other" group before the "java" tag is considered. Thus by the time the "java" tag is considered, jbosseap-6 has already been pulled into the "others" group and is not in the array considered for consideration when matching types against the "java" tag.

This commit changes the algorithm instead to leave types in the array of types for consideration for matching against tags until that type is found to be one of multiple types that match some tag.

This commit fixes bug 1100508.

@Miciah
Copy link
Contributor Author

Miciah commented May 23, 2014

[test]

@openshift-bot
Copy link

@openshift-bot
Copy link

Origin Test Results: SUCCESS (https://50.17.198.52/jenkins/job/test_pull_requests/2411/)

if matches.length > 1
categorized << [tag, matches]
else
uncategorized.concat matches
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appending single matches back to the uncategorized list could change the order of types... not sure if that's a big deal, but would be nice to preserve the order if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liggitt
Copy link
Contributor

liggitt commented May 27, 2014

@Miciah can you add a test for the scenario you describe to test_in_groups_by_tag

Improve the algorithm Console::ModelHelper#in_groups_by_tag to consider
application types for grouping under a tag even for a type that has
previously matched a tag that had only one match.

in_groups_by_tag takes a list of application types and tags and returns two
arrays: One with the application types grouped tags that they match and one
with application types that have no tags that more than one application
type matches.

The algorithm iterates over the array of tags that is passed into it.  For
each tag in this array, the algorithm pulls out each type that matches the
tag.  After iterating over the array of types while matching them against
the iterator tag, the algorithm either creates a new grouping of all types
that matched the tag if there were more than one, or else the algorithm
puts any matching type in the "other" group.

The problem is that jbosseap-6 has the "xpaas" tag, which comes before the
"java" tag in the array of tags provided to in_groups_by_tag.  Thus the
jbosseap-6 cartridge is found to match the "xpaas" tag before the "java"
tag is considered.  However, jbosseap-6 is the only cartridge with the
"xpaas" tag, so it is placed on the "other" group before the "java" tag is
considered.  Thus by the time the "java" tag is considered, jbosseap-6 has
already been pulled into the "others" group and is not in the array
considered for consideration when matching types against the "java" tag.

This commit changes the algorithm instead to leave types in the array of
types for consideration for matching against tags until that type is found
to be one of multiple types that match some tag.

This commit fixes bug 1100508.
Add comments to test_in_groups_by_tag in model_helper_test.rb.
@Miciah
Copy link
Contributor Author

Miciah commented May 30, 2014

I added the test to test_in_groups_by_tag and verified that it detects the bug with the old
in_groups_by_tag algorithm.

[test]

@openshift-bot
Copy link

Evaluated for origin up to fdaf6fb

@liggitt
Copy link
Contributor

liggitt commented May 30, 2014

Looks good, [merge]

@openshift-bot
Copy link

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/5485/) (Image: devenv_4834)

@openshift-bot
Copy link

Evaluated for online up to fdaf6fb

@Miciah
Copy link
Contributor Author

Miciah commented May 30, 2014

Thanks!

openshift-bot pushed a commit that referenced this pull request May 30, 2014
…st-has-jbosseap-6-under-other

Merged by openshift-bot
@openshift-bot openshift-bot merged commit b44bc21 into openshift:master May 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants