From 52d5883b770f2589ef28201adbc7ccb855f78121 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Thu, 21 Jan 2010 19:52:48 -0800
Subject: [PATCH] More exceptions and tests for Asset.pm

---
 lib/WebGUI/Asset.pm | 18 +++++--------
 t/Asset/Asset.t     | 63 ++++++++++++++++++++++++++-------------------
 2 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm
index 55c917ceb..7eaa8d51c 100644
--- a/lib/WebGUI/Asset.pm
+++ b/lib/WebGUI/Asset.pm
@@ -804,8 +804,7 @@ sub getClassById {
 
     return $className if $className;
 
-    $session->errorHandler->error("Couldn't find className for asset '$assetId'");
-    return undef;
+    WebGUI::Error::InvalidParam->throw(error => "Couldn't lookup classname", param => $assetId);
 
 }
 
@@ -1715,7 +1714,7 @@ Must be a valid assetId
 
 =head3 revisionDate
 
-A specific revision date for the asset to retrieve. If not specified, the most recent one will be used.
+An optional, specific revision date for the asset to retrieve. If not specified, the most recent one will be used.
 
 =cut
 
@@ -1723,19 +1722,14 @@ sub newById {
     my $requestedClass  = shift;
     my $session         = shift;
     my $assetId         = shift;
+    if (!$assetId) {
+        WebGUI::Error::InvalidParam->throw(error => 'newById must get an assetId');
+    }
     my $revisionDate    = shift;
- 
-# Some code requires that these situations not die.
-#    confess "newById requires WebGUI::Session" 
-#        unless $session && blessed $session eq 'WebGUI::Session';
-#    confess "newById requires assetId"
-#        unless $assetId;
-# So just return instead
-    return undef unless ( $session && blessed $session eq 'WebGUI::Session' ) 
-        && $assetId;
 
     my $className = WebGUI::Asset->getClassById($session, $assetId);
     my $class     = WebGUI::Asset->loadModule($className);
+
     return $class->new($session, $assetId, $revisionDate);
 }
 
diff --git a/t/Asset/Asset.t b/t/Asset/Asset.t
index 72d7439c3..5ef3df5c9 100644
--- a/t/Asset/Asset.t
+++ b/t/Asset/Asset.t
@@ -176,14 +176,14 @@ note "loadModule";
 
 # Test the default constructor
 my $defaultAsset = WebGUI::Asset->getDefault($session);
-is(ref $defaultAsset, 'WebGUI::Asset::Wobject::Layout', 'default constructor');
+is($defaultAsset, 'WebGUI::Asset::Wobject::Layout');
 
 # Test the new constructor
 my $assetId = "PBnav00000000000000001"; # one of the default nav assets
 
 # - explicit class
 my $asset = WebGUI::Asset->newById($session, $assetId);
-is (ref $asset, 'WebGUI::Asset::Wobject::Navigation','new constructor explicit - ref check');
+isa_ok ($asset, 'WebGUI::Asset::Wobject::Navigation');
 is ($asset->getId, $assetId, 'new constructor explicit - returns correct asset');
 
 # - new by hashref properties
@@ -192,13 +192,13 @@ $asset = WebGUI::Asset->newByPropertyHashRef($session, {
                                                           className=>"WebGUI::Asset::Wobject::Navigation",
 		                                                  assetId=>$assetId
 													    });
-is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'new constructor newByHashref - ref check');
+isa_ok ($asset, 'WebGUI::Asset::Wobject::Navigation');
 is ($asset->getId, $assetId, 'new constructor newByHashref - returns correct asset');
 
 # - implicit class
 $asset = undef;
 $asset = WebGUI::Asset::Wobject::Navigation->new($session, $assetId);
-is (ref $asset, 'WebGUI::Asset::Wobject::Navigation', 'new constructor implicit - ref check');
+isa_ok ($asset, 'WebGUI::Asset::Wobject::Navigation');
 is ($asset->getId, $assetId, 'new constructor implicit - returns correct asset');
 
 # - die gracefully
@@ -224,37 +224,46 @@ isa_ok ($primevalAsset, 'WebGUI::Asset');
 # Test the newById Constructor
 $asset = undef;
 
-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');
+note "new";
+use WebGUI::Asset::Wobject::Navigation;
+$asset = WebGUI::Asset::Wobject::Navigation->new($session, $assetId);
+isa_ok ($asset, 'WebGUI::Asset::Wobject::Navigation');
+is ($asset->getId, $assetId, 'new constructor - returns correct asset when invoked with correct class');
 
 # - die gracefully
 my $deadAsset = 1;
 
 # -- invalid asset id
-$deadAsset = WebGUI::Asset->newById($session, 'RoysNonExistantAssetId');
-is ($deadAsset, undef,'newByDynamicClass constructor with invalid assetId returns undef');
+note "getClassById";
+{
+    my $deadAsset = eval { WebGUI::Asset->getClassById($session, 'RoysNonExistantAssetId'); };
+    my $e = Exception::Class->caught;
+    isa_ok($e, 'WebGUI::Error::InvalidParam', 'getClassById must have a valid assetId');
+    cmp_deeply(
+        $e,
+        methods(
+            error => "Couldn't lookup classname",
+            param => 'RoysNonExistantAssetId',
+        ),
+        '... checking error message',
+    );
+}
 
-# -- no assetId
-is(
-    WebGUI::Asset->newByDynamicClass( $session ),
-    undef,
-    "newByDynamicClass constructor returns 'undef' with no assetId",
-);
+note "newById";
+{
+    my $deadAsset = eval { WebGUI::Asset->newById($session); };
+    my $e = Exception::Class->caught;
+    isa_ok($e, 'WebGUI::Error::InvalidParam', "newById won't work without an assetId");
+    cmp_deeply(
+        $e,
+        methods(
+            error => "newById must get an assetId",
+        ),
+        '... checking error message',
+    );
+}
 
 # -- no session
-is(
-    WebGUI::Asset->newByDynamicClass( ),
-    undef,
-    "newByDynamicClass constructor returns 'undef' with no valid WebGUI::Session",
-);
-is(
-    WebGUI::Asset->newByDynamicClass( "nothing" ),
-    undef,
-    "newByDynamicClass constructor returns 'undef' with no valid WebGUI::Session",
-);
-
 # Root Asset
 isa_ok($rootAsset, 'WebGUI::Asset');
 is($rootAsset->getId, 'PBasset000000000000001', 'Root Asset ID check');