From 52b070e839fd83b60bd1ff4eef797d29bdcb02e7 Mon Sep 17 00:00:00 2001
From: Jordi Riera <547282+foutoucour@users.noreply.github.com>
Date: Fri, 10 Aug 2018 13:26:05 -0400
Subject: [PATCH] auto_backup: allow to change the format of backup (#1333)

---
 auto_backup/models/db_backup.py     | 41 +++++++++++++++++++++--------
 auto_backup/tests/test_db_backup.py | 26 +++++++++---------
 auto_backup/view/db_backup_view.xml |  1 +
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/auto_backup/models/db_backup.py b/auto_backup/models/db_backup.py
index 464215cfb..4c89787ee 100644
--- a/auto_backup/models/db_backup.py
+++ b/auto_backup/models/db_backup.py
@@ -83,6 +83,15 @@ class DbBackup(models.Model):
              "read permissions for that file.",
     )
 
+    backup_format = fields.Selection(
+        [
+            ("zip", "zip (includes filestore)"),
+            ("dump", "pg_dump custom format (without filestore)")
+        ],
+        default='zip',
+        help="Choose the format for this backup."
+    )
+
     @api.model
     def _default_folder(self):
         """Default to ``backups`` folder inside current server datadir."""
@@ -131,11 +140,11 @@ class DbBackup(models.Model):
     def action_backup(self):
         """Run selected backups."""
         backup = None
-        filename = self.filename(datetime.now())
         successful = self.browse()
 
         # Start with local storage
         for rec in self.filtered(lambda r: r.method == "local"):
+            filename = self.filename(datetime.now(), ext=rec.backup_format)
             with rec.backup_log():
                 # Directory must exist
                 try:
@@ -151,21 +160,28 @@ class DbBackup(models.Model):
                             shutil.copyfileobj(cached, destiny)
                     # Generate new backup
                     else:
-                        db.dump_db(self.env.cr.dbname, destiny)
+                        db.dump_db(
+                            self.env.cr.dbname,
+                            destiny,
+                            backup_format=rec.backup_format
+                        )
                         backup = backup or destiny.name
                 successful |= rec
 
         # Ensure a local backup exists if we are going to write it remotely
         sftp = self.filtered(lambda r: r.method == "sftp")
         if sftp:
-            if backup:
-                cached = open(backup)
-            else:
-                cached = db.dump_db(self.env.cr.dbname, None)
+            for rec in sftp:
+                filename = self.filename(datetime.now(), ext=rec.backup_format)
+                with rec.backup_log():
 
-            with cached:
-                for rec in sftp:
-                    with rec.backup_log():
+                    cached = db.dump_db(
+                        self.env.cr.dbname,
+                        None,
+                        backup_format=rec.backup_format
+                    )
+
+                    with cached:
                         with rec.sftp_connection() as remote:
                             # Directory must exist
                             try:
@@ -255,13 +271,16 @@ class DbBackup(models.Model):
                 self.name)
 
     @staticmethod
-    def filename(when):
+    def filename(when, ext='zip'):
         """Generate a file name for a backup.
 
         :param datetime.datetime when:
             Use this datetime instead of :meth:`datetime.datetime.now`.
+        :param str ext: Extension of the file. Default: dump.zip
         """
-        return "{:%Y_%m_%d_%H_%M_%S}.dump.zip".format(when)
+        return "{:%Y_%m_%d_%H_%M_%S}.{ext}".format(
+            when, ext='dump.zip' if ext == 'zip' else ext
+        )
 
     @api.multi
     def sftp_connection(self):
diff --git a/auto_backup/tests/test_db_backup.py b/auto_backup/tests/test_db_backup.py
index eac650c9b..13aa3ff9e 100644
--- a/auto_backup/tests/test_db_backup.py
+++ b/auto_backup/tests/test_db_backup.py
@@ -175,18 +175,6 @@ class TestDbBackup(common.TransactionCase):
                     'wb'
                 )
 
-    def test_action_backup_sftp_remote_open(self):
-        """ It should open remote file w/ proper args """
-        rec_id = self.new_record()
-        with self.mock_assets() as assets:
-            with self.patch_filtered_sftp(rec_id):
-                conn = rec_id.sftp_connection().__enter__()
-                rec_id.action_backup()
-                conn.open.assert_called_once_with(
-                    assets['os'].path.join(),
-                    'wb'
-                )
-
     def test_action_backup_all_search(self):
         """ It should search all records """
         rec_id = self.new_record()
@@ -241,8 +229,20 @@ class TestDbBackup(common.TransactionCase):
             pysftp.Connection(), res,
         )
 
-    def test_filename(self):
+    def test_filename_default(self):
         """ It should not error and should return a .dump.zip file str """
         now = datetime.now()
         res = self.Model.filename(now)
         self.assertTrue(res.endswith(".dump.zip"))
+
+    def test_filename_zip(self):
+        """ It should return a dump.zip filename"""
+        now = datetime.now()
+        res = self.Model.filename(now, ext='zip')
+        self.assertTrue(res.endswith(".dump.zip"))
+
+    def test_filename_dump(self):
+        """ It should return a dump filename"""
+        now = datetime.now()
+        res = self.Model.filename(now, ext='dump')
+        self.assertTrue(res.endswith(".dump"))
diff --git a/auto_backup/view/db_backup_view.xml b/auto_backup/view/db_backup_view.xml
index 42f826feb..ebed80c91 100644
--- a/auto_backup/view/db_backup_view.xml
+++ b/auto_backup/view/db_backup_view.xml
@@ -15,6 +15,7 @@
                     <field name="folder"/>
                     <field name="days_to_keep"/>
                     <field name="method"/>
+                    <field name="backup_format"/>
                 </group>
                 <div attrs="{'invisible': [('method', '!=', 'sftp')]}">
                     <div class="bg-warning text-warning">