From f313992094c2ebcff261fe906433c3d8d55e0067 Mon Sep 17 00:00:00 2001
From: Renata <rvaderna@openlawlib.org>
Date: Tue, 24 Dec 2024 19:07:27 -0500
Subject: [PATCH]  fix: do not update snapshot and timestamp twice

---
 taf/api/metadata.py   | 31 ++++++++++++++++---------------
 taf/tuf/repository.py | 11 ++++++++++-
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/taf/api/metadata.py b/taf/api/metadata.py
index fdfdb67f..91378fbf 100644
--- a/taf/api/metadata.py
+++ b/taf/api/metadata.py
@@ -94,6 +94,7 @@ def update_metadata_expiration_date(
     scheme: Optional[str] = DEFAULT_RSA_SIGNATURE_SCHEME,
     start_date: Optional[datetime] = None,
     commit: Optional[bool] = True,
+    commit_msg: Optional[str] = None,
     prompt_for_keys: Optional[bool] = False,
     push: Optional[bool] = True,
     update_snapshot_and_timestamp: Optional[bool] = True,
@@ -113,6 +114,7 @@ def update_metadata_expiration_date(
         start_date (optional): Date to which expiration interval is added.
             Set to today if not specified.
         commit (optional): Indicates if the changes should be committed and pushed automatically.
+        commit_msg (optional): Custom commit messages.
         prompt_for_keys (optional): Whether to ask the user to enter their key if it is not located inside the keystore directory.
         push (optional): Flag specifying whether to push to remote
 
@@ -128,15 +130,14 @@ def update_metadata_expiration_date(
     if start_date is None:
         start_date = datetime.now()
 
-    commit_msg = git_commit_message("update-expiration-dates", roles=",".join(roles))
+    commit_msg = commit_msg or git_commit_message(
+        "update-expiration-dates", roles=",".join(roles)
+    )
 
     # update the order, snapshot has to be updated before timestamp
     # and all other roles have to be updated before snapshot
     # all other roles can be updated in any order
 
-    if len(roles) == 1 and Timestamp.type in roles:
-        update_snapshot_and_timestamp = False
-
     update_snapshot_expiration_date = Snapshot.type in roles
     update_timestamp_expiration_date = Timestamp.type in roles
 
@@ -151,21 +152,21 @@ def update_metadata_expiration_date(
         commit_msg=commit_msg,
         push=push,
     ):
-        if update_snapshot_and_timestamp:
-            if update_snapshot_expiration_date:
-                auth_repo.add_to_open_metadata(Snapshot.type)
-            if update_timestamp_expiration_date:
-                auth_repo.add_to_open_metadata(Timestamp.type)
+        if update_snapshot_expiration_date:
+            auth_repo.add_to_open_metadata([Snapshot.type])
+        if update_timestamp_expiration_date:
+            auth_repo.add_to_open_metadata([Timestamp.type])
 
         for role in roles:
             auth_repo.set_metadata_expiration_date(
                 role, start_date=start_date, interval=interval
             )
 
-        auth_repo.clear_open_metadata()
+        auth_repo.remove_from_open_metadata([Snapshot.type])
+        # it is important to update snapshot first
+        if update_snapshot_expiration_date or update_snapshot_and_timestamp :
+            auth_repo.do_snapshot(force=True)
 
-        if update_snapshot_and_timestamp:
-            if len(roles) == 1 and Snapshot.type in roles:
-                auth_repo.do_timestamp()
-            else:
-                auth_repo.update_snapshot_and_timestamp()
+        auth_repo.remove_from_open_metadata([Timestamp.type])
+        if update_timestamp_expiration_date or update_snapshot_and_timestamp:
+            auth_repo.do_timestamp(force=True)
diff --git a/taf/tuf/repository.py b/taf/tuf/repository.py
index aaf2f8f6..621e1ab8 100644
--- a/taf/tuf/repository.py
+++ b/taf/tuf/repository.py
@@ -342,7 +342,8 @@ def add_new_roles_to_snapshot(self, roles: List[str]) -> None:
     def add_to_open_metadata(self, roles: List[str]) -> None:
         """
         In order to execute several methods before updating the metadata on disk,
-        these methods need to be added to the _metadata_to_keep_open list.
+        some metadata might need to be kept open, which is done by adding them to
+        _metadata_to_keep_open list.
         This method adds all roles from the provided list to _metadata_to_keep_open.
         """
         self._metadata_to_keep_open.update(roles)
@@ -1407,6 +1408,14 @@ def remove_delegated_paths(self, roles_paths: Dict[str, List[str]]):
                         updated = True
         return updated
 
+    def remove_from_open_metadata(self, roles: List[str]) -> None:
+        """
+        Removes the listed roles from metadata_to_keep_open list
+        """
+        for role in roles:
+            if role in self._metadata_to_keep_open:
+                self._metadata_to_keep_open.remove(role)
+
     def roles_targets_for_filenames(self, target_filenames):
         """Sort target files by roles
         Args: