Re: LiveUser_Auth_Propel Container 0.1

Lukas Kahwe Smith schrieb:
> Georg Gell wrote:
> 
>> I have finished the first version for the LiveUser_Auth_Propel. It is
>> working well for my test configuration.
>> You can find it at
>> http://tools.have2.com/LiveUser/LiveUser/Auth/Propel.phps
>> Can you please check if I have missed to set any properties that
>> LiveUser expects the Auth containers to have.
> 
> 
> Hmm I am not really happpy with the implementation.
> 
> readUserData() is a public method. it needs to be defined because its
> called from the LiveUser class.

ok, I missed that. I found it once in LiveUser::updateProperty(), which
gets the data per primary key. No problem, I can provide this.

> as such I do not see a need to overwrite login(). you should be able to
> do anything inside the init() method that may need to get initialized
> beforehand.

I did this because I wanted to use the advantages of Propel. I mean if I
have already an object with all values, I don't see the need to copy
each property into an array. But I don't see why overriding the login
method would be a problem.

> if you insist to overwrite login() please not the following: your
> login() implementation takes a blacklist approach. i think its better to
> check if the person is active (or if active is not enabled), instead of
> checking the user is not active.

Both are the same: I check if the is_active check is enabled _and_ user
is not active, then i _don't_ log in, you check if if the is_active
check is disabled _or_ the user is active, then you _do_ log in. The
result is the same.

					me		you
is_active not enabled:			login		login
is_active enabled, is_active=true	login		login
is_active enabled, is_active=false	no login	no login

I think my comment is misleading

> 
> hmm reading the source again and remember your comments .. i guess the
> entire story revolves around the propertyValues() array? .. there is
> really no way around it. unless you essentially overwrite every single
> method in the common implementation. its also used in the freeze() and
> unfreeze() method. i am open to finding a better solution, though i dont
> see turning everything into set/get method as a viable solution. if that
> is necessary for propel, then imho its broken.

For me the big advantage of Propel is that the database object itself is
private, and only accessible by get/set methods. So I can never forget
to escape some input or things like that. This is a feature, not a bug ;)
I am already happy with the getProperty method. When I overload this, I
can translate it to a get to the user object. And a quick grep in the
LiveUser code shows that all accesses to $propertyValues for the auth
object use getProperty, so for the auth object I think that I can leave
the propertyValues array to what auth_common sets it. And to un/freeze
the user data:
$propertyValues['Propel']['userObject']=$this->user;
and I keep my Propel user object.
The biggest problem I have is this:
the access to $propertyValues is distributed over so many files that it
is very difficult to find out, what keys/values are needed, and where
they are read/written.
For me the perfect solution would be to use getProperty/setProperty
methods everywhere, because then I would only have to overload one
method, and get the values from $propertyValues or the related Propel
objects.

> 
> hmm maybe we can remove propertyValues from the common implementation,
> so that you can use overloading?

The problem with the common class is that it expects all database
results as arrays. But Propel results are objects.
This will not be a problem for the permission container, because I can
use the interface ArrayAccess for those classes, because i can set my
own base class for those objects, so I think this will be easier.
But for the auth container, the users might need their own base class,
so I think I will just overload login and getProperty. And everything
should work well?

regards
Georg

btw what are the setExternalValues(), externalValuesMatch() used for?
and un/freeze() only to store the properties in a session?