From c047bc48cec495dfba3eaf1146a91acc1e9f8d70 Mon Sep 17 00:00:00 2001 From: dklawren Date: Tue, 13 Feb 2018 10:25:35 -0500 Subject: Bug 1434064 - Refactor Project.pm to use Moo for better type checking --- extensions/PhabBugz/bin/update_project_members.pl | 4 +- extensions/PhabBugz/lib/Policy.pm | 7 +- extensions/PhabBugz/lib/Project.pm | 302 +++++++++++----------- 3 files changed, 154 insertions(+), 159 deletions(-) (limited to 'extensions') diff --git a/extensions/PhabBugz/bin/update_project_members.pl b/extensions/PhabBugz/bin/update_project_members.pl index 06cc55626..a0449a915 100755 --- a/extensions/PhabBugz/bin/update_project_members.pl +++ b/extensions/PhabBugz/bin/update_project_members.pl @@ -55,10 +55,10 @@ my $sync_groups = Bugzilla::Group->match({ name => [ split('[,\s]+', $phab_sync_ foreach my $group (@$sync_groups) { # Create group project if one does not yet exist my $phab_project_name = 'bmo-' . $group->name; - my $project = Bugzilla::Extension::PhabBugz::Project->new({ + my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ name => $phab_project_name }); - if (!$project->id) { + if (!$project) { $project = Bugzilla::Extension::PhabBugz::Project->create({ name => $phab_project_name, description => 'BMO Security Group for ' . $group->name diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm index 3205562c3..23f04b354 100644 --- a/extensions/PhabBugz/lib/Policy.pm +++ b/extensions/PhabBugz/lib/Policy.pm @@ -83,9 +83,8 @@ sub new_from_query { my ($class, $params) = @_; my $result = request('policy.query', $params); if (exists $result->{result}{data} && @{ $result->{result}{data} }) { - return $result->{result}->{data}->[0]; + return $class->new($result->{result}->{data}->[0]); } - return $class->new($result); } sub create { @@ -105,7 +104,7 @@ sub create { if (@$project_names) { my $project_phids = []; foreach my $project_name (@$project_names) { - my $project = Bugzilla::Extension::PhabBugz::Project->new({ name => $project_name }); + my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ name => $project_name }); push @$project_phids, $project->phid if $project; } @@ -134,7 +133,7 @@ sub _build_rule_projects { return [ map { $_->name } grep { $_ } - map { Bugzilla::Extension::PhabBugz::Project->new( { phids => [$_] } ) } + map { Bugzilla::Extension::PhabBugz::Project->new_from_query( { phids => [$_] } ) } @{ $rule->{value} } ]; } diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm index ec00b9532..91dc2133d 100644 --- a/extensions/PhabBugz/lib/Project.pm +++ b/extensions/PhabBugz/lib/Project.pm @@ -8,46 +8,66 @@ package Bugzilla::Extension::PhabBugz::Project; use 5.10.1; -use strict; -use warnings; +use Moo; +use Types::Standard -all; +use Type::Utils; use Bugzilla::Error; use Bugzilla::Util qw(trim); use Bugzilla::Extension::PhabBugz::Util qw( - request - get_phab_bmo_ids + request + get_phab_bmo_ids ); -use Types::Standard -all; -use Type::Utils; +######################### +# Initialization # +######################### + +has id => ( is => 'ro', isa => Int ); +has phid => ( is => 'ro', isa => Str ); +has type => ( is => 'ro', isa => Str ); +has name => ( is => 'ro', isa => Str ); +has description => ( is => 'ro', isa => Str ); +has creation_ts => ( is => 'ro', isa => Str ); +has modification_ts => ( is => 'ro', isa => Str ); +has view_policy => ( is => 'ro', isa => Str ); +has edit_policy => ( is => 'ro', isa => Str ); +has join_policy => ( is => 'ro', isa => Str ); +has members_raw => ( is => 'ro', isa => ArrayRef [ Dict [ phid => Str ] ] ); +has members => ( is => 'lazy', isa => ArrayRef [Object] ); + +sub new_from_query { + my ( $class, $params ) = @_; + + my $data = { + queryKey => 'all', + attachments => { members => 1 }, + constraints => $params + }; + + my $result = request( 'project.search', $data ); + if ( exists $result->{result}{data} && @{ $result->{result}{data} } ) { + return $class->new( $result->{result}{data}[0] ); + } +} + +sub BUILDARGS { + my ( $class, $params ) = @_; + + $params->{name} = $params->{fields}->{name}; + $params->{description} = $params->{fields}->{description}; + $params->{creation_ts} = $params->{fields}->{dateCreated}; + $params->{modification_ts} = $params->{fields}->{dateModified}; + $params->{view_policy} = $params->{fields}->{policy}->{view}; + $params->{edit_policy} = $params->{fields}->{policy}->{edit}; + $params->{join_policy} = $params->{fields}->{policy}->{join}; + $params->{members_raw} = $params->{attachments}->{members}->{members}; -my $SearchResult = Dict[ - id => Int, - type => Str, - phid => Str, - fields => Dict[ - name => Str, - slug => Str, - depth => Int, - milestone => Maybe[Str], - parent => Maybe[Str], - icon => Dict[ key => Str, name => Str, icon => Str ], - color => Dict[ key => Str, name => Str ], - dateCreated => Int, - dateModified => Int, - policy => Dict[ view => Str, edit => Str, join => Str ], - description => Maybe[Str] - ], - attachments => Dict[ - members => Dict[ - members => ArrayRef[ - Dict[ - phid => Str - ], - ], - ], - ], -]; + delete $params->{fields}; + delete $params->{attachments}; + + return $params; +} # { # "data": [ @@ -107,45 +127,15 @@ my $SearchResult = Dict[ # } # } -######################### -# Initialization # -######################### - -sub new { - my ($class, $params) = @_; - my $self = $params ? _load($params) : {}; - $SearchResult->assert_valid($self); - return bless($self, $class); -} - -sub _load { - my ($params) = @_; - - my $data = { - queryKey => 'all', - attachments => { - members => 1 - }, - constraints => $params - }; - - my $result = request('project.search', $data); - if (exists $result->{result}{data} && @{ $result->{result}{data} }) { - return $result->{result}->{data}->[0]; - } - - return $result; -} - ######################### # Modification # ######################### sub create { - my ($class, $params) = @_; + my ( $class, $params ) = @_; - my $name = trim($params->{name}); - $name || ThrowCodeError('param_required', { param => 'name' }); + my $name = trim( $params->{name} ); + $name || ThrowCodeError( 'param_required', { param => 'name' } ); my $description = $params->{description} || 'Need description'; my $view_policy = $params->{view_policy} || 'admin'; @@ -154,19 +144,20 @@ sub create { my $data = { transactions => [ - { type => 'name', value => $name }, + { type => 'name', value => $name }, { type => 'description', value => $description }, { type => 'edit', value => $edit_policy }, { type => 'join', value => $join_policy }, { type => 'view', value => $view_policy }, - { type => 'icon', value => 'group' }, - { type => 'color', value => 'red' } + { type => 'icon', value => 'group' }, + { type => 'color', value => 'red' } ] }; - my $result = request('project.edit', $data); + my $result = request( 'project.edit', $data ); - return $class->new({ phids => $result->{result}{object}{phid} }); + return $class->new_from_query( + { phids => $result->{result}{object}{phid} } ); } sub update { @@ -177,137 +168,142 @@ sub update { transactions => [] }; - if ($self->{set_name}) { - push(@{ $data->{transactions} }, { - type => 'name', - value => $self->{set_name} - }); + if ( $self->{set_name} ) { + push( + @{ $data->{transactions} }, + { + type => 'name', + value => $self->{set_name} + } + ); } - if ($self->{set_description}) { - push(@{ $data->{transactions} }, { - type => 'description', - value => $self->{set_description} - }); + if ( $self->{set_description} ) { + push( + @{ $data->{transactions} }, + { + type => 'description', + value => $self->{set_description} + } + ); } - if ($self->{set_members}) { - push(@{ $data->{transactions} }, { - type => 'members.set', - value => $self->{set_members} - }); + if ( $self->{set_members} ) { + push( + @{ $data->{transactions} }, + { + type => 'members.set', + value => $self->{set_members} + } + ); } else { - if ($self->{add_members}) { - push(@{ $data->{transactions} }, { - type => 'members.add', - value => $self->{add_members} - }); + if ( $self->{add_members} ) { + push( + @{ $data->{transactions} }, + { + type => 'members.add', + value => $self->{add_members} + } + ); } - if ($self->{remove_members}) { - push(@{ $data->{transactions} }, { - type => 'members.remove', - value => $self->{remove_members} - }); + if ( $self->{remove_members} ) { + push( + @{ $data->{transactions} }, + { + type => 'members.remove', + value => $self->{remove_members} + } + ); } } - if ($self->{set_policy}) { - foreach my $name ("view", "edit") { + if ( $self->{set_policy} ) { + foreach my $name ( "view", "edit" ) { next unless $self->{set_policy}->{$name}; - push(@{ $data->{transactions} }, { - type => $name, - value => $self->{set_policy}->{$name} - }); + push( + @{ $data->{transactions} }, + { + type => $name, + value => $self->{set_policy}->{$name} + } + ); } } - my $result = request('project.edit', $data); + my $result = request( 'project.edit', $data ); return $result; } -######################### -# Accessors # -######################### - -sub id { return $_[0]->{id}; } -sub phid { return $_[0]->{phid}; } -sub type { return $_[0]->{type}; } -sub name { return $_[0]->{fields}->{name}; } -sub description { return $_[0]->{fields}->{description}; } -sub creation_ts { return $_[0]->{fields}->{dateCreated}; } -sub modification_ts { return $_[0]->{fields}->{dateModified}; } - -sub view_policy { return $_[0]->{fields}->{policy}->{view}; } -sub edit_policy { return $_[0]->{fields}->{policy}->{edit}; } -sub join_policy { return $_[0]->{fields}->{policy}->{join}; } - -sub members_raw { return $_[0]->{attachments}->{members}->{members}; } - -sub members { - my ($self) = @_; - return $self->{members} if $self->{members}; - - my @phids; - foreach my $member (@{ $self->members_raw }) { - push(@phids, $member->{phid}); - } - - return [] if !@phids; - - my $users = get_phab_bmo_ids({ phids => \@phids }); - - my @members; - foreach my $user (@$users) { - my $member = Bugzilla::User->new({ id => $user->{id}, cache => 1}); - $member->{phab_phid} = $user->{phid}; - push(@members, $member); - } - - return \@members; -} - ######################### # Mutators # ######################### sub set_name { - my ($self, $name) = @_; + my ( $self, $name ) = @_; $name = trim($name); $self->{set_name} = $name; } sub set_description { - my ($self, $description) = @_; + my ( $self, $description ) = @_; $description = trim($description); $self->{set_description} = $description; } sub add_member { - my ($self, $member) = @_; + my ( $self, $member ) = @_; $self->{add_members} ||= []; my $member_phid = blessed $member ? $member->phab_phid : $member; - push(@{ $self->{add_members} }, $member_phid); + push( @{ $self->{add_members} }, $member_phid ); } sub remove_member { - my ($self, $member) = @_; + my ( $self, $member ) = @_; $self->{remove_members} ||= []; my $member_phid = blessed $member ? $member->phab_phid : $member; - push(@{ $self->{remove_members} }, $member_phid); + push( @{ $self->{remove_members} }, $member_phid ); } sub set_members { - my ($self, $members) = @_; + my ( $self, $members ) = @_; $self->{set_members} = [ map { $_->phab_phid } @$members ]; } sub set_policy { - my ($self, $name, $policy) = @_; + my ( $self, $name, $policy ) = @_; $self->{set_policy} ||= {}; $self->{set_policy}->{$name} = $policy; } -1; \ No newline at end of file +############ +# Builders # +############ + +sub _build_members { + my ($self) = @_; + return [] unless $self->members_raw; + + my @phids; + foreach my $member ( @{ $self->members_raw } ) { + push( @phids, $member->{phid} ); + } + + return [] if !@phids; + + my $users = get_phab_bmo_ids( { phids => \@phids } ); + + my @members; + foreach my $user (@$users) { + my $member = Bugzilla::User->new( { id => $user->{id}, cache => 1 } ); + $member->{phab_phid} = $user->{phid}; + push( @members, $member ); + } + + return \@members; +} + +1; + -- cgit v1.2.3-24-g4f1b