Merge pull request #299 from wtsi-npg/devel

pull from devel to master to create Release 3.22.0
wtsi-npg · Jan 26, 2024 · f0e7559 · f0e7559
2 parents 193e034 + ad09080
commit f0e7559
Show file tree

Hide file tree

Showing 11 changed files with 250 additions and 44 deletions.
diff --git a/Build.PL b/Build.PL
@@ -52,7 +52,6 @@ my $build = WTSI::DNAP::Utilities::Build->new
                          },
    recommends =>         {
                           'Net::LDAP'                     => '0',
-                          'WTSI::DNAP::Warehouse::Schema' => '0',
                          });
 
 $build->create_build_script;
diff --git a/Changes b/Changes
@@ -1,3 +1,17 @@
+Release 3.22.0
+
+ - Introduced ss_<STUDY_ID>_human iRODS group to provide limited
+   access to split-out human data. Specific changes:
+     - exposed ensure_group_exists method from iRODS::GroupAdmin
+     - populate_wtsi_irods_groups creates _human iRODS groups
+     - tests fixtures extended to include these new iRODS groups
+     - reimplemented expected_groups from WTSI::NPG::irods::Path
+       to return all groups that are appropriate for an object or
+       a collection; this method consolidates all logic about
+       study-related access to data, this logic was previously split
+       between WTSI::NPG::irods::Path and WTSI::NPG::irods::DataObject
+ - Fixed incorrect example in POD
+
 Release 3.21.0
 
  - Add iRODS 4.3.1 Ubuntu 22.04 as a required test target

diff --git a/bin/populate_wtsi_irods_groups.pl b/bin/populate_wtsi_irods_groups.pl
@@ -44,6 +44,11 @@
 sequencing tracked in the ML warehouse, the iRODS group will be left
 empty (except for the iRODS groupadmin user).
 
+Studies which are marked as have samples contaminated with human which
+should be removed will have an ss_<study_id>_human iRODS group created
+when they do not exist - population of this group is performed outside
+this process and should be tracked in an auditable manner by a ticket.
+
 Script runs to perform such updates when no arguments are given.
 
 Options:
@@ -72,7 +77,7 @@
              exit 0;
            },
            'logconf=s'               => \$log4perl_config,
