merging 7.1.4 changes

docs/changelog/7.x.x.txt

@@ -25,6 +25,7 @@
  - Made Stow's warning a debug message, which is what debug messages are for.
  - fix: WebGUI::Text::splitCsv no longer removes trailing empty fields
  - fix: Product add-to-group would always try to add a user to a group
+ - Made many minor changes recommended by Perl::Critic.
 
 7.1.3
  - fix: SQLReport now returns error if can't find DatabaseLink

lib/WebGUI.pm

@@ -418,11 +418,11 @@ sub uploadsHandler {
 		$path =~ s/^(\/.*\/).*$/$1/;
 		if (-e $path.".wgaccess") {
 			my $fileContents;
-			open(FILE,"<".$path.".wgaccess");
+			open(my $FILE,"<",$path.".wgaccess");
-			while (<FILE>) {
+			while (<$FILE>) {
 				$fileContents .= $_;
 			}
-			close(FILE);
+			close($FILE);
 			my @privs = split("\n",$fileContents);
 			unless ($privs[1] eq "7" || $privs[1] eq "1") {
 				my $s = Apache2::ServerUtil->server;
@@ -459,11 +459,11 @@ The current WebGUI::Session object.
 sub upgrading {
 	my $session = shift;
 	$session->http->sendHeader;
-	open(FILE,"<".$session->config->getWebguiRoot."/docs/maintenance.html");
+	open(my $FILE,"<",$session->config->getWebguiRoot."/docs/maintenance.html");
-	while (<FILE>) {
+	while (<$FILE>) {
 		$session->output->print($_);
 	}
-	close(FILE);
+	close($FILE);
 }
 
 1;

lib/WebGUI/Affiliate.pm

@@ -1,5 +1,7 @@
 package WebGUI::Affiliate;
 
+use strict;
+
 =head1 LEGAL
 
  -------------------------------------------------------------------

lib/WebGUI/Asset.pm

@@ -869,11 +869,11 @@ sub getNotFound {
 	my $session = shift;
 	if ($session->url->getRequestedUrl eq "*give-credit-where-credit-is-due*") {
 		my $content = "";
-		open(FILE,"<".$session->config->getWebguiRoot."/docs/credits.txt");
+		open(my $FILE,"<",$session->config->getWebguiRoot."/docs/credits.txt");
-		while (<FILE>) {
+		while (<$FILE>) {
 			$content .= $_;
 		}
-		close(FILE);
+		close($FILE);
 		return WebGUI::Asset->newByPropertyHashRef($session,{
 			className=>"WebGUI::Asset::Snippet",
 			snippet=> '<pre>'.$content.'</pre>'
@@ -1680,7 +1680,10 @@ sub processTemplate {
 	my $template = shift;
 	$template = WebGUI::Asset->new($self->session, $templateId,"WebGUI::Asset::Template") unless (defined $template);
 	if (defined $template) {
-        	my $meta = $self->getMetaDataFields() if ($self->session->setting->get("metaDataEnabled"));
+        	my $meta = {};
+		if ($self->session->setting->get("metaDataEnabled")) {
+        		$meta = $self->getMetaDataFields();
+		}
         	foreach my $field (keys %$meta) {
 			$var->{$meta->{$field}{fieldName}} = $meta->{$field}{value};
 		}

lib/WebGUI/AssetLineage.pm

@@ -378,7 +378,10 @@ sub getLineage {
 		$where = "asset.state='published'";
 	}
 	## get only approved items or those that i'm currently working on
-	my $archived = " or assetData.status='archived' " if ($rules->{includeArchived});
+	my $archived = "";
+	if ($rules->{includeArchived}) {
+		$archived = " or assetData.status='archived' ";
+	}
 	$where .= " and (assetData.status='approved' $archived or assetData.tagId=".$self->session->db->quote($self->session->scratch->get("versionTag")).")";
 	## class exclusions
 	if (exists $rules->{excludeClasses}) {

lib/WebGUI/AssetPackage.pm

@@ -247,7 +247,10 @@ sub www_importPackage {
 	return $self->session->privilege->insufficient() unless ($self->canEdit && $self->session->user->isInGroup(4));
 	my $storage = WebGUI::Storage->createTemp($self->session);
 	$storage->addFileFromFormPost("packageFile",1);
-	my $error = $self->importPackage($storage) if ($storage->getFileExtension($storage->getFiles->[0]) eq "wgpkg");
+	my $error = "";
+	if ($storage->getFileExtension($storage->getFiles->[0]) eq "wgpkg") {
+		$error = $self->importPackage($storage);
+	}
 	if ($error) {
 		my $i18n = WebGUI::International->new($self->session, "Asset");
 		return $self->session->style->userStyle($i18n->get("package corrupt"));

lib/WebGUI/AssetVersioning.pm

@@ -125,7 +125,10 @@ Optionally specify to get the count based upon the status of the revisions. Opti
 sub getRevisionCount {
 	my $self = shift;
 	my $status = shift;
-	my $statusClause = " and status=".$self->session->db->quote($status) if ($status);
+	my $statusClause = "";
+	if ($status) {
+		$statusClause = " and status=".$self->session->db->quote($status);
+	}
 	my ($count) = $self->session->db->quickArray("select count(*) from assetData where assetId=".$self->session->db->quote($self->getId).$statusClause);
 	return $count;
 }

lib/WebGUI/Cache.pm

@@ -14,6 +14,7 @@ package WebGUI::Cache;
 
 =cut
 
+use strict;
 use File::Path;
 use HTTP::Headers;
 use HTTP::Request;
@@ -63,7 +64,7 @@ An array reference representing the portion of the key to delete. So if you have
 =cut
 
 sub deleteChunk {
-	$self = shift;
+	my $self = shift;
 	$self->flush;
 }
 

lib/WebGUI/Cache/FileCache.pm

@@ -14,7 +14,7 @@ package WebGUI::Cache::FileCache;
 
 =cut
 
-
+use strict;
 use Storable qw(nstore retrieve);
 use File::Path;
 use File::Find;
@@ -107,9 +107,9 @@ sub get {
 	my $self = shift;
 	return undef if ($self->session->config->get("disableCache"));
 	my $folder = $self->getFolder;
-	if (-e $folder."/expires" && -e $folder."/cache" && open(FILE,"<".$folder."/expires")) {
+	if (-e $folder."/expires" && -e $folder."/cache" && open(my $FILE,"<",$folder."/expires")) {
-		my $expires = <FILE>;
+		my $expires = <$FILE>;
-		close(FILE);
+		close($FILE);
 		return undef if ($expires < $self->session->datetime->time());
 		my $value;
 		eval {$value = retrieve($folder."/cache")};
@@ -173,9 +173,9 @@ sub getNamespaceSize {
         File::Find::find({no_chdir=>1, wanted=> sub {
                                 return unless $File::Find::name =~ m/^(.*)expires$/;
                                 my $dir = $1;
-                                if (open(FILE,"<".$dir."/expires")) {
+                                if (open(my $FILE,"<",$dir."/expires")) {
-                                        my $expires = <FILE>;
+                                        my $expires = <$FILE>;
-                                        close(FILE);
+                                        close($FILE);
                                         if ($expires <$self->session->datetime->time()+$expiresModifier) {
                                                 rmtree($dir);
                                         } else {
@@ -254,9 +254,9 @@ sub set {
 		$value = $content;
 	}
 	nstore($value, $path."/cache");
-	open(FILE,">".$path."/expires");
+	open(my $FILE,">",$path."/expires");
-	print FILE$self->session->datetime->time()+$ttl;
+	print $FILE $self->session->datetime->time()+$ttl;
-	close(FILE);
+	close($FILE);
 	umask($oldumask);
 }
 

lib/WebGUI/Config.pm

@@ -132,9 +132,9 @@ sub delete {
 	my $self = shift;
 	my $param = shift;
 	delete $self->{_config}{$param};
-	open(FILE,">".$self->getWebguiRoot.'/etc/'.$self->getFilename);
+	open(my $FILE,">",$self->getWebguiRoot.'/etc/'.$self->getFilename);
-	print FILE "# config-file-type: JSON 1\n".objToJson($self->{_config}, {pretty => 1, indent => 4, autoconv=>0, skipinvalid=>1});
+	print $FILE "# config-file-type: JSON 1\n".objToJson($self->{_config}, {pretty => 1, indent => 4, autoconv=>0, skipinvalid=>1});
-	close(FILE);
+	close($FILE);
 }
 
 #-------------------------------------------------------------------
@@ -334,11 +334,11 @@ sub new {
 		return $config{$filename};
 	} else {
 		my $json = "";
-		if (open(FILE,"<".$webguiPath.'/etc/'.$filename)) {
+		if (open(my $FILE,"<",$webguiPath.'/etc/'.$filename)) {
-			while (my $line = <FILE>) {
+			while (my $line = <$FILE>) {
         			$json .= $line unless ($line =~ /^\s*#/);
 			}
-			close(FILE);
+			close($FILE);
 			my $conf = jsonToObj($json);
 			my $self = {_webguiRoot=>$webguiPath, _configFile=>$filename, _config=>$conf};
 			bless $self, $class;
@@ -403,9 +403,9 @@ sub set {
 	my $param = shift;
 	my $value = shift;
 	$self->{_config}{$param} = $value;
-	open(FILE,">".$self->getWebguiRoot.'/etc/'.$self->getFilename);
+	open(my $FILE,">",$self->getWebguiRoot.'/etc/'.$self->getFilename);
-	print FILE "# config-file-type: JSON 1\n".objToJson($self->{_config}, {pretty => 1, indent => 4, autoconv=>0, skipinvalid=>1});
+	print $FILE "# config-file-type: JSON 1\n".objToJson($self->{_config}, {pretty => 1, indent => 4, autoconv=>0, skipinvalid=>1});
-	close(FILE);
+	close($FILE);
 }
 
 1;

lib/WebGUI/International.pm

@@ -232,11 +232,7 @@ Specify a default language. Defaults to user preference or "English".
 
 sub new {
 	my ($class, $session, $namespace, $language) = @_;
-	my $self = bless( {
+	my $self = bless { _session   => $session, _namespace => $namespace, _language  => ($language || $session->user->profileField('language')) }, $class;
-		_session   => $session,
-		_namespace => $namespace,
-		_language  => ($language || $session->user->profileField('language')),
-	},$class);
 	return $self;
 }
 

lib/WebGUI/Mail/Get.pm

@@ -205,7 +205,10 @@ sub getNextMessage {
 		my $disposition = $part->head->get("Content-Disposition");
 		$disposition =~ m/filename=\"(.*)\"/;
 		my $filename = $1;
-		my $content = $body->as_string if (defined $body);
+		my $content = "";
+ 		if (defined $body) {
+			$content = $body->as_string;
+		}
 		next unless ($content);
 		push(@segments, {
 			filename=>$filename,

lib/WebGUI/Mail/Send.pm

@@ -436,3 +436,4 @@ sub session {
 }
 
 1;
+

lib/WebGUI/Product.pm

@@ -5,12 +5,12 @@ use WebGUI::Asset::Template;
 
 #-------------------------------------------------------------------
 sub _permute {
-	my ($currentSet, @permutations, $permutation, $value, @result);
+	my ($currentSet, @permutations, @result);
 	$currentSet = shift;
 	
 	@permutations = (@_) ? _permute(@_) : [];
-	foreach $permutation (@permutations) {
+	foreach my $permutation (@permutations) {
-		foreach $value (@$currentSet) {
+		foreach my $value (@$currentSet) {
 			push(@result, [$value, @{$permutation}]);
 		}
 	}
@@ -362,7 +362,7 @@ my		$currentOption = $self->getOption($_);
 
 #-------------------------------------------------------------------
 sub updateVariants {
-	my ($self, %variants, @optionSets, @variants, $variant, %var, @composition, $option, @newVariants, $parameterName);
+	my ($self, %variants, @optionSets, @variants, %var, @composition, @newVariants, $parameterName);
 	$self = shift;
 
 	foreach (@{$self->getVariant}) {
@@ -378,7 +378,7 @@ sub updateVariants {
 	
 	@variants = ([]) unless	(@variants);
 	my %newVariants;
-	foreach $variant (@variants) {
+	foreach my $variant (@variants) {
 		my %sku;
 		
 		$var{productId} = $self->get('productId');
@@ -388,7 +388,7 @@ sub updateVariants {
 		$sku{base} = $self->get('sku');
 		@composition = ();
 
-		foreach $option (@{$variant}) {
+		foreach my $option (@{$variant}) {
 			$var{price} += $option->{priceModifier};
 			$var{weight} += $option->{weightModifier};
 			$var{sku} .= $option->{skuModifier};

lib/WebGUI/Search/Index.pm

@@ -209,7 +209,7 @@ A reference to an asset object.
 sub new {
 	my $class = shift;
 	my $asset = shift;
-	my $self = {_asset=>$asset, _session=>$asset->session, _id=>$asset->getId};
+	my $self = {_asset=>$asset, _session=>$asset->session, _id=>$asset->getId}, $class;
 	bless $self;
 }
 

lib/WebGUI/Storage.pm

@@ -254,7 +254,6 @@ sub addFileFromHashref {
 	my $self = shift;
 	my $filename = $self->session->url->makeCompliant(shift);
 	my $hashref = shift;
-	bless $hashref;
         nstore $hashref, $self->getPath($filename) or $self->_addError("Couldn't create file ".$self->getPath($filename)." because ".$!);
 	return $filename;
 }
@@ -279,9 +278,9 @@ sub addFileFromScalar {
 	my $self = shift;
 	my $filename = $self->session->url->makeCompliant(shift);
 	my $content = shift;
-	if (open(FILE,">".$self->getPath($filename))) {
+	if (open(my $FILE,">",$self->getPath($filename))) {
-		print FILE $content;
+		print $FILE $content;
-		close(FILE);
+		close($FILE);
 	} else {
         	$self->_addError("Couldn't create file ".$self->getPath($filename)." because ".$!);
 	}
@@ -476,6 +475,25 @@ sub getErrors {
 }
 
 
+#-------------------------------------------------------------------
+
+=head2 getFileContentsAsHashref ( filename )
+
+Returns a hash reference from the file. Must be used in conjunction with a file that was saved using the addFileFromHashref method.
+
+=head3 filename
+
+The file to retrieve the data from.
+
+=cut
+
+sub getFileContentsAsHashref {
+	my $self = shift;
+	my $filename = shift;
+        return retrieve($self->getPath($filename));
+}
+
+
 #-------------------------------------------------------------------
 
 =head2 getFileContentsAsScalar ( filename )
@@ -492,11 +510,11 @@ sub getFileContentsAsScalar {
 	my $self = shift;
 	my $filename = shift;
 	my $content;
-	open (FILE,"<".$self->getPath($filename));
+	open (my $FILE,"<",$self->getPath($filename));
-	while (<FILE>) {
+	while (<$FILE>) {
 		$content .= $_;
 	}
-	close(FILE);
+	close($FILE);
 	return $content;
 }
 
@@ -594,25 +612,6 @@ sub getFiles {
 
 
 
-#-------------------------------------------------------------------
-
-=head2 getFileContentsAsHashref ( filename )
-
-Returns a hash reference from the file. Must be used in conjunction with a file that was saved using the addFileFromHashref method.
-
-=head3 filename
-
-The file to retrieve the data from.
-
-=cut
-
-sub getHashref {
-	my $self = shift;
-	my $filename = shift;
-        return retrieve($self->getPath($filename));
-}
-
-
 
 #-------------------------------------------------------------------
 

lib/WebGUI/Subscription.pm

@@ -1,5 +1,6 @@
 package WebGUI::Subscription;
 
+use strict;
 use WebGUI::Macro;
 use WebGUI::Utility;
 use WebGUI::Commerce::Payment;

lib/WebGUI/User.pm

@@ -208,7 +208,10 @@ If set to "1" then the listing will not include expired groupings. Defaults to "
 sub getGroups {
 	my $self = shift;
         my $withoutExpired = shift;
-        my $clause = "and expireDate>".$self->session->datetime->time() if ($withoutExpired);
+        my $clause = "";
+	if ($withoutExpired) {
+        	$clause = "and expireDate>".$self->session->datetime->time();
+	}
 	my $gotGroupsForUser = $self->session->stow->get("gotGroupsForUser");
         if (exists $gotGroupsForUser->{$self->userId}) {
                 return $gotGroupsForUser->{$self->userId};