Unverified Commit 6aa1a06d authored by IKEDA Soji's avatar IKEDA Soji Committed by GitHub
Browse files

Deprecate implicit sync_include in Sympa::List constructor (#955)

* review: Won't process sync_include in advance so that processing will be faster.

* Refactoring. Sympa::List::on_the_fly_sync_include() was removed. Instead, "include" request handler had "delay" option.

* Remove unused "last_sync" cache entries.  "last_change" entries remain.

* sync_include will no longer be invoked during Sympa::List->new():
  - No owner defined in a list is no longer treated as error_config.
  - Instead, if no owner defined,
     - If possible, discard incoming message and send back DSN to original sender,
     - or, notifications to owners will be redirected to listmaster(s).

* Refactoring: Remove unused functions

* Small refactoring: mail_tt2/listeditor_notification.tt2 is no longer used.

* Clarify log messages and DSN about undefined owner/editor/listmaster
parent 5b5fd551
...@@ -383,6 +383,7 @@ nobase_default_DATA = \ ...@@ -383,6 +383,7 @@ nobase_default_DATA = \
mail_tt2/request_auth.tt2 \ mail_tt2/request_auth.tt2 \
mail_tt2/report.tt2 \ mail_tt2/report.tt2 \
mail_tt2/review.tt2 \ mail_tt2/review.tt2 \
mail_tt2/shared_moderate.tt2 \
mail_tt2/send_auth.tt2 \ mail_tt2/send_auth.tt2 \
mail_tt2/sendpasswd.tt2 \ mail_tt2/sendpasswd.tt2 \
mail_tt2/sendssopasswd.tt2 \ mail_tt2/sendssopasswd.tt2 \
......
...@@ -11,7 +11,7 @@ Subject: [%"List unknown"|loc|qencode%] ...@@ -11,7 +11,7 @@ Subject: [%"List unknown"|loc|qencode%]
Subject: [%"List unknown"|loc|qencode%] Subject: [%"List unknown"|loc|qencode%]
[% ELSIF status == '5.2.3' -%] [% ELSIF status == '5.2.3' -%]
Subject: [%"Too large message"|loc|qencode%] Subject: [%"Too large message"|loc|qencode%]
[% ELSIF status == '5.3.0' -%] [% ELSIF status == '5.2.4' || status == '5.3.0' -%]
Subject: [%"Message distribution: Internal server error"|loc|qencode%] Subject: [%"Message distribution: Internal server error"|loc|qencode%]
[% ELSIF status == '5.3.5' -%] [% ELSIF status == '5.3.5' -%]
Subject: [%"Message distribution: User error"|loc|qencode%] Subject: [%"Message distribution: User error"|loc|qencode%]
...@@ -59,12 +59,13 @@ Content-Description: Notification ...@@ -59,12 +59,13 @@ Content-Description: Notification
Note: Because binary files have to be encoded in less-efficient ASCII format before being sent over email, the final size of an attachment is often significantly larger that the original file.[%END%] Note: Because binary files have to be encoded in less-efficient ASCII format before being sent over email, the final size of an attachment is often significantly larger that the original file.[%END%]
[% ELSIF status == '5.3.0' -%] [% ELSIF status == '5.2.4' -%]
[% IF entry == 'forward' -%]
[%|loc(list.name,function)%]Impossible to forward your message to '%1-%2' because of an internal server error.[%END%] [%|loc(list.name,function)%]Impossible to forward your message to '%1-%2' because of an internal server error.[%END%]
[% ELSE -%]
[%|loc(list.name,domain)%]For further information, please contact %1-request@%2[%END -%]
[% ELSIF status == '5.3.0' -%]
[%|loc(list.name)%]Impossible to distribute your message for list '%1' because of an internal server error.[%END%] [%|loc(list.name)%]Impossible to distribute your message for list '%1' because of an internal server error.[%END%]
[% END -%]
[%|loc(list.name,domain)%]For further information, please contact %1-request@%2[%END -%] [%|loc(list.name,domain)%]For further information, please contact %1-request@%2[%END -%]
......
[%# listeditor_notification.tt2 ~%] [%# listeditor_notification.tt2 ~%]
To: [% to %] [%# As of 6.2.57b, this template will no longer be used. It is left for
compatibility. ~%]
[% IF type == 'shared_moderated' -%] [% IF type == 'shared_moderated' -%]
Subject: [%"Shared document to be approved for %1"|loc(list.name)|qencode%] [% PROCESS shared_moderate.tt2 %]
[% IF many_files -%]
[%|loc(list.name,filename,who)%]There are new shared documents in list %1:
%2
from %3
To moderate these documents: [%END%]
[%- ELSE -%] [%- ELSE -%]
[%|loc(list.name,filename,who)%]There is a new shared document in list %1: From: [% fromlist %]
%2 To: [%"Moderator"|loc|mailbox("${list.name}-editor@${domain}",list.name)%]
from %3
To moderate this document: [%END%]
[%- END %]
[% 'docindex' | url_abs([list.name]) %]
[% ELSE -%]
Subject: [%"Moderators List %1 / %2"|loc(list.name,type)|qencode%] Subject: [%"Moderators List %1 / %2"|loc(list.name,type)|qencode%]
[% param0 %] [% param0 %]
[% END %] [%- END %]
[%# shared_moderate.tt2 ~%]
From: [% fromlist %]
To: [%"Moderator"|loc|mailbox("${list.name}-editor@${domain}",list.name)%]
Subject: [%"Shared document to be approved for %1"|loc(list.name)|qencode%]
[%|loc(list.name,filename,who)%]There is a new shared document in list %1:
%2
from %3
To moderate this document: [%END%]
[% 'docindex' | url_abs([list.name]) %]
...@@ -10637,8 +10637,7 @@ sub do_edit_list { ...@@ -10637,8 +10637,7 @@ sub do_edit_list {
} }
   
## Reload config to clean some empty entries in $list->{'admin'} ## Reload config to clean some empty entries in $list->{'admin'}
$list = Sympa::List->new($list->{'name'}, $robot, $list = Sympa::List->new($list->{'name'}, $robot, {reload_config => 1});
{'reload_config' => 1, 'force_sync_admin' => 1});
   
unless (defined $list) { unless (defined $list) {
Sympa::WWW::Report::reject_report_web('intern', 'list_reload', {}, Sympa::WWW::Report::reject_report_web('intern', 'list_reload', {},
...@@ -13314,10 +13313,27 @@ sub do_d_create_child { ...@@ -13314,10 +13313,27 @@ sub do_d_create_child {
if ($access{may}{edit} == 0.5 and $type ne 'directory') { if ($access{may}{edit} == 0.5 and $type ne 'directory') {
unless ($child and $child->{moderate}) { unless ($child and $child->{moderate}) {
# Moderated at first time # Moderated at first time
$list->send_notify_to_editor( my @rcpt = $list->get_admins_email('receptive_editor');
'shared_moderated', @rcpt = $list->get_admins_email('actual_editor') unless @rcpt;
{ 'filename' => join('/', @{$new_child->{paths}}), unless (@rcpt) {
'who' => $param->{'user'}{'email'} # Since shared document has already been marked moderated,
# notification to editors should not fail. Fallback to
# listmasters.
$log->syslog(
'notice',
'No editor and owner defined at all in list %s; notification is sent to listmasters',
$list
);
@rcpt = Sympa::get_listmasters_email($list);
}
Sympa::send_file(
$list,
'shared_moderate',
\@rcpt,
{ auto_submitted => 'auto-generated',
filename => join('/', @{$new_child->{paths}}),
who => $param->{'user'}{'email'},
} }
); );
} }
......
...@@ -261,6 +261,8 @@ my %diag_messages = ( ...@@ -261,6 +261,8 @@ my %diag_messages = (
'5.1.2' => 'Bad destination system address', '5.1.2' => 'Bad destination system address',
# too large # too large
'5.2.3' => 'Message length exceeds administrative limit', '5.2.3' => 'Message length exceeds administrative limit',
# no owners defined in list at all, no listmasters defined at all
'5.2.4' => 'Mailing list expansion problem',
# could not store message into spool or mailer # could not store message into spool or mailer
'5.3.0' => 'Other or undefined mail system status', '5.3.0' => 'Other or undefined mail system status',
# misconfigured family list # misconfigured family list
......
...@@ -390,31 +390,6 @@ sub new { ...@@ -390,31 +390,6 @@ sub new {
return undef; return undef;
} }
## Config file was loaded or reloaded
my $pertinent_ttl = $list->{'admin'}{'distribution_ttl'}
|| $list->{'admin'}{'ttl'};
if ( $status
and grep { $list->{'admin'}{'status'} eq $_ } qw(open pending)
and (
( not $options->{'skip_sync_admin'}
and $list->_cache_read_expiry('last_sync_admin_user') <
time - $pertinent_ttl
)
or $options->{'force_sync_admin'}
)
) {
# Update admin_table.
$list->sync_include('owner');
$list->sync_include('editor');
if (not @{$list->get_admins('owner') || []}
and $list->{'admin'}{'status'} ne 'error_config') {
$log->syslog('err', 'The list "%s" has got no owner defined',
$list->{'name'});
$list->set_status_error_config('no_owner_defined');
}
}
return $list; return $list;
} }
...@@ -624,12 +599,8 @@ sub _cache_publish_expiry { ...@@ -624,12 +599,8 @@ sub _cache_publish_expiry {
my $stat_file; my $stat_file;
if ($type eq 'member') { if ($type eq 'member') {
$stat_file = $self->{'dir'} . '/.last_change.member'; $stat_file = $self->{'dir'} . '/.last_change.member';
} elsif ($type eq 'last_sync') {
$stat_file = $self->{'dir'} . '/.last_sync.member';
} elsif ($type eq 'admin_user') { } elsif ($type eq 'admin_user') {
$stat_file = $self->{'dir'} . '/.last_change.admin'; $stat_file = $self->{'dir'} . '/.last_change.admin';
} elsif ($type eq 'last_sync_admin_user') {
$stat_file = $self->{'dir'} . '/.last_sync.admin';
} else { } else {
die 'bug in logic. Ask developer'; die 'bug in logic. Ask developer';
} }
...@@ -649,19 +620,11 @@ sub _cache_read_expiry { ...@@ -649,19 +620,11 @@ sub _cache_read_expiry {
my $stat_file = $self->{'dir'} . '/.last_change.member'; my $stat_file = $self->{'dir'} . '/.last_change.member';
$self->_cache_publish_expiry('member') unless -e $stat_file; $self->_cache_publish_expiry('member') unless -e $stat_file;
return [stat $stat_file]->[9]; return [stat $stat_file]->[9];
} elsif ($type eq 'last_sync') {
# If syncs have never been done, earliest time is assumed.
return Sympa::Tools::File::get_mtime(
$self->{'dir'} . '/.last_sync.member');
} elsif ($type eq 'admin_user') { } elsif ($type eq 'admin_user') {
# If changes have never been done, just now is assumed. # If changes have never been done, just now is assumed.
my $stat_file = $self->{'dir'} . '/.last_change.admin'; my $stat_file = $self->{'dir'} . '/.last_change.admin';
$self->_cache_publish_expiry('admin_user') unless -e $stat_file; $self->_cache_publish_expiry('admin_user') unless -e $stat_file;
return [stat $stat_file]->[9]; return [stat $stat_file]->[9];
} elsif ($type eq 'last_sync_admin_user') {
# If syncs have never been done, earliest time is assumed.
return Sympa::Tools::File::get_mtime(
$self->{'dir'} . '/.last_sync.admin');
} elsif ($type eq 'edit_list_conf') { } elsif ($type eq 'edit_list_conf') {
return [stat Sympa::search_fullpath($self, 'edit_list.conf')]->[9]; return [stat Sympa::search_fullpath($self, 'edit_list.conf')]->[9];
} else { } else {
...@@ -1005,74 +968,26 @@ sub load { ...@@ -1005,74 +968,26 @@ sub load {
## Return a list of hash's owners and their param ## Return a list of hash's owners and their param
#OBSOLETED. Use get_admins(). #OBSOLETED. Use get_admins().
sub get_owners { #sub get_owners;
$log->syslog('debug3', '(%s)', @_);
my $self = shift;
# owners are in the admin_table ; they might come from an include data
# source
return [$self->get_admins('owner')];
}
# OBSOLETED: No longer used. # OBSOLETED: No longer used.
sub get_nb_owners { #sub get_nb_owners;
$log->syslog('debug3', '(%s)', @_);
my $self = shift;
return scalar @{$self->get_admins('owner')};
}
## Return a hash of list's editors and their param(empty if there isn't any ## Return a hash of list's editors and their param(empty if there isn't any
## editor) ## editor)
#OBSOLETED. Use get_admins(). #OBSOLETED. Use get_admins().
sub get_editors { #sub get_editors;
$log->syslog('debug3', '(%s)', @_);
my $self = shift;
# editors are in the admin_table ; they might come from an include data
# source
return [$self->get_admins('editor')];
}
## Returns an array of owners' email addresses ## Returns an array of owners' email addresses
#OBSOLETED: Use get_admins_email('receptive_owner') or #OBSOLETED: Use get_admins_email('receptive_owner') or
# get_admins_email('owner'). # get_admins_email('owner').
sub get_owners_email { #sub get_owners_email;
$log->syslog('debug3', '(%s, %s)', @_);
my $self = shift;
my $param = shift;
my @rcpt;
if ($param->{'ignore_nomail'}) {
@rcpt = map { $_->{'email'} } $self->get_admins('owner');
} else {
@rcpt = map { $_->{'email'} } $self->get_admins('receptive_owner');
}
unless (@rcpt) {
$log->syslog('notice', 'Warning: No owner found for list %s', $self);
}
return @rcpt;
}
## Returns an array of editors' email addresses ## Returns an array of editors' email addresses
# or owners if there isn't any editors' email addresses # or owners if there isn't any editors' email addresses
#OBSOLETED: Use get_admins_email('receptive_editor') or #OBSOLETED: Use get_admins_email('receptive_editor') or
# get_admins_email('actual_editor'). # get_admins_email('actual_editor').
sub get_editors_email { #sub get_editors_email;
$log->syslog('debug3', '(%s, %s)', @_);
my $self = shift;
my $param = shift;
my @rcpt;
if ($param->{'ignore_nomail'}) {
@rcpt = map { $_->{'email'} } $self->get_admins('actual_editor');
} else {
@rcpt = map { $_->{'email'} } $self->get_admins('receptive_editor');
}
return @rcpt;
}
## Returns an object Sympa::Family if the list belongs to a family or undef ## Returns an object Sympa::Family if the list belongs to a family or undef
sub get_family { sub get_family {
...@@ -1712,10 +1627,11 @@ sub send_notify_to_owner { ...@@ -1712,10 +1627,11 @@ sub send_notify_to_owner {
die 'bug in logic. Ask developer' unless defined $operation; die 'bug in logic. Ask developer' unless defined $operation;
my @rcpt = $self->get_admins_email('receptive_owner'); my @rcpt = $self->get_admins_email('receptive_owner');
@rcpt = $self->get_admins_email('owner') unless @rcpt;
unless (@rcpt) { unless (@rcpt) {
$log->syslog( $log->syslog(
'notice', 'notice',
'No owner defined or all of them use nomail option in list %s; using listmasters as default', 'No owner defined at all in list %s; notification is sent to listmasters',
$self $self
); );
@rcpt = Sympa::get_listmasters_email($self); @rcpt = Sympa::get_listmasters_email($self);
...@@ -1892,86 +1808,8 @@ sub delete_list_member_picture { ...@@ -1892,86 +1808,8 @@ sub delete_list_member_picture {
return $ret; return $ret;
} }
#################################################### #No longer used.
# send_notify_to_editor #sub send_notify_to_editor;
####################################################
# Sends a notice to list editor(s) or owner (if no editor)
# by parsing listeditor_notification.tt2 template
#
# IN : -$self (+): ref(List)
# -$operation (+): notification type
# -$param(+) : ref(HASH) | ref(ARRAY)
# values for template parsing
#
# OUT : 1 | undef
#
######################################################
sub send_notify_to_editor {
$log->syslog('debug2', '(%s, %s, %s)', @_);
my ($self, $operation, $param) = @_;
my @rcpt = $self->get_admins_email('receptive_editor');
@rcpt = $self->get_admins_email('actual_editor') unless @rcpt;
my $robot = $self->{'domain'};
$param->{'auto_submitted'} = 'auto-generated';
unless (@rcpt) {
$log->syslog('notice',
'Warning: No editor and owner defined at all in list %s', $self);
return undef;
}
unless (defined $operation) {
die 'missing incoming parameter "$operation"';
}
if (ref($param) eq 'HASH') {
$param->{'to'} = join(',', @rcpt);
$param->{'type'} = $operation;
unless (
Sympa::send_file(
$self, 'listeditor_notification', \@rcpt, $param
)
) {
$log->syslog(
'notice',
'Unable to send template "listeditor_notification" to %s list editor',
$self
);
return undef;
}
} elsif (ref($param) eq 'ARRAY') {
my $data = {
'to' => join(',', @rcpt),
'type' => $operation
};
foreach my $i (0 .. $#{$param}) {
$data->{"param$i"} = $param->[$i];
}
unless (
Sympa::send_file($self, 'listeditor_notification', \@rcpt, $data))
{
$log->syslog('notice',
'Unable to send template "listeditor_notification" to %s list editor'
);
return undef;
}
} else {
$log->syslog(
'err',
'(%s, %s) Error on incoming parameter "$param", it must be a ref on HASH or a ref on ARRAY',
$self,
$operation
);
return undef;
}
return 1;
}
# Moved to Sympa::send_notify_to_user(). # Moved to Sympa::send_notify_to_user().
#sub send_notify_to_user; #sub send_notify_to_user;
...@@ -4758,10 +4596,12 @@ sub _load_include_admin_user_file { ...@@ -4758,10 +4596,12 @@ sub _load_include_admin_user_file {
#sub purge_ca; #sub purge_ca;
# -> Never used. # -> Never used.
# FIXME: Use Sympa::Request::Handler::include handler.
sub sync_include { sub sync_include {
$log->syslog('debug2', '(%s, %s)', @_); $log->syslog('debug2', '(%s, %s)', @_);
my $self = shift; my $self = shift;
my $role = shift; my $role = shift;
my %options = @_;
$role ||= 'member'; # Compat.<=6.2.54 $role ||= 'member'; # Compat.<=6.2.54
...@@ -4773,6 +4613,7 @@ sub sync_include { ...@@ -4773,6 +4613,7 @@ sub sync_include {
context => $self, context => $self,
action => 'include', action => 'include',
role => $role, role => $role,
delay => $options{delay},
scenario_context => {skip => 1}, scenario_context => {skip => 1},
); );
unless ($spindle and $spindle->spin) { unless ($spindle and $spindle->spin) {
...@@ -4789,12 +4630,6 @@ sub sync_include { ...@@ -4789,12 +4630,6 @@ sub sync_include {
return undef; return undef;
} }
# Get and save total of subscribers.
$self->_cache_publish_expiry(
($role eq 'member') ? 'member' : 'admin_user');
$self->_cache_publish_expiry(
($role eq 'member') ? 'last_sync' : 'last_sync_admin_user');
return 1; return 1;
} }
...@@ -4806,29 +4641,9 @@ sub sync_include { ...@@ -4806,29 +4641,9 @@ sub sync_include {
# This one is to be called from anywhere else. This function deletes the # This one is to be called from anywhere else. This function deletes the
# scheduled sync_include task. If this deletion happened in sync_include(), # scheduled sync_include task. If this deletion happened in sync_include(),
# it would disturb the normal task_manager.pl functionning. # it would disturb the normal task_manager.pl functionning.
#
# 6.2.4: Returns 0 if synchronization is not needed. # 6.2.4: Returns 0 if synchronization is not needed.
sub on_the_fly_sync_include { # No longer used. Use sync_include('member', delay => ...);
my $self = shift; #sub on_the_fly_sync_include;
my %options = @_;
my $pertinent_ttl = $self->{'admin'}{'distribution_ttl'}
|| $self->{'admin'}{'ttl'};
$log->syslog('debug2', '(%s)', $pertinent_ttl);
if (not $options{'use_ttl'}
or $self->_cache_read_expiry('last_sync') < time - $pertinent_ttl) {
$log->syslog('notice', "Synchronizing list members...");
my $return_value = $self->sync_include('member');
$log->syslog('notice', "...done");
if ($return_value) {
$self->remove_task('sync_include');
return 1;
} else {
return $return_value;
}
}
return 0;
}
# DEPRECATED. Use sync_include('owner') & sync_include('editor'). # DEPRECATED. Use sync_include('owner') & sync_include('editor').
#sub sync_include_admin; #sub sync_include_admin;
...@@ -5403,8 +5218,7 @@ sub get_lists { ...@@ -5403,8 +5218,7 @@ sub get_lists {
my $list = __PACKAGE__->new( my $list = __PACKAGE__->new(
$listname, $listname,
$robot_id, $robot_id,
{ skip_sync_admin => ($which_role ? 1 : 0), { %options,
%options,
skip_name_check => 1, #ToDo: implement it. skip_name_check => 1, #ToDo: implement it.
} }
); );
...@@ -5486,8 +5300,7 @@ sub get_lists { ...@@ -5486,8 +5300,7 @@ sub get_lists {
my $list = __PACKAGE__->new( my $list = __PACKAGE__->new(
$listname, $listname,
$robot_id, $robot_id,
{ skip_sync_admin => ($which_role ? 1 : 0), { %options,
%options,
skip_name_check => 1, #ToDo: implement it. skip_name_check => 1, #ToDo: implement it.
} }
); );
...@@ -6547,35 +6360,8 @@ sub get_next_delivery_date { ...@@ -6547,35 +6360,8 @@ sub get_next_delivery_date {
# -> No longer used. # -> No longer used.
## Remove a task in the tasks spool ## Remove a task in the tasks spool
sub remove_task { # No longer used.
my $self = shift; #sub remove_task;
my $task = shift;
unless (opendir(DIR, $Conf::Conf{'queuetask'})) {
$log->syslog(
'err',
'Can\'t open dir %s: %m',
$Conf::Conf{'queuetask'}
);
return undef;
}
my @tasks = grep !/^\.\.?$/, readdir DIR;
closedir DIR;
foreach my $task_file (@tasks) {
if ($task_file =~
/^(\d+)\.\w*\.$task\.$self->{'name'}\@$self->{'domain'}$/) {
unless (unlink("$Conf::Conf{'queuetask'}/$task_file")) {
$log->syslog('err', 'Unable to remove task file %s: %m',
$task_file);
return undef;
}
$log->syslog('notice', 'Removing task file %s', $task_file);
}
}
return 1;
}
# Deprecated. Use Sympa::Request::Handler::close_list handler. # Deprecated. Use Sympa::Request::Handler::close_list handler.
#sub close_list; #sub close_list;
......
...@@ -227,12 +227,8 @@ sub _twist { ...@@ -227,12 +227,8 @@ sub _twist {