From 2c246c06cc68a7cd43b0620abfa2da9a5ed77a74 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Sat, 9 Dec 2006 06:05:26 +0000 Subject: [PATCH] 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 ++- lib/WebGUI/Asset/File.pm | 27 ++++++++---- lib/WebGUI/Asset/File/Image.pm | 15 ++++++- t/Asset/File.t | 74 ++++++++++++++++++++++++++++++++ t/Asset/File/Image.t | 77 ++++++++++++++++++++++++++++++++++ t/Macro/Thumbnail.t | 7 +++- 6 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 t/Asset/File.t create mode 100644 t/Asset/File/Image.t diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index ce847d7f5..ff51c01a6 100644 --- a/docs/changelog/7.x.x.txt +++ b/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 diff --git a/lib/WebGUI/Asset/File.pm b/lib/WebGUI/Asset/File.pm index f05fb12b1..ee431d21f 100644 --- a/lib/WebGUI/Asset/File.pm +++ b/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->{_storageLocation} = WebGUI::Storage->create($self->session); - $self->update({storageId=>$self->{_storageLocation}->getId}); - } else { - $self->{_storageLocation} = WebGUI::Storage->get($self->session,$self->get("storageId")); - } + $self->setStorageLocation; } 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")); } diff --git a/lib/WebGUI/Asset/File/Image.pm b/lib/WebGUI/Asset/File/Image.pm index 3d16a444f..ca5f6ba31 100644 --- a/lib/WebGUI/Asset/File/Image.pm +++ b/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; diff --git a/t/Asset/File.t b/t/Asset/File.t new file mode 100644 index 000000000..566457790 --- /dev/null +++ b/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 +} diff --git a/t/Asset/File/Image.t b/t/Asset/File/Image.t new file mode 100644 index 000000000..2415ce0b6 --- /dev/null +++ b/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 +} diff --git a/t/Macro/Thumbnail.t b/t/Macro/Thumbnail.t index 6ecf0ce3b..4f712a2ff 100644 --- a/t/Macro/Thumbnail.t +++ b/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