From 6aed489c2c9c8e09798a24879aaaeefecec4ab13 Mon Sep 17 00:00:00 2001 From: wickedOne Date: Sun, 18 Feb 2024 16:36:38 +0000 Subject: [PATCH] default codeowners (#17) - added support for section default codeowners notation - bumped version to 1.1.2 closes #15 --- lib/GPH.pm | 2 +- lib/GPH/Gitlab.pm | 88 ++++++++++++++++++++++++++------------- t/share/Gitlab/CODEOWNERS | 16 ++++--- t/unit/GPH/Gitlab.t | 63 ++++++++++++++++++++++------ 4 files changed, 120 insertions(+), 49 deletions(-) diff --git a/lib/GPH.pm b/lib/GPH.pm index 6203986..7ef6072 100644 --- a/lib/GPH.pm +++ b/lib/GPH.pm @@ -3,6 +3,6 @@ package GPH; use strict; use warnings FATAL => 'all'; -our $VERSION = '1.1.1'; +our $VERSION = '1.1.2'; 1; \ No newline at end of file diff --git a/lib/GPH/Gitlab.pm b/lib/GPH/Gitlab.pm index e00c217..2f7688b 100755 --- a/lib/GPH/Gitlab.pm +++ b/lib/GPH/Gitlab.pm @@ -15,6 +15,8 @@ package GPH::Gitlab; use strict; use warnings FATAL => 'all'; +use Data::Dumper; + #------------------------------------------------------------------------------ # Construct new class # @@ -49,7 +51,7 @@ sub new { # Returns: reference to Gitlab object sub parseCodeowners { my ($self, %args) = @_; - my ($fh, %codeowners, %excludeHash, %blacklist); + my ($fh, %excludes, $default_owners); open $fh, '<', $args{codeowners} or die "unable to open codeowners file: $!"; my @lines = <$fh>; @@ -58,49 +60,77 @@ sub parseCodeowners { # build excludes hash for quick lookup if (exists($args{excludes})) { foreach my $item (@{$args{excludes}}) { - $excludeHash{$item} = 1; + $excludes{$item} = 1; } } - for my $line (@lines) { - # skip section line. default codeowners not yet supported - next if $line =~ /[\[\]]/; - # skip if line does not contain @ - next unless $line =~ /^.*\s\@[\w]+\/.*$/x; - # replace /**/* with a trailing forward slash - my $pat = quotemeta('/**/* '); - $line =~ s|$pat|/ |; + foreach (@lines) { + next if $_ =~ /^#.*/ or $_ =~ /^[\s]?$/; + my $line = $self->sanitise($_); + + if ($line =~ /\]/) { + $default_owners = ($line =~ /^[\^]?\[[^\]]+\](?:[\[0-9\]]{0,}) (.*)$/) ? $1 : undef; + + next; + } + + my ($class_path, $owners) = split(/\s/, $line, 2); - my ($class_path, $owners) = split(' ', $line, 2); + next if exists $excludes{$class_path}; - # skip if path is excluded - next if exists $excludeHash{$class_path}; + $owners = $owners || $default_owners; - foreach (split(' ', $owners)) { - next unless /(\@[\w\-\/]{0,})$/x; + next unless defined $owners; - if (not exists $codeowners{$1}) { - $codeowners{$1} = []; - $blacklist{$1} = []; + foreach my $owner (split(/\s/, $owners)) { + next unless $owner =~ /@/; + if (not exists $self->{codeowners}{$owner}) { + $self->{codeowners}{$owner} = []; + $self->{blacklist}{$owner} = []; } - push(@{$codeowners{$1}}, $class_path); + push(@{$self->{codeowners}{$owner}}, $class_path); - # check whether less specific path is already defined and add it to the blacklist - foreach my $key (keys %codeowners) { - foreach my $defined (@{$codeowners{$key}}) { - if ($class_path =~ $defined and $class_path ne $defined) { - push(@{$blacklist{$key}}, $class_path); - } - } + $self->blacklist($class_path); + } + } + + return ($self); +} + +#------------------------------------------------------------------------------ +# Check whether less specific path is already defined and add it to the blacklist +# +# Inputs: class_path => (string) path to check and blacklist +# +# Returns: reference to Gitlab object +sub blacklist { + my ($self, $class_path) = @_; + + foreach my $owner (keys %{$self->{codeowners}}) { + foreach my $path (@{$self->{codeowners}{$owner}}) { + if ($class_path =~ $path and $class_path ne $path) { + push(@{$self->{blacklist}{$owner}}, $class_path); } } } - $self->{codeowners} = \%codeowners; - $self->{blacklist} = \%blacklist; + return ($self); +} + +#------------------------------------------------------------------------------ +# Replace /**/* with a trailing forward slash +# +# Inputs: line => (string) line to sanitise +# +# Returns: string +sub sanitise { + my ($self, $line) = @_; + + my $pat = quotemeta('/**/* '); + $line =~ s|$pat|/ |; - return $self; + return ($line); } #------------------------------------------------------------------------------ diff --git a/t/share/Gitlab/CODEOWNERS b/t/share/Gitlab/CODEOWNERS index 245f827..ac41839 100644 --- a/t/share/Gitlab/CODEOWNERS +++ b/t/share/Gitlab/CODEOWNERS @@ -1,9 +1,13 @@ +# Lines that start with `#` are ignored. [alpha] /src/Command/**/* @teams/alpha -/src/Service/ @teams/alpha @#!$%^ -.gitlab-ci.yml @teams/alpha +/src/Service/ @teams/alpha john@doe.com +.gitlab-ci.yml @teams/alpha henkie -[beta] -/src/Command/Config/ConfigPhraseKeyCommand.php @teams/beta -/src/DependencyInjection/ @teams/beta -.gitlab-ci.yml @teams/beta \ No newline at end of file +[beta] @teams/beta +/src/Command/Config/ConfigPhraseKeyCommand.php +/src/DependencyInjection/ +.gitlab-ci.yml + +^[gamma][3] @teams/gamma +/tests/Unit/Service/ \ No newline at end of file diff --git a/t/unit/GPH/Gitlab.t b/t/unit/GPH/Gitlab.t index fbb6516..0477e47 100644 --- a/t/unit/GPH/Gitlab.t +++ b/t/unit/GPH/Gitlab.t @@ -16,8 +16,8 @@ local $SIG{__WARN__} = sub {}; describe "class `$CLASS`" => sub { my %config = ( codeowners => CODEOWNERS_FILE, - owner =>'@teams/alpha', - excludes => ['.gitlab-ci.yml'] + owner => '@teams/alpha', + excludes => [ '.gitlab-ci.yml' ] ); tests 'it can be instantiated' => sub { @@ -25,17 +25,17 @@ describe "class `$CLASS`" => sub { }; tests "codeowners file not found" => sub { - ok(dies{$CLASS->new((codeowners => 'foo.php', owner =>'@teams/alpha'))}, 'died with codeowners not found') or note ($@); + ok(dies {$CLASS->new((codeowners => 'foo.php', owner => '@teams/alpha'))}, 'died with codeowners not found') or note($@); }; tests "mandatory config options" => sub { - ok(dies{$CLASS->new((owner =>'@teams/alpha'))}, 'died with missing codeowners option') or note ($@); - ok(dies{$CLASS->new((codeowners => CODEOWNERS_FILE))}, 'died with missing owner option') or note ($@); - ok(lives{$CLASS->new((owner =>'@teams/alpha', codeowners => CODEOWNERS_FILE))}, 'lived with mandatory options') or note ($@); + ok(dies {$CLASS->new((owner => '@teams/alpha'))}, 'died with missing codeowners option') or note($@); + ok(dies {$CLASS->new((codeowners => CODEOWNERS_FILE))}, 'died with missing owner option') or note($@); + ok(lives {$CLASS->new((owner => '@teams/alpha', codeowners => CODEOWNERS_FILE))}, 'lived with mandatory options') or note($@); }; tests 'owner with blacklist and exclude' => sub { - my ($object, $exception, $warnings, @excludes); + my ($object, $exception, $warnings); $exception = dies { $warnings = warns { @@ -46,12 +46,12 @@ describe "class `$CLASS`" => sub { is($exception, undef, 'no exception thrown'); is($warnings, 0, 'no warnings generated'); - is($object->{blacklist}{'@teams/alpha'}, ['/src/Command/Config/ConfigPhraseKeyCommand.php'], 'blacklist correct'); + is($object->{blacklist}{'@teams/alpha'}, [ '/src/Command/Config/ConfigPhraseKeyCommand.php' ], 'blacklist correct') or diag Dumper($object); is('.gitlab-ci.yml', not_in_set(@{$object->{codeowners}{'@teams/alpha'}}), 'excluded file not defined'); }; tests 'module methods' => sub { - my ($object, $exception, $warnings, @excludes); + my ($object, $exception, $warnings); $exception = dies { $warnings = warns { @@ -68,7 +68,7 @@ describe "class `$CLASS`" => sub { item '/src/Service/'; end; }, - 'GetPaths call correct' + 'GetPaths call correct' ); is($object->getBlacklistPaths(), @@ -76,7 +76,7 @@ describe "class `$CLASS`" => sub { item '/src/Command/Config/ConfigPhraseKeyCommand.php'; end; }, - 'GetBlacklistPaths call correct' + 'GetBlacklistPaths call correct' ); is($object->getCommaSeparatedPathList(), '/src/Command/,/src/Service/', 'GetCommaSeparatedPathList call correct'); @@ -85,12 +85,12 @@ describe "class `$CLASS`" => sub { is($object->intersectCommaSeparatedPathList(@arr), '/src/Command/', 'IntersectToCommaSeparatedPathList call correct'); - is([$object->intersect(@arr)], + is([ $object->intersect(@arr) ], array { item '/src/Command/'; end; }, - 'Intersect call correct' + 'Intersect call correct' ); is($object->match('/src/Service/'), 1, 'Match call match correct'); @@ -101,5 +101,42 @@ describe "class `$CLASS`" => sub { }; }; +describe 'test codeowners syntax' => sub { + my ($object, %config, $owner, $expected_paths, $exception, $warnings); + + case 'section with default owner' => sub { + $owner = '@teams/beta'; + $expected_paths = [ '/src/Command/Config/ConfigPhraseKeyCommand.php', '/src/DependencyInjection/' ]; + }; + + case 'optional section with default owner and required approvals' => sub { + $owner = '@teams/gamma'; + $expected_paths = [ '/tests/Unit/Service/' ]; + }; + + case 'owner with email' => sub { + $owner = 'john@doe.com'; + $expected_paths = [ '/src/Service/' ]; + }; + + tests 'module get paths' => sub { + %config = ( + codeowners => CODEOWNERS_FILE, + owner => $owner, + excludes => [ '.gitlab-ci.yml' ] + ); + + $exception = dies { + $warnings = warns { + $object = $CLASS->new(%config); + }; + }; + + is($exception, undef, 'no exception thrown'); + is($warnings, 0, 'no warnings generated'); + is($object->getPaths(), $expected_paths, 'paths correct') or diag Dumper($object); + }; +}; + done_testing();