Wednesday, July 2, 2008

Four ways to insult a nose

A bit more than two weeks ago, I suggested a small tweak to help keep user-specific data private. A few days ago, I was asked: "Based on the appengine docs, why not use the 'validator' class method, or is that a different thing?" While responding, I tried to figure out if one way was preferable over the other. I consider myself rather pragmatic, but my gut response (do whatever works) did not really cover the advantages or disadvantages of the different approaches. Thus, I have decided to post this follow-up and come up with as many distinct approaches as I could find, taking a page out of Cyrano's book and listing them by category. Bear in mind that my python is still not quite to standard yet, so unlike his over a dozen ways to insult a nose, I can only come up with four. Feel free to post other alternatives as a comment. Same is true if you see any coding errors or simpler ways to accomplish the same goal.




The basic challenge:


How can we modify the following model to throw a BadValueError when data for the wrong user is retrieved?

class SSN(db.Model):
user = db.UserProperty()
ssn = db.StringProperty(required=True)



Hackish: Monkeypatch the UserProperty


Python is a wonderful language in many ways -- one is that a programmer can modify the contract under which a class operates at runtime with only a few lines of code. The following modification replaces the validate-method of db.UserProperty to raise an error if the user does not match:

if not getattr(db.UserProperty,'old_validate', None):
original = db.UserProperty.validate
db.UserProperty.old_validate = original
def validate(self, value):
value = original(self, value)
if value != users.get_current_user():
raise db.BadValueError(
'Property must be the current user')
return value
db.UserProperty.validate = validate



Monkey patching is a powerful tool, because it allows us to bend and extend frameworks to fit out needs. Quite a few of the open source projects out there that make web frameworks work in App Engine heavily utilize this technique. Unfortunately, there are many caveats, as for example described in this wikipedia article. I would recommend to stay away from this and only use it as a last resort. And if one uses it: be more elegant with your patches than I am. This patch is an all-or-nothing approach; I cannot choose to activate it for certain models and deactivate it for others.


Classic OOP: subclass the property


In this version, we create a new subclass, CurrentUserProperty, that overwrites the validate-method. We then modify the model to use the subclass instead of the original UserProperty:

# Enforce the "current user"
class CurrentUserProperty(db.UserProperty):
def validate(self, value):
value = super(CurrentUserProperty, self).validate(value)
if value != users.get_current_user():
raise db.BadValueError(
'Property %s must be the current user' % self.name)
return value

# Data model
class SSN(db.Model):
user = CurrentUserProperty()
ssn = db.StringProperty(required=True)


There are really not a lot of drawbacks to this method, except maybe that one has to change the code for all models that need to use the new subclass. Some people in the favor-composition-over-inheritance crowd (what's that? check out this short description) crowd may cringe at the sight of an actual subclass, but I believe this is actually one of the few perfectly adequate cases where subclassing is philosophically permissible. After all, the CurrentUserProperty really is-a UserProperty.

Nevertheless, thanks to the great designers of the App Engine libraries, people who favor association can easily do so as well. Check out approach #3:


Associative: provide a validator


The idea of this code is to create a custom validation method, checkCurrentUser, and pass it along as an argument in the property's constructor. The validate-method of the Property will detect its presence and execute it on top of the regular validation logic:

# Enforce the "current user"
def checkCurrentUser(value):
if value != users.get_current_user():
raise db.BadValueError(
'Property must be the current user')
return value

# Data model
class SSN(db.Model):
user = db.UserProperty(validator=checkCurrentUser)
ssn = db.StringProperty(required=True)



Better than subclassing or worse? You bet the judge, but I think (in this particular case), it is mostly a matter of taste. Both cases require us to modify the model, so there is no gain in effort either way. Personally, I prefer having a separate classname for readability reasons, but that's just me...


Aspect oriented: Black Magic, python style


I don't know if the consultant crowd has already moved on to greener pastures, but AOP used to be the craze only a few years ago. The idea (please don't flame this blog if I oversimplify) is to take certain "concerns", like logging or transaction control, out of the application logic and "weave" it in separately. We can do the same thing for user control: the following method, enforce_user, will detect all UserProperties in a Model class and patch their validation methods to enforce the current user:

# Define the weaving-code
def enforce_user(modelclass):
for name, prop in modelclass.properties().items():
if isinstance(prop, db.UserProperty) and \
not getattr(prop, 'old_validate', None):
original = prop.validate
def validate(value):
value = original(value)
if value != users.get_current_user():
raise db.BadValueError(
'Property %s must be the current user' % name)
return value
prop.old_validate = original
prop.validate = validate
return modelclass

# Enhance the data model
enforce_user(SSN)



Some out there might say that this is only glorified monkeypatching, and they may be right. Then again, unlike my original hack, we can turn patching on or off for any single model. And if we ever happen to have class decorators available to us, we can even make it look cool by just decorating our model as following:

@enforce_user
class SSN(db.Model):
user = db.UserProperty()
ssn = db.StringProperty(required=True)



In summary


As you can see, there are many ways to skin a cat (what a ghastly idiom by the way! my beloved "Princess" heavily objects!!!). What's the best way to get the job done? I don't know, but I have a gut feeling: do whatever works best for you...

1 comments:

Steve said...

Thanks very much for a great article. Helped me a lot. Went for the Associative approach. Keep up the good work