Make get_all_collections properly recursive (#1361)

* Make `get_all_collections` properly recursive

* Update changelog

---------

Co-authored-by: Pete Gadomski <[email protected]>

Author: jsignell

CHANGELOG.md

@@ -7,6 +7,10 @@
 - Allow object ID as input for getting APILayoutStrategy hrefs and add `items`, `collections`, `search`, `conformance`, `service_desc` and `service_doc` href methods. ([#1335](https://github.com/stac-utils/pystac/pull/1335))
 - Update docstring of `name` argument to `Classification.apply` and `Classification.create` to agree with extension specification. ([#1356](https://github.com/stac-utils/pystac/pull/1356))
 
+### Fixed
+
+- Make `get_all_collections` properly recursive ([#1361](https://github.com/stac-utils/pystac/pull/1361))
+
 ## [v1.10.1] - 2024-05-03
 
 ### Fixed

pystac/catalog.py

@@ -457,7 +457,7 @@ def get_all_collections(self) -> Iterable[Collection]:
         any subcatalogs recursively."""
         yield from self.get_collections()
         for child in self.get_children():
-            yield from child.get_collections()
+            yield from child.get_all_collections()
 
     def get_child_links(self) -> list[Link]:
         """Return all child links of this catalog.

tests/test_catalog.py

@@ -35,7 +35,13 @@
     make_posix_style,
     make_relative_href,
 )
-from tests.utils import ARBITRARY_BBOX, ARBITRARY_GEOM, MockStacIO, TestCases
+from tests.utils import (
+    ARBITRARY_BBOX,
+    ARBITRARY_EXTENT,
+    ARBITRARY_GEOM,
+    MockStacIO,
+    TestCases,
+)
 
 
 class CatalogTypeTest(unittest.TestCase):
@@ -1350,7 +1356,7 @@ def test_get_all_collections(self) -> None:
         catalog = TestCases.case_1()
         all_collections = list(catalog.get_all_collections())
 
-        assert len(all_collections) > 0
+        assert len(all_collections) == 4
         assert all(isinstance(c, pystac.Collection) for c in all_collections)
 
     def test_get_single_links_media_type(self) -> None:
@@ -1639,7 +1645,9 @@ def nested_catalog() -> pystac.Catalog:
     └── variables
         ├── catalog.json
         └── variable_a
-            ├── catalog.json
+            └── catalog.json
+                └── variable_a_1
+                    └── collection.json
     """
     root = pystac.Catalog("root", "root")
     variables = pystac.Catalog("variables", "variables")
@@ -1654,9 +1662,22 @@ def nested_catalog() -> pystac.Catalog:
     variables.add_child(variable_a)
     products.add_child(product_a)
 
+    variable_a_1 = pystac.Collection(
+        "variable_a_1", "variable_a_1", extent=ARBITRARY_EXTENT
+    )
+    variable_a.add_child(variable_a_1)
+
     return root
 
 
+def test_get_all_collections_deeply_nested(nested_catalog: pystac.Catalog) -> None:
+    catalog = nested_catalog
+    all_collections = list(catalog.get_all_collections())
+
+    assert len(all_collections) == 1
+    assert all(isinstance(c, pystac.Collection) for c in all_collections)
+
+
 def test_set_parent_false_stores_in_proper_place_on_normalize_and_save(
     nested_catalog: pystac.Catalog, tmp_path: Path
 ) -> None: