throw exceptions for bad usermod requests

[scpubgit/stemmatology.git] / lib / Text / Tradition / UserStore.pm

diff --git a/lib/Text/Tradition/UserStore.pm b/lib/Text/Tradition/UserStore.pm
index f2b4aea..03082f1 100644 (file)
--- a/lib/Text/Tradition/UserStore.pm
+++ b/lib/Text/Tradition/UserStore.pm
@@ -5,11 +5,12 @@ use warnings;
 
 use Moose;
 use KiokuX::User::Util qw(crypt_password);
+use Text::Tradition::Error;
 
-extends 'KiokuX::Model';
+extends 'Text::Tradition::Directory';
+# extends 'KiokuX::Model';
 
 use Text::Tradition::User;
-# use Text::Tradition::Directory;
 
 =head1 NAME
 
@@ -65,8 +66,6 @@ has MIN_PASS_LEN => ( is => 'ro', isa => 'Num', default => sub { 8 } );
 ## XX_user, but leaving that way for now incase we merge this code
 ## into ::Directory for one-store.
 
-## To die or not to die, on error, this is the question.
-
 =head2 METHODS
 
 =head3 add_user
@@ -79,14 +78,16 @@ Create a new user object, store in the KiokuDB backend, and return it.
 
 sub add_user {
     my ($self, $userinfo) = @_;
-    my $username = $userinfo->{username};
+    my $username = $userinfo->{url} || $userinfo->{username};
     my $password = $userinfo->{password};
 
-    return unless $username && $self->validate_password($password);
-
+       throw( "No username given" ) unless $username;
+       throw( "Invalid password - too short?" )
+               unless ( $self->validate_password($password) || $username =~ /^https?:/ );
+    
     my $user = Text::Tradition::User->new(
         id => $username,
-        password => crypt_password($password),
+        password => ($password ? crypt_password($password) : ''),
     );
 
     my $scope = $self->new_scope;
@@ -95,9 +96,14 @@ sub add_user {
     return $user;
 }
 
+sub create_user {
+    my $self = shift;
+    return $self->add_user(@_);
+}
+
 =head3 find_user
 
-Takes a hashref of C<username>.
+Takes a hashref of C<username>, optionally C<openid_identifier>.
 
 Fetches the user object for the given username and returns it.
 
@@ -105,7 +111,10 @@ Fetches the user object for the given username and returns it.
 
 sub find_user {
     my ($self, $userinfo) = @_;
-    my $username = $userinfo->{username};
+    ## url or display?
+    # 'display' => 'castaway.myopenid.com',
+    # 'url' => 'http://castaway.myopenid.com/',
+    my $username = $userinfo->{url} || $userinfo->{username};
 
     my $scope = $self->new_scope;
     return $self->lookup(Text::Tradition::User->id_for_user($username));
@@ -117,9 +126,8 @@ sub find_user {
 Takes a hashref of C<username> and C<password> (same as add_user).
 
 Retrieves the user, and updates it with the new information. Username
-changing is not currently supported.
-
-Returns the updated user object, or undef if not found.
+changing is not currently supported. Returns the updated user object.
+Throws an error if user not found.
 
 =cut
 
@@ -128,10 +136,11 @@ sub modify_user {
     my $username = $userinfo->{username};
     my $password = $userinfo->{password};
 
-    return unless $username && $self->validate_password($password);
+    throw( "Missing username or bad password" )
+       unless $username && $self->validate_password($password);
 
     my $user = $self->find_user({ username => $username });
-    return unless $user;
+    throw( "Could not find user $username" ) unless $user;
 
     my $scope = $self->new_scope;
     $user->password(crypt_password($password));
@@ -149,7 +158,7 @@ Sets the users C<active> flag to false (0), and sets all traditions
 assigned to them to non-public, updates the storage and returns the
 deactivated user.
 
-Returns undef if user not found.
+Throws an error if user not found.
 
 =cut
 
@@ -157,10 +166,10 @@ sub deactivate_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for deactivation" ) unless $username;
 
     my $user = $self->find_user({ username => $username });
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     $user->active(0);
     foreach my $tradition (@{ $user->traditions }) {
@@ -184,7 +193,7 @@ Takes a hashref of C<username>.
 Returns the user object if already activated. Activates (sets the
 active flag to true (1)), updates the storage and returns the user.
 
-Returns undef if the user is not found.
+Throws an error if the user is not found.
 
 =cut
 
@@ -192,10 +201,10 @@ sub reactivate_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for reactivation" ) unless $username;
 
     my $user = $self->find_user({ username => $username });
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     return $user if $user->active;
 
@@ -212,7 +221,7 @@ CAUTION: Delets actual data!
 
 Takes a hashref of C<username>.
 
-Returns undef if the user doesn't exist.
+Throws an error if the user doesn't exist.
 
 Removes the user from the store and returns 1.
 
@@ -222,10 +231,10 @@ sub delete_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for deletion" ) unless $username;
 
     my $user = $self->find_user({ username => $username });
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     my $scope = $self->new_scope;
 
@@ -256,4 +265,11 @@ sub validate_password {
     return 1;
 }
 
+sub throw {
+       Text::Tradition::Error->throw( 
+               'ident' => 'UserStore error',
+               'message' => $_[0],
+               );
+}
+
 1;