Search.pm should not depend on the CGI

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Query/Bug List
P1
enhancement
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

3.0.2
Bugzilla 4.2
Bug Flags:
approval +

Details

(Whiteboard: [roadmap: 4.0])

Attachments

(1 attachment, 1 obsolete attachment)

v1
18.49 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
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 => [
    [ ], [ ], [ ]
]
(Assignee)

Updated

11 years ago
Blocks: 398281

Comment 1

11 years ago
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

11 years ago
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.
Priority: -- → P2
Target Milestone: --- → Bugzilla 3.2
(Assignee)

Comment 3

11 years ago
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.
(Assignee)

Comment 4

11 years ago
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

11 years ago
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.







(Assignee)

Comment 6

10 years ago
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.
Target Milestone: Bugzilla 3.2 → Bugzilla 4.0

Comment 7

10 years ago
A big task for 4.0, required by WebService, see bug 398281.
Priority: P2 → P1
Whiteboard: [roadmap: 4.0]
(Assignee)

Updated

10 years ago
No longer blocks: 398281
(Assignee)

Updated

9 years ago
Blocks: 475754
Assignee: query-and-buglist → wicked
(Assignee)

Comment 8

8 years ago
Should be fairly easy now that the refactoring is done.
Assignee: wicked → mkanat
Status: NEW → ASSIGNED
Target Milestone: Bugzilla 5.0 → Bugzilla 4.2
(Assignee)

Comment 9

8 years ago
Created attachment 457548 [details] [diff] [review]
Work In Progress
(Assignee)

Comment 10

8 years ago
Created attachment 457745 [details] [diff] [review]
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.
Attachment #457548 - Attachment is obsolete: true
Attachment #457745 - Flags: review+
(Assignee)

Updated

8 years ago
Flags: approval+
(Assignee)

Comment 11

8 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.