justinpaulson / google-api-python-client

Automatically exported from code.google.com/p/google-api-python-client
Other
0 stars 0 forks source link

oauth2client: django_orm.Storage locked_put should try a get/update first #278

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
The problem is that locked_put always instantiate a new class and writes it. 
This makes it impossible to store the credential field in a model that has more 
information (e.g. user profile settings) in addition to the credential field, 
as locked_put wipes all that out on refresh.
Ideally, the locked_put should do a locked_get and if that succeeds, update 
that object. That should preserve anything else in the model.

For example:

class GoogleAPICredentials(models.Model):
    id = models.ForeignKey(User, primary_key=True)
    credential = CredentialsField(editable=False, blank=True, null=True)
    calendarId = models.TextField(blank=True, null=True)
    calendarName = models.TextField(blank=True, null=True)

calendarId and calendarName will get wiped out on auto-refresh of the token.    

Original issue reported on code.google.com by mikesper...@gmail.com on 11 Jun 2013 at 8:09

GoogleCodeExporter commented 8 years ago

Original comment by jcgregorio@google.com on 5 Aug 2013 at 1:58

GoogleCodeExporter commented 8 years ago
I attempted a fix for this in 
https://code.google.com/r/johnscottatlakson-google-api-python-client/source/deta
il?r=a0321a5135af2f9005646e9313ff844120b1ef01#

In my case, this was also completely overwriting any previously obtained 
refresh_tokens (refresh_tokens are rationed, so the existing implementation is 
nearly useless for offline apps). My change does an extra db lookup to retrieve 
the existing credential and pluck out a refresh_token if present before doing 
the locked update using Django's built-in 'select_for_update()'. 

Perhaps there is more that should be preserved from pre-existing credentials 
when refreshing, but this should solve the problem mentioned in this ticket as 
well as the refresh_token overwriting issue I've experienced.

Original comment by john.sco...@gmail.com on 6 Feb 2014 at 11:15

GoogleCodeExporter commented 8 years ago
This is an issue for a project I'm working with. In my case rather than 
additional data fields, I have a "unique=True" field, so the record needs to be 
updated rather than have a new one added -- a refresh throws an IntegrityError 
due to violation of the unique constraint.

I can monkey-patch with John's code, but it would be preferable to have this 
change in trunk since it's more of a bug fix than an enhancement (at least in 
my case).

Original comment by jchamp...@zetacentauri.com on 12 Jul 2014 at 7:47