Avoid loading targets metadata twice

When traversing the delegations tree looking for targets,
avoid re-loading already verified targets metadata.

Signed-off-by: Teodora Sechkova <[email protected]>

Author: sechkova

tests/test_updater_with_simulator.py

@@ -6,11 +6,13 @@
 """Test ngclient Updater using the repository simulator.
 """
 
+import builtins
 import os
 import sys
 import tempfile
 import unittest
 from typing import Optional, Tuple
+from unittest.mock import MagicMock, Mock, patch
 
 from tests import utils
 from tests.repository_simulator import RepositorySimulator
@@ -230,6 +232,32 @@ def test_snapshot_rollback_with_local_snapshot_hash_mismatch(self):
         with self.assertRaises(BadVersionNumberError):
             self._run_refresh()
 
+    @patch.object(builtins, "open", wraps=builtins.open)
+    def test_not_loading_targets_twice(self, wrapped_open: MagicMock):
+        # Do not load targets roles more than once when traversing
+        # the delegations tree
+
+        # Add new delegated targets, update the snapshot
+        spec_version = ".".join(SPECIFICATION_VERSION)
+        targets = Targets(1, spec_version, self.sim.safe_expiry, {}, None)
+        self.sim.add_delegation("targets", "role1", targets, False, ["*"], None)
+        self.sim.update_snapshot()
+
+        # Run refresh, top-level roles are loaded
+        updater = self._run_refresh()
+        # Clean up calls to open during refresh()
+        wrapped_open.reset_mock()
+
+        # First time looking for "somepath", only 'role1' must be loaded
+        updater.get_targetinfo("somepath")
+        wrapped_open.assert_called_once_with(
+            os.path.join(self.metadata_dir, "role1.json"), "rb"
+        )
+        wrapped_open.reset_mock()
+        # Second call to get_targetinfo, all metadata is already loaded
+        updater.get_targetinfo("somepath")
+        wrapped_open.assert_not_called()
+
 
 if __name__ == "__main__":
     if "--dump" in sys.argv:

tuf/ngclient/updater.py

@@ -415,9 +415,10 @@ def _preorder_depth_first_walk(
                 logger.debug("Skipping visited current role %s", role_name)
                 continue
 
-            # The metadata for 'role_name' must be downloaded/updated before
-            # its targets, delegations, and child roles can be inspected.
-            self._load_targets(role_name, parent_role)
+            # The metadata for 'role_name' must be loaded
+            # before its targets and delegations can be inspected.
+            if not self._trusted_set.get(role_name):
+                self._load_targets(role_name, parent_role)
 
             role_metadata: Targets = self._trusted_set[role_name].signed
             target = role_metadata.targets.get(target_filepath)