diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm index dbce4edbb..55c917ceb 100644 --- a/lib/WebGUI/Asset.pm +++ b/lib/WebGUI/Asset.pm @@ -285,13 +285,14 @@ around BUILDARGS => sub { my $revisionDate = shift; unless ($assetId) { - $session->errorHandler->error("Asset constructor new() requires an assetId."); - return undef; + WebGUI::Error::InvalidParam->throw(error => "Asset constructor new() requires an assetId."); } - if ( !$revisionDate ) { + if ( $revisionDate eq '' ) { $revisionDate = $className->getCurrentRevisionDate( $session, $assetId ); - return undef unless $revisionDate; + if ($revisionDate eq '') { + WebGUI::Error::InvalidParam->throw(error => "Cannot find revision date for assetId", param => $assetId); + } } my $properties = eval{$session->cache->get(["asset",$assetId,$revisionDate])}; @@ -332,6 +333,7 @@ use WebGUI::AssetMetaData; use WebGUI::AssetPackage; use WebGUI::AssetTrash; use WebGUI::AssetVersioning; +use WebGUI::Exception; use strict; use Tie::IxHash; use WebGUI::AdminConsole; @@ -1599,22 +1601,25 @@ sub isValidRssItem { 1 } #------------------------------------------------------------------- -=head2 loadModule ( $session, $className ) +=head2 loadModule ( $className ) -Loads an asset module if it's not already in memory. This is a class method. Returns undef on failure to load, otherwise returns the classname. Will only load classes in the WebGUI::Asset namespace. +Loads an asset module if it's not already in memory. This is a class method. Returns +undef on failure to load, otherwise returns the classname. Will only load classes +in the WebGUI::Asset namespace. =cut sub loadModule { - my ($class, $session, $className) = @_; + my ($class, $className) = @_; if ($className !~ /^WebGUI::Asset(?:::\w+)*$/ ) { - return undef; + WebGUI::Error::InvalidParam->throw(param => $className, error => "Not a WebGUI::Asset class",); } (my $module = $className . '.pm') =~ s{::}{/}g; if (eval { require $module; 1 }) { return $className; } - $session->errorHandler->error("Couldn't compile asset package: ".$className.". Root cause: ".$@); + + WebGUI::Error::Compile->throw(class => $className, cause => $@); return undef; } @@ -1730,8 +1735,7 @@ sub newById { && $assetId; my $className = WebGUI::Asset->getClassById($session, $assetId); - my $class = WebGUI::Asset->loadModule($session, $className); - return undef unless $class; + my $class = WebGUI::Asset->loadModule($className); return $class->new($session, $assetId, $revisionDate); } @@ -1761,7 +1765,7 @@ sub newByPropertyHashRef { my $properties = shift || {}; $properties->{className} //= $class; $properties->{session} = $session; - my $className = $class->loadModule($session, $properties->{className}); + my $className = $class->loadModule($properties->{className}); return undef unless (defined $className); my $object = $className->new($properties); return $object; @@ -2408,7 +2412,7 @@ new Asset will inherit security and style properties from the current asset, the sub www_add { my $self = shift; my %prototypeProperties; - my $class = $self->loadModule($self->session, $self->session->form->process("class","className")); + my $class = $self->loadModule($self->session->form->process("class","className")); return undef unless (defined $class); return $self->session->privilege->insufficient() unless ($class->canAdd($self->session)); if ($self->session->form->process('prototype')) { diff --git a/t/Asset/Asset.t b/t/Asset/Asset.t index 25e275a87..72d7439c3 100644 --- a/t/Asset/Asset.t +++ b/t/Asset/Asset.t @@ -151,7 +151,7 @@ $canViewMaker->prepare( }, ); -plan tests => 112 +plan tests => 114 + 2*scalar(@getTitleTests) #same tests used for getTitle and getMenuTitle + $canAddMaker->plan + $canAddMaker2->plan @@ -159,9 +159,24 @@ plan tests => 112 + $canViewMaker->plan ; +note "loadModule"; +{ + my $className = eval { WebGUI::Asset->loadModule('Moose::Asset'); }; + my $e = Exception::Class->caught; + isa_ok($e, 'WebGUI::Error::InvalidParam', 'loadModule must get a WebGUI::Asset class'); + cmp_deeply( + $e, + methods( + error => 'Not a WebGUI::Asset class', + param => 'Moose::Asset', + ), + '... checking error message', + ); +} + # Test the default constructor my $defaultAsset = WebGUI::Asset->getDefault($session); -is(ref $defaultAsset, 'WebGUI::Asset::Wobject::Layout','default constructor'); +is(ref $defaultAsset, 'WebGUI::Asset::Wobject::Layout', 'default constructor'); # Test the new constructor my $assetId = "PBnav00000000000000001"; # one of the default nav assets @@ -187,28 +202,38 @@ is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'new constructor implicit is ($asset->getId, $assetId, 'new constructor implicit - returns correct asset'); # - die gracefully -my $deadAsset = 1; - # -- no asset id -$deadAsset = WebGUI::Asset->new($session, ''); -is ($deadAsset, undef,'new constructor with no assetId returns undef'); +note "new, constructor fails"; +{ + my $deadAsset = eval { WebGUI::Asset->new($session, ''); }; + my $e = Exception::Class->caught; + isa_ok($e, 'WebGUI::Error::InvalidParam', 'new must get an assetId'); + cmp_deeply( + $e, + methods( + error => 'Asset constructor new() requires an assetId.', + ), + '... checking error message', + ); +} # -- no class my $primevalAsset = WebGUI::Asset->new($session, $assetId); isa_ok ($primevalAsset, 'WebGUI::Asset'); -# Test the newByDynamicClass Constructor +# Test the newById Constructor $asset = undef; -$asset = WebGUI::Asset->newByDynamicClass($session, $assetId); -is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'newByDynamicClass constructor - ref check'); -is ($asset->getId, $assetId, 'newByDynamicClass constructor - returns correct asset'); +note "newById"; +$asset = WebGUI::Asset->newById($session, $assetId); +is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'newById constructor - ref check'); +is ($asset->getId, $assetId, 'newById constructor - returns correct asset'); # - die gracefully -$deadAsset = 1; +my $deadAsset = 1; # -- invalid asset id -$deadAsset = WebGUI::Asset->newByDynamicClass($session, 'RoysNonExistantAssetId'); +$deadAsset = WebGUI::Asset->newById($session, 'RoysNonExistantAssetId'); is ($deadAsset, undef,'newByDynamicClass constructor with invalid assetId returns undef'); # -- no assetId