1090493 - Allow ComponentWatching extension to work on either bmo/4.2 or upstream 5.0+

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[David Lawrence [:dkl]]

Currently if you install the component watching extension on upstream trunk (5.0+) it will fail during a new database configuration due to components.id changing to a MEDIUMSERIAL instead of a SMALLSERIAL.

We need to make the extension look to see what size components.id is and adjust accordingly.

dkl

Comment 1

[David Lawrence [:dkl]]

Attachment: 1090493_1.patch

Will not work on upstream 4.2 or 4.4 as it still relies on the fix from bug 1082106 which only went into 5.0/master.

dkl

Comment 3

[:glob ✱]

Comment on attachment 8512970 [details] [diff] [review]
1090493_1.patch

Review of attachment 8512970 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/ComponentWatching/Extension.pm
@@ +39,5 @@
> +        if ($field eq 'id') {
> +            $component_id_type = 'INT3' if $def->{TYPE} eq 'MEDIUMSERIAL';
> +            last;
> +        }
> +    }

dcloning to process the list in pairs is messy.
let's use List::Util's pairfirst instead:

my (undef, $def) = pairfirst { $a eq 'id' } @{ $schema->{components}->{FIELDS} };
my $component_id_type = $def->{TYPE} eq 'MEDIUMSERIAL' ? 'INT3' : 'INT2';

Comment 4

[David Lawrence [:dkl]]

Attachment: 1090493_2.patch

Yeah much cleaner.

dkl

Comment 5

[:glob ✱]

Comment on attachment 8579648 [details] [diff] [review]
1090493_2.patch

Review of attachment 8579648 [details] [diff] [review]:
-----------------------------------------------------------------

gah, sorry, it looks like the version of List::Util we have installed is only 1.21, and pairfirst doesn't exist until 1.30.

there's some other nice things in recent List::Util, so i wouldn't be against bumping our minimum version to 1.41 (and filing an associated infra bug to get it deployed).

or you could rewrite it in a way that doesn't require a dclone or pairfirst.

Comment 6

[David Lawrence [:dkl]]

Attachment: 1090493_3.patch

And here it is. Not pretty, but works :)

dkl

Comment 7

[:glob ✱]

Comment on attachment 8584675 [details] [diff] [review]
1090493_3.patch

Review of attachment 8584675 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob with changes on commit

::: extensions/ComponentWatching/Extension.pm
@@ +34,5 @@
> +    # Bugzilla 5.0+, the components.id type
> +    # is INT3, while earlier versions used INT2
> +    my $component_id_type = 'INT2';
> +    my $len = scalar @{ $args->{schema}->{components}->{FIELDS} };
> +    for (my $i = 0; $i < $len; $i++) {

that should be |$i < $len - 1| because you're accessing $i+1
and since they are pairs, use $i += 2.

Comment 8

[David Lawrence [:dkl]]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   426d9ce..634f31d  master -> master