From 1c4e0827558d220fb624fe1af3245f4f386bee86 Mon Sep 17 00:00:00 2001 From: Aleks-Daniel Jakimenko Date: Fri, 4 Sep 2015 04:55:48 +0300 Subject: [PATCH 1/4] Return objects where it begs for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sub ParseData is fully backwards compatible. If some module runs it in list context, then it will get listified hash like previously. New code should always run it in scalar context though (everything in our code base was changed according to that). sub GetTextRevision is not backwards compatible (don't let “wantarray” usage to confuse you). Most modules do not touch that subroutine, so we are probably fine (modules from our git repo that do use were changed accordingly). “EncodePage(%$page)” looks wrong. It seems like we should change it to accept hash ref. --- modules/aggregate.pl | 14 ++-- modules/ban-contributors.pl | 2 +- modules/despam.pl | 6 +- modules/drafts.pl | 4 +- modules/enclosure.pl | 4 +- modules/gd_security_image.pl | 6 +- modules/joiner.pl | 141 +++++++++++++++++------------------ modules/live-templates.pl | 6 +- modules/namespaces.pl | 3 +- modules/near-links.pl | 6 +- modules/olocalmap.pl | 6 +- modules/private-pages.pl | 12 +-- modules/static-copy.pl | 4 +- modules/static-hybrid.pl | 14 ++-- modules/svg-edit.pl | 2 +- modules/thumbs.pl | 2 +- modules/translations.pl | 3 +- wiki.pl | 88 +++++++++++----------- 18 files changed, 163 insertions(+), 160 deletions(-) diff --git a/modules/aggregate.pl b/modules/aggregate.pl index c9031951..fcdd3f4f 100644 --- a/modules/aggregate.pl +++ b/modules/aggregate.pl @@ -126,8 +126,8 @@ sub DoAggregate { } } foreach my $id (@pages) { - my %data = ParseData(ReadFileOrDie(GetPageFile(FreeToNormal($id)))); - my $page = $data{text}; + my $data = ParseData(ReadFileOrDie(GetPageFile(FreeToNormal($id)))); + my $page = $data->{text}; my $size = length($page); my $i = index($page, "\n="); my $j = index($page, "\n----"); @@ -136,13 +136,13 @@ sub DoAggregate { $page =~ s/^=.*\n//; # if it starts with a header my $name = $id; $name =~ s/_/ /g; - my $date = TimeToRFC822($data{ts}); - my $host = $data{host}; - my $username = $data{username}; + my $date = TimeToRFC822($data->{ts}); + my $host = $data->{host}; + my $username = $data->{username}; $username = QuoteHtml($username); $username = $host unless $username; - my $minor = $data{minor}; - my $revision = $data{revision}; + my $minor = $data->{minor}; + my $revision = $data->{revision}; my $cluster = GetCluster($page); my $description = ToString(sub { ApplyRules(QuoteHtml($page), 1, 0, undef, 'p') }); $description .= $q->p(GetPageLink($id, T('Learn more...'))) diff --git a/modules/ban-contributors.pl b/modules/ban-contributors.pl index ad384315..8c947047 100644 --- a/modules/ban-contributors.pl +++ b/modules/ban-contributors.pl @@ -124,7 +124,7 @@ sub NewBanContributorsWriteRcLog { and $OpenPageName eq $id and UserIsAdmin()) { # we currently have the clean page loaded, so we need to reload # the spammed revision (there is a possible race condition here) - my ($old) = GetTextRevision($Page{revision}-1, 1); + my $old = GetTextRevision($Page{revision} - 1, 1)->{text}; my %urls = map {$_ => 1 } $old =~ /$UrlPattern/g; # we open the file again to force a load of the despammed page foreach my $url ($Page{text} =~ /$UrlPattern/g) { diff --git a/modules/despam.pl b/modules/despam.pl index 522946dc..d7a35fdb 100644 --- a/modules/despam.pl +++ b/modules/despam.pl @@ -133,14 +133,14 @@ sub DespamPage { # from DoHistory() my @revisions = sort {$b <=> $a} map { m|/([0-9]+).kp$|; $1; } GetKeepFiles($OpenPageName); foreach my $revision (@revisions) { - my ($text, $rev) = GetTextRevision($revision, 1); # quiet + my ($revisionPage, $rev) = GetTextRevision($revision, 1); # quiet if (not $rev) { print ': ' . Ts('Cannot find revision %s.', $revision); return; - } elsif (not DespamBannedContent($text)) { + } elsif (not DespamBannedContent($revisionPage->{text})) { my $summary = Tss('Revert to revision %1: %2', $revision, $rule); print ': ' . $summary; - Save($OpenPageName, $text, $summary) unless GetParam('debug', 0); + Save($OpenPageName, $revisionPage->{text}, $summary) unless GetParam('debug', 0); return; } } diff --git a/modules/drafts.pl b/modules/drafts.pl index d4a7e13b..01958438 100644 --- a/modules/drafts.pl +++ b/modules/drafts.pl @@ -48,10 +48,10 @@ sub DoDraft { SetParam('msg', T('Draft saved')); # invalidate cache print GetHttpHeader('', T('Draft saved'), '204 NO CONTENT'); } elsif (-f $draft) { - my %data = ParseData(ReadFileOrDie($draft)); + my $data = ParseData(ReadFileOrDie($draft)); unlink ($draft); $Message .= $q->p(T('Draft recovered')); - DoEdit($data{id}, $data{text}, 1); + DoEdit($data->{id}, $data->{text}, 1); } else { ReportError(T('No draft available to recover'), '404 NOT FOUND'); } diff --git a/modules/enclosure.pl b/modules/enclosure.pl index 45aca628..3d36211d 100644 --- a/modules/enclosure.pl +++ b/modules/enclosure.pl @@ -56,8 +56,8 @@ sub NewEnclosureRssItem { my $id = shift; my $rss = OldEnclosureRssItem($id, @_); require MIME::Base64; - my %data = ParseData(ReadFileOrDie(GetPageFile($id))); - my @enclosures = split(' ', $data{enclosures}); + my $data = ParseData(ReadFileOrDie(GetPageFile($id))); + my @enclosures = split(' ', $data->{enclosures}); my $enclosures = ''; foreach my $enclosure (@enclosures) { # Don't add the enclosure if the page has been deleted in the mean diff --git a/modules/gd_security_image.pl b/modules/gd_security_image.pl index d52129d0..f10524d8 100644 --- a/modules/gd_security_image.pl +++ b/modules/gd_security_image.pl @@ -240,9 +240,9 @@ sub GdSecurityImageCheck { if ($answer ne '' && GdSecurityImageIsValidId($id)) { my ($status, $data) = ReadFile(GdSecurityImageGetTicketFile($id)); if ($status) { - my %page = ParseData($data); - if ($page{generation_time} + $GdSecurityImageDuration > $Now) { - if ($answer eq $page{string}) { + my $page = ParseData($data); + if ($page->{generation_time} + $GdSecurityImageDuration > $Now) { + if ($answer eq $page->{string}) { $GdSecurityImageId = ''; if (!$GdSecurityImageRememberAnswer) { SetParam('gd_security_image_id', ''); diff --git a/modules/joiner.pl b/modules/joiner.pl index 6f908e0e..23245fcd 100644 --- a/modules/joiner.pl +++ b/modules/joiner.pl @@ -174,18 +174,17 @@ sub JoinerCreateAccount { } my ($email_status, $email_data) = ReadFile(JoinerGetEmailFile($email)); - my %email_page = (); if ($email_status) { - %email_page = ParseData($email_data); - if ($email_page{confirmed}) { + my $email_page = ParseData($email_data); + if ($email_page->{confirmed}) { return Ts('The email address %s has already been used.', $email); } - if ($email_page{registration_time} + $JoinerWait > $Now) { - my $min = 1 + int(($email_page{registration_time} + $JoinerWait - $Now) / 60); + if ($email_page->{registration_time} + $JoinerWait > $Now) { + my $min = 1 + int(($email_page->{registration_time} + $JoinerWait - $Now) / 60); return Ts('Wait %s minutes before try again.', $min); } } - %email_page = (); + my %email_page = (); $email_page{username} = $username; $email_page{email} = $email; $email_page{confirmed} = 0; @@ -468,37 +467,37 @@ sub JoinerDoConfirmRegistration { JoinerShowRegistrationConfirmationFailed(); return; } - my %page = ParseData($data); + my $page = ParseData($data); - if ($key ne $page{key}) { + if ($key ne $page->{key}) { $JoinerMessage = T('Invalid key.'); JoinerShowRegistrationConfirmationFailed(); return; } - if ($page{registration_time} + $JoinerWait < $Now) { + if ($page->{registration_time} + $JoinerWait < $Now) { $JoinerMessage = T('The key expired.'); JoinerShowRegistrationConfirmationFailed(); return; } - $page{key} = ''; - $page{confirmed} = 1; + $page->{key} = ''; + $page->{confirmed} = 1; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); - my $email = $page{email}; + my $email = $page->{email}; JoinerRequestLockOrError('joiner'); my ($email_status, $email_data) = ReadFile(JoinerGetEmailFile($email)); ReleaseLockDir('joiner'); if ($email_status) { - my %email_page = ParseData($email_data); - $email_page{confirmed} = 1; + my $email_page = ParseData($email_data); + $email_page->{confirmed} = 1; JoinerRequestLockOrError('joiner'); CreateDir($JoinerEmailDir); - WriteStringToFile(JoinerGetEmailFile($email), EncodePage(%email_page)); + WriteStringToFile(JoinerGetEmailFile($email), EncodePage(%$email_page)); ReleaseLockDir('joiner'); } @@ -570,41 +569,41 @@ sub JoinerDoProcessLogin { JoinerDoLogin(); return; } - my %page = ParseData($data); + my $page = ParseData($data); my $hash = JoinerGetPasswordHash($password); - if ($hash eq $page{password}) { - $page{recover} = 0; + if ($hash eq $page->{password}) { + $page->{recover} = 0; SetParam('joiner_recover', 0); - } elsif ($key ne '' && $key eq $page{recover_key}) { - if ($page{recover_time} + $JoinerWait < $Now) { + } elsif ($key ne '' && $key eq $page->{recover_key}) { + if ($page->{recover_time} + $JoinerWait < $Now) { $JoinerMessage = T('The key expired.'); JoinerDoLogin(); return; } - $page{recover} = 1; + $page->{recover} = 1; SetParam('joiner_recover', 1); } else { $JoinerMessage = T('Login failed.'); JoinerDoLogin(); return; } - if ($page{banned}) { + if ($page->{banned}) { $JoinerMessage = T('You are banned.'); JoinerDoLogin(); return; } - if (!$page{confirmed}) { + if (!$page->{confirmed}) { $JoinerMessage = T('You must confirm email address.'); JoinerDoLogin(); return; } my $session = Digest::MD5::md5_hex(rand()); - $page{session} = $session; + $page->{session} = $session; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); SetParam('username', $username); @@ -617,7 +616,7 @@ sub JoinerDoProcessLogin { print Ts('%s has logged in.', $username); print $q->end_p(); - if ($page{recover}) { + if ($page->{recover}) { print $q->start_p(); print T('You should set new password immediately.'); print $q->end_p(); @@ -735,9 +734,9 @@ sub JoinerDoProcessChangePassword { JoinerDoChangePassword(); return; } - my %page = ParseData($data); + my $page = ParseData($data); my $hash = JoinerGetPasswordHash($current_password); - if (!$page{recover} && $hash ne $page{password}) { + if (!$page->{recover} && $hash ne $page->{password}) { $JoinerMessage = T('Current Password:') . ' ' . T('Password is wrong.'); JoinerDoChangePassword(); return; @@ -754,12 +753,12 @@ sub JoinerDoProcessChangePassword { return; } - $page{password} = JoinerGetPasswordHash($new_password); - $page{key} = ''; - $page{recover} = ''; + $page->{password} = JoinerGetPasswordHash($new_password); + $page->{key} = ''; + $page->{recover} = ''; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); SetParam('joiner_recover', 0); @@ -823,9 +822,9 @@ sub JoinerDoProcessForgotPassword { JoinerDoForgotPassword(); return; } - my %email_page = ParseData($email_data); + my $email_page = ParseData($email_data); - my $username = $email_page{username}; + my $username = $email_page->{username}; JoinerRequestLockOrError('joiner'); my ($status, $data) = ReadFile(JoinerGetAccountFile($username)); ReleaseLockDir('joiner'); @@ -834,27 +833,27 @@ sub JoinerDoProcessForgotPassword { JoinerDoForgotPassword(); return; } - my %page = ParseData($data); + my $page = ParseData($data); - if ($email ne $page{email}) { + if ($email ne $page->{email}) { $JoinerMessage = T('The mail address is not valid anymore.'); JoinerDoForgotPassword(); return; } - if ($page{recover_time} + $JoinerWait > $Now) { - my $min = 1 + int(($page{recover_time} + $JoinerWait - $Now) / 60); + if ($page->{recover_time} + $JoinerWait > $Now) { + my $min = 1 + int(($page->{recover_time} + $JoinerWait - $Now) / 60); $JoinerMessage = Ts('Wait %s minutes before try again.', $min); JoinerDoForgotPassword(); return; } my $key = Digest::MD5::md5_hex($JoinerGeneratorSalt . rand()); - $page{recover_time} = $Now; - $page{recover_key} = $key; + $page->{recover_time} = $Now; + $page->{recover_key} = $key; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); JoinerSendRecoverAccountEmail($email, $username, $key); @@ -922,8 +921,8 @@ sub JoinerDoProcessChangeEmail { my ($email_status, $email_data) = ReadFile(JoinerGetEmailFile($email)); ReleaseLockDir('joiner'); if ($email_status) { - my %email_page = ParseData($email_data); - if ($email_page{confirmed} && $email_page{username} ne $username) { + my $email_page = ParseData($email_data); + if ($email_page->{confirmed} && $email_page->{username} ne $username) { $JoinerMessage = T('Email:') . ' ' . Ts('The email address %s has already been used.', $email); JoinerDoChangeEmail(); @@ -939,29 +938,29 @@ sub JoinerDoProcessChangeEmail { JoinerDoChangeEmail(); return; } - my %page = ParseData($data); + my $page = ParseData($data); - if ($page{change_email_time} + $JoinerWait > $Now) { - my $min = 1 + int(($page{change_email_time} + $JoinerWait - $Now) / 60); + if ($page->{change_email_time} + $JoinerWait > $Now) { + my $min = 1 + int(($page->{change_email_time} + $JoinerWait - $Now) / 60); $JoinerMessage = Ts('Wait %s minutes before try again.', $min); JoinerDoChangeEmail(); return; } my $hash = JoinerGetPasswordHash($password); - if ($hash ne $page{password}) { + if ($hash ne $page->{password}) { $JoinerMessage = T('Password:') . ' ' . T('Password is wrong.'); JoinerDoChangeEmail(); return; } my $key = Digest::MD5::md5_hex(rand()); - $page{change_email} = $email; - $page{change_email_key} = $key; - $page{change_email_time} = $Now; + $page->{change_email} = $email; + $page->{change_email_key} = $key; + $page->{change_email_time} = $Now; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); JoinerSendChangeEmailEmail($email, $username, $key); @@ -1012,22 +1011,22 @@ sub JoinerDoConfirmEmail { JoinerShowConfirmEmailFailed(); return; } - my %page = ParseData($data); + my $page = ParseData($data); - if ($key ne $page{change_email_key}) { + if ($key ne $page->{change_email_key}) { $JoinerMessage = T('Invalid key.'); JoinerShowConfirmEmailFailed(); return; } - my $new_email = $page{change_email}; - $page{email} = $new_email; - $page{change_email} = ''; - $page{change_email_key} = ''; - $page{change_email_time} = ''; + my $new_email = $page->{change_email}; + $page->{email} = $new_email; + $page->{change_email} = ''; + $page->{change_email_key} = ''; + $page->{change_email_time} = ''; JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); my %email_page = (); @@ -1128,30 +1127,30 @@ sub JoinerDoProcessBan { JoinerDoBan(); return; } - my %page = ParseData($data); + my $page = ParseData($data); if ($ban) { - if ($page{banned}) { + if ($page->{banned}) { $JoinerMessage = Ts('%s is already banned.', $username); JoinerDoBan(); return; } - $page{banned} = 1; - $page{session} = ''; + $page->{banned} = 1; + $page->{session} = ''; $JoinerMessage = Ts('%s has been banned.', $username); } else { - if (!$page{banned}) { + if (!$page->{banned}) { $JoinerMessage = Ts('%s is not banned.', $username); JoinerDoBan(); return; } - $page{banned} = 0; + $page->{banned} = 0; $JoinerMessage = Ts('%s has been unbanned.', $username); } JoinerRequestLockOrError('joiner'); CreateDir($JoinerDir); - WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%page)); + WriteStringToFile(JoinerGetAccountFile($username), EncodePage(%$page)); ReleaseLockDir('joiner'); JoinerDoBan(); @@ -1178,16 +1177,16 @@ sub JoinerIsLoggedIn { $JoinerLoggedIn = 0; return $JoinerLoggedIn; } - my %page = ParseData($data); - if (!$page{confirmed}) { + my $page = ParseData($data); + if (!$page->{confirmed}) { $JoinerLoggedIn = 0; return $JoinerLoggedIn; } - if ($session ne $page{session}) { + if ($session ne $page->{session}) { $JoinerLoggedIn = 0; return $JoinerLoggedIn; } - if ($page{banned}) { + if ($page->{banned}) { $JoinerLoggedIn = 0; return $JoinerLoggedIn; } diff --git a/modules/live-templates.pl b/modules/live-templates.pl index 3d785f60..048a34cd 100644 --- a/modules/live-templates.pl +++ b/modules/live-templates.pl @@ -35,12 +35,12 @@ sub LiveTemplateRule { Dirty($str); my $oldpos = pos; my $old_ = $_; - my %hash = ParseData($2); + my $hash = ParseData($2); my $text = GetPageContent($template); return $q->p($q->strong(Ts('The template %s is either empty or does not exist.', $template))) . AddHtmlEnvironment('p') unless $text; - foreach my $key (keys %hash) { - $text =~ s/\$$key\$/$hash{$key}/g; + foreach my $key (keys %$hash) { + $text =~ s/\$$key\$/$hash->{$key}/g; } print "
"; ApplyRules(QuoteHtml($text), 1, 1, undef, 'p'); diff --git a/modules/namespaces.pl b/modules/namespaces.pl index 421b6a26..86c895e7 100644 --- a/modules/namespaces.pl +++ b/modules/namespaces.pl @@ -381,7 +381,8 @@ sub NewNamespaceBrowsePage { #REDIRECT into different namespaces my ($id, $raw, $comment, $status) = @_; OpenPage($id); - my ($text, $revision) = GetTextRevision(GetParam('revision', ''), 1); + my ($revisionPage, $revision) = GetTextRevision(GetParam('revision', ''), 1); + my $text = $revisionPage->{text}; my $oldId = GetParam('oldid', ''); if (not $oldId and not $revision and (substr($text, 0, 10) eq '#REDIRECT ') and (($WikiLinks and $text =~ /^\#REDIRECT\s+(($InterSitePattern:)?$InterLinkPattern)/) diff --git a/modules/near-links.pl b/modules/near-links.pl index f56516eb..8f36c496 100644 --- a/modules/near-links.pl +++ b/modules/near-links.pl @@ -271,11 +271,11 @@ sub SearchNearPages { my @entries = split(/\n\n+/, $data); shift @entries; # skip head foreach my $entry (@entries) { - my %entry = ParseData($entry); # need to pass reference - my $name = $entry{title}; + my $entryPage = ParseData($entry); # need to pass reference + my $name = $entryPage->{title}; next if $found{$name}; # do not duplicate local pages $found{$name} = 1; - PrintSearchResultEntry(\%entry, $regex); # with context and full search! + PrintSearchResultEntry($entryPage, $regex); # with context and full search! } } } diff --git a/modules/olocalmap.pl b/modules/olocalmap.pl index eb96f899..a192f81d 100644 --- a/modules/olocalmap.pl +++ b/modules/olocalmap.pl @@ -99,9 +99,9 @@ sub LocalMapWorkHorse { my $retval_children = ''; if ($depth > 0) { - my %data = ParseData(ReadFileOrDie(GetPageFile($id))); - my @flags = split(/$FS/, $data{'flags'}); - my @blocks = split(/$FS/, $data{'blocks'}); + my $data = ParseData(ReadFileOrDie(GetPageFile($id))); + my @flags = split(/$FS/, $data->{'flags'}); + my @blocks = split(/$FS/, $data->{'blocks'}); my @subpages; # Iterate over blocks, operate only on "dirty" ones diff --git a/modules/private-pages.pl b/modules/private-pages.pl index 7eda59e4..6823e72c 100644 --- a/modules/private-pages.pl +++ b/modules/private-pages.pl @@ -69,8 +69,8 @@ sub NewPrivatePagesUserCanEdit { my $result = OldPrivatePagesUserCanEdit($id, $editing, @rest); # bypass OpenPage and GetPageContent (these are redefined below) if ($result > 0 and $editing and $IndexHash{$id}) { - my %data = ParseData(ReadFileOrDie(GetPageFile($id))); - if (PrivatePageLocked($data{text})) { + my $data = ParseData(ReadFileOrDie(GetPageFile($id))); + if (PrivatePageLocked($data->{text})) { return 0; } } @@ -128,11 +128,11 @@ sub NewPrivatePagesGetPageContent { *GetTextRevision = \&NewPrivatePagesGetTextRevision; sub NewPrivatePagesGetTextRevision { - my ($text, $revision) = OldPrivatePagesGetTextRevision(@_); - if (PrivatePageLocked($text)) { - return (NewPrivatePageNewText(), $revision); + my ($page, $revision) = OldPrivatePagesGetTextRevision(@_); + if (PrivatePageLocked($page->{text})) { + return ({text => NewPrivatePageNewText()}, $revision); # XXX faking a page object like this is not good } - return ($text, $revision); + return wantarray ? ($page, $revision) : $page; } # hide #PASSWORD diff --git a/modules/static-copy.pl b/modules/static-copy.pl index 63df70f8..d84a9b1d 100644 --- a/modules/static-copy.pl +++ b/modules/static-copy.pl @@ -117,9 +117,9 @@ sub StaticFileName { my ($status, $data) = ReadFile(GetPageFile(UrlDecode($id))); # If the link points to a wanted page, we cannot make this static. return $id unless $status; - my %hash = ParseData($data); + my $hash = ParseData($data); my $ext = '.html'; - if ($hash{text} =~ /^\#FILE ([^ \n]+ ?[^ \n]*)\n(.*)/s) { + if ($hash->{text} =~ /^\#FILE ([^ \n]+ ?[^ \n]*)\n(.*)/s) { %StaticMimeTypes = StaticMimeTypes() unless %StaticMimeTypes; $ext = $StaticMimeTypes{"$1"}; $ext = '.' . $ext if $ext; diff --git a/modules/static-hybrid.pl b/modules/static-hybrid.pl index ff439969..b2771c29 100644 --- a/modules/static-hybrid.pl +++ b/modules/static-hybrid.pl @@ -113,9 +113,9 @@ sub StaticFileName { return $StaticFiles{$id} if $StaticFiles{$id}; # cache filenames my ($status, $data) = ReadFile(GetPageFile(StaticUrlDecode($id))); print "cannot read " . GetPageFile(StaticUrlDecode($id)) . $q->br() unless $status; - my %hash = ParseData($data); + my $hash = ParseData($data); my $ext = '.html'; - if ($hash{text} =~ /^\#FILE ([^ \n]+)\n(.*)/s) { + if ($hash->{text} =~ /^\#FILE ([^ \n]+)\n(.*)/s) { $ext = $StaticMimeTypes{$1}; $ext = '.' . $ext if $ext; } @@ -445,15 +445,15 @@ sub StaticNewDespamPage { # from DoHistory() my @revisions = sort {$b <=> $a} map { m|/([0-9]+).kp$|; $1; } GetKeepFiles($OpenPageName); foreach my $revision (@revisions) { - my ($text, $rev) = GetTextRevision($revision, 1); # quiet + my ($revisionPage, $rev) = GetTextRevision($revision, 1); # quiet if (not $rev) { print ': ' . Ts('Cannot find revision %s.', $revision); return; - } elsif (not DespamBannedContent($text)) { + } elsif (not DespamBannedContent($revisionPage->{text})) { my $summary = Tss('Revert to revision %1: %2', $revision, $rule); print ': ' . $summary; - Save($OpenPageName, $text, $summary) unless GetParam('debug', 0); - StaticDeleteFile($OpenPageName); + Save($OpenPageName, $revisionPage->{text}, $summary) unless GetParam('debug', 0); + StaticDeleteFile($OpenPageName); return; } } @@ -461,7 +461,7 @@ sub StaticNewDespamPage { my $summary = Ts($rule). ' ' . Ts('Marked as %s.', $DeletedPage); print ': ' . $summary; Save($OpenPageName, $DeletedPage, $summary) unless GetParam('debug', 0); - StaticDeleteFile($OpenPageName); + StaticDeleteFile($OpenPageName); } else { print ': ' . T('Cannot find unspammed revision.'); } diff --git a/modules/svg-edit.pl b/modules/svg-edit.pl index d8955668..76ecf62e 100644 --- a/modules/svg-edit.pl +++ b/modules/svg-edit.pl @@ -42,7 +42,7 @@ sub NewSvgGetDownloadLink { local (%Page, $OpenPageName); OpenPage($name); if ($revision) { - ($data) = GetTextRevision($revision); # ignore revision reset + $data = GetTextRevision($revision)->{text}; # ignore revision reset } else { $data = $Page{text}; } diff --git a/modules/thumbs.pl b/modules/thumbs.pl index f68ecb02..e1191034 100644 --- a/modules/thumbs.pl +++ b/modules/thumbs.pl @@ -190,7 +190,7 @@ sub GenerateThumbNail { # Check MIME type supported # Check is a file - my ($text, $revision) = GetTextRevision(GetParam('revision', '')); # maybe revision reset! + my $text = GetTextRevision(GetParam('revision', ''))->{text}; # maybe revision reset! my ($type) = TextIsFile($text); # MIME type if an uploaded file my $data = substr($text, index($text, "\n") + 1); diff --git a/modules/translations.pl b/modules/translations.pl index df330a4e..7b8cea95 100644 --- a/modules/translations.pl +++ b/modules/translations.pl @@ -37,8 +37,7 @@ sub TranslationRule { sub GetCurrentPageRevision { my $id = shift; - my %page = ParseData(ReadFileOrDie(GetPageFile($id))); - return $page{revision}; + return ParseData(ReadFileOrDie(GetPageFile($id)))->{revision}; } sub GetTranslationLink { diff --git a/wiki.pl b/wiki.pl index ac81fb57..47ec8507 100755 --- a/wiki.pl +++ b/wiki.pl @@ -231,8 +231,8 @@ sub InitConfig { } if ($ConfigPage) { # $FS and $MaxPost must be set in config file! my ($status, $data) = ReadFile(GetPageFile(FreeToNormal($ConfigPage))); - my %data = ParseData($data); # before InitVariables so GetPageContent won't work - eval $data{text} if $data{text}; # perlcritic dislikes the use of eval here but we really mean it + my $page = ParseData($data); # before InitVariables so GetPageContent won't work + eval $page->{text} if $page->{text}; # perlcritic dislikes the use of eval here but we really mean it $Message .= CGI::p("$ConfigPage: $@") if $@; } } @@ -1384,7 +1384,8 @@ sub NewText { sub BrowsePage { my ($id, $raw, $comment, $status) = @_; OpenPage($id); - my ($text, $revision, $summary) = GetTextRevision(GetParam('revision', '')); + my ($revisionPage, $revision) = GetTextRevision(GetParam('revision', '')); + my $text = $revisionPage->{text}; $text = NewText($id) unless $revision or $Page{revision}; # new text for new pages # handle a single-level redirect my $oldId = GetParam('oldid', ''); @@ -1414,7 +1415,7 @@ sub BrowsePage { print GetHeader($id, NormalToFree($id), $oldId, undef, $status); my $showDiff = GetParam('diff', 0); if ($UseDiff and $showDiff) { - PrintHtmlDiff($showDiff, GetParam('diffrevision', $revision), $revision, $text, $summary); + PrintHtmlDiff($showDiff, GetParam('diffrevision', $revision), $revision, $text, $revisionPage->{summary}); print $q->hr(); } PrintPageContent($text, $revision, $comment); @@ -1963,10 +1964,10 @@ sub DoRawHistory { RcTextRevision($id, $Page{ts}, $Page{host}, $Page{username}, $Page{summary}, $Page{minor}, $Page{revision}, \@languages, undef, 1); foreach my $revision (GetKeepRevisions($OpenPageName)) { - my %keep = GetKeptRevision($revision); - @languages = split(/,/, $keep{languages}); - RcTextRevision($id, $keep{ts}, $keep{host}, $keep{username}, - $keep{summary}, $keep{minor}, $keep{revision}, \@languages); + my $keep = GetKeptRevision($revision); + @languages = split(/,/, $keep->{languages}); + RcTextRevision($id, $keep->{ts}, $keep->{host}, $keep->{username}, + $keep->{summary}, $keep->{minor}, $keep->{revision}, \@languages); } } @@ -1979,9 +1980,9 @@ sub DoHtmlHistory { my $date = CalcDay($Page{ts}); my @html = (GetHistoryLine($id, \%Page, $row++, $rollback, $date, 1)); foreach my $revision (GetKeepRevisions($OpenPageName)) { - my %keep = GetKeptRevision($revision); - my $new = CalcDay($keep{ts}); - push(@html, GetHistoryLine($id, \%keep, $row++, $rollback, + my $keep = GetKeptRevision($revision); + my $new = CalcDay($keep->{ts}); + push(@html, GetHistoryLine($id, $keep, $row++, $rollback, $new, $new ne $date)); $date = $new; } @@ -2536,8 +2537,12 @@ sub PrintHtmlDiff { if (not $old and (not $diff or GetParam('cache', $UseCache) < 1)) { if ($type == 1) { $old = $Page{lastmajor} - 1; - ($text, $new, $summary) = GetTextRevision($Page{lastmajor}, 1) - unless $new or $Page{lastmajor} == $Page{revision}; + if (not $new and $Page{lastmajor} != $Page{revision}) { + my $revisionPage; + ($revisionPage, $new) = GetTextRevision($Page{lastmajor}, 1); + $text = $revisionPage->{text}; + $summary = $revisionPage->{summary}; + } } elsif ($new) { $old = $new - 1; } else { @@ -2549,8 +2554,7 @@ sub PrintHtmlDiff { $intro = Tss('Difference between revision %1 and %2', $old, $new ? Ts('revision %s', $new) : T('current revision')); } elsif ($type == 1 and $Page{lastmajor} and $Page{lastmajor} != $Page{revision}) { - my %keep = GetKeptRevision($Page{lastmajor}); - $summary = $keep{summary}; + $summary = GetKeptRevision($Page{lastmajor})->{summary}; $intro = Ts('Last major edit (%s)', ScriptLinkDiff(1, $OpenPageName, T('later minor edits'), undef, $Page{lastmajor} || 1)); } @@ -2571,10 +2575,10 @@ sub GetCacheDiff { sub GetKeptDiff { my ($new, $revision) = @_; $revision ||= 1; - my ($old, $rev) = GetTextRevision($revision, 1); + my ($revisionPage, $rev) = GetTextRevision($revision, 1); return '' unless $rev; - return T("The two revisions are the same.") if $old eq $new; - return GetDiff($old, $new, $rev); + return T("The two revisions are the same.") if $revisionPage->{text} eq $new; + return GetDiff($revisionPage->{text}, $new, $rev); } sub DoDiff { # Actualy call the diff program @@ -2687,14 +2691,15 @@ sub ParseData { $value =~ s/\n\t/\n/g; $result{$key} = $value; } - return %result; + # return unless %result; # undef instead of empty hash # TODO should we do that? + return wantarray ? %result : \%result; # return list sometimes for compatibility } sub OpenPage { # Sets global variables my $id = shift; return if $OpenPageName eq $id; if ($IndexHash{$id}) { - %Page = ParseData(ReadFileOrDie(GetPageFile($id))); + %Page = %{ParseData(ReadFileOrDie(GetPageFile($id)))}; } else { %Page = (); $Page{ts} = $Now; @@ -2709,42 +2714,40 @@ sub GetTextAtTime { # call with opened page, return $minor if all pages between my $minor = $Page{minor}; return ($Page{text}, $minor, 0) if $Page{ts} <= $ts; # current page is old enough return ($DeletedPage, $minor, 0) if $Page{revision} == 1 and $Page{ts} > $ts; # created after $ts - my %keep = (); # info may be needed after the loop + my $keep = {}; # info may be needed after the loop foreach my $revision (GetKeepRevisions($OpenPageName)) { - %keep = GetKeptRevision($revision); - $minor = 0 if not $keep{minor} and $keep{ts} >= $ts; # ignore keep{minor} if keep{ts} is too old - return ($keep{text}, $minor, 0) if $keep{ts} <= $ts; + $keep = GetKeptRevision($revision); + # $minor = 0 unless defined $keep; # TODO? + $minor = 0 if not $keep->{minor} and $keep->{ts} >= $ts; # ignore keep{minor} if keep{ts} is too old + return ($keep->{text}, $minor, 0) if $keep->{ts} <= $ts; } - return ($DeletedPage, $minor, 0) if $keep{revision} == 1; # then the page was created after $ts! - return ($keep{text}, $minor, $keep{ts}); # the oldest revision available is not old enough + return ($DeletedPage, $minor, 0) if $keep->{revision} == 1; # then the page was created after $ts! + return ($keep->{text}, $minor, $keep->{ts}); # the oldest revision available is not old enough } sub GetTextRevision { my ($revision, $quiet) = @_; $revision =~ s/\D//g; # Remove non-numeric chars - return ($Page{text}, $revision, $Page{summary}) unless $revision and $revision ne $Page{revision}; - my %keep = GetKeptRevision($revision); - if (not %keep) { + return wantarray ? (\%Page, $revision) : \%Page unless $revision and $revision ne $Page{revision}; + my $keep = GetKeptRevision($revision); + if (not defined $keep) { $Message .= $q->p(Ts('Revision %s not available', $revision) . ' (' . T('showing current revision instead') . ')') unless $quiet; - return ($Page{text}, '', ''); + return wantarray ? (\%Page, '') : \%Page; } $Message .= $q->p(Ts('Showing revision %s', $revision)) unless $quiet; - return ($keep{text}, $revision, $keep{summary}); + return wantarray ? ($keep, $revision) : $keep; } sub GetPageContent { my $id = shift; - if ($IndexHash{$id}) { - my %data = ParseData(ReadFileOrDie(GetPageFile($id))); - return $data{text}; - } + return ParseData(ReadFileOrDie(GetPageFile($id)))->{text} if $IndexHash{$id}; return ''; } sub GetKeptRevision { # Call after OpenPage my ($status, $data) = ReadFile(GetKeepFile($OpenPageName, (shift))); - return () unless $status; + return unless $status; return ParseData($data); } @@ -2822,9 +2825,9 @@ sub ExpireKeepFiles { # call with opened page return unless $KeepDays; my $expirets = $Now - ($KeepDays * 86400); # 24*60*60 foreach my $revision (GetKeepRevisions($OpenPageName)) { - my %keep = GetKeptRevision($revision); - next if $keep{'keep-ts'} >= $expirets; - next if $KeepMajor and $keep{revision} == $Page{lastmajor}; + my $keep = GetKeptRevision($revision); + next if $keep->{'keep-ts'} >= $expirets; + next if $KeepMajor and $keep->{revision} == $Page{lastmajor}; unlink GetKeepFile($OpenPageName, $revision); } } @@ -3031,8 +3034,8 @@ sub DoEdit { ReportError(T('Only administrators can upload files.'), '403 FORBIDDEN'); } OpenPage($id); - my ($text, $revision) = GetTextRevision(GetParam('revision', ''), 1); # maybe revision reset! - my $oldText = $preview ? $newText : $text; + my ($revisionPage, $revision) = GetTextRevision(GetParam('revision', ''), 1); # maybe revision reset! + my $oldText = $preview ? $newText : $revisionPage->{text}; my $isFile = TextIsFile($oldText); $upload //= $isFile; if ($upload and not $UploadAllowed and not UserIsAdmin()) { @@ -3110,7 +3113,8 @@ sub DoDownload { my $id = shift; OpenPage($id) if ValidIdOrDie($id); print $q->header(-status=>'304 NOT MODIFIED') and return if FileFresh(); # FileFresh needs an OpenPage! - my ($text, $revision) = GetTextRevision(GetParam('revision', '')); # maybe revision reset! + my ($revisionPage, $revision) = GetTextRevision(GetParam('revision', '')); # maybe revision reset! + my $text = $revisionPage->{text}; if (my ($type, $encoding) = TextIsFile($text)) { my ($data) = $text =~ /^[^\n]*\n(.*)/s; my %allowed = map {$_ => 1} @UploadTypes; From 9d7e5b43c0cff28fab0cdf1d0a4faee489c6ce6e Mon Sep 17 00:00:00 2001 From: Aleks-Daniel Jakimenko Date: Sat, 5 Sep 2015 23:54:43 +0300 Subject: [PATCH 2/4] Test for Issue #1 on github --- t/revisions.t | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/t/revisions.t b/t/revisions.t index 7c3e0f7d..fb9baaee 100644 --- a/t/revisions.t +++ b/t/revisions.t @@ -18,12 +18,14 @@ require 't/test.pl'; package OddMuse; -use Test::More tests => 16; +use Test::More tests => 17; ## Test revision and diff stuff update_page('KeptRevisions', 'first'); -update_page('KeptRevisions', 'second'); +#sleep 120; # TODO implement fake time! +update_page('KeptRevisions', 'second', '', 0, 0, 'username=BestContributorEver'); +#sleep 120; # TODO implement fake time! update_page('KeptRevisions', 'third'); update_page('KeptRevisions', 'fourth', '', 1); update_page('KeptRevisions', 'fifth', '', 1); @@ -50,6 +52,10 @@ test_page(get_page('action=browse revision=9 id=KeptRevisions'), 'Revision 9 not available \(showing current revision instead\)', 'fifth'); +my ($ts2) = get_page('action=browse revision=2 id=KeptRevisions') =~ /edited (.*?) diff/ix; +my ($ts3) = get_page('action=browse revision=3 id=KeptRevisions') =~ /edited (.*?) diff/ix; +ok($ts2 ne $ts3, 'Revision timestamp or author is different'); + # Disable cache and request the correct last major diff test_page(get_page('action=browse diff=1 id=KeptRevisions cache=0'), 'Difference between revision 2 and revision 3', From 1cd33b691c5bb69453fa33c6ff19a32117d6292d Mon Sep 17 00:00:00 2001 From: Aleks-Daniel Jakimenko Date: Sun, 6 Sep 2015 01:10:29 +0300 Subject: [PATCH 3/4] Fix for issue #1 on github Changing everything to return objects is a worthy goal, but for now we have taken enough destructive steps towards it. Therefore, this commit fixes the problem in backwards compatible way (by adding one more parameter to the signatures). Note that this additional parameter is NOT a timestamp, it is a whole page object. Which means that we are still moving towards our goal of using page objects everywhere, this commit is just doing it in a backwards-compatible way. --- wiki.pl | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/wiki.pl b/wiki.pl index 47ec8507..b3fdef1e 100755 --- a/wiki.pl +++ b/wiki.pl @@ -1421,7 +1421,7 @@ sub BrowsePage { PrintPageContent($text, $revision, $comment); SetParam('rcclusteronly', $id) if FreeToNormal(GetCluster($text)) eq $id; # automatically filter by cluster PrintRcHtml($id); - PrintFooter($id, $revision, $comment); + PrintFooter($id, $revision, $comment, $revisionPage); } sub ReBrowsePage { @@ -2384,7 +2384,7 @@ sub PrintPageContent { } sub PrintFooter { - my ($id, $rev, $comment) = @_; + my ($id, $rev, $comment, $page) = @_; if (GetParam('embed', $EmbedWiki)) { print $q->end_html, "\n"; return; @@ -2401,11 +2401,11 @@ sub WrapperEnd { # called via @MyFooters } sub DefaultFooter { # called via @MyFooters - my ($id, $rev, $comment) = @_; + my ($id, $rev, $comment, $page) = @_; my $html = $q->start_div({-class=>'footer'}) . $q->hr(); $html .= GetGotoBar($id) if GetParam('toplinkbar', $TopLinkBar) != 1; $html .= GetFooterLinks($id, $rev); - $html .= GetFooterTimestamp($id, $rev); + $html .= GetFooterTimestamp($id, $rev, $page); $html .= GetSearchForm() if GetParam('topsearchform', $TopSearchForm) != 1; if ($DataDir =~ m|/tmp/|) { $html .= $q->p($q->strong(T('Warning') . ': ') @@ -2418,11 +2418,12 @@ sub DefaultFooter { # called via @MyFooters } sub GetFooterTimestamp { - my ($id, $rev) = @_; - if ($id and $rev ne 'history' and $rev ne 'edit' and $Page{revision}) { - my @elements = (($rev eq '' ? T('Last edited') : T('Edited')), TimeToText($Page{ts}), - Ts('by %s', GetAuthorLink($Page{host}, $Page{username}))); - push(@elements, ScriptLinkDiff(2, $id, T('(diff)'), $rev)) if $UseDiff and $Page{revision} > 1; + my ($id, $rev, $page) = @_; + $page //= \%Page; + if ($id and $rev ne 'history' and $rev ne 'edit' and $page->{revision}) { + my @elements = (($rev eq '' ? T('Last edited') : T('Edited')), TimeToText($page->{ts}), + Ts('by %s', GetAuthorLink($page->{host}, $page->{username}))); + push(@elements, ScriptLinkDiff(2, $id, T('(diff)'), $rev)) if $UseDiff and $page->{revision} > 1; return $q->div({-class=>'time'}, @elements); } return ''; From f8ac7a281831591af0ed1d28d9e68a48bfecaf62 Mon Sep 17 00:00:00 2001 From: Aleks-Daniel Jakimenko Date: Sun, 6 Sep 2015 02:55:36 +0300 Subject: [PATCH 4/4] aawrapperdiv.pl: wrap PrintFooter correctly --- modules/aawrapperdiv.pl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/aawrapperdiv.pl b/modules/aawrapperdiv.pl index d52f264a..edb82e19 100644 --- a/modules/aawrapperdiv.pl +++ b/modules/aawrapperdiv.pl @@ -36,8 +36,7 @@ sub WrapperGetHeader { *PrintFooter = \&WrapperPrintFooter; sub WrapperPrintFooter { - my ($id, $rev, $comment) = @_; print $q->start_div({-class=>'wrapper close'}); print $q->end_div(), $q->end_div(); - OldPrintFooter($id, $rev, $comment); + OldPrintFooter(@_); }