fix(transaction): no transaction created if file already exists from … (

#19)

pdbstore/cli/commands/add.py

@@ -23,10 +23,11 @@
 
 def add_text_formatter(summary: Summary) -> None:
     """Print output text for add command as simple text"""
+    nb_deleted = summary.linked.count(True) if summary.linked else 0
     cli_out_write(f"Number of files stored = {summary.success(False)}")
     cli_out_write(f"Number of errors = {summary.failed(False)}")
     cli_out_write(f"Number of files ignored = {summary.skipped(False)}")
-    cli_out_write(f"Number of transactions deleted = {summary.count(True)-1}")
+    cli_out_write(f"Number of transactions deleted = {nb_deleted}")
 
     if summary.failed(True):
         raise PDBAbortExecution(summary.failed(True))
@@ -73,6 +74,7 @@ def add(parser: PDBStoreArgumentParser, *args: Any) -> Any:
         "--force",
         dest="force",
         action="store_true",
+        default=False,
         help="""Overwrite any existing file from the store. uses file's hash
         to check if it's already exists in the store. Defaults to False.""",
     )
@@ -119,7 +121,7 @@ def add(parser: PDBStoreArgumentParser, *args: Any) -> Any:
     if not input_files:
         raise CommandLineError("no file or directory given")
 
-    compress: bool = opts.compress or False
+    compress: bool = opts.compress
     if compress and not pdbstore.io.is_compression_supported():
         raise CompressionNotSupportedError()
     store = Store(store_dir)
@@ -154,14 +156,16 @@ def add(parser: PDBStoreArgumentParser, *args: Any) -> Any:
     if success > 0:
         # Commit modifications to the disk
         try:
-            summary = store.commit(new_transaction, opts.force or False)
+            summary = store.commit(new_transaction, opts.force)
         except PDBStoreException as exc:
             output.error(exc)
             return Summary(new_transaction.id, OpStatus.FAILED, TransactionType.ADD)
-        except Exception:  # pylint: disable=broad-except
+        except Exception as exc2:  # pylint: disable=broad-except
+            print(exc2)
             output.error(
                 "unexpected error when filling Transaction object",
             )
+            return Summary(new_transaction.id, OpStatus.FAILED, TransactionType.ADD)
     else:
         summary = Summary(None, OpStatus.SKIPPED, TransactionType.ADD)
 

pdbstore/cli/commands/del.py

@@ -65,9 +65,8 @@ def delete(parser: PDBStoreArgumentParser, *args: Any) -> Any:
     store.next_transaction_id  # pylint: disable=pointless-statement
 
     # Delete the transaction from the store
-    summary: Summary
+    summary: Optional[Summary] = None
     summary_head: Optional[Summary] = None
-
     for trans_id in transaction_id if isinstance(transaction_id, list) else [transaction_id]:
         try:
             summary_del: Summary = store.delete_transaction(trans_id, opts.dry_run)
@@ -79,10 +78,10 @@ def delete(parser: PDBStoreArgumentParser, *args: Any) -> Any:
             output.error(
                 f"unexpected error when deleting {trans_id} transaction",
             )
-        if summary_head:
-            summary.linked = summary_del  # noqa: F821 # pylint: disable=used-before-assignment
+        if summary:
+            summary.linked = summary_del
         else:
             summary_head = summary_del
         summary = summary_del
 
-    return summary
+    return summary_head

pdbstore/cli/commands/promote.py

@@ -82,7 +82,7 @@ def promote(parser: PDBStoreArgumentParser, *args: Any) -> Any:
     store_in = Store(input_store_dir)
     store_out = Store(output_store_dir)
 
-    summary: Summary
+    summary: Optional[Summary] = None
     summary_head: Optional[Summary] = None
 
     for trans_id in transaction_id if isinstance(transaction_id, list) else [transaction_id]:
@@ -96,10 +96,10 @@ def promote(parser: PDBStoreArgumentParser, *args: Any) -> Any:
             output.error(f"unexpected error when promoting {trans_id}")
             output.error(exc)
 
