From 04ef1ab8a025751e0f6ab79b94f002ce5ad64753 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 09:46:31 +1000 Subject: [PATCH] More aggressive searching for local AleaIdField(unique=True) when translating between meteor/object identifiers. --- dddp/api.py | 22 +++++++++ dddp/models.py | 121 +++++++++++++++++++++++++++++++++---------------- 2 files changed, 105 insertions(+), 38 deletions(-) diff --git a/dddp/api.py b/dddp/api.py index 9153c43..0cabe85 100644 --- a/dddp/api.py +++ b/dddp/api.py @@ -409,6 +409,15 @@ class Collection(APIMixin): ) elif isinstance(field, django.contrib.postgres.fields.ArrayField): fields[field.name] = field.to_python(fields.pop(field.name)) + elif ( + isinstance(field, AleaIdField) + ) and ( + not field.null + ) and ( + field.name == 'aid' + ): + # This will be sent as the `id`, don't send it in `fields`. + fields.pop(field.name) for field in meta.local_many_to_many: fields['%s_ids' % field.name] = get_meteor_ids( field.rel.to, fields.pop(field.name), @@ -616,6 +625,19 @@ class DDP(APIMixin): ) if isinstance(col.model._meta.pk, AleaIdField): meteor_ids = None + elif len([ + field + for field + in col.model._meta.local_fields + if ( + isinstance(field, AleaIdField) + ) and ( + field.unique + ) and ( + not field.null + ) + ]) == 1: + meteor_ids = None else: meteor_ids = get_meteor_ids( qs.model, qs.values_list('pk', flat=True), diff --git a/dddp/models.py b/dddp/models.py index b812a5c..f181842 100644 --- a/dddp/models.py +++ b/dddp/models.py @@ -2,6 +2,7 @@ from __future__ import absolute_import import collections +import os from django.db import models, transaction from django.db.models.fields import NOT_PROVIDED @@ -16,7 +17,6 @@ import ejson from dddp import meteor_random_id -@transaction.atomic def get_meteor_id(obj_or_model, obj_pk=None): """Return an Alea ID for the given object.""" if obj_or_model is None: @@ -33,17 +33,27 @@ def get_meteor_id(obj_or_model, obj_pk=None): # obj_or_model is an instance, not a model. if isinstance(meta.pk, AleaIdField): return obj_or_model.pk - alea_unique_fields = [ - field - for field in meta.local_fields - if isinstance(field, AleaIdField) and field.unique - ] - if len(alea_unique_fields) == 1: - # found an AleaIdField with unique=True, assume it's got the value. - return getattr(obj_or_model, alea_unique_fields[0].attname) if obj_pk is None: # fall back to primary key, but coerce as string type for lookup. obj_pk = str(obj_or_model.pk) + alea_unique_fields = [ + field + for field in meta.local_fields + if isinstance(field, AleaIdField) and field.unique + ] + if len(alea_unique_fields) == 1: + # found an AleaIdField with unique=True, assume it's got the value. + aid = alea_unique_fields[0].attname + if isinstance(obj_or_model, model): + val = getattr(obj_or_model, aid) + elif obj_pk is None: + val = None + else: + val = model.objects.values_list(aid, flat=True).get( + pk=obj_pk, + ) + if val: + return val if obj_pk is None: # bail out if args are (model, pk) but pk is None. @@ -67,38 +77,44 @@ def get_meteor_id(obj_or_model, obj_pk=None): get_meteor_id.short_description = 'DDP ID' # nice title for admin list_display -@transaction.atomic def get_meteor_ids(model, object_ids): """Return Alea ID mapping for all given ids of specified model.""" - content_type = ContentType.objects.get_for_model(model) # Django model._meta is now public API -> pylint: disable=W0212 meta = model._meta - if isinstance(meta.pk, AleaIdField): - # primary_key is an AleaIdField, use it. - return collections.OrderedDict( - (obj_pk, obj_pk) for obj_pk in object_ids - ) result = collections.OrderedDict( (str(obj_pk), None) for obj_pk in object_ids ) - for obj_pk, meteor_id in ObjectMapping.objects.filter( + if isinstance(meta.pk, AleaIdField): + # primary_key is an AleaIdField, use it. + return collections.OrderedDict( + (obj_pk, obj_pk) for obj_pk in object_ids + ) + alea_unique_fields = [ + field + for field in meta.local_fields + if isinstance(field, AleaIdField) and field.unique and not field.null + ] + if len(alea_unique_fields) == 1: + aid = alea_unique_fields[0].name + query = model.objects.filter( + pk__in=object_ids, + ).values_list('pk', aid) + else: + content_type = ContentType.objects.get_for_model(model) + query = ObjectMapping.objects.filter( content_type=content_type, object_id__in=list(result) - ).values_list('object_id', 'meteor_id'): + ).values_list('object_id', 'meteor_id') + for obj_pk, meteor_id in query: result[obj_pk] = meteor_id for obj_pk, meteor_id in result.items(): if meteor_id is None: - result[obj_pk] = ObjectMapping.objects.create( - content_type=content_type, - object_id=obj_pk, - meteor_id=meteor_random_id('/collection/%s' % meta), - ).meteor_id + result[obj_pk] = get_meteor_id(model, obj_pk) return result -@transaction.atomic def get_object_id(model, meteor_id): """Return an object ID for the given meteor_id.""" if meteor_id is None: @@ -121,11 +137,13 @@ def get_object_id(model, meteor_id): ] if len(alea_unique_fields) == 1: # found an AleaIdField with unique=True, assume it's got the value. - return model.objects.values_list( + val = model.objects.values_list( 'pk', flat=True, ).get(**{ alea_unique_fields[0].attname: meteor_id, }) + if val: + return val content_type = ContentType.objects.get_for_model(model) return ObjectMapping.objects.filter( @@ -134,27 +152,39 @@ def get_object_id(model, meteor_id): ).values_list('object_id', flat=True).get() -@transaction.atomic def get_object_ids(model, meteor_ids): """Return all object IDs for the given meteor_ids.""" if model is ObjectMapping: # this doesn't make sense - raise TypeError raise TypeError("Can't map ObjectMapping instances through self.") - content_type = ContentType.objects.get_for_model(model) + # Django model._meta is now public API -> pylint: disable=W0212 + meta = model._meta + alea_unique_fields = [ + field + for field in meta.local_fields + if isinstance(field, AleaIdField) and field.unique and not field.null + ] result = collections.OrderedDict( (str(meteor_id), None) for meteor_id in meteor_ids ) - for meteor_id, object_id in ObjectMapping.objects.filter( - content_type=content_type, - meteor_id__in=meteor_ids, - ).values_list('meteor_id', 'object_id'): + if len(alea_unique_fields) == 1: + aid = alea_unique_fields[0].name + query = model.objects.filter(**{ + '%s__in' % aid: meteor_ids, + }).values_list(aid, 'pk') + else: + content_type = ContentType.objects.get_for_model(model) + query = ObjectMapping.objects.filter( + content_type=content_type, + meteor_id__in=meteor_ids, + ).values_list('meteor_id', 'object_id') + for meteor_id, object_id in query: result[meteor_id] = object_id return result -@transaction.atomic def get_object(model, meteor_id, *args, **kwargs): """Return an object for the given meteor_id.""" # Django model._meta is now public API -> pylint: disable=W0212 @@ -163,6 +193,16 @@ def get_object(model, meteor_id, *args, **kwargs): # meteor_id is the primary key return model.objects.filter(*args, **kwargs).get(pk=meteor_id) + alea_unique_fields = [ + field + for field in meta.local_fields + if isinstance(field, AleaIdField) and field.unique and not field.null + ] + if len(alea_unique_fields) == 1: + return model.objects.filter(*args, **kwargs).get(**{ + alea_unique_fields[0].name: meteor_id, + }) + return model.objects.filter(*args, **kwargs).get( pk=get_object_id(model, meteor_id), ) @@ -174,7 +214,7 @@ class AleaIdField(models.CharField): def __init__(self, *args, **kwargs): """Assume max_length of 17 to match Meteor implementation.""" - kwargs.setdefault('editable', False) + kwargs.setdefault('verbose_name', 'Alea ID') kwargs.setdefault('max_length', 17) super(AleaIdField, self).__init__(*args, **kwargs) @@ -195,19 +235,24 @@ class AleaIdField(models.CharField): def pre_save(self, model_instance, add): """Generate ID if required.""" value = super(AleaIdField, self).pre_save(model_instance, add) - if (not value) and self.default is NOT_PROVIDED: + if (not value) and self.default in (meteor_random_id, NOT_PROVIDED): value = self.get_seeded_value(model_instance) setattr(model_instance, self.attname, value) return value +# Please don't hate me... +AID_KWARGS = {} + +if os.environ.get('AID_MIGRATE_STEP', '') == '1': + AID_KWARGS['null'] = True # ...please? + + class AleaIdMixin(models.Model): - """Django model mixin that provides AleaIdField field (as _id).""" + """Django model mixin that provides AleaIdField field (as aid).""" - id = AleaIdField( - primary_key=True, - ) + aid = AleaIdField(unique=True, editable=True, **AID_KWARGS) class Meta(object):