Fix bug where first_instance_id was no longer populated on central …

…issues

Reviewed By: yuhshin-oss

Differential Revision: D61398769

fbshipit-source-id: a12a851937858e3e896a2f020e97bd9696190ac9

sapp/bulk_saver.py

@@ -77,6 +77,7 @@ def __init__(
         self.saving: Dict[str, Any] = {}
         for cls in self.saving_classes_order:
             self.saving[cls.__name__] = []
+        self.prepare_all_done = False
 
     # pyre-fixme[2]: Parameter must be annotated.
     def add(self, item) -> None:
@@ -98,9 +99,7 @@ def add_all(self, items) -> None:
     def get_items_to_add(self, cls):
         return self.saving[cls.__name__]
 
-    def save_all(
-        self, database: DB, before_save: Optional[Callable[[], None]] = None
-    ) -> None:
+    def prepare_all(self, database: DB) -> None:
         saving_classes = [
             cls
             for cls in self.saving_classes_order
@@ -123,13 +122,20 @@ def save_all(
             )
             self._prepare(database, cls, pk_gen)
 
-        # Used by unit tests to simulate races
-        if before_save:
-            before_save()
+        self.prepare_all_done = True
+
+    def save_all(self, database: DB) -> None:
+        assert self.prepare_all_done, "prepare_all must succeed before calling save_all"
+
+        saving_classes = [
+            cls
+            for cls in self.saving_classes_order
+            if len(self.saving[cls.__name__]) != 0
+        ]
 
         for cls in saving_classes:
             log.info(f"Saving {len(self.saving[cls.__name__])} {cls.__name__}s...")
-            self._save(database, cls, pk_gen)
+            self._save(database, cls, self.primary_key_generator)
 
     @log_time
     # pyre-fixme[2]: Parameter must be annotated.

sapp/pipeline/database_saver.py

@@ -129,9 +129,16 @@ def _save(self) -> RunSummary:
                 run_id = self.summary["run"].id.resolved()
                 log.info("Created run: %d", run_id)
 
-            self._save_central_issues_and_sync_local_issues(
-                self.summary["run"], self.bulk_saver.get_items_to_add(Issue)
-            )
+            # Get full list of issues before bulk_saver removes existing issues
+            # in prepare_all
+            issues = self.bulk_saver.get_items_to_add(Issue)
+
+            self.bulk_saver.prepare_all(self.database)
+
+            # Save central issues after ids are reserved so they can reference
+            # `first_instance_id` but before local issues are saved as they
+            # may be modified here
+            self._save_central_issues_and_sync_local_issues(self.summary["run"], issues)
 
             self.bulk_saver.save_all(self.database)
 

sapp/tests/fake_object_generator.py

@@ -48,7 +48,10 @@ def reinit(self, run_id) -> None:
     def save_all(self, db, before_save: Optional[Callable[[], None]] = None) -> None:
         if self.graph:
             self.graph.update_bulk_saver(self.saver)
-        self.saver.save_all(db, before_save=before_save)
+        self.saver.prepare_all(db)
+        if before_save is not None:
+            before_save()
+        self.saver.save_all(db)
         self.saver = BulkSaver()
 
     def issue(