-        if summary_head:
-            summary.linked = summary_trans  # noqa: F821 # pylint: disable=used-before-assignment
+        if summary:
+            summary.linked = summary_trans
         else:
             summary_head = summary_trans
         summary = summary_trans
 
-    return summary
+    return summary_head

pdbstore/store/entry.py

@@ -88,7 +88,8 @@ def is_compressed(self) -> bool:
     def commit(
         self,
         force: Optional[bool] = False,
-        store: Optional["Store"] = None,  # type: ignore[name-defined] # noqa F821
+        store: Optional["Store"] = None,  # type: ignore[name-defined] # noqa F821,
+        skip_if_exists: Optional[bool] = False,
     ) -> bool:
         """Commit transaction entry by storing the required filse into the symbol store.
 
@@ -97,18 +98,22 @@ def commit(
         :class:`this entry object <TransactionEntry>` and stored them in ``store``
         as a new entry.
 
-        :param force: True to overwrite any existing file from the store, else False.
+        :param force: `True` to overwrite any existing file from the store, else `False`.
         :param store: Optional :class:`Store <pdbstore.store.store.Store>` object.
-
-        :return: True if the file is stored successfully, else False if the file was
+        :param skip_if_exists: `True` to skip entry creation if the file already exists
+            in the store, else `False`
+        :return: `True` if the file is stored successfully, else `False` if the file was
                  alredy present.
         :raise:
             :CabCompressionError: if an error occurs during compressed file operation
             :CopyFileError: if an error occurs during file storage without compression
         """
-        if not force and self.is_committed():
+        if self.is_committed():
             # The file is already present, so keep it as it is
-            return False
+            if skip_if_exists:
+                return False
+            if not force:
+                return True
 
         dest_dir = self._stored_dir()
 

pdbstore/store/store.py

@@ -177,7 +177,9 @@ def commit(
         :class:`Store <pdbstore.store.store.Store>` object.
 
         :param transaction: The transaction to be committed.
-        :param force: True to overwrite any existing file from the store, else False.
+        :param force: If **True** and a file is already present in the store, the existing
+            file will be overwritten and the file will be associated to ``transaction``, else
+            this function will only make the associated between the file and ``transaction``.
         :param store: Optional :class:`Store <pdbstore.store.store.Store>` object
         :return: A :class:`Summary <pdbstore.store.summary.Summary>` object
         :raise:

pdbstore/store/transaction.py

@@ -247,7 +247,9 @@ def commit(
         :param transaction_id: The transaction ID
         :param timestamp: The transaction date/time
         :param store: Optional :class:`Store <pdbstore.store.store.Store>` object
-        :param force: True to overwrite any existing file from the store, else False.
+        :param force: If **True** and a file is already present in the store, the existing
+            file will be overwritten and the file will be associated to ``transaction``, else
+            this function will only make the associated between the file and ``transaction``.
         :return: True if successful, else False
         :raise:
             :WriteFileError: Failed to update history file
@@ -282,7 +284,7 @@ def commit(
                         result[1],
                         TransactionType.ADD,
                     )
-                    if summary.status == OpStatus.SKIPPED:
+                    if result[1] == OpStatus.SUCCESS:
                         summary.status = OpStatus.SUCCESS
                 else:
                     summary.status = OpStatus.FAILED

tests/unit/test_transaction_entry.py

@@ -61,8 +61,10 @@ def test_commit_success(tmp_store, test_data_native_dir):
         False,
     )
     assert entry.commit() is True
-    assert entry.commit() is False
-    assert entry.commit(True) is True
+    assert entry.commit(skip_if_exists=True) is False
+    assert entry.commit(force=True) is True
+    assert entry.commit(force=True, skip_if_exists=True) is False
+    assert entry.commit(force=True, skip_if_exists=False) is True
 
 
 def test_commit_failure(tmp_store, test_data_native_dir):