public inbox for [email protected]  
help / color / mirror / Atom feed
From: Khushboo Vashi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject:  Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.
Date: Mon, 12 Mar 2018 11:30:50 +0530
Message-ID: <CAFOhELftpncVyoK=H4L-yZco_XWbk+x0XwRx1A5QgLkgXpu2MQ@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoz-N=7QMVh05R6dpQxugyx-z=+4TPOYB6z4vhZA_Ks3tA@mail.gmail.com>
References: <CAFOhELf74n8cKE9y_4ppNs31Qi87WDLry+TV7K_bpAmdczM2bg@mail.gmail.com>
	<CA+OCxozdsAg3-jv4yhv9G9Kre4KmO+zWYfkzmVTGrfjX=OkvWA@mail.gmail.com>
	<CAFOhELfcJSs+nE3Z5bgFfNkVVmuavi8GvQ-uwrKPO7Y+hVf79w@mail.gmail.com>
	<CA+OCxowgspEsUwg8cGcyZH=whCS8n26NePw0cPycSJ=H+PZT8Q@mail.gmail.com>
	<CAFOhELdv_EFFctXmLNVTeH4dj7EdQ_oabAUxeC+oho+S4Q-Jww@mail.gmail.com>
	<CA+OCxoxKoZ7Mijapqb=oFh34jFNrs5xeuRWNH+gZhMtxkGFu3w@mail.gmail.com>
	<CA+OCxoz-N=7QMVh05R6dpQxugyx-z=+4TPOYB6z4vhZA_Ks3tA@mail.gmail.com>

Hi Dave,

On Fri, Mar 9, 2018 at 9:09 PM, Dave Page <[email protected]> wrote:

> Hi
>
> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix below issues:
>>>
>>> 1. #2963 - Backup database, Restore database and Maintenance Database
>>> failed for é object
>>> 2. #3157 - Process viewer doesn't show complete command executed.
>>>
>>> Test cases are not included for these fixes as we don't have test cases
>>> for these modules (backup, restore, maintenance).
>>> I will create one separate RM for the same which will cover this.
>>>
>>
>> Interesting that you fix these together, as together they also exhibit
>> another bug :-). Backing up the é database displays the following
>> command:
>>
>> /usr/local/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host
>> "localhost" --port "5432" --username "postgres" --no-password --verbose
>> --format=c --blobs "&#233;"
>>
>
> I can reproduce this issue only with notification dialogue (which I have
fixed in the attached patch) not with the details dialogue. Please refer
the screenshots for the same.

> Also, what tests can we add for backup/restore? We have nothing at all at
> the moment, and it is pretty troublesome. I'd like to ensure that we can
> backup and restore a database correctly, and ensure that the displayed
> commands are what we expect and that we get valid output from
> pg_dump/pg_restore (though, it may change from PG version to PG version, so
> maybe we should just check for something small and generic). I guess this
> might need some config parameters for the tests to specify the pg_* utility
> paths for each server.
>
> I'd suggest maybe having a feature test that opens the prefs, sets the
> appropriate path, then runs a backup, waits for it to finish, checks the
> process monitor output, then restores the same backup to a new database,
> checking the process monitor output again, and then checking that the
> restored database contains at least one object from the original database
> (we don't need to check all of pg_dump/pg_restore, just that something
> expected was restored). We should use a (partial) database name and backup
> filename from the advanced test config file, and I think both should
> default to some interesting non-ASCII strings to ensure quoting works.
>
I was thinking of writing the unit test cases for the processes.py file as
all the major functionalities for backup/restore/maintenance jobs done by
this file, but by this we can not achieve the front-end string validation
esp for non-ASCII strings.
So, I am thinking of writing feature tests (as you have suggested) first
and after that if needed I will write unit test cases.

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Attachments:

  [text/x-patch] RM_2963_3157_ver1.patch (5.1K, 3-RM_2963_3157_ver1.patch)
  download | inline diff:
diff --git a/web/pgadmin/misc/bgprocess/processes.py b/web/pgadmin/misc/bgprocess/processes.py
index 02b953d..385b7bd 100644
--- a/web/pgadmin/misc/bgprocess/processes.py
+++ b/web/pgadmin/misc/bgprocess/processes.py
@@ -78,10 +78,20 @@ class BatchProcess(object):
                 _("Could not find a process with the specified ID.")
             )
 
+        try:
+            tmp_desc = loads(p.desc.encode('latin-1')) if \
+                IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+        except UnicodeDecodeError:
+            tmp_desc = loads(p.desc.encode('utf-8')) if \
+                IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+        except Exception as e:
+            tmp_desc = loads(p.desc.encode('utf-8', 'ignore')) if \
+                IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+
         # ID
         self.id = _id
         # Description
-        self.desc = loads(p.desc)
+        self.desc = tmp_desc
         # Status Acknowledged time
         self.atime = p.acknowledge
         # Command
