fix a bug where updating the storageId in File Type assets did not update the cached storageLocation. Also fixed the Thumbnail Macro test

docs/changelog/7.x.x.txt

@@ -6,7 +6,11 @@
  - Storage deletes were throwing fatals when they should throw warnings.
  - Fixed a bug in WebGUI::ProfileField->getCategory which caused it to always 
    return undef. (Martin Kamerbeek / Procolix)
-
+ - Fixed a bug in WebGUI::Asset::File where update did not update the
+   internally cached storage object inside of _storageLocation.
+   This is probably only a real problem in persistent code, like Workflow
+   Activities and tests.
+   Added tests for File and Image assets to verify that this happens correctly.
 
 7.3.0
  - NOTICE: The Template Managers group is deprecated.  It has not been removed

lib/WebGUI/Asset/File.pm

@@ -199,15 +199,11 @@ sub getIcon {
 
 
 #-------------------------------------------------------------------
+
 sub getStorageLocation {
 	my $self = shift;
 	unless (exists $self->{_storageLocation}) {
-		if ($self->get("storageId") eq "") {
+		$self->setStorageLocation;
-			$self->{_storageLocation} = WebGUI::Storage->create($self->session);
-			$self->update({storageId=>$self->{_storageLocation}->getId});
-		} else {
-			$self->{_storageLocation} = WebGUI::Storage->get($self->session,$self->get("storageId"));
-		}
 	}
 	return $self->{_storageLocation};
 }
@@ -313,6 +309,18 @@ sub setSize {
 
 #-------------------------------------------------------------------
 
+sub setStorageLocation {
+	my $self = shift;
+	if ($self->get("storageId") eq "") {
+		$self->{_storageLocation} = WebGUI::Storage->create($self->session);
+		$self->update({storageId=>$self->{_storageLocation}->getId});
+	} else {
+		$self->{_storageLocation} = WebGUI::Storage->get($self->session,$self->get("storageId"));
+	}
+}
+
+#-------------------------------------------------------------------
+
 =head2 update
 
 We override the update method from WebGUI::Asset in order to handle file system privileges.
@@ -324,9 +332,14 @@ sub update {
 	my %before = (
 		owner => $self->get("ownerUserId"),
 		view => $self->get("groupIdView"),
-		edit => $self->get("groupIdEdit")
+		edit => $self->get("groupIdEdit"),
+		storageId => $self->get('storageId'),
 	);
 	$self->SUPER::update(@_);
+	##update may have entered a new storageId.  Reset the cached one just in case.
+	if ($self->get("storageId") ne $before{storageId}) {
+		$self->setStorageLocation;
+	}
 	if ($self->get("ownerUserId") ne $before{owner} || $self->get("groupIdEdit") ne $before{edit} || $self->get("groupIdView") ne $before{view}) {
 		$self->getStorageLocation->setPrivileges($self->get("ownerUserId"),$self->get("groupIdView"),$self->get("groupIdEdit"));
 	}

lib/WebGUI/Asset/File/Image.pm

@@ -148,11 +148,12 @@ sub getEditForm {
 
 
 #-------------------------------------------------------------------
+
 sub getStorageLocation {
 	my $self = shift;
 	unless (exists $self->{_storageLocation}) {
 		if ($self->get("storageId") eq "") {
-		$self->{_storageLocation} = WebGUI::Storage::Image->create($self->session);
+			$self->{_storageLocation} = WebGUI::Storage::Image->create($self->session);
 			$self->update({storageId=>$self->{_storageLocation}->getId});
 		} else {
 			$self->{_storageLocation} = WebGUI::Storage::Image->get($self->session,$self->get("storageId"));
@@ -222,6 +223,18 @@ sub setSize {
 	return $self->SUPER::setSize($size);
 }
 
+#-------------------------------------------------------------------
+
+sub setStorageLocation {
+	my $self = shift;
+	if ($self->get("storageId") eq "") {
+		$self->{_storageLocation} = WebGUI::Storage::Image->create($self->session);
+		$self->update({storageId=>$self->{_storageLocation}->getId});
+	} else {
+		$self->{_storageLocation} = WebGUI::Storage::Image->get($self->session,$self->get("storageId"));
+	}
+}
+
 #-------------------------------------------------------------------
 sub view {
 	my $self = shift;

t/Asset/File.t

@@ -0,0 +1,74 @@
+#-------------------------------------------------------------------
+# WebGUI is Copyright 2001-2006 Plain Black Corporation.
+#-------------------------------------------------------------------
+# Please read the legal notices (docs/legal.txt) and the license
+# (docs/license.txt) that came with this distribution before using
+# this software.
+#-------------------------------------------------------------------
+# http://www.plainblack.com                     info@plainblack.com
+#-------------------------------------------------------------------
+
+use FindBin;
+use strict;
+use lib "$FindBin::Bin/../lib";
+
+use WebGUI::Test;
+use WebGUI::Session;
+use WebGUI::Storage;
+use WebGUI::Asset::File;
+
+use Image::Magick;
+
+use Test::More; # increment this value for each test you create
+use Test::Deep;
+plan tests => 7;
+
+my $session = WebGUI::Test->session;
+
+##Create a storage location
+my $storage = WebGUI::Storage->create($session);
+
+##Save the image to the location
+my $filename = "someScalarFile.txt";
+$storage->addFileFromScalar($filename, $filename);
+
+##Do a file existance check.
+
+ok((-e $storage->getPath and -d $storage->getPath), 'Storage location created and is a directory');
+cmp_bag($storage->getFiles, ['someScalarFile.txt'], 'Only 1 file in storage with correct name');
+
+##Initialize an Image Asset with that filename and storage location
+
+$session->user({userId=>3});
+my $versionTag = WebGUI::VersionTag->getWorking($session);
+$versionTag->set({name=>"File Asset test"});
+my $properties = {
+	#     '1234567890123456789012'
+	id => 'FileAssetTest000000012',
+	title => 'File Asset Test',
+	className => 'WebGUI::Asset::File',
+	url => 'file-asset-test',
+};
+my $defaultAsset = WebGUI::Asset->getDefault($session);
+my $asset = $defaultAsset->addChild($properties, $properties->{id});
+
+ok($asset->get('storageId'), 'File Asset created with initial storage location');
+ok($asset->getStorageLocation, 'File Asset getStorageLocation initialized');
+is($asset->get('storageId'), $asset->getStorageLocation->getId, 'Asset storageId and cached storageId agree');
+
+$asset->update({
+	storageId => $storage->getId,
+	filename => $filename,
+});
+
+is($storage->getId, $asset->get('storageId'), 'Asset updated with correct new storageId');
+is($storage->getId, $asset->getStorageLocation->getId, 'Cached Asset storage location updated with correct new storageId');
+
+$versionTag->commit;
+
+END {
+	if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') {
+		$versionTag->rollback;
+	}
+	##Storage is cleaned up by rolling back the version tag
+}

t/Asset/File/Image.t

@@ -0,0 +1,77 @@
+#-------------------------------------------------------------------
+# WebGUI is Copyright 2001-2006 Plain Black Corporation.
+#-------------------------------------------------------------------
+# Please read the legal notices (docs/legal.txt) and the license
+# (docs/license.txt) that came with this distribution before using
+# this software.
+#-------------------------------------------------------------------
+# http://www.plainblack.com                     info@plainblack.com
+#-------------------------------------------------------------------
+
+use FindBin;
+use strict;
+use lib "$FindBin::Bin/../../lib";
+
+use WebGUI::Test;
+use WebGUI::Session;
+use WebGUI::Image;
+use WebGUI::Storage::Image;
+use WebGUI::Asset::File::Image;
+
+use Image::Magick;
+
+use Test::More; # increment this value for each test you create
+use Test::Deep;
+plan tests => 7;
+
+my $session = WebGUI::Test->session;
+
+my $square = WebGUI::Image->new($session, 100, 100);
+$square->setBackgroundColor('#0000FF');
+
+##Create a storage location
+my $storage = WebGUI::Storage::Image->create($session);
+
+##Save the image to the location
+$square->saveToStorageLocation($storage, 'square.png');
+
+##Do a file existance check.
+
+ok((-e $storage->getPath and -d $storage->getPath), 'Storage location created and is a directory');
+cmp_bag($storage->getFiles, ['square.png'], 'Only 1 file in storage with correct name');
+
+##Initialize an Image Asset with that filename and storage location
+
+$session->user({userId=>3});
+my $versionTag = WebGUI::VersionTag->getWorking($session);
+$versionTag->set({name=>"Image Asset test"});
+my $properties = {
+	#     '1234567890123456789012'
+	id => 'ImageAssetTest00000001',
+	title => 'Image Asset Test',
+	className => 'WebGUI::Asset::File::Image',
+	url => 'image-asset-test',
+};
+my $defaultAsset = WebGUI::Asset->getDefault($session);
+my $asset = $defaultAsset->addChild($properties, $properties->{id});
+
+ok($asset->get('storageId'), 'Image Asset created with initial storage location');
+ok($asset->getStorageLocation, 'Image Asset getStorageLocation initialized');
+is($asset->get('storageId'), $asset->getStorageLocation->getId, 'Asset storageId and cached storageId agree');
+
+$asset->update({
+	storageId => $storage->getId,
+	filename => 'square.png',
+});
+
+is($storage->getId, $asset->get('storageId'), 'Asset updated with correct new storageId');
+is($storage->getId, $asset->getStorageLocation->getId, 'Cached Asset storage location updated with correct new storageId');
+
+$versionTag->commit;
+
+END {
+	if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') {
+		$versionTag->rollback;
+	}
+	##Storage is cleaned up by rolling back the version tag
+}

t/Macro/Thumbnail.t

@@ -57,14 +57,17 @@ my $properties = {
 	title => 'Thumbnail macro test',
 	className => 'WebGUI::Asset::File::Image',
 	url => 'thumbnail-test',
-	storageId => $storage->getId,
-	filename => 'square.png',
 };
 my $defaultAsset = WebGUI::Asset->getDefault($session);
 $session->asset($defaultAsset);
 my $asset = $defaultAsset->addChild($properties, $properties->{id});
+$asset->update({
+	storageId => $storage->getId,
+	filename => 'square.png',
+});
 $versionTag->commit;
 
+
 $asset->generateThumbnail();
 
 ##Call the Thumbnail Macro with that Asset's URL and see if it returns