475151 - Refactor the XML-RPC server stuff out of Bugzilla::WebService

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Max Kanat-Alexander]

Attachment: v1

When I started working on the JSON bug (bug 432907), I realized that a lot of refactoring was going to be necessary. Anyhow, to have multiple different WebService frontends, we'll need to refactor the actual RPC server out of Bugzilla::WebService.

I figured I'd split this patch out of the JSON patch into a separate bug because otherwise the JSON patch would be way too large.

Comment 1

[Max Kanat-Alexander]

Attachment: v2

I realized that handle_login needs to be used by both JSON and XML-RPC, so I factored it out into Bugzilla::WebService::Server.

Comment 2

[Max Kanat-Alexander]

Attachment: v3

Oops. Previous patch accidentally broke webservice extensions.

Comment 3

[Max Kanat-Alexander]

Comment on attachment 358591 [details] [diff] [review]
v3

I'll take review from whoever gets to this first.

Comment 4

[David Lawrence [:dkl]]

t/002goodperl........36/486 
#   Failed test 'Bugzilla/WebService/Server.pm DOES NOT use strict --WARNING'
#   at t/002goodperl.t line 99.
# Looks like you failed 1 test of 486.
t/002goodperl........ Dubious, test returned 1 (wstat 256, 0x100)
 Failed 1/486 subtests

Comment 5

[Max Kanat-Alexander]

Attachment: v4

Fixed.

Comment 6

[David Lawrence [:dkl]]

In Bugzilla/Webservice.pm:

sub type {
    my ($self, $type, $value) = @_;
    if ($type eq 'dateTime') {
        $value = $self->datetime_format($value);
    }
    return XMLRPC::Data->type($type)->value($value);
}

This seems very specific to XMLRPC as written. Should this be moved to Bugzilla/WebService/Server/XMLRPC.pm?

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> This seems very specific to XMLRPC as written. Should this be moved to
> Bugzilla/WebService/Server/XMLRPC.pm?

  Yes, it is on the dependent patch, I believe.

Comment 8

[David Lawrence [:dkl]]

(In reply to comment #7)
> (In reply to comment #6)
> > This seems very specific to XMLRPC as written. Should this be moved to
> > Bugzilla/WebService/Server/XMLRPC.pm?
> 
>   Yes, it is on the dependent patch, I believe.

Ok cool.

As for the cookie issue. I am still debugging but I did notice that 'on_dispatch' is not being called correctly when set via Bugzilla::WebService::Server::XMLRPC::initialize. If I change the line in xmlrpc.cgi to the following, cookie auth works again as before.

-Bugzilla::WebService::Server::XMLRPC->handle();
+Bugzilla::WebService::Server::XMLRPC
+    ->on_action(sub { +Bugzilla::WebService::Server::XMLRPC->handle_login(WS_DISPATCH, @_) } )
+    ->handle();

Maybe you will see something related to this before I get it.

Dave

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> As for the cookie issue. I am still debugging but I did notice that
> 'on_dispatch' is not being called correctly when set via
> Bugzilla::WebService::Server::XMLRPC::initialize. 

  Actually, I can't reproduce this. What version of SOAP::Lite are you using? Have you tried upgrading?

Comment 10

[Max Kanat-Alexander]

Attachment: v5

Oh, actually, you were right. I did reproduce it, and here's the fix for xmlrpc.cgi.

Comment 11

[David Lawrence [:dkl]]

(In reply to comment #7)
> (In reply to comment #6)
> > This seems very specific to XMLRPC as written. Should this be moved to
> > Bugzilla/WebService/Server/XMLRPC.pm?
> 
>   Yes, it is on the dependent patch, I believe.

/me baffles. I am not able to see which bug id this is addressed in. Should this be another bug or fixed in this one?

Dave

Comment 12

[Max Kanat-Alexander]

(In reply to comment #11)
> /me baffles. I am not able to see which bug id this is addressed in. Should
> this be another bug or fixed in this one?

  Oh, no, actually it belongs in Bugzilla::WebService. See the README for why.

Comment 13

[David Lawrence [:dkl]]

Comment on attachment 361687 [details] [diff] [review]
v5

Ok, I understand now about the type() question. Also in IRC we discussed that issues related to POD documention will be addressed the in a different bug which will be checked in after this.
This is also dependent on the USAGE_MODE_WEBSERVICE issue being addressed in the JSON patch.

r=dkl

Comment 14

[Max Kanat-Alexander]

Checking in xmlrpc.cgi;
/cvsroot/mozilla/webtools/bugzilla/xmlrpc.cgi,v  <--  xmlrpc.cgi
new revision: 1.11; previous revision: 1.10
done
Checking in Bugzilla/WebService.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService.pm,v  <--  WebService.pm
new revision: 1.17; previous revision: 1.16
done
Checking in Bugzilla/WebService/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Constants.pm,v  <--  Constants.pm
new revision: 1.24; previous revision: 1.23
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Server.pm,v
done
Checking in Bugzilla/WebService/Server.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Server.pm,v  <--  Server.pm
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Server/XMLRPC.pm,v
done
Checking in Bugzilla/WebService/Server/XMLRPC.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Server/XMLRPC.pm,v  <--  XMLRPC.pm
initial revision: 1.1
done