398308 - Search.pm should not depend on the CGI

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement, P1)

Description

[Max Kanat-Alexander]

Right now Search.pm depends on $params being a CGI object, and depends on the whole idea that we're running inside a CGI.

Instead it should take normal named arguments. Ideally, these named arguments should be extremely simple. Also, the boolean chart arguments should be simplified. Something like:

charts => [
    [ ], [ ], [ ]
]

Comment 1

[Rémi Zara]

Why not invent a Bugzilla Query Language (BQL), which would look like SQL, but be independant of the actual SQL schema. Something like:

SELECT bugid, summary, reporter... WHERE (cond1 AND (cond2 OR cond3)) ORDER BY reporter ASC

buglist.cgi would construct such a query (or a "precompiled" form) and pass it to Search.pm
XML-RPC would take such a query...

Comment 2

[Frédéric Buclin]

I really want bug 398281 to be implemented in 3.2, so this makes this bug a rather high priority. Note that removing CGI dependencies means going back to the use of %FORM in Bugzilla 2.18 and older. An alternative could be to build the CGI object from an hash and pass it to Search.pm as we currently do, which is probably easier to implement. Something like:

my $cgi_string = "1=1";
foreach my $key (keys %hash) {
    $cgi_string .= "&$key=" . join("&$key=", @{$hash{$key}});
}
my $cgi_object = new Bugzilla::CGI($cgi_string);

Then you can pass this to Search.pm:
  new Bugzilla::Search({params => $cgi_object})

That's pretty much all you need to use Search.pm from an non-CGI script. We could even put these few lines of code in a function in Util.pm or CGI.pm so that we can use it for flags and attachments too.

Comment 3

[Max Kanat-Alexander]

We already have that as part of QuickSearch.

That's not what I'm talking about here, I'm talking about the API of a library.

Comment 4

[Max Kanat-Alexander]

My last comment was a reply to Remi.

What I'm talking about is re-working the API of Search.pm so that it takes its arguments as a hashref. It should not take a CGI object at all.

Comment 5

[Clement Chan]

It's a coincidence, I exposed the Boolean Charts in bug 398281 exactly like Max what described in the first comment. 

Except, I have to pack the rowref from the boolean chart back into query string tokens and then feed them to the CGI object before passing them to the contructor of Bugzilla::Search.pm

Because the constructor takes a fields key in the hash, I can specify a single column 'bugs.bug_id'. Then executing the getSQL() statement from Search will yield a list of bug ids. 

Then I passed the bug list into Bugzilla::WebService::Bug::get_bugs({ids=>\@ids}), to create the list of Bugzilla::Bug objects. 

But the thing is that all is done under SOAP, I don't know it can be done as a regular object though.

Comment 6

[Max Kanat-Alexander]

Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set the "blocking3.2" flag to "?", and either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.

Comment 7

[Frédéric Buclin]

A big task for 4.0, required by WebService, see bug 398281.

Comment 8

[Max Kanat-Alexander]

Should be fairly easy now that the refactoring is done.

Comment 9

[Max Kanat-Alexander]

Attachment: Work In Progress

Comment 10

[Max Kanat-Alexander]

Attachment: v1

Okay, to keep things simple for now, I've retained the interface identically, I've just made it take a hashref instead of a CGI object. I also had to move _convert_old_params into Search.pm instead of it being in Bugzilla::CGI, and update the tests to use the new system instead of a fake CGI object.

Comment 11

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified buglist.cgi
modified collectstats.pl
modified report.cgi
modified whine.pl
modified Bugzilla/CGI.pm
modified Bugzilla/Search.pm
modified xt/lib/Bugzilla/Test/Search/AndTest.pm
missing xt/lib/Bugzilla/Test/Search/FakeCGI.pm
modified xt/lib/Bugzilla/Test/Search/FakeCGI.pm
modified xt/lib/Bugzilla/Test/Search/FieldTest.pm
modified xt/lib/Bugzilla/Test/Search/OrTest.pm
Committed revision 7375.