diff --git a/digital_land/phase/lookup.py b/digital_land/phase/lookup.py index 56b44a59..15bd4727 100644 --- a/digital_land/phase/lookup.py +++ b/digital_land/phase/lookup.py @@ -152,18 +152,29 @@ def _check_entity_range(self, entity, line_number=0): ) def redirect_entity(self, entity): - """ - Given an entity number can check the redirect lookups to see if the entity - has been removed or redirected. - """ - if self.redirect_lookups: - redirect_entity = self.redirect_lookups.get(entity, "") - if redirect_entity: - if redirect_entity["status"] == "301": - return redirect_entity["entity"] - elif redirect_entity["status"] == "410": - return "" - return entity + """Follow redirects until the entity has no further redirect.""" + current_entity = entity + visited_entities = set() + + while current_entity in self.redirect_lookups: + if current_entity in visited_entities: + raise ValueError( + f"Cycle found while following entity redirects from {entity}" + ) + + visited_entities.add(current_entity) + redirect = self.redirect_lookups[current_entity] + status = redirect["status"] + + if status == "410": + return "" + + if status != "301": + return current_entity + + current_entity = redirect["entity"] + + return current_entity def _log_unknown_entity(self, reference, curie, line_number): """ diff --git a/tests/unit/phase/test_lookup.py b/tests/unit/phase/test_lookup.py index b99977b6..a9887c8a 100644 --- a/tests/unit/phase/test_lookup.py +++ b/tests/unit/phase/test_lookup.py @@ -49,6 +49,7 @@ def get_lookup(): class TestLookupPhase: + def test_redirect_entity_entity_is_not_redirected(self): redirects = {"101": {"entity": "101", "status": "310"}} phase = LookupPhase(redirect_lookups=redirects) @@ -258,6 +259,24 @@ def test_lookup_rule_fallback_in_get_entity(self): } assert phase.get_entity(block) == "1100" + def test_redirect_entity_follows_multiple_redirects(self): + redirects = { + "101": {"entity": "202", "status": "301"}, + "202": {"entity": "303", "status": "301"}, + } + phase = LookupPhase(redirect_lookups=redirects) + + assert phase.redirect_entity("101") == "303" + + def test_redirect_entity_returns_empty_when_chain_ends_in_410(self): + redirects = { + "101": {"entity": "202", "status": "301"}, + "202": {"entity": "", "status": "410"}, + } + phase = LookupPhase(redirect_lookups=redirects) + + assert phase.redirect_entity("101") == "" + class TestPrintLookupPhase: def test_process_produces_lookup_without_organisation(self):