View Issue Details

IDProjectCategoryView StatusLast Update
0011061mantisbtapi soappublic2010-02-22 14:35
Reporterrombert Assigned Tovboctor  
PrioritylowSeverityminorReproducibilityalways
Status closedResolutionfixed 
Fixed in Version1.2.0 
Summary0011061: SOAP issue tests fail
Description

Out of the box, 9 of the SOAP tests fail. All of them seem to be related to

<pre>SoapFault: Error Type: SYSTEM NOTICE,
Error Description:
Undefined index: handler,
Stack Trace:
mc_api.php L0 mc_error_handler()
nusoap.php L0 mc_issue_add()
nusoap.php L3997 call_user_func_array()
nusoap.php L3686 invoke_method()
mantisconnect.php L1390 service()
mantisconnect.php L0 {main}()</pre>

Tagspatch
Attached Files
0001-Don-t-require-the-handler-to-be-set.patch (966 bytes)   
From 234c1324de546077759888c61490057ced9b40a7 Mon Sep 17 00:00:00 2001
From: Robert Munteanu <robert.munteanu@gmail.com>
Date: Wed, 21 Oct 2009 10:39:34 +0300
Subject: [PATCH] Don't require the handler to be set

---
 api/soap/mc_issue_api.php |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php
index 266c13b..e320f07 100644
--- a/api/soap/mc_issue_api.php
+++ b/api/soap/mc_issue_api.php
@@ -415,7 +415,7 @@ function mc_issue_add( $p_username, $p_password, $p_issue ) {
 		return new soap_fault( 'Client', '', 'Access Denied' );
 	}
 
-	$t_handler_id = mci_get_user_id( $p_issue['handler'] );
+	$t_handler_id = isset ( $p_issue['handler'] ) ?  mci_get_user_id( $p_issue['handler'] ) : 0 ;
 	$t_priority_id = mci_get_priority_id( $p_issue['priority'] );
 	$t_severity_id = mci_get_severity_id( $p_issue['severity'] );
 	$t_status_id = mci_get_status_id( $p_issue['status'] );
-- 
1.6.4.2

0001-Set-default-values-for-fields-in-mc_issue_add.patch (1,823 bytes)   
From 4cf793bcfb9262bac79e795962a84a615c3bc23e Mon Sep 17 00:00:00 2001
From: Robert Munteanu <robert.munteanu@gmail.com>
Date: Wed, 21 Oct 2009 11:14:24 +0300
Subject: [PATCH] Set default values for fields in mc_issue_add

---
 api/soap/mc_issue_api.php |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php
index 266c13b..dd80a91 100644
--- a/api/soap/mc_issue_api.php
+++ b/api/soap/mc_issue_api.php
@@ -415,11 +415,11 @@ function mc_issue_add( $p_username, $p_password, $p_issue ) {
 		return new soap_fault( 'Client', '', 'Access Denied' );
 	}
 
