3:10 p.m.
V2
Implement enhancements proposed by Aline in code review:
- UI does validation in the report name
- Automatic name is generated in backend now
- Name not required. User can send empty string or none
V1
The two patch fix a problem reported by internal test team.
Rodrigo Trujillo (2):
Fix debug report naming problem (backend)
Fix debug report naming problem (UI)
src/kimchi/API.json | 12 ++++++++++++
src/kimchi/control/base.py | 1 +
src/kimchi/i18n.py | 2 ++
src/kimchi/model/debugreports.py | 5 ++++-
ui/js/src/kimchi.report_add_main.js | 21 +++++++++++++--------
ui/pages/i18n.html.tmpl | 2 +-
ui/pages/report-add.html.tmpl | 2 +-
7 files changed, 34 insertions(+), 11 deletions(-)
--
1.8.5.3
3:10 p.m.
New subject: [PATCH V2 1/2] Fix debug report naming problem (backend)
The tool sosreport only accepts letters, digits and hyphen in the name
of the report, non alphanumeric characteres are removed from the file
name and then Kimchi is not able to find the file.
This patch fixes the problem in the backend, adding json schema
verification.
Implement name generation in backend
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
---
src/kimchi/API.json | 12 ++++++++++++
src/kimchi/control/base.py | 1 +
src/kimchi/i18n.py | 2 ++
src/kimchi/model/debugreports.py | 5 ++++-
4 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index 1189c01..e393324 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -29,6 +29,18 @@
}
},
"properties": {
+ "debugreports_create": {
+ "type": "object",
+ "error": "KCHDEBREP0006E",
+ "properties": {
+ "name": {
+ "description": "The name for the debug report
file.",
+ "type": "string",
+ "pattern": "^[A-Za-z0-9-]*$",
+ "error": "KCHDEBREP0007E"
+ }
+ }
+ },
"storagepools_create": {
"type": "object",
"error": "KCHPOOL0026E",
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
index 048dd34..2cfcfd7 100644
--- a/src/kimchi/control/base.py
+++ b/src/kimchi/control/base.py
@@ -307,6 +307,7 @@ class AsyncCollection(Collection):
get_class_name(self)})
raise cherrypy.HTTPError(405, e.message)
+ validate_params(params, self, 'create')
args = self.model_args + [params]
task = create(*args)
cherrypy.response.status = 202
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index dfc1b2b..7ca4060 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -173,6 +173,8 @@ messages = {
"KCHDR0003E": _("Unable to create debug report %(name)s. Details:
%(err)s."),
"KCHDR0004E": _("Can not find any generated debug report matching name
%(name)s"),
"KCHDR0005E": _("Unable to generate debug report %(name)s. Details:
%(err)s"),
+ "KCHDR0006E": _("You must give a name for the debug file
report."),
+ "KCHDR0007E": _("Name must be a string. Only letters, digits and
hyphen ('-') are allowed."),
"KCHSR0001E": _("Storage server %(server)s was not used by
Kimchi"),
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py
index 7dbd69f..aa70a6b 100644
--- a/src/kimchi/model/debugreports.py
+++ b/src/kimchi/model/debugreports.py
@@ -41,7 +41,10 @@ class DebugReportsModel(object):
self.task = TaskModel(**kargs)
def create(self, params):
- ident = params['name']
+ ident = params.get('name')
+ # Generate a name with time and millisec precision, if necessary
+ if ident is None or ident == "":
+ ident = 'report-' + str(int(time.time()*1000))
taskid = self._gen_debugreport_file(ident)
return self.task.lookup(taskid)
--
1.8.5.3
6:59 a.m.
New subject: [PATCH V2 1/2] Fix debug report naming problem (backend)
On 02/21/2014 06:10 PM, Rodrigo Trujillo wrote:
The tool sosreport only accepts letters, digits and hyphen in the
name
of the report, non alphanumeric characteres are removed from the file
name and then Kimchi is not able to find the file.
This patch fixes the problem in the backend, adding json schema
verification.
Implement name generation in backend
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
---
src/kimchi/API.json | 12 ++++++++++++
src/kimchi/control/base.py | 1 +
src/kimchi/i18n.py | 2 ++
src/kimchi/model/debugreports.py | 5 ++++-
4 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index 1189c01..e393324 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -29,6 +29,18 @@
}
},
"properties": {
+ "debugreports_create": {
+ "type": "object",
+ "error": "KCHDEBREP0006E",
+ "properties": {
+ "name": {
+ "description": "The name for the debug report
file.",
+ "type": "string",
+ "pattern": "^[A-Za-z0-9-]*$",
+ "error": "KCHDEBREP0007E"
+ }
+ }
+ },
"storagepools_create": {
"type": "object",
"error": "KCHPOOL0026E",
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
index 048dd34..2cfcfd7 100644
--- a/src/kimchi/control/base.py
+++ b/src/kimchi/control/base.py
@@ -307,6 +307,7 @@ class AsyncCollection(Collection):
get_class_name(self)})
raise cherrypy.HTTPError(405, e.message)
+ validate_params(params, self, 'create')
args = self.model_args + [params]
task = create(*args)
cherrypy.response.status = 202
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index dfc1b2b..7ca4060 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -173,6 +173,8 @@ messages = {
"KCHDR0003E": _("Unable to create debug report %(name)s. Details:
%(err)s."),
"KCHDR0004E": _("Can not find any generated debug report matching
name %(name)s"),
"KCHDR0005E": _("Unable to generate debug report %(name)s. Details:
%(err)s"),
+ "KCHDR0006E": _("You must give a name for the debug file
report."),
The above message isn't being used.
+ "KCHDR0007E": _("Name must be a string. Only
letters, digits and hyphen ('-') are allowed."),
"KCHSR0001E": _("Storage server %(server)s was not used by
Kimchi"),
diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py
index 7dbd69f..aa70a6b 100644
--- a/src/kimchi/model/debugreports.py
+++ b/src/kimchi/model/debugreports.py
@@ -41,7 +41,10 @@ class DebugReportsModel(object):
self.task = TaskModel(**kargs)
def create(self, params):
- ident = params['name']
+ ident = params.get('name')
You need to strip() the name value. Otherwise we will accept " " like
a name
+ # Generate a name with time and millisec precision, if
necessary
+ if ident is None or ident == "":
+ ident = 'report-' + str(int(time.time()*1000))
taskid = self._gen_debugreport_file(ident)
return self.task.lookup(taskid)
8:20 a.m.
New subject: [PATCH V2 1/2] Fix debug report naming problem (backend)
On 02/24/2014 09:59 AM, Aline Manera wrote:
On 02/21/2014 06:10 PM, Rodrigo Trujillo wrote:
> The tool sosreport only accepts letters, digits and hyphen in the name
> of the report, non alphanumeric characteres are removed from the file
> name and then Kimchi is not able to find the file.
>
> This patch fixes the problem in the backend, adding json schema
> verification.
>
> Implement name generation in backend
>
> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
> ---
> src/kimchi/API.json | 12 ++++++++++++
> src/kimchi/control/base.py | 1 +
> src/kimchi/i18n.py | 2 ++
> src/kimchi/model/debugreports.py | 5 ++++-
> 4 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 1189c01..e393324 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -29,6 +29,18 @@
> }
> },
> "properties": {
> + "debugreports_create": {
> + "type": "object",
> + "error": "KCHDEBREP0006E",
> + "properties": {
> + "name": {
> + "description": "The name for the debug report
> file.",
> + "type": "string",
> + "pattern": "^[A-Za-z0-9-]*$",
> + "error": "KCHDEBREP0007E"
> + }
> + }
> + },
> "storagepools_create": {
> "type": "object",
> "error": "KCHPOOL0026E",
> diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
> index 048dd34..2cfcfd7 100644
> --- a/src/kimchi/control/base.py
> +++ b/src/kimchi/control/base.py
> @@ -307,6 +307,7 @@ class AsyncCollection(Collection):
> get_class_name(self)})
> raise cherrypy.HTTPError(405, e.message)
>
> + validate_params(params, self, 'create')
> args = self.model_args + [params]
> task = create(*args)
> cherrypy.response.status = 202
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index dfc1b2b..7ca4060 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -173,6 +173,8 @@ messages = {
> "KCHDR0003E": _("Unable to create debug report %(name)s.
> Details: %(err)s."),
> "KCHDR0004E": _("Can not find any generated debug report
> matching name %(name)s"),
> "KCHDR0005E": _("Unable to generate debug report %(name)s.
> Details: %(err)s"),
> + "KCHDR0006E": _("You must give a name for the debug file
report."),
The above message isn't being used.
Actually it is used in error return of
json schema, but it was wrongly
written there. I fixed this
> + "KCHDR0007E": _("Name must be a string. Only letters, digits and
> hyphen ('-') are allowed."),
>
> "KCHSR0001E": _("Storage server %(server)s was not used by
> Kimchi"),
>
> diff --git a/src/kimchi/model/debugreports.py
> b/src/kimchi/model/debugreports.py
> index 7dbd69f..aa70a6b 100644
> --- a/src/kimchi/model/debugreports.py
> +++ b/src/kimchi/model/debugreports.py
> @@ -41,7 +41,10 @@ class DebugReportsModel(object):
> self.task = TaskModel(**kargs)
>
> def create(self, params):
> - ident = params['name']
> + ident = params.get('name')
You need to strip() the name value. Otherwise we will accept " "
like a name
The api schema will not allow spaces in the name. But I am going to add
the strip, then we avoid problems in the case of
someone calls the function directly.
> + # Generate a name with time and millisec precision, if
> necessary
> + if ident is None or ident == "":
> + ident = 'report-' + str(int(time.time()*1000))
> taskid = self._gen_debugreport_file(ident)
> return self.task.lookup(taskid)
>
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
3:10 p.m.
New subject: [PATCH V2 2/2] Fix debug report naming problem (UI)
The tool sosreport only accepts letters, digits and hyphen in the name
of the report, non alphanumeric characteres are removed from the file
name and then Kimchi is not able to find the file.
This patch modifies the UI html and js in order to show the right
information and catch the right return error message.
Implements name checking in frontend.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
---
ui/js/src/kimchi.report_add_main.js | 21 +++++++++++++--------
ui/pages/i18n.html.tmpl | 2 +-
ui/pages/report-add.html.tmpl | 2 +-
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js
index f893d85..757ee21 100644
--- a/ui/js/src/kimchi.report_add_main.js
+++ b/ui/js/src/kimchi.report_add_main.js
@@ -1,8 +1,4 @@
kimchi.report_add_main = function() {
- var generateReportName = function() {
- return 'report-' + new Date().getTime();
- };
-
var addReportForm = $('#form-report-add');
var submitButton = $('#button-report-add');
var nameTextbox = $('input[name="name"]', addReportForm);
@@ -11,8 +7,12 @@ kimchi.report_add_main = function() {
if(submitButton.prop('disabled')) {
return false;
}
- var reportName = nameTextbox.val() || generateReportName();
- nameTextbox.val(reportName);
+ var reportName = nameTextbox.val();
+ var validator = RegExp("^[A-Za-z0-9-]*$");
+ if (!validator.test(reportName)) {
+ errorMessage.text(i18n['KCHDR6011M']);
+ return false;
+ }
var formData = addReportForm.serializeObject();
errorMessage.text('');
submitButton
@@ -25,8 +25,13 @@ kimchi.report_add_main = function() {
result: result
});
}, function(result) {
- result && result['reason'] &&
- $('#report-error-message').text(result['reason']);
+ if (result['reason']) {
+ var errText = result['reason'];
+ }
+ else {
+ var errText = result['responseJSON']['reason'];
+ }
+ result && $('#report-error-message').text(errText);
submitButton
.text(i18n['KCHDR6006M'])
.prop('disabled', false);
diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl
index 613b707..eabf897 100644
--- a/ui/pages/i18n.html.tmpl
+++ b/ui/pages/i18n.html.tmpl
@@ -92,7 +92,7 @@ var i18n = {
'KCHDR6008M': "$_("Rename")",
'KCHDR6009M': "$_("Remove")",
'KCHDR6010M': "$_("Download")",
-
+ 'KCHDR6011M': "$_("Report name should contain only letters, digits
and/or hyphen ('-').")",
'KCHVM6001M': "$_("This will delete the virtual machine and its
virtual disks. This operation cannot be undone. Would you like to continue?")",
diff --git a/ui/pages/report-add.html.tmpl b/ui/pages/report-add.html.tmpl
index 2a962d4..383175f 100644
--- a/ui/pages/report-add.html.tmpl
+++ b/ui/pages/report-add.html.tmpl
@@ -38,7 +38,7 @@
</h2>
<div class="field">
<span>
- $_("The name used to identify the report. If omitted, a name
will be chosen based on current time. Name can contain: letters, digits, \"-\",
\"_\", or \".\".")
+ $_("The name used to identify the report. If omitted, a name
will be chosen based on current time. Name can contain: letters, digits and hyphen
(\"-\").")
</span>
<input type="text" class="text"
id="report-name-textbox" name="name" />
<span id="report-error-message"></span>
--
1.8.5.3
7:02 a.m.
New subject: [PATCH V2 2/2] Fix debug report naming problem (UI)
Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
On 02/21/2014 06:10 PM, Rodrigo Trujillo wrote:
The tool sosreport only accepts letters, digits and hyphen in the
name
of the report, non alphanumeric characteres are removed from the file
name and then Kimchi is not able to find the file.
This patch modifies the UI html and js in order to show the right
information and catch the right return error message.
Implements name checking in frontend.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com>
---
ui/js/src/kimchi.report_add_main.js | 21 +++++++++++++--------
ui/pages/i18n.html.tmpl | 2 +-
ui/pages/report-add.html.tmpl | 2 +-
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/ui/js/src/kimchi.report_add_main.js b/ui/js/src/kimchi.report_add_main.js
index f893d85..757ee21 100644
--- a/ui/js/src/kimchi.report_add_main.js
+++ b/ui/js/src/kimchi.report_add_main.js
@@ -1,8 +1,4 @@
kimchi.report_add_main = function() {
- var generateReportName = function() {
- return 'report-' + new Date().getTime();
- };
-
var addReportForm = $('#form-report-add');
var submitButton = $('#button-report-add');
var nameTextbox = $('input[name="name"]', addReportForm);
@@ -11,8 +7,12 @@ kimchi.report_add_main = function() {
if(submitButton.prop('disabled')) {
return false;
}
- var reportName = nameTextbox.val() || generateReportName();
- nameTextbox.val(reportName);
+ var reportName = nameTextbox.val();
+ var validator = RegExp("^[A-Za-z0-9-]*$");
+ if (!validator.test(reportName)) {
+ errorMessage.text(i18n['KCHDR6011M']);
+ return false;
+ }
var formData = addReportForm.serializeObject();
errorMessage.text('');
submitButton
@@ -25,8 +25,13 @@ kimchi.report_add_main = function() {
result: result
});
}, function(result) {
- result && result['reason'] &&
- $('#report-error-message').text(result['reason']);
+ if (result['reason']) {
+ var errText = result['reason'];
+ }
+ else {
+ var errText = result['responseJSON']['reason'];
+ }
+ result && $('#report-error-message').text(errText);
submitButton
.text(i18n['KCHDR6006M'])
.prop('disabled', false);
diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl
index 613b707..eabf897 100644
--- a/ui/pages/i18n.html.tmpl
+++ b/ui/pages/i18n.html.tmpl
@@ -92,7 +92,7 @@ var i18n = {
'KCHDR6008M': "$_("Rename")",
'KCHDR6009M': "$_("Remove")",
'KCHDR6010M': "$_("Download")",
-
+ 'KCHDR6011M': "$_("Report name should contain only letters, digits
and/or hyphen ('-').")",
'KCHVM6001M': "$_("This will delete the virtual machine and its
virtual disks. This operation cannot be undone. Would you like to continue?")",
diff --git a/ui/pages/report-add.html.tmpl b/ui/pages/report-add.html.tmpl
index 2a962d4..383175f 100644
--- a/ui/pages/report-add.html.tmpl
+++ b/ui/pages/report-add.html.tmpl
@@ -38,7 +38,7 @@
</h2>
<div class="field">
<span>
- $_("The name used to identify the report. If omitted, a
name will be chosen based on current time. Name can contain: letters, digits,
\"-\", \"_\", or \".\".")
+ $_("The name used to identify the report. If omitted, a
name will be chosen based on current time. Name can contain: letters, digits and hyphen
(\"-\").")
</span>
<input type="text" class="text"
id="report-name-textbox" name="name" />
<span id="report-error-message"></span>
3953
days inactive
3956
days old
5 comments
2 participants
participants (2)
-
Aline Manera -
Rodrigo Trujillo