From 84ed67d8806a9e2c9bca43c62984e504219ca34c Mon Sep 17 00:00:00 2001 From: "ian%hixie.ch" Date: Thu, 24 May 2001 07:06:41 +0000 Subject: [PATCH] Added some minor corrections and clarifications to the documentation. Added some comments to various user-related files. Factored out some code that started becoming common when inserting fields in Service/User.pm. Added code to deal with adding new contact details. Added code to support removing fields from a user. git-svn-id: svn://10.0.0.236/trunk@95862 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/webtools/PLIF/Documentation.txt | 13 +++- mozilla/webtools/PLIF/PLIF/DataSource/User.pm | 2 +- mozilla/webtools/PLIF/PLIF/Service/User.pm | 59 +++++++++++++++---- .../webtools/PLIF/PLIF/Service/UserField.pm | 11 ++++ 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/mozilla/webtools/PLIF/Documentation.txt b/mozilla/webtools/PLIF/Documentation.txt index d3569daade1..6907cdd358a 100644 --- a/mozilla/webtools/PLIF/Documentation.txt +++ b/mozilla/webtools/PLIF/Documentation.txt @@ -193,7 +193,10 @@ five debugging aids: it, not the people who use it on a daily basis) can change the debugging level that is printed, so if you have a lot of dump(9,'verbose debugging information') calls they can easily turn - them off. 0 is the most serious, 9 is the most trivial. + them off. 0 is the most serious, 9 is the most trivial. Actually, + 10 is the most trivial, but 10 is so trivial as to be unusable + unless you are trying to get a headache -- the core PLIF code calls + dump(10,'...') for almost every method call that goes through it. warn(level, message) Same as dump(), but includes a stack trace. @@ -221,6 +224,12 @@ They also allow us to later reimplement the debugging code to add better support for debuggers or pretty printing or mailing errors to admins or whatever. +If you've been paying attention you'll notice there were actually six +debugging aids, and not five. This is just to demonstrate that they +will only actually help if you use them, not just if you look at +them. Clearly here I should have been using them if I wanted to catch +that fence-post error. + Unfortunately, using these utility methods to report errors can result in suboptimal feedback to the user, and so should only be used to report errors that you really were not expecting, such as missing @@ -364,7 +373,7 @@ initialise the amount of money to zero (as opposed to undefined). Wowee, lots of PLIFisms there! Let's look at each one in turn. The first line of the body sets the $self variable to be the reference to -the object. If you are familiar with JavaScript, think "this". +the object. If you are familiar with JavaScript or C++, think "this". The second line calls the inherited constructor with the same arguments as was passed to _this_ constructor. diff --git a/mozilla/webtools/PLIF/PLIF/DataSource/User.pm b/mozilla/webtools/PLIF/PLIF/DataSource/User.pm index be706767294..22e881db880 100644 --- a/mozilla/webtools/PLIF/PLIF/DataSource/User.pm +++ b/mozilla/webtools/PLIF/PLIF/DataSource/User.pm @@ -100,7 +100,7 @@ sub setField { my $self = shift; my($app, $fieldID, $category, $name, $type, $data) = @_; # if fieldID is undefined, then add a new entry and return the - # fieldID. + # fieldID. Typically data will be undefined then too. $self->notImplemented(); } diff --git a/mozilla/webtools/PLIF/PLIF/Service/User.pm b/mozilla/webtools/PLIF/PLIF/Service/User.pm index f43afc5b7df..494307aad27 100644 --- a/mozilla/webtools/PLIF/PLIF/Service/User.pm +++ b/mozilla/webtools/PLIF/PLIF/Service/User.pm @@ -33,6 +33,11 @@ use PLIF::Service::Session; @ISA = qw(PLIF::Service::Session); 1; +# XXX It would be interesting to implement crack detection (time since +# last incorrect login, number of login attempts performed with time +# since last incorrect login < a global delta, address changing +# timeout, etc). + sub provides { my $class = shift; my($service) = @_; @@ -91,11 +96,10 @@ sub objectInit { $self->newFieldPassword($newFieldPassword); $self->fields({}); $self->fieldsByID({}); - my $fieldFactory = $self->app->getService('user.field.factory'); + # don't forget to update the 'hash' function if you add more fields + my $fieldFactory = $app->getService('user.field.factory'); foreach my $fieldID (keys($fields)) { - my $field = $fieldFactory->createFieldByID($app, $self, $fieldID, $fields->{$fieldID}); - $self->fields->{$field->category}->{$field->name} = $field; - $self->fieldsByID->{$field->fieldID} = $field; + $self->insertField($fieldFactory->createFieldByID($app, $self, $fieldID, $fields->{$fieldID})); } $self->groups({%$groups}); # hash of groupID => groupName $self->rights(map {$_ => 1} @$rights); # map a list of strings into a hash for easy access @@ -129,6 +133,14 @@ sub getField { return $field; } +sub insertField { + my $self = shift; + my($field) = @_; + $self->fields->{$field->category}->{$field->name} = $field; + $self->fieldsByID->{$field->fieldID} = $field; + return $field; +} + sub getAddress { my $self = shift; my($protocol) = @_; @@ -155,16 +167,33 @@ sub prepareAddressChange { } } +sub prepareAddressAddition { + my $self = shift; + my($fieldName, $newAddress, $password) = @_; + my $field = $self->insertField($self->app->getService('user.field.factory')->createFieldByName($self->app, $self, 'contact', $fieldName, undef)); + + if ($field->validate($newAddress)) { + $self->newFieldID($field->fieldID); + $self->newFieldValue($newAddress); + $self->newFieldPassword($password); + return $self->objectCreate($self->app, $self->userID, $self->disabled, $self->adminMessage, + $self->newFieldID, $self->newFieldValue, $self->newFieldPassword, + {$field->FieldID => $newAddress}, $self->{'groups'}, keys(%{$self->rights})); + } else { + return undef; + } +} + sub doAddressChange { my $self = shift; my($password) = @_; - if ($self->newFieldID and $self->newFieldValue) { - my $field; - if ($self->app->getService('service.passwords')->checkPassword($self->newFieldPassword, $password)) { - $field = $self->fieldsByID->{$self->newFieldID}; - if (defined($field)) { - $field->data = $self->newFieldValue; - } # else XXX create it? How do we enable the user to add contact fields? + if ($self->newFieldID) { + my $field = $self->fieldsByID->{$self->newFieldID}; + $self->assert(defined($field), 1, 'Database integrity error: newFieldID doesn\'t map to a field!'); + if (defined($password) and ($self->app->getService('service.passwords')->checkPassword($self->newFieldPassword, $password))) { + $field->data = $self->newFieldValue; + } elsif (not defined($field->data)) { + $field->remove(); } $self->newFieldID(undef); $self->newFieldValue(undef); @@ -175,6 +204,14 @@ sub doAddressChange { } } +sub resetAddressChange { + my $self = shift; + # calling the doAddressChange() function with no arguments is the + # same as calling doAddressChange() with the wrong password, which + # resets the fields (to prevent multiple attempts) + return defined($self->doAddressChange()); +} + sub hash { my $self = shift; return %$self; # XXX should expand fields too diff --git a/mozilla/webtools/PLIF/PLIF/Service/UserField.pm b/mozilla/webtools/PLIF/PLIF/Service/UserField.pm index e371bfa1852..d24d2e1d35f 100644 --- a/mozilla/webtools/PLIF/PLIF/Service/UserField.pm +++ b/mozilla/webtools/PLIF/PLIF/Service/UserField.pm @@ -48,6 +48,7 @@ sub init { $self->typeData($fieldTypeData); # change this at your peril $self->name($fieldName); # change this at your peril $self->data($fieldData); # this is the only thing you should be changing + # don't forget to update the user's 'hash' function if you add more fields $self->{'_DIRTY'} = 0; } @@ -65,6 +66,13 @@ sub validate { return 1; } +# deletes this field from the database +sub remove { + my $self = shift; + $self->{'_DELETE'} = 1; + $self->{'_DIRTY'} = 1; +} + # Contact fields should generate usernames (for their 'username' # member function) with the syntax "ServiceName: username" e.g., my # AIM username would be "AIM: HixieDaPixie". Services are fully @@ -85,8 +93,11 @@ sub DESTROY { if ($self->{'_DIRTY'}) { $self->write(); } + $self->SUPER::DESTROY(@_); } sub write { # XXX + # check $self->{'_DELETE'} to see if we have to remove it altogether + # otherwise, commit the changes }