diff --git a/CHANGES.rst b/CHANGES.rst index 2eae416..8a63954 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,15 @@ Change Log ========== +0.12.1 +------ +* Add `AleaIdMixin` which provides `aid = AleaIdField(unique=True)` to + models. +* Use `AleaIdField(unique=True)` wherever possible when translating + between Meteor style identifiers and Django primary keys, reducing + round trips to the database and hence drastically improving + performance when such fields are available. + 0.12.0 ------ * Get path to `star.json` from view config (defined in your urls.py) diff --git a/dddp/__init__.py b/dddp/__init__.py index db0f1c4..db33c8f 100644 --- a/dddp/__init__.py +++ b/dddp/__init__.py @@ -107,12 +107,12 @@ THREAD_LOCAL = ThreadLocal() METEOR_ID_CHARS = u'23456789ABCDEFGHJKLMNPQRSTWXYZabcdefghijkmnopqrstuvwxyz' -def meteor_random_id(name=None): +def meteor_random_id(name=None, length=17): if name is None: stream = THREAD_LOCAL.alea_random else: stream = THREAD_LOCAL.random_streams[name] - return stream.random_string(17, METEOR_ID_CHARS) + return stream.random_string(length, METEOR_ID_CHARS) def autodiscover(): diff --git a/dddp/api.py b/dddp/api.py index b3d8d97..0cabe85 100644 --- a/dddp/api.py +++ b/dddp/api.py @@ -29,7 +29,9 @@ import ejson from dddp import ( AlreadyRegistered, THREAD_LOCAL as this, ADDED, CHANGED, REMOVED, ) -from dddp.models import Connection, Subscription, get_meteor_id, get_meteor_ids +from dddp.models import ( + AleaIdField, Connection, Subscription, get_meteor_id, get_meteor_ids, +) XMIN = {'select': {'xmin': "'xmin'"}} @@ -407,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), @@ -612,9 +623,25 @@ class DDP(APIMixin): model_name=model_name(qs.model), collection_name=col.name, ) - meteor_ids = get_meteor_ids( - qs.model, qs.values_list('pk', flat=True), - ) + 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), + ) for obj in qs: payload = col.obj_change_as_msg(obj, ADDED, meteor_ids) this.send(payload) @@ -632,9 +659,12 @@ class DDP(APIMixin): connection=this.ws.connection, sub_id=id_, ) for col, qs in self.sub_unique_objects(sub): - meteor_ids = get_meteor_ids( - qs.model, qs.values_list('pk', flat=True), - ) + if isinstance(col.model._meta.pk, AleaIdField): + meteor_ids = None + else: + meteor_ids = get_meteor_ids( + qs.model, qs.values_list('pk', flat=True), + ) for obj in qs: payload = col.obj_change_as_msg(obj, REMOVED, meteor_ids) this.send(payload) diff --git a/dddp/logging.py b/dddp/logging.py index 147873f..121e542 100644 --- a/dddp/logging.py +++ b/dddp/logging.py @@ -13,7 +13,6 @@ class DDPHandler(logging.Handler): """Logging handler that streams log events via DDP to the current client.""" def __init__(self, *args, **kwargs): - print(self.__class__, args, kwargs) self.logger = logging.getLogger('django.db.backends') self.logger.info('Test') super(DDPHandler, self).__init__(*args, **kwargs) diff --git a/dddp/migrations/0009_auto_20150812_0856.py b/dddp/migrations/0009_auto_20150812_0856.py new file mode 100644 index 0000000..3247443 --- /dev/null +++ b/dddp/migrations/0009_auto_20150812_0856.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations +import dddp.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('dddp', '0008_remove_subscription_publication_class'), + ] + + operations = [ + migrations.AlterField( + model_name='connection', + name='connection_id', + field=dddp.models.AleaIdField(max_length=17, verbose_name=b'Alea ID'), + ), + ] diff --git a/dddp/migrations/__init__.py b/dddp/migrations/__init__.py index 29844c7..51b84bc 100644 --- a/dddp/migrations/__init__.py +++ b/dddp/migrations/__init__.py @@ -1,4 +1,7 @@ +import functools +from django.db import migrations from django.db.migrations.operations.base import Operation +from dddp.models import AleaIdField, get_meteor_id class TruncateOperation(Operation): @@ -35,3 +38,48 @@ class TruncateOperation(Operation): def describe(self): """Describe what the operation does in console output.""" return "Truncate tables" + + +def set_default_forwards(app_name, operation, apps, schema_editor): + """Set default value for AleaIdField.""" + model = apps.get_model(app_name, operation.model_name) + for obj_pk in model.objects.values_list('pk', flat=True): + model.objects.filter(pk=obj_pk).update(**{ + operation.name: get_meteor_id(model, obj_pk), + }) + + +def set_default_reverse(app_name, operation, apps, schema_editor): + """Unset default value for AleaIdField.""" + model = apps.get_model(app_name, operation.model_name) + for obj_pk in model.objects.values_list('pk', flat=True): + get_meteor_id(model, obj_pk) + + +class DefaultAleaIdOperations(object): + + def __init__(self, app_name): + self.app_name = app_name + + def __add__(self, operations): + default_operations = [] + for operation in operations: + if not isinstance(operation, migrations.AlterField): + continue + if not isinstance(operation.field, AleaIdField): + continue + if operation.name != 'aid': + continue + if operation.field.null: + continue + default_operations.append( + migrations.RunPython( + code=functools.partial( + set_default_forwards, self.app_name, operation, + ), + reverse_code=functools.partial( + set_default_reverse, self.app_name, operation, + ), + ) + ) + return default_operations + operations diff --git a/dddp/models.py b/dddp/models.py index b4515f0..f181842 100644 --- a/dddp/models.py +++ b/dddp/models.py @@ -2,8 +2,10 @@ from __future__ import absolute_import import collections +import os from django.db import models, transaction +from django.db.models.fields import NOT_PROVIDED from django.conf import settings from django.contrib.contenttypes.fields import ( GenericRelation, GenericForeignKey, @@ -15,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: @@ -26,10 +27,39 @@ def get_meteor_id(obj_or_model, obj_pk=None): if model is ObjectMapping: # this doesn't make sense - raise TypeError raise TypeError("Can't map ObjectMapping instances through self.") - if obj_or_model is not model and obj_pk is None: - obj_pk = str(obj_or_model.pk) + + # try getting value of AleaIdField straight from instance if possible + if isinstance(obj_or_model, model): + # obj_or_model is an instance, not a model. + if isinstance(meta.pk, AleaIdField): + return obj_or_model.pk + 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. return None + + # fallback to using AleaIdField from ObjectMapping model. content_type = ContentType.objects.get_for_model(model) try: return ObjectMapping.objects.values_list( @@ -47,37 +77,74 @@ 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 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: - # Django model._meta is now public API -> pylint: disable=W0212 - result[obj_pk] = ObjectMapping.objects.create( - content_type=content_type, - object_id=obj_pk, - meteor_id=meteor_random_id('/collection/%s' % model._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: + return None + # Django model._meta is now public API -> pylint: disable=W0212 + meta = model._meta + if model is ObjectMapping: # this doesn't make sense - raise TypeError raise TypeError("Can't map ObjectMapping instances through self.") + + if isinstance(meta.pk, AleaIdField): + # meteor_id is the primary key + return meteor_id + + 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. + 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( content_type=content_type, @@ -85,29 +152,57 @@ 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 + meta = model._meta + if isinstance(meta.pk, AleaIdField): + # 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), ) @@ -119,12 +214,52 @@ class AleaIdField(models.CharField): def __init__(self, *args, **kwargs): """Assume max_length of 17 to match Meteor implementation.""" - kwargs.update( - default=meteor_random_id, - max_length=17, - ) + kwargs.setdefault('verbose_name', 'Alea ID') + kwargs.setdefault('max_length', 17) super(AleaIdField, self).__init__(*args, **kwargs) + def get_seeded_value(self, instance): + """Generate a syncronised value.""" + # Django model._meta is public API -> pylint: disable=W0212 + return meteor_random_id( + '/collection/%s' % instance._meta, self.max_length, + ) + + def get_pk_value_on_save(self, instance): + """Generate ID if required.""" + value = super(AleaIdField, self).get_pk_value_on_save(instance) + if not value: + value = self.get_seeded_value(instance) + return value + + 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 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 aid).""" + + aid = AleaIdField(unique=True, editable=True, **AID_KWARGS) + + class Meta(object): + + """Model meta options for AleaIdMixin.""" + + abstract = True + @python_2_unicode_compatible class ObjectMapping(models.Model): diff --git a/setup.py b/setup.py index c84f740..1126824 100644 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ from setuptools import setup, find_packages setup( name='django-ddp', - version='0.12.0', + version='0.12.1', description=__doc__, long_description=open('README.rst').read(), author='Tyson Clugg',