-	$t_handler_id = mci_get_user_id( $p_issue['handler'] );
-	$t_priority_id = mci_get_priority_id( $p_issue['priority'] );
-	$t_severity_id = mci_get_severity_id( $p_issue['severity'] );
-	$t_status_id = mci_get_status_id( $p_issue['status'] );
-	$t_reproducibility_id = mci_get_reproducibility_id( $p_issue['reproducibility'] );
+	$t_handler_id = isset( $p_issue['handler'] ) ? mci_get_user_id( $p_issue['handler'] ) : 0;
+	$t_priority_id = isset( $p_issue['priority'] ) ? mci_get_priority_id( $p_issue['priority'] ) : config_get( 'default_bug_priority' );
+	$t_severity_id = isset( $p_issue['severity'] ) ?  mci_get_severity_id( $p_issue['severity'] ) : config_get( 'default_bug_severity' );
+	$t_status_id = isset ( $p_issue['status'] ) ? mci_get_status_id( $p_issue['status'] ) : config_get( 'bug_submit_status' );
+	$t_reproducibility_id = isset ( $p_issue['reproducibility'] ?  mci_get_reproducibility_id( $p_issue['reproducibility'] ) : config_get( 'default_bug_reproducibility' );
 	$t_resolution_id = mci_get_resolution_id( $p_issue['resolution'] );
 	$t_projection_id = mci_get_projection_id( $p_issue['projection'] );
 	$t_eta_id = mci_get_eta_id( $p_issue['eta'] );
-- 
1.6.4.2

Activities

rombert

rombert

2009-10-21 03:41

reporter   ~0023257

If I apply the attached patch then I get further errors, such as

<pre>SoapFault: Error Type: SYSTEM NOTICE,
Error Description:
Undefined index: priority,
Stack Trace:
mc_api.php L0 mc_error_handler()
nusoap.php L0 mc_issue_add()
nusoap.php L3997 call_user_func_array()
nusoap.php L3686 invoke_method()
mantisconnect.php L1390 service()
mantisconnect.php L0 {main}()</pre>

Apparently we don't validate for all the required fields being set. I'm not sure what the correct behaviour is here:

<ol>
<li>Fail if the required fields are not set</li>
<li>Set hardcoded default, e.g lowest priority, no handler</li>
<li>Read the default configured settings, if any</li>
</ol>

Thoughts?

vboctor

vboctor

2009-10-21 03:50

manager   ~0023259

At one point methods like mci_get_userid() were taking a reference (&) and the isset check was within the method. At one point such references were removed causing the parameter to be evaluated while it is not set. The fix is to make sure isset is called on all optional parameters before calling the mci* methods.

Relating to the correct behavior, you can look inside the mci_get_xxx() method and get the move out the isset() -> return blah code from it.

rombert

rombert

2009-10-21 04:17

reporter   ~0023262

Please ignore the second patch, it has a syntax error.

rombert

rombert

2009-10-21 04:41

reporter   ~0023263

Now I have the IssueAdd and IssueNote tests passing. The IssueUpdate tests are still failing, with similar errors. I'm not sure what the behaviour should be here. I can read the default settings for the update as well, but this would overwrite the existing data ( see 0010323 ).

rombert

rombert

2009-10-21 05:17

reporter   ~0023264

The following changes since commit b74ae4e512d106ce33cad82abb768dcfcae88d67:
Robert Munteanu (1):
Document the testing process

are available in the git repository at:

git://github.com/rombert/mantisbt.git soap_tests

Robert Munteanu (1):
Fixes 0011061: SOAP issue tests fail

api/soap/mc_issue_api.php | 48 ++++++++++++++++++++++----------------------
1 files changed, 24 insertions(+), 24 deletions(-)

vboctor

vboctor

2009-10-21 23:52

manager   ~0023274

Robert, I've pulled from your branch and applies to master and master-1.2.x. Here are my questions / comments:

  1. Shouldn't we remove the isset() checks from inside the mci_get_xxx() methods?

  2. Should this issue be resolved based on your patch? Are all tests passing now?

  3. There should be no spaces between function names and (). For example, "isset ( xxx )" should be "isset( xxx )".

rombert

rombert

2009-10-22 02:21

reporter   ~0023277

  1. Yes, we should.
  2. All tests pass now for me.
  3. Understood.

I'll prep an additional patch with issues 1 and 3 later on today. Suggest you keep this open so that I can attach the patch.

rombert

rombert

2009-10-22 10:41

reporter   ~0023280

This should address your comments.

The following changes since commit 112122eddea6ee18cfa36b636239abb580793d5e:
Robert Munteanu (1):
Fixes 0011061: SOAP issue tests fail

are available in the git repository at:

git://github.com/rombert/mantisbt.git soap_tests

Robert Munteanu (1):
Issue 0011061: SOAP issue tests fail

api/soap/mc_api.php | 35 -----------------------------------
api/soap/mc_issue_api.php | 38 +++++++++++++++++++-------------------
2 files changed, 19 insertions(+), 54 deletions(-)

vboctor

vboctor

2009-10-23 00:04

manager   ~0023288

Thanks @rombert.

Related Changesets

MantisBT: master-1.2.x ef8e2a39

2009-10-21 04:14

rombert

Committer: vboctor


Details Diff
Fixes 0011061: SOAP issue tests fail

Values not passed to mc_issue_add or mc_issue_update now have values
set to default.

According to vboctor at one point methods like mci_get_user_id() were taking
a reference (&) and the isset check was within the method. At one point
such references were removed causing the parameter to be evaluated while
it is not set.
Affected Issues
0011061
mod - api/soap/mc_issue_api.php Diff File

MantisBT: master 112122ed

2009-10-21 04:14

rombert


Details Diff
Fixes 0011061: SOAP issue tests fail

Values not passed to mc_issue_add or mc_issue_update now have values
set to default.

According to vboctor at one point methods like mci_get_user_id() were taking
a reference (&) and the isset check was within the method. At one point
such references were removed causing the parameter to be evaluated while
it is not set.
Affected Issues
0011061
mod - api/soap/mc_issue_api.php Diff File

MantisBT: master-1.2.x bfd2eae2

2009-10-22 04:18

rombert

Committer: vboctor


Details Diff
Issue 0011061: SOAP issue tests fail

Fix according to review:

- style fixes: there should be no spaces between function names and ()
- removed isset checks from some of the mci_get functions called
by mc_issue_add and mc_issue_update since they are no longer used.
Affected Issues
0011061
mod - api/soap/mc_api.php Diff File
mod - api/soap/mc_issue_api.php Diff File

MantisBT: master 85bcf015

2009-10-22 04:18

rombert

Committer: vboctor


Details Diff
Issue 0011061: SOAP issue tests fail

Fix according to review:

- style fixes: there should be no spaces between function names and ()
- removed isset checks from some of the mci_get functions called
by mc_issue_add and mc_issue_update since they are no longer used.
Affected Issues
0011061
mod - api/soap/mc_issue_api.php Diff File
mod - api/soap/mc_api.php Diff File