@@ -171,6 +181,16 @@ class BatchProcess(object):
             csv_writer.writerow(_args)
 
         args_val = args_csv_io.getvalue().strip(str('\r\n'))
+        tmp_desc = dumps(self.desc)
+        try:
+            tmp_desc =tmp_desc.decode('utf-8') if\
+                IS_PY2 and hasattr(tmp_desc, 'decode') else tmp_desc
+        except UnicodeDecodeError:
+            tmp_desc = tmp_desc.decode('latin-1') if \
+                IS_PY2 and hasattr(tmp_desc, 'decode') else tmp_desc
+        except Exception:
+            tmp_desc = tmp_desc.decode('utf-8', 'ignore') if \
+                IS_PY2 and hasattr(tmp_desc, 'decode') else tmp_desc
 
         j = Process(
             pid=int(id),
@@ -178,7 +198,7 @@ class BatchProcess(object):
             arguments=args_val.decode('utf-8', 'replace')
             if IS_PY2 and hasattr(args_val, 'decode') else args_val,
             logdir=log_dir,
-            desc=dumps(self.desc),
+            desc=tmp_desc,
             user_id=current_user.id
         )
         db.session.add(j)
@@ -534,7 +554,17 @@ class BatchProcess(object):
             etime = parser.parse(p.end_time or get_current_time())
 
             execution_time = (etime - stime).total_seconds()
-            desc = loads(p.desc)
+            desc = ""
+            try:
+                desc = loads(p.desc.encode('latin-1')) if \
+                    IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+            except UnicodeDecodeError:
+                desc = loads(p.desc.encode('utf-8')) if \
+                    IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+            except Exception:
+                desc = loads(p.desc.encode('utf-8', 'ignore')) if \
+                    IS_PY2 and hasattr(p.desc, 'encode') else loads(p.desc)
+
             details = desc
 
             if isinstance(desc, IProcessDesc):
diff --git a/web/pgadmin/misc/bgprocess/static/js/bgprocess.js b/web/pgadmin/misc/bgprocess/static/js/bgprocess.js
index 5e30590..0934ae4 100644
--- a/web/pgadmin/misc/bgprocess/static/js/bgprocess.js
+++ b/web/pgadmin/misc/bgprocess/static/js/bgprocess.js
@@ -248,7 +248,7 @@ define('misc.bgprocess', [
           if (!self.notifier) {
             var header = $('<div></div>', {
                 class: 'h5 pg-bg-notify-header',
-              }).append($('<span></span>').text(self.desc)),
+              }).append($('<span></span>').text(_.unescape(self.desc))),
               content = $('<div class="pg-bg-bgprocess row"></div>').append(
                 header
               ).append(
@@ -366,7 +366,7 @@ define('misc.bgprocess', [
             self.logs[0].scrollTop = self.logs[0].scrollHeight;
           });
           // set bgprocess detailed description
-          $header.find('.bg-detailed-desc').html(self.detailed_desc);
+          $header.find('.bg-detailed-desc').html(_.unescape(self.detailed_desc));
         }
 
         // set bgprocess start time
@@ -562,4 +562,4 @@ define('misc.bgprocess', [
     });
 
   return pgBrowser.BackgroundProcessObsorver;
-});
\ No newline at end of file
+});
diff --git a/web/pgadmin/tools/backup/__init__.py b/web/pgadmin/tools/backup/__init__.py
index 715ff6e..9e70c4f 100644
--- a/web/pgadmin/tools/backup/__init__.py
+++ b/web/pgadmin/tools/backup/__init__.py
@@ -97,11 +97,9 @@ class BackupMessage(IProcessDesc):
 
         def cmdArg(x):
             if x:
-                # x = html.safe_str(x)
                 x = x.replace('\\', '\\\\')
                 x = x.replace('"', '\\"')
                 x = x.replace('""', '\\"')
-
                 return ' "' + x + '"'
             return ''
 
@@ -111,6 +109,7 @@ class BackupMessage(IProcessDesc):
             else:
                 self.cmd += cmdArg(arg)
 
+
     @property
     def message(self):
         # Fetch the server details like hostname, port, roles etc
@@ -189,7 +188,7 @@ class BackupMessage(IProcessDesc):
         res += '</div><div class="h5">'
         res += _("Running command:")
         res += '</b><br><span class="pg-bg-cmd enable-selection">'
-        res += html.safe_str(self.cmd)
+        res += html.safe_str(cmd + self.cmd)
         res += '</span></div>'
 
         return res


  [image/png] detail_dialogue.png (366.6K, 4-detail_dialogue.png)
  download | view image

  [image/png] notification_dialogue.png (397.3K, 5-notification_dialogue.png)
  download | view image

view thread (19+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re:  Re: [pgAdmin4][Patch]: RM #2963 - Backup database, Restore database and Maintenance Database failed for é object.
  In-Reply-To: <CAFOhELftpncVyoK=H4L-yZco_XWbk+x0XwRx1A5QgLkgXpu2MQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox