Changing undef to exceptions in Asset.pm

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.");
+        WebGUI::Error::InvalidParam->throw(error => "Asset constructor new() requires an assetId.");
-        return undef;
     }
 
-    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);
+    my $class     = WebGUI::Asset->loadModule($className);
-    return undef unless $class;
     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')) {

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, '');
+note "new, constructor fails";
-is ($deadAsset, undef,'new constructor with no assetId returns undef');
+{
+    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);
+note "newById";
-is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'newByDynamicClass constructor - ref check');
+$asset = WebGUI::Asset->newById($session, $assetId);
-is ($asset->getId, $assetId, 'newByDynamicClass constructor - returns correct asset');
+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