-           'study=i'                 => \@study_ids,
+           'study_id=i'              => \@study_ids,
            'verbose'                 => \$verbose) or die "\n$what_on_earth\n";
 
 if ($log4perl_config) {
@@ -141,7 +146,7 @@ sub _uid_to_irods_uid {
 my $studies = $mlwh->resultset('Study')->search($query,
                                                 {order_by => 'id_study_lims'});
 
-my ($group_count, $altered_count) = (0, 0);
+my ($group_count, $altered_count, $altered_human_count) = (0, 0, 0);
 while (my $study = $studies->next){
   my $study_id = $study->id_study_lims;
   my $dag_str  = $study->data_access_group || q();
@@ -172,14 +177,21 @@ sub _uid_to_irods_uid {
     $altered_count++;
   }
 
+  if ($study->contaminated_human_dna) {
+    $altered_human_count += $iga->ensure_group_exists("ss_$study_id".'_human');
+  }
+
   $group_count++;
 }
 
 $log->debug("Altered $altered_count groups");
+$log->debug("Created $altered_human_count _human groups");
 
 $log->info("When considering $group_count Sequencescape studies, ",
-           "$altered_count iRODS groups were created or their ",
-           'membership altered (by ', $iga->_user, ')');
+           $altered_count.' iRODS "ss_*" groups were created or their ',
+           'membership altered, and '.$altered_human_count,
+           ' "ss_?????_human" groups were created (by ',
+           $iga->_user, ')');
 
 # Find both gid and member uids for each group
 sub find_group_ids {

diff --git a/lib/WTSI/NPG/iRODS/DataObject.pm b/lib/WTSI/NPG/iRODS/DataObject.pm
@@ -9,8 +9,6 @@ use Set::Scalar;
 use Try::Tiny;
 
 use WTSI::NPG::iRODS;
-use WTSI::NPG::iRODS::Metadata qw($SAMPLE_CONSENT
-                                  $SAMPLE_CONSENT_WITHDRAWN);
 use WTSI::NPG::iRODS::Replicate;
 use WTSI::NPG::iRODS::Types qw(ArrayRefOfReplicate);
 
@@ -426,7 +424,7 @@ sub set_permissions {
                $WTSI::NPG::iRODS::OWN_PERMISSION or
                $WTSI::NPG::iRODS::NULL_PERMISSION. Optional.
 
-  Example    : $obj->get_object_groups($WTSI::NPG::iRODS::READ_PERMISSION)
+  Example    : $obj->get_groups($WTSI::NPG::iRODS::READ_PERMISSION)
   Description: Return a list of the data access groups in the object's ACL.
                If a permission level argument is supplied, only groups with
                that level of access will be returned. Only groups having a
@@ -452,19 +450,14 @@ sub get_groups {
                logged only.
 
   Example    : $obj->update_group_permissions
-  Description: Modify a data objects ACL with respect to its study_id and
-               sample_consent / consent_withdrawn metadata and return the
-               data object.
+  Description: Modify a data objects ACL.
 
                The target group membership is determined by the result of
                calling $self->expected_groups. The current group membership
                is determined and any difference calculated. Unwanted
                group memberships are pruned, then missing group memberships
                are added.
 
-               If there are sample_consent or consent_withdrawn metadata,
-               access for all groups is removed.
-
                This method does not add or remove access for the 'public'
                group.
   Returntype : WTSI::NPG::iRODS::DataObject
@@ -488,16 +481,6 @@ sub update_group_permissions {
   $self->debug('Updated annotations: [', join(', ', @groups_annotated), ']');
 
   my $path = $self->str;
-
-  my $true  = 1;
-  my $false = 0;
-  if ($self->get_avu($SAMPLE_CONSENT,          $false) or
-      $self->get_avu($SAMPLE_CONSENT_WITHDRAWN, $true)) {
-    $self->info('Data is marked as CONSENT WITHDRAWN; ',
-                'all permissions will be withdrawn');
-    @groups_annotated = (); # Emptying this means all will be removed
-  }
-
   my $perms = Set::Scalar->new(@groups_permissions);
   my $annot = Set::Scalar->new(@groups_annotated);
   my @to_remove = $perms->difference($annot)->members;
@@ -638,7 +621,7 @@ Keith James <[email protected]>
 
 =head1 COPYRIGHT AND DISCLAIMER
 
-Copyright (C) 2013, 2014, 2015, 2016, 2021 Genome Research Limited. All
+Copyright (C) 2013, 2014, 2015, 2016, 2021, 2023 Genome Research Limited. All
 Rights Reserved.
 
 This program is free software: you can redistribute it and/or modify

diff --git a/lib/WTSI/NPG/iRODS/GroupAdmin.pm b/lib/WTSI/NPG/iRODS/GroupAdmin.pm
@@ -191,15 +191,22 @@ sub _op_g_u {
   return;
 }
 
-sub _ensure_existence_of_group {
+=head2 ensure_group_exists
+
+Given a group ensure that it exists in iRODS by making it if it does not, and adding this admin user to it to ensure admin rights on it are retained. Return true if a group is created.
+
+=cut
+
+sub ensure_group_exists {
   my($self,$group)=@_;
   $self->__croak_on_bad_group_name($group);
   if ( any {$group eq $_} $self->lg){ return;}
   if ($self->dry_run) {
-    $self->info("Dry run: mkgroup '$group'");
+    $self->info("Dry run: mkgroup '$group' and then atg...");
   }
   else {
     $self->_push_pump_trim_split(qq(mkgroup "$group"\n));
+    $self->_op_g_u('atg',$group, $self->_user); #add this user to empty group (first) so admin rights to operate on it are retained
   }
   return 1; #return true if we make a group
 }
@@ -212,7 +219,7 @@ Given a group and list of members will ensure that the group exists and contains
 
 sub set_group_membership {
   my($self,$group,@members)=@_;
-  my $altered = $self->_ensure_existence_of_group($group);
+  my $altered = $self->ensure_group_exists($group);
   my @orig_members = $self->lg($group);
   $self->debug("Members of $group: ", join q(, ), @orig_members);
   if (@orig_members){

diff --git a/lib/WTSI/NPG/iRODS/Path.pm b/lib/WTSI/NPG/iRODS/Path.pm
@@ -6,7 +6,10 @@ use List::MoreUtils qw(any notall uniq);
 use Moose::Role;
 
 use WTSI::NPG::iRODS;
-use WTSI::NPG::iRODS::Metadata qw($STUDY_ID);
+use WTSI::NPG::iRODS::Metadata qw($STUDY_ID
+                                  $ALIGNMENT_FILTER
+                                  $SAMPLE_CONSENT
+                                  $SAMPLE_CONSENT_WITHDRAWN);
 
 our $VERSION = '';
 
@@ -276,22 +279,61 @@ sub supersede_multivalue_avus {
   Arg [1]    : None
 
   Example    : @groups = $path->expected_groups
-  Description: Return an array of iRODS group names given metadata containing
-               >=1 study_id.
-  Returntype : Array
+  Description: Return a list of iRODS group names. The list might be empty or
+               contain either one or multiple iRODS group names.
+
+               An empty list is returned if the concent has been withdrawn or
+               for split-out xa-human data, for which the consent does not
+               exist by definition, or for split-out human data that is
+               associated with multiple studies.
+
+               Special study-related 'human' group ss_<STUDY_ID>_human is
+               returned for split-out human data associated with a single study.
+
+               In all other cases if data is associated with a list of studies,
+               a list of groups is returned, a group per study, the group name
+               pattern being ss_<STUDY_ID>.
+
+               The logic is based on examining such iRODS metadata as
+               'study_id', 'alignment_filter', 'sample_consent',
+               'sample_consent_withdrawn'. Neither object's path nor file name
+               is considered.
+       
+  Returntype : List
 
 =cut
 
 sub expected_groups {
   my ($self) = @_;
 
+  my @af_avus = $self->find_in_metadata($ALIGNMENT_FILTER);
   my @ss_study_avus = $self->find_in_metadata($STUDY_ID);
+  my $human_subset   = any { $_->{value} eq 'human' }   @af_avus;
+  my $xahuman_subset = any { $_->{value} eq 'xahuman' } @af_avus;
 
+  my $info;
+  my $true  = 1;
+  my $false = 0;
   my @groups;
-  foreach my $avu (@ss_study_avus) {
-    my $study_id = $avu->{value};
-    my $group = $self->irods->make_group_name($study_id);
-    push @groups, $group;
+  if ($self->get_avu($SAMPLE_CONSENT,          $false) ||
+      $self->get_avu($SAMPLE_CONSENT_WITHDRAWN, $true)) {
+    $info = 'Data is marked as CONSENT WITHDRAWN';
+  } elsif ($xahuman_subset) {
+    $info = 'Data belongs to xahuman subset';
+  } elsif ($human_subset && (@ss_study_avus > 1)) {
+    $info = 'Data belongs to human subset and multiple studies';
+  } else {
+    @groups = map { $self->irods->make_group_name($_) }
+              map { $_->{value} }
+              @ss_study_avus;
+    if (@groups == 1 and $human_subset) {
+      $self->info('Data belongs to human subset');
+      @groups = ($groups[0] . '_human'); # Reset the list
+    }
+  }
+
+  if ($info) {
+    $self->info("${info}:\n no study-associated iRODS groups are applicable");
   }
 
   return @groups;
@@ -448,7 +490,7 @@ Keith James <[email protected]>
 
 =head1 COPYRIGHT AND DISCLAIMER
 
-Copyright (C) 2013, 2014, 2015, 2016 Genome Research Limited. All
+Copyright (C) 2013, 2014, 2015, 2016, 2023, 2024 Genome Research Limited. All
 Rights Reserved.
 
 This program is free software: you can redistribute it and/or modify

diff --git a/t/lib/WTSI/NPG/DriRODSTest.pm b/t/lib/WTSI/NPG/DriRODSTest.pm
@@ -31,7 +31,8 @@ my $have_admin_rights =
 # Prefix for test iRODS data access groups
 my $group_prefix = 'ss_';
 # Groups to be added to the test iRODS
-my @irods_groups = map { $group_prefix . $_ } (0);
+my @irods_groups = map { $group_prefix . $_, $group_prefix . $_ . '_human' }
+                   (0);
 # Groups added to the test iRODS in fixture setup
 my @groups_added;
 # Enable group tests

diff --git a/t/lib/WTSI/NPG/iRODS/CollectionTest.pm b/t/lib/WTSI/NPG/iRODS/CollectionTest.pm
@@ -27,7 +27,8 @@ my $have_admin_rights =
 # Prefix for test iRODS data access groups
 my $group_prefix = 'ss_';
 # Groups to be added to the test iRODS
-my @irods_groups = map { $group_prefix . $_ } (10, 100);
+my @irods_groups = map { $group_prefix . $_, $group_prefix . $_ . '_human' }
+                   (10, 100);
 # Groups added to the test iRODS in fixture setup
 my @groups_added;