From 2b916fccae0df60b350369c6fc827c1c9ce1030e Mon Sep 17 00:00:00 2001 From: dklawren Date: Wed, 7 Feb 2018 14:09:25 -0500 Subject: Bug 1430259 - Update policy code in BMO PhabBugz extension to update custom policy if a private bugs groups have changed. --- extensions/PhabBugz/lib/Feed.pm | 147 +++++++++++++++++++++++-------------- extensions/PhabBugz/lib/Policy.pm | 142 +++++++++++++++++++++++++++++++++++ extensions/PhabBugz/lib/Project.pm | 97 ++++++++++++++---------- 3 files changed, 295 insertions(+), 91 deletions(-) create mode 100644 extensions/PhabBugz/lib/Policy.pm diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index 8b6801e91..670bc5d24 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -14,7 +14,10 @@ use List::MoreUtils qw(any); use Moo; use Bugzilla::Constants; +use Bugzilla::Util qw(diff_arrays); + use Bugzilla::Extension::PhabBugz::Constants; +use Bugzilla::Extension::PhabBugz::Policy; use Bugzilla::Extension::PhabBugz::Revision; use Bugzilla::Extension::PhabBugz::Util qw( add_security_sync_comments @@ -104,25 +107,7 @@ sub feed_query { } } - my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [$object_phid] }); - - if (!$revision->bug_id) { - if ($story_text =~ /\s+created\s+D\d+/) { - # If new revision and bug id was omitted, make revision public - $self->logger->debug("No bug associated with new revision. Marking public."); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', 'users'); - $revision->update(); - $self->logger->info("SUCCESS"); - } - else { - $self->logger->debug("SKIPPING: No bug associated with revision change"); - } - $self->save_feed_last_id($story_id); - next; - } - - $self->process_revision_change($revision, $story_text); + $self->process_revision_change($object_phid, $story_text); $self->save_feed_last_id($story_id); } } @@ -136,18 +121,28 @@ sub save_feed_last_id { } sub process_revision_change { - my ($self, $revision, $story_text) = @_; + my ($self, $revision_phid, $story_text) = @_; - # Pre setup before making changes - my $old_user = set_phab_user(); + # Load the revision from Phabricator + my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [ $revision_phid ] }); - my $is_shadow_db = Bugzilla->is_shadow_db; - Bugzilla->switch_to_main_db if $is_shadow_db; + # NO BUG ID - my $dbh = Bugzilla->dbh; - $dbh->bz_start_transaction; - - my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); + if (!$revision->bug_id) { + if ($story_text =~ /\s+created\s+D\d+/) { + # If new revision and bug id was omitted, make revision public + $self->logger->debug("No bug associated with new revision. Marking public."); + $revision->set_policy('view', 'public'); + $revision->set_policy('edit', 'users'); + $revision->update(); + $self->logger->info("SUCCESS"); + return; + } + else { + $self->logger->debug("SKIPPING: No bug associated with revision change"); + return; + } + } my $log_message = sprintf( "REVISION CHANGE FOUND: D%d: %s | bug: %d | %s", @@ -157,39 +152,73 @@ sub process_revision_change { $story_text); $self->logger->info($log_message); + # Pre setup before making changes + my $old_user = set_phab_user(); + + my $is_shadow_db = Bugzilla->is_shadow_db; + Bugzilla->switch_to_main_db if $is_shadow_db; + + my $dbh = Bugzilla->dbh; + $dbh->bz_start_transaction; + my $bug = Bugzilla::Bug->new({ id => $revision->bug_id, cache => 1 }); # REVISION SECURITY POLICY - # Do not set policy if a custom policy has already been set - # This keeps from setting new custom policy everytime a change - # is made. - unless ($revision->view_policy =~ /^PHID-PLCY/) { - - # If bug is public then remove privacy policy - if (!@{ $bug->groups_in }) { - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', 'users'); + # If bug is public then remove privacy policy + if (!@{ $bug->groups_in }) { + $self->logger->debug('Bug is public so setting view/edit public'); + $revision->set_policy('view', 'public'); + $revision->set_policy('edit', 'users'); + } + # else bug is private. + else { + my @set_groups = get_security_sync_groups($bug); + + # If bug privacy groups do not have any matching synchronized groups, + # then leave revision private and it will have be dealt with manually. + if (!@set_groups) { + $self->logger->debug('No matching groups. Adding comments to bug and revision'); + add_security_sync_comments([$revision], $bug); } - # else bug is private + # Otherwise, we create a new custom policy containing the project + # groups that are mapped to bugzilla groups. else { - my @set_groups = get_security_sync_groups($bug); + my @set_projects = map { "bmo-" . $_ } @set_groups; + + # If current policy projects matches what we want to set, then + # we leave the current policy alone. + my $current_policy; + if ($revision->view_policy =~ /^PHID-PLCY/) { + $self->logger->debug("Loading current policy: " . $revision->view_policy); + $current_policy + = Bugzilla::Extension::PhabBugz::Policy->new_from_query({ phids => [ $revision->view_policy ]}); + my $current_projects = $current_policy->rule_projects; + $self->logger->debug("Current policy projects: " . join(", ", @$current_projects)); + my ($added, $removed) = diff_arrays($current_projects, \@set_projects); + if (@$added || @$removed) { + $self->logger->debug('Project groups do not match. Need new custom policy'); + $current_policy= undef; + } + else { + $self->logger->debug('Project groups match. Leaving current policy as-is'); + } + } - # If bug privacy groups do not have any matching synchronized groups, - # then leave revision private and it will have be dealt with manually. - if (!@set_groups) { - add_security_sync_comments([$revision], $bug); + if (!$current_policy) { + $self->logger->debug("Creating new custom policy: " . join(", ", @set_projects)); + my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects); + $revision->set_policy('view', $new_policy->phid); + $revision->set_policy('edit', $new_policy->phid); } - my $policy_phid = create_private_revision_policy($bug, \@set_groups); my $subscribers = get_bug_role_phids($bug); - - $revision->set_policy('view', $policy_phid); - $revision->set_policy('edit', $policy_phid); $revision->set_subscribers($subscribers); } } + my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); + my $attachment = create_revision_attachment($bug, $revision->id, $revision->title, $timestamp); # ATTACHMENT OBSOLETES @@ -203,24 +232,28 @@ sub process_revision_change { next if $attach_revision_id != $revision->id; my $make_obsolete = $revision->status eq 'abandoned' ? 1 : 0; + $self->logger->debug('Updating obsolete status on attachmment ' . $attachment->id); $attachment->set_is_obsolete($make_obsolete); - if ($revision->id == $attach_revision_id - && $revision->title ne $attachment->description) { + if ($revision->title ne $attachment->description) { + $self->logger->debug('Updating description on attachment ' . $attachment->id); $attachment->set_description($revision->title); } $attachment->update($timestamp); - last; } # fixup attachments with same revision id but on different bugs + my %other_bugs; my $other_attachments = Bugzilla::Attachment->match({ mimetype => PHAB_CONTENT_TYPE, filename => 'phabricator-D' . $revision->id . '-url.txt', WHERE => { 'bug_id != ? AND NOT isobsolete' => $bug->id } }); foreach my $attachment (@$other_attachments) { + $other_bugs{$attachment->bug_id}++; + $self->logger->debug('Updating obsolete status on attachment ' . + $attachment->id . " for bug " . $attachment->bug_id); $attachment->set_is_obsolete(1); $attachment->update($timestamp); } @@ -228,9 +261,11 @@ sub process_revision_change { # REVIEWER STATUSES my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids); - foreach my $reviewer (@{ $revision->reviewers }) { - push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted'; - push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected'; + unless ($revision->status eq 'changes-planned' || $revision->status eq 'needs-review') { + foreach my $reviewer (@{ $revision->reviewers }) { + push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted'; + push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected'; + } } my $phab_users = get_phab_bmo_ids({ phids => \@accepted_phids }); @@ -301,7 +336,11 @@ sub process_revision_change { $bug->update($timestamp); $revision->update(); - Bugzilla::BugMail::Send($revision->bug_id, { changer => Bugzilla->user }); + # Email changes for this revisions bug and also for any other + # bugs that previously had these revision attachments + foreach my $bug_id ($revision->bug_id, keys %other_bugs) { + Bugzilla::BugMail::Send($bug_id, { changer => Bugzilla->user }); + } $dbh->bz_commit_transaction; Bugzilla->switch_to_shadow_db if $is_shadow_db; diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm new file mode 100644 index 000000000..3205562c3 --- /dev/null +++ b/extensions/PhabBugz/lib/Policy.pm @@ -0,0 +1,142 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::PhabBugz::Policy; + +use 5.10.1; +use Moo; + +use Bugzilla::Error; +use Bugzilla::Extension::PhabBugz::Util qw(request); +use Bugzilla::Extension::PhabBugz::Project; + +use List::Util qw(first); + +use Types::Standard -all; +use Type::Utils; + +has 'phid' => ( is => 'ro', isa => Str ); +has 'type' => ( is => 'ro', isa => Str ); +has 'name' => ( is => 'ro', isa => Str ); +has 'shortName' => ( is => 'ro', isa => Str ); +has 'fullName' => ( is => 'ro', isa => Str ); +has 'href' => ( is => 'ro', isa => Maybe[Str] ); +has 'workflow' => ( is => 'ro', isa => Maybe[Str] ); +has 'icon' => ( is => 'ro', isa => Str ); +has 'default' => ( is => 'ro', isa => Str ); +has 'rules' => ( + is => 'ro', + isa => ArrayRef[ + Dict[ + action => Str, + rule => Str, + value => Maybe[ArrayRef[Str]] + ] + ] +); + +has 'rule_projects' => ( + is => 'lazy', + isa => ArrayRef[Str], +); + +# { +# "data": [ +# { +# "phid": "PHID-PLCY-l2mt4yeq4byqgcot7x4j", +# "type": "custom", +# "name": "Custom Policy", +# "shortName": "Custom Policy", +# "fullName": "Custom Policy", +# "href": null, +# "workflow": null, +# "icon": "fa-certificate", +# "default": "deny", +# "rules": [ +# { +# "action": "allow", +# "rule": "PhabricatorSubscriptionsSubscribersPolicyRule", +# "value": null +# }, +# { +# "action": "allow", +# "rule": "PhabricatorProjectsPolicyRule", +# "value": [ +# "PHID-PROJ-cvurjiwfvh756mv2vhvi" +# ] +# } +# ] +# } +# ], +# "cursor": { +# "limit": 100, +# "after": null, +# "before": null +# } +# } + +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); +} + +sub create { + my ($class, $project_names) = @_; + + my $data = { + objectType => 'DREV', + default => 'deny', + policy => [ + { + action => 'allow', + rule => 'PhabricatorSubscriptionsSubscribersPolicyRule', + } + ] + }; + + if (@$project_names) { + my $project_phids = []; + foreach my $project_name (@$project_names) { + my $project = Bugzilla::Extension::PhabBugz::Project->new({ name => $project_name }); + push @$project_phids, $project->phid if $project; + } + + ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids; + + push @{ $data->{policy} }, { + action => 'allow', + rule => 'PhabricatorProjectsPolicyRule', + value => $project_phids, + }; + } + else { + push @{ $data->{policy} }, { action => 'allow', value => 'admin' }; + } + + my $result = request('policy.create', $data); + return $class->new_from_query({ phids => [ $result->{result}{phid} ] }); +} + +sub _build_rule_projects { + my ($self) = @_; + + return [] unless $self->rules; + my $rule = first { $_->{rule} eq 'PhabricatorProjectsPolicyRule'} @{ $self->rules }; + return [] unless $rule; + return [ + map { $_->name } + grep { $_ } + map { Bugzilla::Extension::PhabBugz::Project->new( { phids => [$_] } ) } + @{ $rule->{value} } + ]; +} + +1; \ No newline at end of file diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm index 3ad9558ff..ec00b9532 100644 --- a/extensions/PhabBugz/lib/Project.pm +++ b/extensions/PhabBugz/lib/Project.pm @@ -18,37 +18,36 @@ use Bugzilla::Extension::PhabBugz::Util qw( get_phab_bmo_ids ); -######################### -# Initialization # -######################### - -sub new { - my ($class, $params) = @_; - my $self = $params ? _load($params) : {}; - bless($self, $class); - return $self; -} - -sub _load { - my ($params) = @_; - - my $data = { - queryKey => 'all', - attachments => { - projects => 1, - reviewers => 1, - subscribers => 1 - }, - constraints => $params - }; - - my $result = request('project.search', $data); - if (exists $result->{result}{data} && @{ $result->{result}{data} }) { - return $result->{result}->{data}->[0]; - } - - return $result; -} +use Types::Standard -all; +use Type::Utils; + +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 + ], + ], + ], + ], +]; # { # "data": [ @@ -90,12 +89,6 @@ sub _load { # "phid": "PHID-USER-uif2miph2poiehjeqn5q" # } # ] -# }, -# "ancestors": { -# "ancestors": [] -# }, -# "watchers": { -# "watchers": [] # } # } # } @@ -114,6 +107,36 @@ sub _load { # } # } +######################### +# 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 # ######################### -- cgit v1.2.3-24-g4f1b