339385 - Make Bugzilla::Version use Bugzilla::Object

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

Comment 1

[Frédéric Buclin]

Versions have neither an ID nor a name. How do you plan to use Bugzilla::Object in this case?

Comment 2

[Max Kanat-Alexander]

Oh, I could come up with some way. Maybe we could add a product_id to Bugzilla::Object, and we could make "value" into the name field.

Comment 3

[Frédéric Buclin]

We are in "soft freeze" mode to prepare 3.0 RC1.

Comment 4

[Max Kanat-Alexander]

Retargeting to 3.0 for perf reasons.

Comment 5

[Max Kanat-Alexander]

Attachment: v1

This one requires the component-object patch to be applied first.

Comment 6

[Olav Vitters]

Attachment: dbi.prof non-cached from 2.23.3+ (patch applied)

The Component patch was applied as well.

Comment 7

[Olav Vitters]

Attachment: dbiprof --number 25 for 2.23.3+ non-cached

Decreased --number to 25. The rest is just noise anyway.

Comment 8

[Olav Vitters]

From attachment 249037 [details] you can see the *total* queries ran by query.cgi for my testcase is a staggering 7021 (this is after applying the patch in bug 339380). After applying the patch, query.cgi now runs 4458 queries in total (see attachment 249043 [details]). So just this one patch decreases the total number of queries executed by query.cgi (for 345 products) by 2563. See the attachments for details.

The total time taken to run all queries executed by query.cgi decreased from 7.62 to 4.80 (2.82 seconds faster). This is of course not the total execution time of query.cgi (just the time spent on database calls).

Comment 9

[Frédéric Buclin]

Comment on attachment 249033 [details] [diff] [review]
v1

>diff -u Bugzilla/Object.pm Bugzilla/Object.pm

>-sub id                { return $_[0]->{'id'};          }
>-sub name              { return $_[0]->{'name'};        }
>+sub id   { return $_[0]->{$_[0]->ID_FIELD};   }
>+sub name { return $_[0]->{$_[0]->NAME_FIELD}; }

This change seems to break User.pm. When viewing a bug with this patch applied, no reporter and no assignee is displayed, my saved searches are gone and the UI is the one you get when being logged out, despite I'm logged in as an admin. Maybe some conflict with

use constant NAME_FIELD => 'login_name';
use constant ID_FIELD   => 'userid';

defined in User.pm?

Comment 10

[Frédéric Buclin]

My web server error log if full of:

show_bug.cgi: Use of uninitialized value in numeric eq (==) at Bugzilla/Bug.pm line 1890.
show_bug.cgi: Use of uninitialized value in numeric eq (==) at Bugzilla/Bug.pm line 1934.

which are respectively:

if ($self->{'assigned_to_id'} == $user->id)
and
if (!$self->{'error'} && $self->{'reporter_id'} == $user->id)

so the problem seems related to $class->ID_FIELD vs 'id'.

Comment 11

[Max Kanat-Alexander]

Attachment: v2

Okay, I haven't had a chance to test this, but it should fix it.

Comment 12

[Frédéric Buclin]

Comment on attachment 249051 [details] [diff] [review]
v2

>Index: Bugzilla/Version.pm

>@@ -110,7 +108,7 @@

>-    my $version = new Bugzilla::Version($self->product_id, $name);
>+    my $version = new Bugzilla::Version($self->id);

This change is wrong. We cannot edit versions anymore, because $version will always exists and an error will be thrown. $name is the *new* name for the version, not the current one, so you really want to check if the new $name is not already in use. You must write:

 { product => $product, name => $name }


r=LpSolit with this fix on checkin.

Comment 13

[Max Kanat-Alexander]

Okay, I did the checkin fix.

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.69; previous revision: 1.68
done
Checking in Bugzilla/Product.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v  <--  Product.pm
new revision: 1.23; previous revision: 1.22
done
Checking in Bugzilla/Version.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Version.pm,v  <--  Version.pm
new revision: 1.11; previous revision: 1.10
done
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.79; previous revision: 1.78
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.24; previous revision: 1.23
done