Red Hat Bugzilla – Bug 118313
Category#getDefaultAscendants should not add an order to the query
Last modified: 2007-04-18 13:04:18 EDT
Description of problem: The method: public CategoryCollection getDefaultAscendants() { Explicitly adds an ordering based on 'defaultAncestors' before returning the CategoryCollection. This is bogus, since the method has no means of knowing what the caller will be doing with the collection & thus whether this ordering is applicable. Version-Release number of selected component (if applicable): How reproducible: Always Steps to Reproduce: 1. Call cat.getAscendants() 2. Add order based on title 3. Iterate over results Actual results: Results are ordered by ancestors Expected results: Results are ordered by title Additional info:
Dan, I have a quick question. I agree that Category#getDefaultAscendants() should add an order clause to the returned CategoryCollection. While it's easy to remove the offending line of code, we have a small amount of code that seems to expect the returned collection to be ordered by default_ancestors. These clients are //cms/dev/src/com/arsdigita/cms/ui/CategoryForm.java public static String getCategoryPath(Category) //cms/dev/src/com/arsdigita/cms/ui/authoring/ItemCategorySummary.java generateXML //core-platform/dev/src/com/arsdigita/categorization/CategorizedObject.java public Collection getParentCategories(String purposeKey) (The latter is irrelevant and is list only for the sake of completeness.) There is also //cms/dev/src/com/arsdigita/cms/ui/item/Summary.java, but I can't tell from a cursory look whether it makes any assumptions about the ordering of the collection returned by getDefaultAncestors. Do you think it would be sufficient to fix the above two known uses of getDefaultAncestors such they explicitly add the ordering they require? Is it ok not to worry about other unknown uses of getDefaultAncestors that may or may not rely on the (undocumented) fact that the returned collection is currently ordered?
Yes, lets just move the existing ordering into these 3 places which need it. Since we've not officially released this API yet we don't have to care about breaking other peoples code here ;-) I'll check APLAWS codebase too.
Fixed on the trunk in change 41366.